| From 9e2ef73d9d72ab312bef34ba318fdcb77facb1f0 Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Mon, 27 Aug 2018 09:47:42 +0200 |
| Subject: [PATCH 1/7] 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 <andriy.shevchenko@linux.intel.com> |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| |
| drivers/pwm/pwm-lpss-platform.c | 1 + |
| 1 file changed, 1 insertion(+) |
| |
| diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c |
| index 5561b9e190f8..7304f36ee715 100644 |
| |
| |
| @@ -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 }, |
| { }, |
| }; |
| -- |
| 2.19.1 |
| |
| From f215ee5bd62ab40ee34c318df1af61991dead98d Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Tue, 11 Sep 2018 16:07:41 +0200 |
| Subject: [PATCH 2/7] pwm: lpss: Move struct pwm_lpss_chip definition to the |
| header file |
| |
| Move struct pwm_lpss_chip definition from pwm-lpss.c to pwm-lpss.h, |
| so that the pci/platform drivers can access the info member |
| (struct pwm_lpss_boardinfo *). |
| |
| This is a preparation patch for adding platform specific quirks, which |
| the drivers need access to, to pwm_lpss_boardinfo. |
| |
| Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| |
| Changes in v4: |
| -No changes in v4 of this patch-set |
| |
| Changes in v3: |
| -There was no v3, but I accidentally put v3 in the Subject of the v2 |
| patches, so lets skip v3 |
| |
| Changes in v2: |
| -No changes in v2 of this patch-set |
| |
| drivers/pwm/pwm-lpss.c | 9 --------- |
| drivers/pwm/pwm-lpss.h | 9 ++++++++- |
| 2 files changed, 8 insertions(+), 10 deletions(-) |
| |
| diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c |
| index 4721a264bac2..e602835fd6de 100644 |
| |
| |
| @@ -32,15 +32,6 @@ |
| /* Size of each PWM register space if multiple */ |
| #define PWM_SIZE 0x400 |
| |
| -#define MAX_PWMS 4 |
| - |
| -struct pwm_lpss_chip { |
| - struct pwm_chip chip; |
| - void __iomem *regs; |
| - const struct pwm_lpss_boardinfo *info; |
| - u32 saved_ctrl[MAX_PWMS]; |
| -}; |
| - |
| static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip) |
| { |
| return container_of(chip, struct pwm_lpss_chip, chip); |
| diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h |
| index 7a4238ad1fcb..8f029ed263af 100644 |
| |
| |
| @@ -16,7 +16,14 @@ |
| #include <linux/device.h> |
| #include <linux/pwm.h> |
| |
| -struct pwm_lpss_chip; |
| +#define MAX_PWMS 4 |
| + |
| +struct pwm_lpss_chip { |
| + struct pwm_chip chip; |
| + void __iomem *regs; |
| + const struct pwm_lpss_boardinfo *info; |
| + u32 saved_ctrl[MAX_PWMS]; |
| +}; |
| |
| struct pwm_lpss_boardinfo { |
| unsigned long clk_rate; |
| -- |
| 2.19.1 |
| |
| From eb73756876f92ad0da4259400bce50881cb332b7 Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Mon, 10 Sep 2018 15:30:58 +0200 |
| Subject: [PATCH 3/7] 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 <andriy.shevchenko@linux.intel.com> |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| |
| Changes in v4: |
| -Use acpi_device_get_power() instead of manually calling _PSC |
| |
| Changes in v3: |
| -There was no v3, but I accidentally put v3 in the Subject of the v2 |
| patches, so lets skip v3 |
| |
| Changes in v2: |
| -Do the pm_runtime_en/disable before/after checking the power-state |
| |
| drivers/pwm/pwm-lpss-platform.c | 25 ++++++++++++++++++++++--- |
| drivers/pwm/pwm-lpss.h | 2 ++ |
| 2 files changed, 24 insertions(+), 3 deletions(-) |
| |
| diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c |
| index 7304f36ee715..b6edf8af26cc 100644 |
| |
| |
| @@ -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 |
| |
| |
| @@ -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, |
| -- |
| 2.19.1 |
| |
| From 0a2e85765305e9fc376d0153aa9747b5d58cc804 Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Mon, 24 Sep 2018 20:57:43 +0200 |
| Subject: [PATCH 4/7] pwm: lpss: Release runtime-pm reference from the driver's |
| remove callback |
| |
| For each pwm output which gets enabled through pwm_lpss_apply(), we do a |
| pm_runtime_get_sync(). |
| |
| This commit adds pm_runtime_put() calls to pwm_lpss_remove() to balance |
| these when the driver gets removed with some of the outputs still enabled. |
| |
| Fixes: f080be27d7d9 ("pwm: lpss: Add support for runtime PM") |
| Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| |
| Changes in v2: |
| -New patch in v2 of this patch-set replacing "pwm: lpss: Add |
| pwm_lpss_get_put_runtime_pm helper function" |
| |
| drivers/pwm/pwm-lpss.c | 6 ++++++ |
| 1 file changed, 6 insertions(+) |
| |
| diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c |
| index e602835fd6de..723ca9de8325 100644 |
| |
| |
| @@ -205,6 +205,12 @@ EXPORT_SYMBOL_GPL(pwm_lpss_probe); |
| |
| int pwm_lpss_remove(struct pwm_lpss_chip *lpwm) |
| { |
| + int i; |
| + |
| + for (i = 0; i < lpwm->info->npwm; i++) { |
| + if (pwm_is_enabled(&lpwm->chip.pwms[i])) |
| + pm_runtime_put(lpwm->chip.dev); |
| + } |
| return pwmchip_remove(&lpwm->chip); |
| } |
| EXPORT_SYMBOL_GPL(pwm_lpss_remove); |
| -- |
| 2.19.1 |
| |
| From c3ffc28eeb4f9974380c4a85abfbb387d6d1cd8d Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Fri, 25 Nov 2016 09:45:19 +0100 |
| Subject: [PATCH 5/7] pwm: lpss: Add get_state callback |
| |
| Add a get_state callback so that the initial state correctly reflects |
| the actual hardware state. |
| |
| Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> |
| Acked-by: Jani Nikula <jani.nikula@intel.com> |
| Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| |
| Changes in v2: |
| -Stop using the dropped pwm_lpss_get_put_runtime_pm() helper |
| |
| drivers/pwm/pwm-lpss.c | 34 ++++++++++++++++++++++++++++++++++ |
| 1 file changed, 34 insertions(+) |
| |
| diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c |
| index 723ca9de8325..ea93ef9f3672 100644 |
| |
| |
| @@ -159,8 +159,42 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, |
| return 0; |
| } |
| |
| +/* This function gets called once from pwmchip_add to get the initial state */ |
| +static void pwm_lpss_get_state(struct pwm_chip *chip, struct pwm_device *pwm, |
| + struct pwm_state *state) |
| +{ |
| + struct pwm_lpss_chip *lpwm = to_lpwm(chip); |
| + unsigned long base_unit_range; |
| + unsigned long long base_unit, freq, on_time_div; |
| + u32 ctrl; |
| + |
| + base_unit_range = BIT(lpwm->info->base_unit_bits); |
| + |
| + ctrl = pwm_lpss_read(pwm); |
| + on_time_div = 255 - (ctrl & PWM_ON_TIME_DIV_MASK); |
| + base_unit = (ctrl >> PWM_BASE_UNIT_SHIFT) & (base_unit_range - 1); |
| + |
| + freq = base_unit * lpwm->info->clk_rate; |
| + do_div(freq, base_unit_range); |
| + if (freq == 0) |
| + state->period = NSEC_PER_SEC; |
| + else |
| + state->period = NSEC_PER_SEC / (unsigned long)freq; |
| + |
| + on_time_div *= state->period; |
| + do_div(on_time_div, 255); |
| + state->duty_cycle = on_time_div; |
| + |
| + state->polarity = PWM_POLARITY_NORMAL; |
| + state->enabled = !!(ctrl & PWM_ENABLE); |
| + |
| + if (state->enabled) |
| + pm_runtime_get(chip->dev); |
| +} |
| + |
| static const struct pwm_ops pwm_lpss_ops = { |
| .apply = pwm_lpss_apply, |
| + .get_state = pwm_lpss_get_state, |
| .owner = THIS_MODULE, |
| }; |
| |
| -- |
| 2.19.1 |
| |
| From e96509f196d1229cf66b19ad9f3d7cd43a86bc9d Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Sat, 13 Oct 2018 00:04:12 +0200 |
| Subject: [PATCH 6/7] 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 <hdegoede@redhat.com> |
| |
| drivers/pwm/pwm-lpss-platform.c | 24 +++++++++++------------- |
| drivers/pwm/pwm-lpss.h | 7 +++++-- |
| 2 files changed, 16 insertions(+), 15 deletions(-) |
| |
| diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c |
| index b6edf8af26cc..757230e1f575 100644 |
| |
| |
| @@ -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 |
| |
| |
| @@ -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, |
| -- |
| 2.19.1 |
| |
| From f249418a3a4f123a37c389378f289a7baea95332 Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Fri, 12 Oct 2018 21:39:53 +0200 |
| Subject: [PATCH 7/7] pwm: lpss: Only set update bit if we are actually |
| changing the settings |
| |
| According to the datasheet the update bit must be set if the on-time-div |
| or the base-unit changes. |
| |
| Now that we properly order device resume on Cherry Trail so that the GFX0 |
| _PS0 method no longer exits with an error, we end up with a sequence of |
| events where we are writing the same values twice in a row. |
| |
| First the _PS0 method restores the duty cycle of 0% the GPU driver set |
| on suspend and then the GPU driver first updates just the enabled bit in |
| the pwm_state from 0 to 1, causing us to write the same values again, |
| before restoring the pre-suspend duty-cycle in a separate pwm_apply call. |
| |
| When writing the update bit the second time, without changing any of |
| the values the update bit clears immediately / instantly, instead of |
| staying 1 for a while as usual. After this the next setting of the update |
| bit seems to be ignored, causing the restoring of the pre-suspend |
| duty-cycle to not get applied. This makes the backlight come up with |
| a 0% dutycycle after suspend/resume. |
| |
| Any further brightness changes after this do work. |
| |
| This commit moves the setting of the update bit into pwm_lpss_prepare() |
| and only sets the bit if we have actually changed any of the values. |
| |
| This avoids the setting of the update bit the second time we configure |
| the PWM to 0% dutycycle, this fixes the backlight coming up with 0% |
| duty-cycle after a suspend/resume. |
| |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| |
| drivers/pwm/pwm-lpss.c | 12 +++++++----- |
| 1 file changed, 7 insertions(+), 5 deletions(-) |
| |
| diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c |
| index ea93ef9f3672..2ac3a2aa9e53 100644 |
| |
| |
| @@ -88,7 +88,7 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, |
| unsigned long long on_time_div; |
| unsigned long c = lpwm->info->clk_rate, base_unit_range; |
| unsigned long long base_unit, freq = NSEC_PER_SEC; |
| - u32 ctrl; |
| + u32 orig_ctrl, ctrl; |
| |
| do_div(freq, period_ns); |
| |
| @@ -105,13 +105,17 @@ static void pwm_lpss_prepare(struct pwm_lpss_chip *lpwm, struct pwm_device *pwm, |
| do_div(on_time_div, period_ns); |
| on_time_div = 255ULL - on_time_div; |
| |
| - ctrl = pwm_lpss_read(pwm); |
| + orig_ctrl = ctrl = pwm_lpss_read(pwm); |
| ctrl &= ~PWM_ON_TIME_DIV_MASK; |
| ctrl &= ~(base_unit_range << PWM_BASE_UNIT_SHIFT); |
| base_unit &= base_unit_range; |
| ctrl |= (u32) base_unit << PWM_BASE_UNIT_SHIFT; |
| ctrl |= on_time_div; |
| - pwm_lpss_write(pwm, ctrl); |
| + |
| + if (orig_ctrl != ctrl) { |
| + pwm_lpss_write(pwm, ctrl); |
| + pwm_lpss_write(pwm, ctrl | PWM_SW_UPDATE); |
| + } |
| } |
| |
| static inline void pwm_lpss_cond_enable(struct pwm_device *pwm, bool cond) |
| @@ -135,7 +139,6 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, |
| return ret; |
| } |
| pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); |
| - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); |
| pwm_lpss_cond_enable(pwm, lpwm->info->bypass == false); |
| ret = pwm_lpss_wait_for_update(pwm); |
| if (ret) { |
| @@ -148,7 +151,6 @@ static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm, |
| if (ret) |
| return ret; |
| pwm_lpss_prepare(lpwm, pwm, state->duty_cycle, state->period); |
| - pwm_lpss_write(pwm, pwm_lpss_read(pwm) | PWM_SW_UPDATE); |
| return pwm_lpss_wait_for_update(pwm); |
| } |
| } else if (pwm_is_enabled(pwm)) { |
| -- |
| 2.19.1 |
| |