diff mbox

[2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes

Message ID 1430379455-21244-2-git-send-email-vandana.kannan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

vandana.kannan@intel.com April 30, 2015, 7:37 a.m. UTC 
BXT does not have PP_DIV register. Making changes to handle this.

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |  2 ++
 drivers/gpu/drm/i915/i915_suspend.c |  8 ++++--
 drivers/gpu/drm/i915/intel_dp.c     | 56 +++++++++++++++++++++++++++++--------
 3 files changed, 52 insertions(+), 14 deletions(-)

Comments

Jani Nikula April 30, 2015, 8:57 a.m. UTC | #1 
On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> BXT does not have PP_DIV register. Making changes to handle this.
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     |  2 ++
>  drivers/gpu/drm/i915/i915_suspend.c |  8 ++++--
>  drivers/gpu/drm/i915/intel_dp.c     | 56 +++++++++++++++++++++++++++++--------
>  3 files changed, 52 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 36805b6..580f5cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6315,6 +6315,8 @@ enum skl_disp_power_wells {
>  #define PCH_PP_CONTROL		0xc7204
>  #define  PANEL_UNLOCK_REGS	(0xabcd << 16)
>  #define  PANEL_UNLOCK_MASK	(0xffff << 16)
> +#define  BXT_POWER_CYCLE_DELAY_MASK	(0x1f0)
> +#define  BXT_POWER_CYCLE_DELAY_SHIFT	4
>  #define  EDP_FORCE_VDD		(1 << 3)
>  #define  EDP_BLC_ENABLE		(1 << 2)
>  #define  PANEL_POWER_RESET	(1 << 1)
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index e91d637..24e0b06 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -48,7 +48,9 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
>  		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
> -		dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
> +		if (!IS_BROXTON(dev))
> +			dev_priv->regfile.savePP_DIVISOR =
> +				I915_READ(PCH_PP_DIVISOR);
>  	} else if (!IS_VALLEYVIEW(dev)) {
>  		dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
>  		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
> @@ -82,7 +84,9 @@ static void i915_restore_display(struct drm_device *dev)
>  	if (!HAS_GMCH_DISPLAY(dev)) {
>  		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>  		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> -		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> +		if (!IS_BROXTON(dev))
> +			I915_WRITE(PCH_PP_DIVISOR,
> +					dev_priv->regfile.savePP_DIVISOR);

Again, no need to do this in i915_suspend.c.

>  		I915_WRITE(PCH_PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
>  	} else if (!IS_VALLEYVIEW(dev)) {
>  		I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 68e10c1..37514d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4954,8 +4954,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct edp_power_seq cur, vbt, spec,
>  		*final = &intel_dp->pps_delays;
> -	u32 pp_on, pp_off, pp_div, pp;
> -	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
> +	u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0, pp;
> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> @@ -4964,10 +4964,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  		return;
>  
>  	if (!HAS_GMCH_DISPLAY(dev)) {
> +		/*
> +		 * TODO: BXT has 2 sets of PPS registers.
> +		 * Correct Register for Broxton need to be identified
> +		 * using VBT. hardcoding for now
> +		 */

This comment really belongs in the previous patch, which would add the
if (IS_BROXTON(dev)) branch as I suggested.

>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> -		pp_div_reg = PCH_PP_DIVISOR;
> +		if (!IS_BROXTON(dev))
> +			pp_div_reg = PCH_PP_DIVISOR;

And thus this is naturally included in the previous patch as well.

The rest will probably need some updates - maybe it needs to be fully
incorporated into the previous patch.

>  	} else {
>  		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
>  
> @@ -4984,7 +4990,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  
>  	pp_on = I915_READ(pp_on_reg);
>  	pp_off = I915_READ(pp_off_reg);
> -	pp_div = I915_READ(pp_div_reg);
> +	if (!IS_BROXTON(dev))
> +		pp_div = I915_READ(pp_div_reg);
> +	else
> +		pp_ctl = I915_READ(pp_ctrl_reg);
>  
>  	/* Pull timing values out of registers */
>  	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
> @@ -4999,8 +5008,12 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	cur.t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
>  		PANEL_POWER_DOWN_DELAY_SHIFT;
>  
> -	cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
> -		       PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
> +	if (IS_BROXTON(dev))
> +		cur.t11_t12 = (((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
> +			BXT_POWER_CYCLE_DELAY_SHIFT) - 1) * 1000;
> +	else
> +		cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
> +			PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
>  
>  	DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
>  		      cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
> @@ -5057,16 +5070,22 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 pp_on, pp_off, pp_div, port_sel = 0;
>  	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
> -	int pp_on_reg, pp_off_reg, pp_div_reg;
> +	int pp_on_reg, pp_off_reg, pp_div_reg = 0, pp_ctrl_reg;
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	const struct edp_power_seq *seq = &intel_dp->pps_delays;
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
>  	if (!HAS_GMCH_DISPLAY(dev)) {
> +		/*
> +		 * TODO: Correct Register for Broxton need to be identified
> +		 * using VBT. hardcoding for now
> +		 */
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> -		pp_div_reg = PCH_PP_DIVISOR;
> +		pp_ctrl_reg = PCH_PP_CONTROL;
> +		if (!IS_BROXTON(dev))
> +			pp_div_reg = PCH_PP_DIVISOR;
>  	} else {
>  		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
>  
> @@ -5089,9 +5108,16 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  		 (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
>  	/* Compute the divisor for the pp clock, simply match the Bspec
>  	 * formula. */
> -	pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
> -	pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
> -			<< PANEL_POWER_CYCLE_DELAY_SHIFT);
> +	if (IS_BROXTON(dev)) {
> +		pp_div = I915_READ(pp_ctrl_reg);
> +		pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
> +		pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000)
> +				<< BXT_POWER_CYCLE_DELAY_SHIFT);
> +	} else {
> +		pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
> +		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
> +				<< PANEL_POWER_CYCLE_DELAY_SHIFT);
> +	}
>  
>  	/* Haswell doesn't have any port selection bits for the panel
>  	 * power sequencer any more. */
> @@ -5108,11 +5134,17 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  
>  	I915_WRITE(pp_on_reg, pp_on);
>  	I915_WRITE(pp_off_reg, pp_off);
> -	I915_WRITE(pp_div_reg, pp_div);
> +	if (IS_BROXTON(dev))
> +		I915_WRITE(pp_ctrl_reg, pp_div);
> +	else
> +		I915_WRITE(pp_div_reg, pp_div);
>  
>  	DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
>  		      I915_READ(pp_on_reg),
>  		      I915_READ(pp_off_reg),
> +		      IS_BROXTON(dev) ?
> +		      ((I915_READ(pp_ctrl_reg) && BXT_POWER_CYCLE_DELAY_MASK)
> +		       >> BXT_POWER_CYCLE_DELAY_SHIFT) :
>  		      I915_READ(pp_div_reg));
>  }
>  
> -- 
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
David Weinehall April 30, 2015, 9:27 a.m. UTC | #2 
On Thu, Apr 30, 2015 at 01:07:34PM +0530, Vandana Kannan wrote:
> BXT does not have PP_DIV register. Making changes to handle this.

[snip]

> +		if (!IS_BROXTON(dev))
> +			dev_priv->regfile.savePP_DIVISOR =
> +				I915_READ(PCH_PP_DIVISOR);

In case some future products also lack this register this will quickly
end up messy.  Maybe it'd be better to add a HAS_PP_DIVISOR(dev) macro
or something like that?


Kind regards, David Weinehall
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..580f5cb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6315,6 +6315,8 @@  enum skl_disp_power_wells {
 #define PCH_PP_CONTROL		0xc7204
 #define  PANEL_UNLOCK_REGS	(0xabcd << 16)
 #define  PANEL_UNLOCK_MASK	(0xffff << 16)
+#define  BXT_POWER_CYCLE_DELAY_MASK	(0x1f0)
+#define  BXT_POWER_CYCLE_DELAY_SHIFT	4
 #define  EDP_FORCE_VDD		(1 << 3)
 #define  EDP_BLC_ENABLE		(1 << 2)
 #define  PANEL_POWER_RESET	(1 << 1)
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index e91d637..24e0b06 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -48,7 +48,9 @@  static void i915_save_display(struct drm_device *dev)
 		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
 		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
 		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
-		dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
+		if (!IS_BROXTON(dev))
+			dev_priv->regfile.savePP_DIVISOR =
+				I915_READ(PCH_PP_DIVISOR);
 	} else if (!IS_VALLEYVIEW(dev)) {
 		dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
 		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
@@ -82,7 +84,9 @@  static void i915_restore_display(struct drm_device *dev)
 	if (!HAS_GMCH_DISPLAY(dev)) {
 		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
 		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
-		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
+		if (!IS_BROXTON(dev))
+			I915_WRITE(PCH_PP_DIVISOR,
+					dev_priv->regfile.savePP_DIVISOR);
 		I915_WRITE(PCH_PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
 	} else if (!IS_VALLEYVIEW(dev)) {
 		I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 68e10c1..37514d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4954,8 +4954,8 @@  intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct edp_power_seq cur, vbt, spec,
 		*final = &intel_dp->pps_delays;
-	u32 pp_on, pp_off, pp_div, pp;
-	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
+	u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0, pp;
+	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -4964,10 +4964,16 @@  intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 		return;
 
 	if (!HAS_GMCH_DISPLAY(dev)) {
+		/*
+		 * TODO: BXT has 2 sets of PPS registers.
+		 * Correct Register for Broxton need to be identified
+		 * using VBT. hardcoding for now
+		 */
 		pp_ctrl_reg = PCH_PP_CONTROL;
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
-		pp_div_reg = PCH_PP_DIVISOR;
+		if (!IS_BROXTON(dev))
+			pp_div_reg = PCH_PP_DIVISOR;
 	} else {
 		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
 
@@ -4984,7 +4990,10 @@  intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 
 	pp_on = I915_READ(pp_on_reg);
 	pp_off = I915_READ(pp_off_reg);
-	pp_div = I915_READ(pp_div_reg);
+	if (!IS_BROXTON(dev))
+		pp_div = I915_READ(pp_div_reg);
+	else
+		pp_ctl = I915_READ(pp_ctrl_reg);
 
 	/* Pull timing values out of registers */
 	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
@@ -4999,8 +5008,12 @@  intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	cur.t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
 		PANEL_POWER_DOWN_DELAY_SHIFT;
 
-	cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
-		       PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
+	if (IS_BROXTON(dev))
+		cur.t11_t12 = (((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
+			BXT_POWER_CYCLE_DELAY_SHIFT) - 1) * 1000;
+	else
+		cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
+			PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
 
 	DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
 		      cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
@@ -5057,16 +5070,22 @@  intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp_on, pp_off, pp_div, port_sel = 0;
 	int div = HAS_PCH_SPLIT(dev) ? intel_pch_rawclk(dev) : intel_hrawclk(dev);
-	int pp_on_reg, pp_off_reg, pp_div_reg;
+	int pp_on_reg, pp_off_reg, pp_div_reg = 0, pp_ctrl_reg;
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	const struct edp_power_seq *seq = &intel_dp->pps_delays;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
 	if (!HAS_GMCH_DISPLAY(dev)) {
+		/*
+		 * TODO: Correct Register for Broxton need to be identified
+		 * using VBT. hardcoding for now
+		 */
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
-		pp_div_reg = PCH_PP_DIVISOR;
+		pp_ctrl_reg = PCH_PP_CONTROL;
+		if (!IS_BROXTON(dev))
+			pp_div_reg = PCH_PP_DIVISOR;
 	} else {
 		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
 
@@ -5089,9 +5108,16 @@  intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 		 (seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
 	/* Compute the divisor for the pp clock, simply match the Bspec
 	 * formula. */
-	pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
-	pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
-			<< PANEL_POWER_CYCLE_DELAY_SHIFT);
+	if (IS_BROXTON(dev)) {
+		pp_div = I915_READ(pp_ctrl_reg);
+		pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
+		pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000)
+				<< BXT_POWER_CYCLE_DELAY_SHIFT);
+	} else {
+		pp_div = ((100 * div)/2 - 1) << PP_REFERENCE_DIVIDER_SHIFT;
+		pp_div |= (DIV_ROUND_UP(seq->t11_t12, 1000)
+				<< PANEL_POWER_CYCLE_DELAY_SHIFT);
+	}
 
 	/* Haswell doesn't have any port selection bits for the panel
 	 * power sequencer any more. */
@@ -5108,11 +5134,17 @@  intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 
 	I915_WRITE(pp_on_reg, pp_on);
 	I915_WRITE(pp_off_reg, pp_off);
-	I915_WRITE(pp_div_reg, pp_div);
+	if (IS_BROXTON(dev))
+		I915_WRITE(pp_ctrl_reg, pp_div);
+	else
+		I915_WRITE(pp_div_reg, pp_div);
 
 	DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
 		      I915_READ(pp_on_reg),
 		      I915_READ(pp_off_reg),
+		      IS_BROXTON(dev) ?
+		      ((I915_READ(pp_ctrl_reg) && BXT_POWER_CYCLE_DELAY_MASK)
+		       >> BXT_POWER_CYCLE_DELAY_SHIFT) :
 		      I915_READ(pp_div_reg));
 }