| Message ID | 20200514191001.457441-1-sam@ravnborg.org |
|---|---|
| Headers | show |
| Series | backlight updates | expand |
Hi Daniel. On Thu, May 14, 2020 at 09:41:16PM +0200, Daniel Vetter wrote: > On Thu, May 14, 2020 at 09:09:51PM +0200, Sam Ravnborg wrote: > > The backlight support has two properties that express the state: > > - power > > - state > > > > It is un-documented and easy to get wrong. > > Add backlight_is_blank() helper to make it simpler for drivers > > to get the check of the state correct. > > > > A lot of drivers also includes checks for fb_blank. > > This check is redundant when the state is checked > > as thus not needed in this helper function. > > Rolling out this helper to all relevant backlight drivers > > will eliminate almost all accesses to fb_blank. > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > Cc: Lee Jones <lee.jones@linaro.org> > > Cc: Daniel Thompson <daniel.thompson@linaro.org> > > Cc: Jingoo Han <jingoohan1@gmail.com> > > --- > > include/linux/backlight.h | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h > > index b7839ea9d00a..e67e926de1e2 100644 > > --- a/include/linux/backlight.h > > +++ b/include/linux/backlight.h > > @@ -165,6 +165,23 @@ static inline int backlight_disable(struct backlight_device *bd) > > return backlight_update_status(bd); > > } > > > > +/** > > + * backlight_is_blank - Return true if display is expected to be blank > > + * @bd: the backlight device > > + * > > + * Display is expected to be blank if any of these is true:: > > + * > > + * 1) if power in not UNBLANK > > + * 2) if state indicate BLANK or SUSPENDED > > + * > > + * Returns true if display is expected to be blank, false otherwise. > > + */ > > +static inline bool backlight_is_blank(struct backlight_device *bd) > > +{ > > + return bd->props.power != FB_BLANK_UNBLANK || > > + bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK); > > This definition here doesn't match backlight_enabled/disable() functions > we added. I think to avoid lots of pondering and surprises we should try > to make sure these are all matching, so that once we rolled them out > everywhere, we can just replace the complicated state with one flag. Will add it in v2. When all user of fb_blank is dropped we can safely remove it then. And as you said on irc, the risk of introducing regressions is lower as we see some creative uses in the drivers today. I already did some kind of audit - but I may have missed something. Sam > > > +} > > + > > extern struct backlight_device *backlight_device_register(const char *name, > > struct device *dev, void *devdata, const struct backlight_ops *ops, > > const struct backlight_properties *props); > > -- > > 2.25.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 14/05/2020 22:09, Sam Ravnborg wrote: > Look up backlight device using devm_of_find_backlight(). > This simplifies the code and prevents us from hardcoding > the node name in the driver. > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > Cc: Jyri Sarha <jsarha@ti.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 17 ++++++----------- > 1 file changed, 6 insertions(+), 11 deletions(-) Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Tomi
Hi all. i ... > Sam Ravnborg (18): > drm/omap: display: use devm_of_find_backlight > drm/tilcdc: use devm_of_find_backlight Tomi - thanks for the prompt review of the above two patches. > video: amba-clcd: use devm_of_find_backlight Any takes for review/ack of this patch? > backlight: make of_find_backlight static > backlight: drop backlight_put() > backlight: make of_find_backlight_by_node() static The above three patches are moved to the end of the patchset in v2. They cannot be applied before the users are gone. As we will remove users of of_find_backlight_by_node() in drm-misc-next, we can only merge the "make static" patches in the backlight tree after the merge window. So move them to the end so we may apply other patches before. I plan to send v2 end of the weekend, and hope for some feedback on the doc pathches until then. Sam > backlight: refactor fb_notifier_callback() > backlight: add backlight_is_blank() > backlight: improve backlight_ops documentation > backlight: improve backlight_properties documentation > backlight: improve backlight_device documentation > backlight: document inline functions in backlight.h > backlight: document enums in backlight.h > backlight: remove the unused backlight_bl driver > backlight: drop extern from prototypes > backlight: add overview and update doc > backlight: wire up kernel-doc documentation > backlight: use backlight_is_blank() in all backlight drivers > > Documentation/gpu/backlight.rst | 12 + > Documentation/gpu/index.rst | 1 + > drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 18 +- > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 17 +- > drivers/video/backlight/88pm860x_bl.c | 8 +- > drivers/video/backlight/Kconfig | 8 - > drivers/video/backlight/Makefile | 1 - > drivers/video/backlight/adp5520_bl.c | 5 +- > drivers/video/backlight/adp8860_bl.c | 5 +- > drivers/video/backlight/adp8870_bl.c | 5 +- > drivers/video/backlight/as3711_bl.c | 8 +- > drivers/video/backlight/backlight.c | 237 ++++++++------ > drivers/video/backlight/bd6107.c | 4 +- > drivers/video/backlight/corgi_lcd.c | 5 +- > drivers/video/backlight/cr_bllcd.c | 22 +- > drivers/video/backlight/da903x_bl.c | 8 +- > drivers/video/backlight/ep93xx_bl.c | 3 +- > drivers/video/backlight/generic_bl.c | 110 ------- > drivers/video/backlight/gpio_backlight.c | 4 +- > drivers/video/backlight/hp680_bl.c | 4 +- > drivers/video/backlight/jornada720_bl.c | 2 +- > drivers/video/backlight/kb3886_bl.c | 4 +- > drivers/video/backlight/led_bl.c | 4 +- > drivers/video/backlight/lm3533_bl.c | 4 +- > drivers/video/backlight/locomolcd.c | 4 +- > drivers/video/backlight/lv5207lp.c | 4 +- > drivers/video/backlight/max8925_bl.c | 8 +- > drivers/video/backlight/pwm_bl.c | 4 +- > drivers/video/backlight/qcom-wled.c | 4 +- > drivers/video/backlight/tps65217_bl.c | 4 +- > drivers/video/backlight/wm831x_bl.c | 8 +- > drivers/video/fbdev/amba-clcd.c | 19 +- > include/linux/backlight.h | 415 +++++++++++++++++++----- > 33 files changed, 536 insertions(+), 433 deletions(-) >
On Fri, May 15, 2020 at 11:24:53AM +0300, Tomi Valkeinen wrote: > On 14/05/2020 22:09, Sam Ravnborg wrote: > > Look up backlight device using devm_of_find_backlight(). > > This simplifies the code and prevents us from hardcoding > > the node name in the driver. > > > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > > Cc: Zheng Bin <zhengbin13@huawei.com> > > Cc: Kate Stewart <kstewart@linuxfoundation.org> > > Cc: Enrico Weigelt <info@metux.net> > > Cc: Allison Randal <allison@lohutok.net> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Kefeng Wang <wangkefeng.wang@huawei.com> > > Cc: Sam Ravnborg <sam@ravnborg.org> > > --- > > .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Thanks, pushed to drm-misc-next. Sam > > Tomi > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
The following series touches a lot of backlight things. It starts by migrating users of of_find_backlight_by_node() over to devm_of_find_backlight() to simplify code and to use the preferred way to register backlight. All the functions in the backlight core that is no longer used by any drivers are then marked static to avoid adding new users. Then a new helper backlight_is_blank() is added. This helper will simplify the implementation of update_status() in almost all backlight drivers. While surfing the code I really missed some documentation. So I got a bit carried away and updated the documentation for the backlight core and added it to kernel-doc. The documentation express my current understanding. Everything from spelling errors to outright wrong content shall be anticipated - so please review! We are all best helped if the documentation is correct and up-to-date and it is readable. In this process I identified that the backlight_bl driver was no longer is use - so drop it. The last patch is for now just an RFC patch that shows the potential simplifications by introducing the use of the backlight_is_blank() helper. I may split it up later. I have local patches to introduce backlight_is_blank() in the remaining backlight drivers. But they will await that this patch set matures a bit. Everything builds, but so far no run-time testing. Sam Sam Ravnborg (18): drm/omap: display: use devm_of_find_backlight drm/tilcdc: use devm_of_find_backlight video: amba-clcd: use devm_of_find_backlight backlight: make of_find_backlight static backlight: drop backlight_put() backlight: make of_find_backlight_by_node() static backlight: refactor fb_notifier_callback() backlight: add backlight_is_blank() backlight: improve backlight_ops documentation backlight: improve backlight_properties documentation backlight: improve backlight_device documentation backlight: document inline functions in backlight.h backlight: document enums in backlight.h backlight: remove the unused backlight_bl driver backlight: drop extern from prototypes backlight: add overview and update doc backlight: wire up kernel-doc documentation backlight: use backlight_is_blank() in all backlight drivers Documentation/gpu/backlight.rst | 12 + Documentation/gpu/index.rst | 1 + drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c | 18 +- drivers/gpu/drm/tilcdc/tilcdc_panel.c | 17 +- drivers/video/backlight/88pm860x_bl.c | 8 +- drivers/video/backlight/Kconfig | 8 - drivers/video/backlight/Makefile | 1 - drivers/video/backlight/adp5520_bl.c | 5 +- drivers/video/backlight/adp8860_bl.c | 5 +- drivers/video/backlight/adp8870_bl.c | 5 +- drivers/video/backlight/as3711_bl.c | 8 +- drivers/video/backlight/backlight.c | 237 ++++++++------ drivers/video/backlight/bd6107.c | 4 +- drivers/video/backlight/corgi_lcd.c | 5 +- drivers/video/backlight/cr_bllcd.c | 22 +- drivers/video/backlight/da903x_bl.c | 8 +- drivers/video/backlight/ep93xx_bl.c | 3 +- drivers/video/backlight/generic_bl.c | 110 ------- drivers/video/backlight/gpio_backlight.c | 4 +- drivers/video/backlight/hp680_bl.c | 4 +- drivers/video/backlight/jornada720_bl.c | 2 +- drivers/video/backlight/kb3886_bl.c | 4 +- drivers/video/backlight/led_bl.c | 4 +- drivers/video/backlight/lm3533_bl.c | 4 +- drivers/video/backlight/locomolcd.c | 4 +- drivers/video/backlight/lv5207lp.c | 4 +- drivers/video/backlight/max8925_bl.c | 8 +- drivers/video/backlight/pwm_bl.c | 4 +- drivers/video/backlight/qcom-wled.c | 4 +- drivers/video/backlight/tps65217_bl.c | 4 +- drivers/video/backlight/wm831x_bl.c | 8 +- drivers/video/fbdev/amba-clcd.c | 19 +- include/linux/backlight.h | 415 +++++++++++++++++++----- 33 files changed, 536 insertions(+), 433 deletions(-)