| Message ID | 20230106164856.1453819-2-steve@sk2.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [RESEND,1/4] backlight: aat2870: Use backlight helper | expand |
On Fri, Jan 06, 2023 at 05:48:52PM +0100, Stephen Kitt wrote: > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Signed-off-by: Stephen Kitt <steve@sk2.org> Lee/Daniel, will you pick these up, or should I smash them all into drm-misc-next for 6.3? Stephen, I see to be missing 3/4 here, but maybe it'll show up. -Daniel > --- > drivers/video/backlight/aat2870_bl.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c > index a7af9adafad6..1cbb303e9c88 100644 > --- a/drivers/video/backlight/aat2870_bl.c > +++ b/drivers/video/backlight/aat2870_bl.c > @@ -59,7 +59,7 @@ static int aat2870_bl_update_status(struct backlight_device *bd) > struct aat2870_bl_driver_data *aat2870_bl = bl_get_data(bd); > struct aat2870_data *aat2870 = > dev_get_drvdata(aat2870_bl->pdev->dev.parent); > - int brightness = bd->props.brightness; > + int brightness = backlight_get_brightness(bd); > int ret; > > if ((brightness < 0) || (bd->props.max_brightness < brightness)) { > @@ -70,11 +70,6 @@ static int aat2870_bl_update_status(struct backlight_device *bd) > dev_dbg(&bd->dev, "brightness=%d, power=%d, state=%d\n", > bd->props.brightness, bd->props.power, bd->props.state); > > - if ((bd->props.power != FB_BLANK_UNBLANK) || > - (bd->props.state & BL_CORE_FBBLANK) || > - (bd->props.state & BL_CORE_SUSPENDED)) > - brightness = 0; > - > ret = aat2870->write(aat2870, AAT2870_BLM, > (u8)aat2870_brightness(aat2870_bl, brightness)); > if (ret < 0) > -- > 2.30.2 >
Hi Stephen, On Fri, Jan 06, 2023 at 05:48:52PM +0100, Stephen Kitt wrote: > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Signed-off-by: Stephen Kitt <steve@sk2.org> Nice cleanup. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/video/backlight/aat2870_bl.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c > index a7af9adafad6..1cbb303e9c88 100644 > --- a/drivers/video/backlight/aat2870_bl.c > +++ b/drivers/video/backlight/aat2870_bl.c > @@ -59,7 +59,7 @@ static int aat2870_bl_update_status(struct backlight_device *bd) > struct aat2870_bl_driver_data *aat2870_bl = bl_get_data(bd); > struct aat2870_data *aat2870 = > dev_get_drvdata(aat2870_bl->pdev->dev.parent); > - int brightness = bd->props.brightness; > + int brightness = backlight_get_brightness(bd); > int ret; > > if ((brightness < 0) || (bd->props.max_brightness < brightness)) { > @@ -70,11 +70,6 @@ static int aat2870_bl_update_status(struct backlight_device *bd) > dev_dbg(&bd->dev, "brightness=%d, power=%d, state=%d\n", > bd->props.brightness, bd->props.power, bd->props.state); > > - if ((bd->props.power != FB_BLANK_UNBLANK) || > - (bd->props.state & BL_CORE_FBBLANK) || > - (bd->props.state & BL_CORE_SUSPENDED)) > - brightness = 0; > - > ret = aat2870->write(aat2870, AAT2870_BLM, > (u8)aat2870_brightness(aat2870_bl, brightness)); > if (ret < 0) > -- > 2.30.2
Hi Daniel, On Fri, 6 Jan 2023 18:43:36 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Jan 06, 2023 at 05:48:52PM +0100, Stephen Kitt wrote: > > Instead of retrieving the backlight brightness in struct > > backlight_properties manually, and then checking whether the backlight > > should be on at all, use backlight_get_brightness() which does all > > this and insulates this from future changes. > > > > Signed-off-by: Stephen Kitt <steve@sk2.org> > > Lee/Daniel, will you pick these up, or should I smash them all into > drm-misc-next for 6.3? > > Stephen, I see to be missing 3/4 here, but maybe it'll show up. It seems to have been delayed in transit, it showed up some time after all the others on the mailing lists. If you haven’t got it yet I can resend it! Regards, Stephen
On Fri, Jan 06, 2023 at 05:48:52PM +0100, Stephen Kitt wrote: > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Signed-off-by: Stephen Kitt <steve@sk2.org> This RESEND seems to have dropped the Reviewed-By tags provided the last time is was circulated: https://lore.kernel.org/all/20220608143233.l5pbopafqc4n6uwq@maple.lan/ There's only four patches so I guess I can add them again so the tools can pick them up. Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> Daniel.
On Fri, Jan 06, 2023 at 06:43:36PM +0100, Daniel Vetter wrote: > On Fri, Jan 06, 2023 at 05:48:52PM +0100, Stephen Kitt wrote: > > Instead of retrieving the backlight brightness in struct > > backlight_properties manually, and then checking whether the backlight > > should be on at all, use backlight_get_brightness() which does all > > this and insulates this from future changes. > > > > Signed-off-by: Stephen Kitt <steve@sk2.org> > > Lee/Daniel, will you pick these up, or should I smash them all into > drm-misc-next for 6.3? Outside of holiday, Lee usually hoovers up the backlight patches (and negotiates about who merges what). Daniel. > > --- > > drivers/video/backlight/aat2870_bl.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c > > index a7af9adafad6..1cbb303e9c88 100644 > > --- a/drivers/video/backlight/aat2870_bl.c > > +++ b/drivers/video/backlight/aat2870_bl.c > > @@ -59,7 +59,7 @@ static int aat2870_bl_update_status(struct backlight_device *bd) > > struct aat2870_bl_driver_data *aat2870_bl = bl_get_data(bd); > > struct aat2870_data *aat2870 = > > dev_get_drvdata(aat2870_bl->pdev->dev.parent); > > - int brightness = bd->props.brightness; > > + int brightness = backlight_get_brightness(bd); > > int ret; > > > > if ((brightness < 0) || (bd->props.max_brightness < brightness)) { > > @@ -70,11 +70,6 @@ static int aat2870_bl_update_status(struct backlight_device *bd) > > dev_dbg(&bd->dev, "brightness=%d, power=%d, state=%d\n", > > bd->props.brightness, bd->props.power, bd->props.state); > > > > - if ((bd->props.power != FB_BLANK_UNBLANK) || > > - (bd->props.state & BL_CORE_FBBLANK) || > > - (bd->props.state & BL_CORE_SUSPENDED)) > > - brightness = 0; > > - > > ret = aat2870->write(aat2870, AAT2870_BLM, > > (u8)aat2870_brightness(aat2870_bl, brightness)); > > if (ret < 0) > > -- > > 2.30.2 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Mon, 09 Jan 2023, Daniel Thompson wrote: > On Fri, Jan 06, 2023 at 06:43:36PM +0100, Daniel Vetter wrote: > > On Fri, Jan 06, 2023 at 05:48:52PM +0100, Stephen Kitt wrote: > > > Instead of retrieving the backlight brightness in struct > > > backlight_properties manually, and then checking whether the backlight > > > should be on at all, use backlight_get_brightness() which does all > > > this and insulates this from future changes. > > > > > > Signed-off-by: Stephen Kitt <steve@sk2.org> > > > > Lee/Daniel, will you pick these up, or should I smash them all into > > drm-misc-next for 6.3? > > Outside of holiday, Lee usually hoovers up the backlight patches (and > negotiates about who merges what). Right, I'll collect these into the Backlight tree if there aren't any build deps involved. If there are, we'll need to devise a strategy that will likely involve immutable branches.
On Fri, 06 Jan 2023, Stephen Kitt wrote: > Instead of retrieving the backlight brightness in struct > backlight_properties manually, and then checking whether the backlight > should be on at all, use backlight_get_brightness() which does all > this and insulates this from future changes. > > Signed-off-by: Stephen Kitt <steve@sk2.org> > --- > drivers/video/backlight/aat2870_bl.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) Applied, thanks
diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c index a7af9adafad6..1cbb303e9c88 100644 --- a/drivers/video/backlight/aat2870_bl.c +++ b/drivers/video/backlight/aat2870_bl.c @@ -59,7 +59,7 @@ static int aat2870_bl_update_status(struct backlight_device *bd) struct aat2870_bl_driver_data *aat2870_bl = bl_get_data(bd); struct aat2870_data *aat2870 = dev_get_drvdata(aat2870_bl->pdev->dev.parent); - int brightness = bd->props.brightness; + int brightness = backlight_get_brightness(bd); int ret; if ((brightness < 0) || (bd->props.max_brightness < brightness)) { @@ -70,11 +70,6 @@ static int aat2870_bl_update_status(struct backlight_device *bd) dev_dbg(&bd->dev, "brightness=%d, power=%d, state=%d\n", bd->props.brightness, bd->props.power, bd->props.state); - if ((bd->props.power != FB_BLANK_UNBLANK) || - (bd->props.state & BL_CORE_FBBLANK) || - (bd->props.state & BL_CORE_SUSPENDED)) - brightness = 0; - ret = aat2870->write(aat2870, AAT2870_BLM, (u8)aat2870_brightness(aat2870_bl, brightness)); if (ret < 0)
Instead of retrieving the backlight brightness in struct backlight_properties manually, and then checking whether the backlight should be on at all, use backlight_get_brightness() which does all this and insulates this from future changes. Signed-off-by: Stephen Kitt <steve@sk2.org> --- drivers/video/backlight/aat2870_bl.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)