Hello Abel,
On Thu, Feb 27, 2025 at 06:50:38PM +0200, Abel Vesa wrote:
> On 25-02-27 16:51:15, Uwe Kleine-König wrote:
> > On Thu, Feb 27, 2025 at 03:07:21PM +0200, Abel Vesa wrote:
> > > On 25-02-26 17:34:50, 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?
> > >
> > > The PWM hardware is Qualcomm PMK8550 PMIC. The way the duty cycle is
> > > controlled is described in the following comment found in lpg_calc_freq
> > > of the leds-qcom-lpg driver:
> > >
> > > /*
> > > * The PWM period is determined by:
> > > *
> > > * resolution * pre_div * 2^M
> > > * period = --------------------------
> > > * refclk
> > > *
> > > * Resolution = 2^9 bits for PWM or
> > > * 2^{8, 9, 10, 11, 12, 13, 14, 15} bits for high resolution
> > > PWM
> > > * pre_div = {1, 3, 5, 6} and
> > > * M = [0..7].
> > > *
> > > * This allows for periods between 27uS and 384s for PWM channels and
> > > periods between
> > > * 3uS and 24576s for high resolution PWMs.
> > > * The PWM framework wants a period of equal or lower length than
> > > requested,
> > > * reject anything below minimum period.
> > > */
> > >
> > > So if we request a period of 5MHz, that will not ever be reached no
> > > matter what config
> > > is used. Instead, the 4.26 MHz is selected as closest possible.
> >
> > The trace in the other mail thread suggest that you asked for a period
> > of 5 ms, not 5 MHz. And that results in a period of 4.26 ms.
>
> OK. So unit is ms. Got it.
>
> >
> > > Now, the pwm_bl is not aware of this limitation and will request duty
> > > cycle values that
> > > go above 4.26MHz.
> >
> > It requests .period = 5 ms + .duty_cycle = 5 ms. This is fine, and
> > according to the trace this results in both values becoming 4.26 ms in
> > real life. Seems fine to me.
>
> Right, but as I keep trying to explain is that, the consumer keeps
> asking for duty cycles that go over the 4.26ms, which is the period that
> the provider decided it can do instead of 5ms.
I see no problem here. Yes, requests are not implemented exactly in
general, but there is no way around that. For some use-cases the picked
configuration is sensible, for others less so. There is also no way
around that.
> > > > > This could be easily fixed if the pwm_apply*() API family would allow
> > > > > overriding the period within the PWM state that's used for providing
> > > > > the
> > > > > duty cycle. But that is currently not the case.
> > > >
> > > > I don't understand what you mean here.
> > >
> > > What I was trying to say is that the PWM generic framework currently
> > > doesn't
> > > allow overriding the PWM state's period with one provided by the consumer,
> > > when calling pwm_apply_might_sleep().
> >
> > Either I still don't understand what you want, or that is impossible or
> > useless. If you target .period = 5 ms and the hardware can only do 4.26
> > ms, why would you want to override period to 5 ms?
>
> Meaning the consumer should become aware of the period the provider can
> do before asking for a duty cycle.
There is pwm_round_waveform_might_sleep() that you could use. Downside
is that is currently only works for two lowlevel drivers.
> If you look at the other mail thread, the trace there shows the
> following sequence for every new backlight update request:
>
> 1. pwm_apply with consumer's period (5ms)
> 2. pwm_get reads the provider's period (4.25ms)
> - which is what the provider is able to do instead of 5ms
> 3. pwm_apply (due to debug) which uses the state from 2.
> 4. pwm_get reads back exactly as 2.
>
> So we can ignore 3 and 4 for now as they are there due to debug,
> but the step 1 still requests a value over the 4.26ms (5ms),
> which in the provider will translate to a pwm value higher than allowed
> by the selected configuration.
The lowlevel driver sees the request e.g.
.period = 5000
.duty_cycle = 4600
and is supposed to implement (I guess)
.period = 4260
.duty_cycle = 4260
. I don't see a technical problem here (apart from you being unlucky
about the choice made?).
Best regards
Uwe
signature.asc
Description: PGP signature
