Re: [PATCH] backlight: pwm_bl: apply the initial backlight state with sane defaults

[Michael Grzeschik <mgr@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]

On Fri, Nov 07, 2025 at 02:51:15PM +0000, Daniel Thompson wrote:
>On Fri, Nov 07, 2025 at 09:00:33AM +0100, Uwe Kleine-König wrote:
>> On Thu, Oct 30, 2025 at 11:51:07AM +0000, Daniel Thompson wrote:
>> > On Thu, Jul 31, 2025 at 10:47:18AM +0200, Michael Grzeschik wrote:
>> > > Currently when calling pwm_apply_might_sleep in the probe routine
>> > > the pwm will be configured with an not fully defined state.
>> > >
>> > > The duty_cycle is not yet set in that moment. There is a final
>> > > backlight_update_status call that will have a properly setup state.
>> > > However this change in the backlight can create a short flicker if the
>> > > backlight was already preinitialised.
>> > >
>> > > We fix the flicker by moving the pwm_apply after the default duty_cycle
>> > > can be calculated.
>> > >
>> > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> >
>> > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
>>
>> I guess this tag resulted in Lee picking up the change. I wonder if you
>> share my concern and who's responsibility it is now to address it.
>
>You mean the ordering the backlight registration versus setting the
>properties in the probe method?
>
>I definitely share the concern that there's a short window where
>something could request a brightness via sysfs and then have it
>overwritten by the remains of the probe method. Likewise I can't see
>why there would be any problem moving the call to
>pwm_backlight_initial_power_state() before the backlight is registered.
>Thus I'd be happy to see the backlight registered later in the probe
>method.
>
>On the other hand I don't see any problem calling
>backlight_update_status() from the probe method. This is a relatively
>common approach in backlight drivers to impose the initial brightness
>on the hardware without needing extra code paths.
>backlight_update_status() is guarded with a mutex and should be
>idempotent for most drivers. Therefore it is OK even if something gets
>in via sysfs and provokes an update before the probe method has started
>it's own update.
>
>In terms of who should follow up I've got no strong opinions on that.
>It's worth noting that I don't own any hardware that uses a PWM
>backlight so I'm not in a position to test it!

Depending on the setup of the hardware, calling pwm_apply_might_sleep
inbetween before having a fixed definition of how the pwm should be
setup, makes the backlight flicker. Therefor it is better to touch
it as late as possible.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]