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

--- 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 },

 	{ },

 };

-- 

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

--- a/drivers/pwm/pwm-lpss.c

+++ b/drivers/pwm/pwm-lpss.c

@@ -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

--- a/drivers/pwm/pwm-lpss.h

+++ b/drivers/pwm/pwm-lpss.h

@@ -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

--- 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,

-- 

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

--- a/drivers/pwm/pwm-lpss.c

+++ b/drivers/pwm/pwm-lpss.c

@@ -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

--- a/drivers/pwm/pwm-lpss.c

+++ b/drivers/pwm/pwm-lpss.c

@@ -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

--- 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,

-- 

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

--- a/drivers/pwm/pwm-lpss.c

+++ b/drivers/pwm/pwm-lpss.c

@@ -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