| Message ID | 20201030094832.18297-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | pwm: lpss: Misc. cleanups / improvements | expand |
On Fri, Oct 30, 2020 at 11:48 AM Hans de Goede <hdegoede@redhat.com> wrote: > > As the comment above the code setting the DPM_FLAG_NO_DIRECT_COMPLETE > flag explains: > > /* > * On Cherry Trail devices the GFX0._PS0 AML checks if the controller > * is on and if it is not on it turns it on and restores what it > * believes is the correct state to the PWM controller. > * Because of this we must disallow direct-complete, which keeps the > * controller (runtime)suspended, on resume to avoid 2 issues: > * 1. The controller getting turned on without the linux-pm code > * knowing about this. On devices where the controller is unused > * this causes it to stay on during the next suspend causing high > * battery drain (because S0i3 is not reached) > * 2. The state restoring code unexpectedly messing with the controller > */ > > The pm-core must not skip resume to avoid the GFX0._PS0 AML code messing > with the PWM controller behind our back. But > leaving the controller > runtime-suspended (skipping runtime-resume + normal-suspend) during > suspend is fine. This paragraph is good to have in the comment of the code I think. > Set the DPM_FLAG_SMART_SUSPEND flag to allow this. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/pwm/pwm-lpss-platform.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c > index ac33861edb48..38d17e2e2b4c 100644 > --- a/drivers/pwm/pwm-lpss-platform.c > +++ b/drivers/pwm/pwm-lpss-platform.c > @@ -71,7 +71,8 @@ static int pwm_lpss_probe_platform(struct platform_device *pdev) > * 2. The state restoring code unexpectedly messing with the controller > */ > if (info->other_devices_aml_touches_pwm_regs) > - dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE); > + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE| > + DPM_FLAG_SMART_SUSPEND); > > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > -- > 2.28.0 > -- With Best Regards, Andy Shevchenko
On Fri, Oct 30, 2020 at 11:48 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi All, > > Now that the pwm-lpss / pwm-crc / i915 atomic PWM conversion has landed > in 5.10-rc1 here is a small follow up series with some misc. improvements. One nit against 3/3, in any case looks very good to me, Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> -- With Best Regards, Andy Shevchenko
Hi, On 10/30/20 1:45 PM, Andy Shevchenko wrote: > On Fri, Oct 30, 2020 at 11:48 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> As the comment above the code setting the DPM_FLAG_NO_DIRECT_COMPLETE >> flag explains: >> >> /* >> * On Cherry Trail devices the GFX0._PS0 AML checks if the controller >> * is on and if it is not on it turns it on and restores what it >> * believes is the correct state to the PWM controller. >> * Because of this we must disallow direct-complete, which keeps the >> * controller (runtime)suspended, on resume to avoid 2 issues: >> * 1. The controller getting turned on without the linux-pm code >> * knowing about this. On devices where the controller is unused >> * this causes it to stay on during the next suspend causing high >> * battery drain (because S0i3 is not reached) >> * 2. The state restoring code unexpectedly messing with the controller >> */ >> >> The pm-core must not skip resume to avoid the GFX0._PS0 AML code messing >> with the PWM controller behind our back. But > >> leaving the controller >> runtime-suspended (skipping runtime-resume + normal-suspend) during >> suspend is fine. > > This paragraph is good to have in the comment of the code I think. Agreed, I've added this as comment for v2 of this patch set and I will send out a v2 of the entire series with your reviewed-by added. Thanks & Regards, Hans > >> Set the DPM_FLAG_SMART_SUSPEND flag to allow this. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/pwm/pwm-lpss-platform.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c >> index ac33861edb48..38d17e2e2b4c 100644 >> --- a/drivers/pwm/pwm-lpss-platform.c >> +++ b/drivers/pwm/pwm-lpss-platform.c >> @@ -71,7 +71,8 @@ static int pwm_lpss_probe_platform(struct platform_device *pdev) >> * 2. The state restoring code unexpectedly messing with the controller >> */ >> if (info->other_devices_aml_touches_pwm_regs) >> - dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE); >> + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE| >> + DPM_FLAG_SMART_SUSPEND); >> >> pm_runtime_set_active(&pdev->dev); >> pm_runtime_enable(&pdev->dev); >> -- >> 2.28.0 >> > >