linux-kernel - Re: [PATCH RFC] backlight: pwm_bl: Read back PWM period from provider

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <eexaex3ped44yszqaiedh23hjsivddmpjtij2pjciayt2z2o3l@bd4jlol6nvuq>
Date: Wed, 30 Apr 2025 14:25:00 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Daniel Thompson <danielt@...nel.org>
Cc: Sebastian Reichel <sre@...nel.org>, Abel Vesa <abel.vesa@...aro.org>, 
	Lee Jones <lee@...nel.org>, Jingoo Han <jingoohan1@...il.com>, Helge Deller <deller@....de>, 
	Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>, 
	Johan Hovold <johan@...nel.org>, linux-pwm@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linux-arm-msm@...r.kernel.org, linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] backlight: pwm_bl: Read back PWM period from provider

Hello Daniel,

On Tue, Mar 18, 2025 at 10:05:55AM +0000, Daniel Thompson wrote:
> On Thu, Feb 27, 2025 at 04:06:47AM +0100, Sebastian Reichel wrote:
> > On Wed, Feb 26, 2025 at 05:34:50PM +0100, Uwe Kleine-König wrote:
> > > On Wed, Feb 26, 2025 at 05:31:08PM +0200, Abel Vesa wrote:
> > > > The current implementation assumes that the PWM provider will be able to
> > > > meet the requested period, but that is not always the case. Some PWM
> > > > providers have limited HW configuration capabilities and can only
> > > > provide a period that is somewhat close to the requested one. This
> > > > simply means that the duty cycle requested might either be above the
> > > > PWM's maximum value or the 100% duty cycle is never reached.
> > >
> > > If you request a state with 100% relative duty cycle you should get 100%
> > > unless the hardware cannot do that. Which PWM hardware are you using?
> > > Which requests are you actually doing that don't match your expectation?
> >
> > drivers/leds/rgb/leds-qcom-lpg.c (which probably should at least get
> > a MAINTAINERS entry to have you CC'd considering all the PWM bits in
> > it). See the following discussion (I point you to my message in the
> > middle of a thread, which has a summary and probably is a good
> > starting point):
> >
> > https://lore.kernel.org/all/vc7irlp7nuy5yvkxwb5m7wy7j7jzgpg73zmajbmq2zjcd67pd2@cz2dcracta6w/
> 
> I had a quick glance at this thread.
> 
> It sounded to me like the PWM driver was scaling the requested period
> to match h/ware capability but then neglected to scale the requested
> duty cycle accordingly.

Well, I'd not call the period adaption "scaling", it just gets fitted to
the hardware capabilities. The same happens for duty_cycle, it's just
that the absolute duty_cycle value is reduced to the next value that is
possible to implement. Obviously that modifies the ratio between
duty_cycle and period (requested vs. implemented), but you cannot
prevent that anyhow and it makes handling easier for the lowlevel driver
with less corner cases. And whatever policy is chosen to be the right
one, it becomes ridiculous in the corner cases, so picking the simplest
to implement is the sane option in my eyes.

> That means the qcomm PWM driver programming a
> fractional value into the hardware that was not being anywhere close
> to duty_cycle / period.
> 
> So the recommendation was to fix the PWM driver rather than have
> pwm_bl.c work around it?

No, the lowlevel driver is fine.

With the new-style driver callbacks it becomes possible to query the
hardware capabilities enough to implement a helper that determines the
actually implementable waveform that is best for your use-case, whatever
"best" means here.

Best regards
Uwe

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