diff mbox series

[v3,01/15] ACPI / LPSS: Resume Cherry Trail PWM controller in no-irq phase

Message ID 20200620121758.14836-2-hdegoede@redhat.com
State Not Applicable
Headers show
Series acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API | expand

Commit Message

Hans de Goede June 20, 2020, 12:17 p.m. UTC
The DSDTs on most Cherry Trail devices have an ugly clutch where the PWM
controller gets poked from the _PS0 method of the graphics-card device:

	Local0 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
	If (((Local0 & 0x03) == 0x03))
	{
	    PSAT &= 0xFFFFFFFC
	    Local1 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
	    RSTA = Zero
	    RSTF = Zero
	    RSTA = One
	    RSTF = One
	    PWMB |= 0xC0000000
	    PWMC = PWMB /* \_SB_.PCI0.GFX0.PWMB */
	}

Where PSAT is the power-status register of the PWM controller, so if it
is in D3 when the GFX0 device's PS0 method runs then it will turn it on
and restore the PWM ctrl register value it saved from its PS3 handler.
Note not only does it restore it, it ors it with 0xC0000000 turning it
on at a time where we may not want it to get turned on at all.

The pwm_get call which the i915 driver does to get a reference to the
PWM controller, already adds a device-link making the GFX0 device a
consumer of the PWM device. So it should already have been resumed when
the above AML runs and the AML should thus not do its undesirable poking
of the PWM controller register.

But the PCI core powers on PCI devices in the no-irq resume phase and
thus calls the troublesome PS0 method in the no-irq resume phase.
Where as LPSS devices by default are resumed in the early resume phase.

This commit sets the resume_from_noirq flag in the bsw_pwm_dev_desc
struct, so that Cherry Trail PWM controllers will be resumed in the
no-irq phase. Together with the device-link added by the pwm-get this
ensures that the PWM controller will be on when the troublesome PS0
method runs, which stops it from poking the PWM controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/acpi_lpss.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Rafael J. Wysocki June 22, 2020, 4:03 p.m. UTC | #1
On Sat, Jun 20, 2020 at 2:18 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The DSDTs on most Cherry Trail devices have an ugly clutch where the PWM
> controller gets poked from the _PS0 method of the graphics-card device:
>
>         Local0 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
>         If (((Local0 & 0x03) == 0x03))
>         {
>             PSAT &= 0xFFFFFFFC
>             Local1 = PSAT /* \_SB_.PCI0.GFX0.PSAT */
>             RSTA = Zero
>             RSTF = Zero
>             RSTA = One
>             RSTF = One
>             PWMB |= 0xC0000000
>             PWMC = PWMB /* \_SB_.PCI0.GFX0.PWMB */
>         }
>
> Where PSAT is the power-status register of the PWM controller, so if it
> is in D3 when the GFX0 device's PS0 method runs then it will turn it on
> and restore the PWM ctrl register value it saved from its PS3 handler.
> Note not only does it restore it, it ors it with 0xC0000000 turning it
> on at a time where we may not want it to get turned on at all.
>
> The pwm_get call which the i915 driver does to get a reference to the
> PWM controller, already adds a device-link making the GFX0 device a
> consumer of the PWM device. So it should already have been resumed when
> the above AML runs and the AML should thus not do its undesirable poking
> of the PWM controller register.
>
> But the PCI core powers on PCI devices in the no-irq resume phase and
> thus calls the troublesome PS0 method in the no-irq resume phase.
> Where as LPSS devices by default are resumed in the early resume phase.
>
> This commit sets the resume_from_noirq flag in the bsw_pwm_dev_desc
> struct, so that Cherry Trail PWM controllers will be resumed in the
> no-irq phase. Together with the device-link added by the pwm-get this
> ensures that the PWM controller will be on when the troublesome PS0
> method runs, which stops it from poking the PWM controller.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/acpi/acpi_lpss.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index c5a5a179f49d..446e666b3466 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -257,6 +257,7 @@ static const struct lpss_device_desc bsw_pwm_dev_desc = {
>         .flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY,
>         .prv_offset = 0x800,
>         .setup = bsw_pwm_setup,
> +       .resume_from_noirq = true,
>  };
>
>  static const struct lpss_device_desc byt_uart_dev_desc = {
> --
> 2.26.2
>
diff mbox series

Patch

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index c5a5a179f49d..446e666b3466 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -257,6 +257,7 @@  static const struct lpss_device_desc bsw_pwm_dev_desc = {
 	.flags = LPSS_SAVE_CTX | LPSS_NO_D3_DELAY,
 	.prv_offset = 0x800,
 	.setup = bsw_pwm_setup,
+	.resume_from_noirq = true,
 };
 
 static const struct lpss_device_desc byt_uart_dev_desc = {