diff mbox

drm/i915 : Restore PWM_GRANULARITY after resume

Message ID 1473329920-4449-1-git-send-email-shawn.c.lee@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lee, Shawn C Sept. 8, 2016, 10:18 a.m. UTC
From: "Lee, Shawn C" <shawn.c.lee@intel.com>

SPT_PWM_GRANULARITY (SOUTH_CHICKEN1, bit 0) controls the granularity
(minimum increment) of the PWM backlight control counter. PWM frequency
adjustment on 128 clock increments when this bit was 1. And 16 clock
increments when it was 0.

PWM frequency multiple octuple (from 200Hz to 1.6KHz) due to
SPT_PWM_GRANULARITY was clear to 0 after S3. This patch save
SOUTH_CHICKEN1 register value before suspend. And will restore
it after i915 resume.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97486

Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Cooper Chiou <cooper.chiou@intel.com>
Reviewed-by: Wei Shun Chen <wei.shun.chang@intel.com>
Reviewed-by: Gary C Wang <gary.c.wang@intel.com>
Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h   |    1 +
 drivers/gpu/drm/i915/intel_panel.c |   21 +++++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Jani Nikula Sept. 9, 2016, 7:46 a.m. UTC | #1
On Thu, 08 Sep 2016, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> From: "Lee, Shawn C" <shawn.c.lee@intel.com>
>
> SPT_PWM_GRANULARITY (SOUTH_CHICKEN1, bit 0) controls the granularity
> (minimum increment) of the PWM backlight control counter. PWM frequency
> adjustment on 128 clock increments when this bit was 1. And 16 clock
> increments when it was 0.
>
> PWM frequency multiple octuple (from 200Hz to 1.6KHz) due to
> SPT_PWM_GRANULARITY was clear to 0 after S3. This patch save
> SOUTH_CHICKEN1 register value before suspend. And will restore
> it after i915 resume.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97486
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Cooper Chiou <cooper.chiou@intel.com>
> Reviewed-by: Wei Shun Chen <wei.shun.chang@intel.com>
> Reviewed-by: Gary C Wang <gary.c.wang@intel.com>
> Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h   |    1 +
>  drivers/gpu/drm/i915/intel_panel.c |   21 +++++++++++++++++++--
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 570a7ca..4c28692 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -256,6 +256,7 @@ struct intel_panel {
>  		u32 level;
>  		u32 min;
>  		u32 max;
> +		u32 pwm_granularity;

None of the fields here store register information directly. Please
don't do so here either. Make this a bool. Call it
pwm_alternate_increment or something.

>  		bool enabled;
>  		bool combination_mode;	/* gen 2/4 only */
>  		bool active_low_pwm;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index c10e9b0..720fd5a 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -841,7 +841,17 @@ static void lpt_enable_backlight(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	u32 pch_ctl1, pch_ctl2;
> +	u32 pch_ctl1, pch_ctl2, mul;
> +
> +	if (HAS_PCH_LPT(dev_priv)) {
> +		mul = I915_READ(SOUTH_CHICKEN2);
> +		mul &= ~LPT_PWM_GRANULARITY;
> +		I915_WRITE(SOUTH_CHICKEN2, mul | panel->backlight.pwm_granularity);
> +	} else {
> +		mul = I915_READ(SOUTH_CHICKEN1);
> +		mul &= ~SPT_PWM_GRANULARITY;
> +		I915_WRITE(SOUTH_CHICKEN1, mul | panel->backlight.pwm_granularity);
> +	}

It's probably safer to modify the granularity after we've ensured the
backlight is disabled below.

>  
>  	pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
>  	if (pch_ctl1 & BLM_PCH_PWM_ENABLE) {
> @@ -1413,7 +1423,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
> -	u32 pch_ctl1, pch_ctl2, val;
> +	u32 pch_ctl1, pch_ctl2, val, mul;
>  
>  	pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
>  	panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
> @@ -1421,6 +1431,13 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  	pch_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
>  	panel->backlight.max = pch_ctl2 >> 16;
>  
> +	if (HAS_PCH_LPT(dev_priv))
> +		mul = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> +	else
> +		mul = I915_READ(SOUTH_CHICKEN1) & SPT_PWM_GRANULARITY;
> +
> +	panel->backlight.pwm_granularity = mul;

In the function, there's a sequence of figuring out the max backlight,
please don't put this stuff in the middle of it. You can just make this
the first thing.

As follow-up, you can then use the field in spt_hz_to_pwm and
lpt_hz_to_pwm instead of re-reading the chicken bits there.

> +
>  	if (!panel->backlight.max)
>  		panel->backlight.max = get_backlight_max_vbt(connector);
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 570a7ca..4c28692 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -256,6 +256,7 @@  struct intel_panel {
 		u32 level;
 		u32 min;
 		u32 max;
+		u32 pwm_granularity;
 		bool enabled;
 		bool combination_mode;	/* gen 2/4 only */
 		bool active_low_pwm;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index c10e9b0..720fd5a 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -841,7 +841,17 @@  static void lpt_enable_backlight(struct intel_connector *connector)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	u32 pch_ctl1, pch_ctl2;
+	u32 pch_ctl1, pch_ctl2, mul;
+
+	if (HAS_PCH_LPT(dev_priv)) {
+		mul = I915_READ(SOUTH_CHICKEN2);
+		mul &= ~LPT_PWM_GRANULARITY;
+		I915_WRITE(SOUTH_CHICKEN2, mul | panel->backlight.pwm_granularity);
+	} else {
+		mul = I915_READ(SOUTH_CHICKEN1);
+		mul &= ~SPT_PWM_GRANULARITY;
+		I915_WRITE(SOUTH_CHICKEN1, mul | panel->backlight.pwm_granularity);
+	}
 
 	pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
 	if (pch_ctl1 & BLM_PCH_PWM_ENABLE) {
@@ -1413,7 +1423,7 @@  static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
-	u32 pch_ctl1, pch_ctl2, val;
+	u32 pch_ctl1, pch_ctl2, val, mul;
 
 	pch_ctl1 = I915_READ(BLC_PWM_PCH_CTL1);
 	panel->backlight.active_low_pwm = pch_ctl1 & BLM_PCH_POLARITY;
@@ -1421,6 +1431,13 @@  static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
 	pch_ctl2 = I915_READ(BLC_PWM_PCH_CTL2);
 	panel->backlight.max = pch_ctl2 >> 16;
 
+	if (HAS_PCH_LPT(dev_priv))
+		mul = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
+	else
+		mul = I915_READ(SOUTH_CHICKEN1) & SPT_PWM_GRANULARITY;
+
+	panel->backlight.pwm_granularity = mul;
+
 	if (!panel->backlight.max)
 		panel->backlight.max = get_backlight_max_vbt(connector);