From 1688c8717118f37191d824862a006c8373d261de Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 12 Oct 2018 12:12:25 +0200 Subject: pwm: lpss: Add ACPI HID for second PWM controller on Cherry Trail devices The second PWM controller on Cherry Trail devices uses a separate ACPI HID: "80862289", add this so that the driver will properly bind to the second PWM controller. The second PWM controller is usually not used, the main thing gained by this is properly putting the PWM controller in D3 on suspend. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss-platform.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/pwm/pwm-lpss-platform.c') diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index 5561b9e190f8..7304f36ee715 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -81,6 +81,7 @@ static SIMPLE_DEV_PM_OPS(pwm_lpss_platform_pm_ops, static const struct acpi_device_id pwm_lpss_acpi_match[] = { { "80860F09", (unsigned long)&pwm_lpss_byt_info }, { "80862288", (unsigned long)&pwm_lpss_bsw_info }, + { "80862289", (unsigned long)&pwm_lpss_bsw_info }, { "80865AC8", (unsigned long)&pwm_lpss_bxt_info }, { }, }; -- cgit v1.2.3 From 6a425ecd19a2c0e19a501323216ecf987de07841 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 12 Oct 2018 12:12:27 +0200 Subject: pwm: lpss: Check PWM powerstate after resume on Cherry Trail devices The _PS0 method for the integrated graphics on some Cherry Trail devices (observed on a HP Pavilion X2 10-p0XX) turns on the PWM chip (puts it in D0), causing an inconsistency between the state the pm-core thinks it is in (left runtime suspended as it was before the suspend/resume) and the state it actually is in. Interestingly enough this is done on a device where the pwm controller is not used for the backlight at all, since it uses an eDP panel. On devices where the PWM is used this is not a problem since we will resume it ourselves anyways. This inconsistency causes us to never suspend the pwm controller again, which causes the device to not be able to reach S0ix states when suspended. This commit adds a resume-complete handler, which when we think the device is still run-time suspended checks the actual power-state and if necessary updates the rpm-core's internal state. This fixes the Pavilion X2 10-p0XX not reaching S0ix states when suspended. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss-platform.c | 25 ++++++++++++++++++++++--- drivers/pwm/pwm-lpss.h | 2 ++ 2 files changed, 24 insertions(+), 3 deletions(-) (limited to 'drivers/pwm/pwm-lpss-platform.c') diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index 7304f36ee715..b6edf8af26cc 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -30,6 +30,7 @@ static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { .clk_rate = 19200000, .npwm = 1, .base_unit_bits = 16, + .check_power_on_resume = true, }; /* Broxton */ @@ -74,9 +75,27 @@ static int pwm_lpss_remove_platform(struct platform_device *pdev) return pwm_lpss_remove(lpwm); } -static SIMPLE_DEV_PM_OPS(pwm_lpss_platform_pm_ops, - pwm_lpss_suspend, - pwm_lpss_resume); +static void pwm_lpss_complete(struct device *dev) +{ + struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev); + int ret, state; + + /* The PWM may be turned on by AML code, update our state to match */ + if (pm_runtime_suspended(dev) && lpwm->info->check_power_on_resume) { + pm_runtime_disable(dev); + + ret = acpi_device_get_power(ACPI_COMPANION(dev), &state); + if (ret == 0 && state == ACPI_STATE_D0) + pm_runtime_set_active(dev); + + pm_runtime_enable(dev); + } +} + +static const struct dev_pm_ops pwm_lpss_platform_pm_ops = { + .complete = pwm_lpss_complete, + SET_SYSTEM_SLEEP_PM_OPS(pwm_lpss_suspend, pwm_lpss_resume) +}; static const struct acpi_device_id pwm_lpss_acpi_match[] = { { "80860F09", (unsigned long)&pwm_lpss_byt_info }, diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h index 8f029ed263af..1a2575d25bea 100644 --- a/drivers/pwm/pwm-lpss.h +++ b/drivers/pwm/pwm-lpss.h @@ -30,6 +30,8 @@ struct pwm_lpss_boardinfo { unsigned int npwm; unsigned long base_unit_bits; bool bypass; + /* Some devices have AML code messing with the state underneath us */ + bool check_power_on_resume; }; struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, -- cgit v1.2.3 From 4743765babb278a7d399df5733fc8a6b6bbedf3e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 14 Oct 2018 17:12:01 +0200 Subject: pwm: lpss: Force runtime-resume on suspend on Cherry Trail On Cherry Trail devices under Windows the PWM controller used for the backlight is considered part of the GPU even though it is part of the LPSS block and thus is an entirely different independent hardware unit. Because of this on Cherry Trail the GPU's (GFX0 ACPI node) _PS3 and _PS0 methods save and restore the PWM controller registers. If userspace blanks the screen before suspending, such as e.g. GNOME does, then the PWM controller will be runtime-suspended when the suspend starts. This causes the GFX0 _PS? methods to save a value of 0xffffffff for the PWM control register and to restore this value on resume. 0xffffffff is not a valid value for the register and writing this causes problems such as e.g. a flickering backlight. This commit adds a prepare method to the dev_pm_ops and makes it return 0 on Cherry Trail devices forcing a runtime-resume before other device's suspend methods run. This fixes the reading and writing back of 0xffffffff. Since we now always runtime-resume the device on suspend, it will be resumed on resume too and we no longer need to check for the GFX0 _PS0 method having resumed it underneath us, so this commit removes the now no longer necessary complete dev_pm_op. Signed-off-by: Hans de Goede Signed-off-by: Thierry Reding --- drivers/pwm/pwm-lpss-platform.c | 24 +++++++++++------------- drivers/pwm/pwm-lpss.h | 7 +++++-- 2 files changed, 16 insertions(+), 15 deletions(-) (limited to 'drivers/pwm/pwm-lpss-platform.c') diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index b6edf8af26cc..757230e1f575 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -30,7 +30,7 @@ static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { .clk_rate = 19200000, .npwm = 1, .base_unit_bits = 16, - .check_power_on_resume = true, + .other_devices_aml_touches_pwm_regs = true, }; /* Broxton */ @@ -61,6 +61,7 @@ static int pwm_lpss_probe_platform(struct platform_device *pdev) platform_set_drvdata(pdev, lpwm); + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); @@ -75,25 +76,22 @@ static int pwm_lpss_remove_platform(struct platform_device *pdev) return pwm_lpss_remove(lpwm); } -static void pwm_lpss_complete(struct device *dev) +static int pwm_lpss_prepare(struct device *dev) { struct pwm_lpss_chip *lpwm = dev_get_drvdata(dev); - int ret, state; - /* The PWM may be turned on by AML code, update our state to match */ - if (pm_runtime_suspended(dev) && lpwm->info->check_power_on_resume) { - pm_runtime_disable(dev); + /* + * If other device's AML code touches the PWM regs on suspend/resume + * force runtime-resume the PWM controller to allow this. + */ + if (lpwm->info->other_devices_aml_touches_pwm_regs) + return 0; /* Force runtime-resume */ - ret = acpi_device_get_power(ACPI_COMPANION(dev), &state); - if (ret == 0 && state == ACPI_STATE_D0) - pm_runtime_set_active(dev); - - pm_runtime_enable(dev); - } + return 1; /* If runtime-suspended leave as is */ } static const struct dev_pm_ops pwm_lpss_platform_pm_ops = { - .complete = pwm_lpss_complete, + .prepare = pwm_lpss_prepare, SET_SYSTEM_SLEEP_PM_OPS(pwm_lpss_suspend, pwm_lpss_resume) }; diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h index 1a2575d25bea..3236be835bd9 100644 --- a/drivers/pwm/pwm-lpss.h +++ b/drivers/pwm/pwm-lpss.h @@ -30,8 +30,11 @@ struct pwm_lpss_boardinfo { unsigned int npwm; unsigned long base_unit_bits; bool bypass; - /* Some devices have AML code messing with the state underneath us */ - bool check_power_on_resume; + /* + * On some devices the _PS0/_PS3 AML code of the GPU (GFX0) device + * messes with the PWM0 controllers state, + */ + bool other_devices_aml_touches_pwm_regs; }; struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, -- cgit v1.2.3