lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aQ4U3LHBQ7WcYKUd@pengutronix.de>
Date: Fri, 7 Nov 2025 16:48:44 +0100
From: Michael Grzeschik <mgr@...gutronix.de>
To: Daniel Thompson <daniel@...cstar.com>
Cc: Uwe Kleine-König <ukleinek@...nel.org>,
	Daniel Thompson <danielt@...nel.org>, Lee Jones <lee@...nel.org>,
	Jingoo Han <jingoohan1@...il.com>, Helge Deller <deller@....de>,
	Pengutronix <kernel@...gutronix.de>, linux-pwm@...r.kernel.org,
	dri-devel@...ts.freedesktop.org, linux-fbdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] backlight: pwm_bl: apply the initial backlight state
 with sane defaults

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@...gutronix.de>
>> >
>> > Reviewed-by: Daniel Thompson (RISCstar) <danielt@...nel.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 |

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ