Hi Uwe,
> From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> Sent: 2023年3月22日 15:04
>
> Hello,
>
> hmm, the subject is wrong, this is about commit deaeeda2051f
> ("backlight: pwm_bl: Don't rely on a disabled PWM emiting inactive
> state") and not 00e7e698bff1 ("backlight: pwm_bl: Configure pwm only once
> per backlight toggle"). I fixed it accordingly.
I just double checked that only revert deaeeda2051f can't fix the issue.
I have to revert 00e7e698bff1 as well.
Below are my top commits:
2c8b0985593a (HEAD -> next-20230315) Revert "backlight: pwm_bl: Configure pwm only once per backlight toggle"
e6d0aba366a7 Revert "backlight: pwm_bl: Don't rely on a disabled PWM emiting inactive state"
225b6b81afe6 (tag: next-20230315, linux-next-cu/master) Add linux-next specific files for 20230315
...
>
> On Wed, Mar 22, 2023 at 05:13:24AM +0000, Aisheng Dong wrote:
> > It seems this patch changed the behavior of pwm_backlight_suspend a
> > little bit in
> > pwm_backlight_power_off() that pwm state keep unchanged during
> suspend.
> > Then pwm_imx_tpm_suspend() will return -Ebusy due to
> tpm->enable_count > 0.
> > Was this intended behavior? Should we fix pwm core or the driver?
>
> A I see. The problem is the combination of the following facts:
>
> - Some PWMs don't emit a constant inactive signal when disabled, so a
> consumer who wants a constant inactive signal must not disable the
> PWM.
>
> - A used PWM is supposed to be disabled by its consumer on suspend.
> (This is right IMHO because on suspend the PWM is likely to stop
> oscillating and if the consumer requested some output wave form a
> suspend usually stops to adhere to the consumer's request.)
>
> So the options to fix this are (in order of my preference):
>
> a) Improve the check in the pwm_bl driver when it's safe to disable the
> PWM.
>
> b) Disable the PWM on suspend in the pwm_bl driver.
>
> c) If the pwm-imx-tpm's PWM output is configured with duty_cycle = 0 and
> is known not to continue driving a constant inactive signal on
> suspend, it might continue to suspend.
>
> I think a) is not possible in general. To determine that: Which machine do you
> experience this regression on?
Imx7ulp evk.
>
> b) is the right one from the PWM framework's POV. If you have a PWM like
> pwm-imx27 that might result in the backlight going on on suspend. That's bad,
> but compared to the pre-deaeeda2051f state it's still an improvement (as
> there the backlight went on on disable *and* suspend).
> Depending on the machine the backlight might or might not go off again later
> when suspend progresses.
>
This seems like the previous working behavior on mx7ulp without this patch.
Would you help make a patch to fix it?
Regards
Aisheng
> c) isn't that nice because that's an a bit special behaviour and people who
> intend to write code that is correct for all PWMs but only have an
> pwm-imx-tpm to test might fail to do so.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König
> |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
