public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-13  9:43 ` [PATCH v4] " Vandana Kannan
@ 2015-05-13  9:22   ` Kannan, Vandana
  2015-06-03 12:01     ` Kannan, Vandana
  2015-05-14  9:13   ` shuang.he
  1 sibling, 1 reply; 10+ messages in thread
From: Kannan, Vandana @ 2015-05-13  9:22 UTC (permalink / raw)
  To: jani.nikula; +Cc: intel-gfx



On 5/13/2015 3:13 PM, Vandana Kannan wrote:
> Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
> registers for BXT.
> BXT does not have PP_DIV register. Making changes to handle this.
> Second set of PPS registers have been defined but will be used when VBT
> provides a selection between the 2 sets of registers.
>
> v2:
> [Jani] Added 2nd set of PPS registers and the macro
> Jani's review comments
> 	- remove reference in i915_suspend.c
> 	- Use BXT PP macro
> Squashing all PPS related patches into one.
>
> v3: Jani's review comments addressed
> 	- Use pp_ctl instead of pp
> 	- ironlake_get_pp_control() is not required for BXT
> 	- correct the use of && in the print statement
> 	- drop the shift in the print statement
>
> v4: Jani's comments
> 	- modify ironlake_get_pp_control() - dont set unlock key for bxt
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
>   drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
>   2 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1b31238..777c51a 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)
> @@ -6343,6 +6345,17 @@ enum skl_disp_power_wells {
>   #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
>   #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
>
> +/* BXT PPS changes - 2nd set of PPS registers */
> +#define _BXT_PP_STATUS2		0xc7300
> +#define _BXT_PP_CONTROL2 	0xc7304
> +#define _BXT_PP_ON_DELAYS2	0xc7308
> +#define _BXT_PP_OFF_DELAYS2	0xc730c
> +
> +#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
> +#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
> +#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
> +#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
> +
>   #define PCH_DP_B		0xe4100
>   #define PCH_DPB_AUX_CH_CTL	0xe4110
>   #define PCH_DPB_AUX_CH_DATA1	0xe4114
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bacdec5..c7432a2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -559,7 +559,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>   {
>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> -	if (HAS_PCH_SPLIT(dev))
> +	if (IS_BROXTON(dev))
> +		return BXT_PP_CONTROL(0);
> +	else if (HAS_PCH_SPLIT(dev))
>   		return PCH_PP_CONTROL;
>   	else
>   		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -569,7 +571,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>   {
>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> -	if (HAS_PCH_SPLIT(dev))
> +	if (IS_BROXTON(dev))
> +		return BXT_PP_STATUS(0);
> +	else if (HAS_PCH_SPLIT(dev))
>   		return PCH_PP_STATUS;
>   	else
>   		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -1681,8 +1685,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
>   	lockdep_assert_held(&dev_priv->pps_mutex);
>
>   	control = I915_READ(_pp_ctrl_reg(intel_dp));
> -	control &= ~PANEL_UNLOCK_MASK;
> -	control |= PANEL_UNLOCK_REGS;
> +	if (!IS_BROXTON(dev)) {
> +		control &= ~PANEL_UNLOCK_MASK;
> +		control |= PANEL_UNLOCK_REGS;
> +	}
>   	return control;
>   }
>
> @@ -4969,8 +4975,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;
> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>
>   	lockdep_assert_held(&dev_priv->pps_mutex);
>
> @@ -4978,7 +4984,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>   	if (final->t11_t12 != 0)
>   		return;
>
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> +	} else if (HAS_PCH_SPLIT(dev)) {
>   		pp_ctrl_reg = PCH_PP_CONTROL;
>   		pp_on_reg = PCH_PP_ON_DELAYS;
>   		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -4994,12 +5009,13 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>
>   	/* Workaround: Need to write PP_CONTROL with the unlock key as
>   	 * the very first thing. */
> -	pp = ironlake_get_pp_control(intel_dp);
> -	I915_WRITE(pp_ctrl_reg, pp);
> +	pp_ctl = ironlake_get_pp_control(intel_dp);
> +	I915_WRITE(pp_ctrl_reg, pp_ctl);
>
This write may not be required for BXT.

- Vandana
>   	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);
>
>   	/* Pull timing values out of registers */
>   	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
> @@ -5014,7 +5030,11 @@ 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) >>
> +	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",
> @@ -5072,13 +5092,23 @@ 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_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> +
> +	} else if (HAS_PCH_SPLIT(dev)) {
>   		pp_on_reg = PCH_PP_ON_DELAYS;
>   		pp_off_reg = PCH_PP_OFF_DELAYS;
>   		pp_div_reg = PCH_PP_DIVISOR;
> @@ -5104,9 +5134,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. */
> @@ -5123,11 +5160,16 @@ 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) :
>   		      I915_READ(pp_div_reg));
>   }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-11 14:34 [PATCH v3] " Kannan, Vandana
@ 2015-05-13  9:43 ` Vandana Kannan
  2015-05-13  9:22   ` Kannan, Vandana
  2015-05-14  9:13   ` shuang.he
  0 siblings, 2 replies; 10+ messages in thread
From: Vandana Kannan @ 2015-05-13  9:43 UTC (permalink / raw)
  To: intel-gfx

Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
registers for BXT.
BXT does not have PP_DIV register. Making changes to handle this.
Second set of PPS registers have been defined but will be used when VBT
provides a selection between the 2 sets of registers.

v2:
[Jani] Added 2nd set of PPS registers and the macro
Jani's review comments
	- remove reference in i915_suspend.c
	- Use BXT PP macro
Squashing all PPS related patches into one.

v3: Jani's review comments addressed
	- Use pp_ctl instead of pp
	- ironlake_get_pp_control() is not required for BXT
	- correct the use of && in the print statement
	- drop the shift in the print statement

v4: Jani's comments
	- modify ironlake_get_pp_control() - dont set unlock key for bxt

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
 drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
 2 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1b31238..777c51a 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)
@@ -6343,6 +6345,17 @@ enum skl_disp_power_wells {
 #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
 #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
 
+/* BXT PPS changes - 2nd set of PPS registers */
+#define _BXT_PP_STATUS2		0xc7300
+#define _BXT_PP_CONTROL2 	0xc7304
+#define _BXT_PP_ON_DELAYS2	0xc7308
+#define _BXT_PP_OFF_DELAYS2	0xc730c
+
+#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
+#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
+#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
+#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
+
 #define PCH_DP_B		0xe4100
 #define PCH_DPB_AUX_CH_CTL	0xe4110
 #define PCH_DPB_AUX_CH_DATA1	0xe4114
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bacdec5..c7432a2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -559,7 +559,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_BROXTON(dev))
+		return BXT_PP_CONTROL(0);
+	else if (HAS_PCH_SPLIT(dev))
 		return PCH_PP_CONTROL;
 	else
 		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
@@ -569,7 +571,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_BROXTON(dev))
+		return BXT_PP_STATUS(0);
+	else if (HAS_PCH_SPLIT(dev))
 		return PCH_PP_STATUS;
 	else
 		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
@@ -1681,8 +1685,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
 	control = I915_READ(_pp_ctrl_reg(intel_dp));
-	control &= ~PANEL_UNLOCK_MASK;
-	control |= PANEL_UNLOCK_REGS;
+	if (!IS_BROXTON(dev)) {
+		control &= ~PANEL_UNLOCK_MASK;
+		control |= PANEL_UNLOCK_REGS;
+	}
 	return control;
 }
 
@@ -4969,8 +4975,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;
+	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -4978,7 +4984,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	if (final->t11_t12 != 0)
 		return;
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
+		pp_on_reg = BXT_PP_ON_DELAYS(0);
+		pp_off_reg = BXT_PP_OFF_DELAYS(0);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		pp_ctrl_reg = PCH_PP_CONTROL;
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
@@ -4994,12 +5009,13 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 
 	/* Workaround: Need to write PP_CONTROL with the unlock key as
 	 * the very first thing. */
-	pp = ironlake_get_pp_control(intel_dp);
-	I915_WRITE(pp_ctrl_reg, pp);
+	pp_ctl = ironlake_get_pp_control(intel_dp);
+	I915_WRITE(pp_ctrl_reg, pp_ctl);
 
 	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);
 
 	/* Pull timing values out of registers */
 	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
@@ -5014,7 +5030,11 @@ 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) >>
+	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",
@@ -5072,13 +5092,23 @@ 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_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
+		pp_on_reg = BXT_PP_ON_DELAYS(0);
+		pp_off_reg = BXT_PP_OFF_DELAYS(0);
+
+	} else if (HAS_PCH_SPLIT(dev)) {
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
 		pp_div_reg = PCH_PP_DIVISOR;
@@ -5104,9 +5134,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. */
@@ -5123,11 +5160,16 @@ 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) :
 		      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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-13  9:43 ` [PATCH v4] " Vandana Kannan
  2015-05-13  9:22   ` Kannan, Vandana
@ 2015-05-14  9:13   ` shuang.he
  1 sibling, 0 replies; 10+ messages in thread
From: shuang.he @ 2015-05-14  9:13 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, vandana.kannan

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6406
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  234/234              234/234
ILK                                  262/262              262/262
SNB                 -1              282/282              281/282
IVB                 -1              300/300              299/300
BYT                                  254/254              254/254
BDW                 -1              275/275              274/275
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(2)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
 IVB  igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip      FAIL(2)PASS(1)      FAIL(1)
*BDW  igt@gem_dummy_reloc_loop@bsd      PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane
assertion_failure@assertion failure
WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-13  9:22   ` Kannan, Vandana
@ 2015-06-03 12:01     ` Kannan, Vandana
  0 siblings, 0 replies; 10+ messages in thread
From: Kannan, Vandana @ 2015-06-03 12:01 UTC (permalink / raw)
  To: intel-gfx@lists.freedesktop.org

Please help review this patch.

- Vandana

On 5/13/2015 2:52 PM, Kannan, Vandana wrote:
>
>
> On 5/13/2015 3:13 PM, Vandana Kannan wrote:
>> Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
>> registers for BXT.
>> BXT does not have PP_DIV register. Making changes to handle this.
>> Second set of PPS registers have been defined but will be used when VBT
>> provides a selection between the 2 sets of registers.
>>
>> v2:
>> [Jani] Added 2nd set of PPS registers and the macro
>> Jani's review comments
>> 	- remove reference in i915_suspend.c
>> 	- Use BXT PP macro
>> Squashing all PPS related patches into one.
>>
>> v3: Jani's review comments addressed
>> 	- Use pp_ctl instead of pp
>> 	- ironlake_get_pp_control() is not required for BXT
>> 	- correct the use of && in the print statement
>> 	- drop the shift in the print statement
>>
>> v4: Jani's comments
>> 	- modify ironlake_get_pp_control() - dont set unlock key for bxt
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>> ---
>>    drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
>>    drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
>>    2 files changed, 72 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 1b31238..777c51a 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)
>> @@ -6343,6 +6345,17 @@ enum skl_disp_power_wells {
>>    #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
>>    #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
>>
>> +/* BXT PPS changes - 2nd set of PPS registers */
>> +#define _BXT_PP_STATUS2		0xc7300
>> +#define _BXT_PP_CONTROL2 	0xc7304
>> +#define _BXT_PP_ON_DELAYS2	0xc7308
>> +#define _BXT_PP_OFF_DELAYS2	0xc730c
>> +
>> +#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
>> +#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
>> +#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
>> +#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
>> +
>>    #define PCH_DP_B		0xe4100
>>    #define PCH_DPB_AUX_CH_CTL	0xe4110
>>    #define PCH_DPB_AUX_CH_DATA1	0xe4114
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index bacdec5..c7432a2 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -559,7 +559,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>>    {
>>    	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>
>> -	if (HAS_PCH_SPLIT(dev))
>> +	if (IS_BROXTON(dev))
>> +		return BXT_PP_CONTROL(0);
>> +	else if (HAS_PCH_SPLIT(dev))
>>    		return PCH_PP_CONTROL;
>>    	else
>>    		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
>> @@ -569,7 +571,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>>    {
>>    	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>
>> -	if (HAS_PCH_SPLIT(dev))
>> +	if (IS_BROXTON(dev))
>> +		return BXT_PP_STATUS(0);
>> +	else if (HAS_PCH_SPLIT(dev))
>>    		return PCH_PP_STATUS;
>>    	else
>>    		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> @@ -1681,8 +1685,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
>>    	lockdep_assert_held(&dev_priv->pps_mutex);
>>
>>    	control = I915_READ(_pp_ctrl_reg(intel_dp));
>> -	control &= ~PANEL_UNLOCK_MASK;
>> -	control |= PANEL_UNLOCK_REGS;
>> +	if (!IS_BROXTON(dev)) {
>> +		control &= ~PANEL_UNLOCK_MASK;
>> +		control |= PANEL_UNLOCK_REGS;
>> +	}
>>    	return control;
>>    }
>>
>> @@ -4969,8 +4975,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;
>> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>>
>>    	lockdep_assert_held(&dev_priv->pps_mutex);
>>
>> @@ -4978,7 +4984,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>    	if (final->t11_t12 != 0)
>>    		return;
>>
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
>> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
>> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
>> +	} else if (HAS_PCH_SPLIT(dev)) {
>>    		pp_ctrl_reg = PCH_PP_CONTROL;
>>    		pp_on_reg = PCH_PP_ON_DELAYS;
>>    		pp_off_reg = PCH_PP_OFF_DELAYS;
>> @@ -4994,12 +5009,13 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>
>>    	/* Workaround: Need to write PP_CONTROL with the unlock key as
>>    	 * the very first thing. */
>> -	pp = ironlake_get_pp_control(intel_dp);
>> -	I915_WRITE(pp_ctrl_reg, pp);
>> +	pp_ctl = ironlake_get_pp_control(intel_dp);
>> +	I915_WRITE(pp_ctrl_reg, pp_ctl);
>>
> This write may not be required for BXT.
>
> - Vandana
>>    	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);
>>
>>    	/* Pull timing values out of registers */
>>    	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
>> @@ -5014,7 +5030,11 @@ 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) >>
>> +	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",
>> @@ -5072,13 +5092,23 @@ 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_PCH_SPLIT(dev)) {
>> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
>> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
>> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
>> +
>> +	} else if (HAS_PCH_SPLIT(dev)) {
>>    		pp_on_reg = PCH_PP_ON_DELAYS;
>>    		pp_off_reg = PCH_PP_OFF_DELAYS;
>>    		pp_div_reg = PCH_PP_DIVISOR;
>> @@ -5104,9 +5134,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. */
>> @@ -5123,11 +5160,16 @@ 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) :
>>    		      I915_READ(pp_div_reg));
>>    }
>>
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
@ 2015-06-12 10:27 Vandana Kannan
  2015-06-17  9:15 ` Jindal, Sonika
  0 siblings, 1 reply; 10+ messages in thread
From: Vandana Kannan @ 2015-06-12 10:27 UTC (permalink / raw)
  To: intel-gfx

Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
registers for BXT.
BXT does not have PP_DIV register. Making changes to handle this.
Second set of PPS registers have been defined but will be used when VBT
provides a selection between the 2 sets of registers.

v2:
[Jani] Added 2nd set of PPS registers and the macro
Jani's review comments
	- remove reference in i915_suspend.c
	- Use BXT PP macro
Squashing all PPS related patches into one.

v3: Jani's review comments addressed
	- Use pp_ctl instead of pp
	- ironlake_get_pp_control() is not required for BXT
	- correct the use of && in the print statement
	- drop the shift in the print statement

v4: Jani's comments
	- modify ironlake_get_pp_control() - dont set unlock key for bxt

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
 drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
 2 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7213224..cc03b5b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6377,6 +6377,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)
@@ -6405,6 +6407,17 @@ enum skl_disp_power_wells {
 #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
 #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
 
+/* BXT PPS changes - 2nd set of PPS registers */
+#define _BXT_PP_STATUS2		0xc7300
+#define _BXT_PP_CONTROL2 	0xc7304
+#define _BXT_PP_ON_DELAYS2	0xc7308
+#define _BXT_PP_OFF_DELAYS2	0xc730c
+
+#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
+#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
+#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
+#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
+
 #define PCH_DP_B		0xe4100
 #define PCH_DPB_AUX_CH_CTL	0xe4110
 #define PCH_DPB_AUX_CH_DATA1	0xe4114
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 15e8892..ad9cb0e9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_BROXTON(dev))
+		return BXT_PP_CONTROL(0);
+	else if (HAS_PCH_SPLIT(dev))
 		return PCH_PP_CONTROL;
 	else
 		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
@@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_BROXTON(dev))
+		return BXT_PP_STATUS(0);
+	else if (HAS_PCH_SPLIT(dev))
 		return PCH_PP_STATUS;
 	else
 		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
@@ -1701,8 +1705,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
 	control = I915_READ(_pp_ctrl_reg(intel_dp));
-	control &= ~PANEL_UNLOCK_MASK;
-	control |= PANEL_UNLOCK_REGS;
+	if (!IS_BROXTON(dev)) {
+		control &= ~PANEL_UNLOCK_MASK;
+		control |= PANEL_UNLOCK_REGS;
+	}
 	return control;
 }
 
@@ -5091,8 +5097,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;
+	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -5100,7 +5106,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	if (final->t11_t12 != 0)
 		return;
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
+		pp_on_reg = BXT_PP_ON_DELAYS(0);
+		pp_off_reg = BXT_PP_OFF_DELAYS(0);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		pp_ctrl_reg = PCH_PP_CONTROL;
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
@@ -5116,12 +5131,13 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 
 	/* Workaround: Need to write PP_CONTROL with the unlock key as
 	 * the very first thing. */
-	pp = ironlake_get_pp_control(intel_dp);
-	I915_WRITE(pp_ctrl_reg, pp);
+	pp_ctl = ironlake_get_pp_control(intel_dp);
+	I915_WRITE(pp_ctrl_reg, pp_ctl);
 
 	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);
 
 	/* Pull timing values out of registers */
 	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
@@ -5136,7 +5152,11 @@ 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) >>
+	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",
@@ -5194,13 +5214,23 @@ 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_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
+		pp_on_reg = BXT_PP_ON_DELAYS(0);
+		pp_off_reg = BXT_PP_OFF_DELAYS(0);
+
+	} else if (HAS_PCH_SPLIT(dev)) {
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
 		pp_div_reg = PCH_PP_DIVISOR;
@@ -5226,9 +5256,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. */
@@ -5245,11 +5282,16 @@ 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) :
 		      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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-06-12 10:27 [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing Vandana Kannan
@ 2015-06-17  9:15 ` Jindal, Sonika
  2015-06-17  9:22   ` Kannan, Vandana
  0 siblings, 1 reply; 10+ messages in thread
From: Jindal, Sonika @ 2015-06-17  9:15 UTC (permalink / raw)
  To: Vandana Kannan, intel-gfx@lists.freedesktop.org



On 6/12/2015 3:57 PM, Vandana Kannan wrote:
> Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
> registers for BXT.
> BXT does not have PP_DIV register. Making changes to handle this.
> Second set of PPS registers have been defined but will be used when VBT
> provides a selection between the 2 sets of registers.
>
> v2:
> [Jani] Added 2nd set of PPS registers and the macro
> Jani's review comments
> 	- remove reference in i915_suspend.c
> 	- Use BXT PP macro
> Squashing all PPS related patches into one.
>
> v3: Jani's review comments addressed
> 	- Use pp_ctl instead of pp
> 	- ironlake_get_pp_control() is not required for BXT
> 	- correct the use of && in the print statement
> 	- drop the shift in the print statement
>
> v4: Jani's comments
> 	- modify ironlake_get_pp_control() - dont set unlock key for bxt
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
>   drivers/gpu/drm/i915/intel_dp.c | 76 ++++++++++++++++++++++++++++++++---------
>   2 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7213224..cc03b5b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6377,6 +6377,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)
> @@ -6405,6 +6407,17 @@ enum skl_disp_power_wells {
>   #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
>   #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
>
> +/* BXT PPS changes - 2nd set of PPS registers */
> +#define _BXT_PP_STATUS2		0xc7300
Not aligned?

> +#define _BXT_PP_CONTROL2 	0xc7304
> +#define _BXT_PP_ON_DELAYS2	0xc7308
> +#define _BXT_PP_OFF_DELAYS2	0xc730c
> +
> +#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
> +#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
> +#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
> +#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
> +
>   #define PCH_DP_B		0xe4100
>   #define PCH_DPB_AUX_CH_CTL	0xe4110
>   #define PCH_DPB_AUX_CH_DATA1	0xe4114
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 15e8892..ad9cb0e9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>   {
>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> -	if (HAS_PCH_SPLIT(dev))
> +	if (IS_BROXTON(dev))
> +		return BXT_PP_CONTROL(0);
> +	else if (HAS_PCH_SPLIT(dev))
>   		return PCH_PP_CONTROL;
>   	else
>   		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>   {
>   	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> -	if (HAS_PCH_SPLIT(dev))
> +	if (IS_BROXTON(dev))
> +		return BXT_PP_STATUS(0);
> +	else if (HAS_PCH_SPLIT(dev))
>   		return PCH_PP_STATUS;
>   	else
>   		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -1701,8 +1705,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
>   	lockdep_assert_held(&dev_priv->pps_mutex);
>
>   	control = I915_READ(_pp_ctrl_reg(intel_dp));
> -	control &= ~PANEL_UNLOCK_MASK;
> -	control |= PANEL_UNLOCK_REGS;
> +	if (!IS_BROXTON(dev)) {
> +		control &= ~PANEL_UNLOCK_MASK;
> +		control |= PANEL_UNLOCK_REGS;
> +	}
>   	return control;
>   }
>
> @@ -5091,8 +5097,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;
> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>
>   	lockdep_assert_held(&dev_priv->pps_mutex);
>
> @@ -5100,7 +5106,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>   	if (final->t11_t12 != 0)
>   		return;
>
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> +	} else if (HAS_PCH_SPLIT(dev)) {
>   		pp_ctrl_reg = PCH_PP_CONTROL;
>   		pp_on_reg = PCH_PP_ON_DELAYS;
>   		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5116,12 +5131,13 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>
>   	/* Workaround: Need to write PP_CONTROL with the unlock key as
>   	 * the very first thing. */
> -	pp = ironlake_get_pp_control(intel_dp);
> -	I915_WRITE(pp_ctrl_reg, pp);
> +	pp_ctl = ironlake_get_pp_control(intel_dp);
> +	I915_WRITE(pp_ctrl_reg, pp_ctl);
Since you are not doing anything in ironlake_get_pp_control, you can 
move this write to !IS_BROXTON below

>
>   	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);
>
>   	/* Pull timing values out of registers */
>   	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
> @@ -5136,7 +5152,11 @@ 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) >>
> +	if (IS_BROXTON(dev))
> +		cur.t11_t12 = (((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
> +			BXT_POWER_CYCLE_DELAY_SHIFT) - 1) * 1000;
Why a -1 here for broxton and not for other platforms?
Also, since 0 is also a valid value and cur.t11_t12 is u16, this -1 will 
result in undefined value.

Regards,
Sonika
> +	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",
> @@ -5194,13 +5214,23 @@ 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_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> +
> +	} else if (HAS_PCH_SPLIT(dev)) {
>   		pp_on_reg = PCH_PP_ON_DELAYS;
>   		pp_off_reg = PCH_PP_OFF_DELAYS;
>   		pp_div_reg = PCH_PP_DIVISOR;
> @@ -5226,9 +5256,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. */
> @@ -5245,11 +5282,16 @@ 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) :
>   		      I915_READ(pp_div_reg));
>   }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-06-17  9:15 ` Jindal, Sonika
@ 2015-06-17  9:22   ` Kannan, Vandana
  2015-06-18  5:30     ` [PATCH v5] " Vandana Kannan
  0 siblings, 1 reply; 10+ messages in thread
From: Kannan, Vandana @ 2015-06-17  9:22 UTC (permalink / raw)
  To: Jindal, Sonika, intel-gfx@lists.freedesktop.org



On 6/17/2015 2:45 PM, Jindal, Sonika wrote:
>
>
> On 6/12/2015 3:57 PM, Vandana Kannan wrote:
>> Changes for BXT - added a IS_BROXTON check to use the macro related to
>> PPS
>> registers for BXT.
>> BXT does not have PP_DIV register. Making changes to handle this.
>> Second set of PPS registers have been defined but will be used when VBT
>> provides a selection between the 2 sets of registers.
>>
>> v2:
>> [Jani] Added 2nd set of PPS registers and the macro
>> Jani's review comments
>>     - remove reference in i915_suspend.c
>>     - Use BXT PP macro
>> Squashing all PPS related patches into one.
>>
>> v3: Jani's review comments addressed
>>     - Use pp_ctl instead of pp
>>     - ironlake_get_pp_control() is not required for BXT
>>     - correct the use of && in the print statement
>>     - drop the shift in the print statement
>>
>> v4: Jani's comments
>>     - modify ironlake_get_pp_control() - dont set unlock key for bxt
>>
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
>>   drivers/gpu/drm/i915/intel_dp.c | 76
>> ++++++++++++++++++++++++++++++++---------
>>   2 files changed, 72 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 7213224..cc03b5b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6377,6 +6377,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)
>> @@ -6405,6 +6407,17 @@ enum skl_disp_power_wells {
>>   #define  PANEL_POWER_CYCLE_DELAY_MASK    (0x1f)
>>   #define  PANEL_POWER_CYCLE_DELAY_SHIFT    0
>>
>> +/* BXT PPS changes - 2nd set of PPS registers */
>> +#define _BXT_PP_STATUS2        0xc7300
> Not aligned?
>
>> +#define _BXT_PP_CONTROL2     0xc7304
>> +#define _BXT_PP_ON_DELAYS2    0xc7308
>> +#define _BXT_PP_OFF_DELAYS2    0xc730c
>> +
>> +#define BXT_PP_STATUS(n)    ((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
>> +#define BXT_PP_CONTROL(n)    ((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
>> +#define BXT_PP_ON_DELAYS(n)    ((!n) ? PCH_PP_ON_DELAYS :
>> _BXT_PP_ON_DELAYS2)
>> +#define BXT_PP_OFF_DELAYS(n)    ((!n) ? PCH_PP_OFF_DELAYS :
>> _BXT_PP_OFF_DELAYS2)
>> +
>>   #define PCH_DP_B        0xe4100
>>   #define PCH_DPB_AUX_CH_CTL    0xe4110
>>   #define PCH_DPB_AUX_CH_DATA1    0xe4114
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 15e8892..ad9cb0e9 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
>>   {
>>       struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>
>> -    if (HAS_PCH_SPLIT(dev))
>> +    if (IS_BROXTON(dev))
>> +        return BXT_PP_CONTROL(0);
>> +    else if (HAS_PCH_SPLIT(dev))
>>           return PCH_PP_CONTROL;
>>       else
>>           return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
>> @@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>>   {
>>       struct drm_device *dev = intel_dp_to_dev(intel_dp);
>>
>> -    if (HAS_PCH_SPLIT(dev))
>> +    if (IS_BROXTON(dev))
>> +        return BXT_PP_STATUS(0);
>> +    else if (HAS_PCH_SPLIT(dev))
>>           return PCH_PP_STATUS;
>>       else
>>           return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> @@ -1701,8 +1705,10 @@ static  u32 ironlake_get_pp_control(struct
>> intel_dp *intel_dp)
>>       lockdep_assert_held(&dev_priv->pps_mutex);
>>
>>       control = I915_READ(_pp_ctrl_reg(intel_dp));
>> -    control &= ~PANEL_UNLOCK_MASK;
>> -    control |= PANEL_UNLOCK_REGS;
>> +    if (!IS_BROXTON(dev)) {
>> +        control &= ~PANEL_UNLOCK_MASK;
>> +        control |= PANEL_UNLOCK_REGS;
>> +    }
>>       return control;
>>   }
>>
>> @@ -5091,8 +5097,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;
>> +    int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
>>
>>       lockdep_assert_held(&dev_priv->pps_mutex);
>>
>> @@ -5100,7 +5106,16 @@ intel_dp_init_panel_power_sequencer(struct
>> drm_device *dev,
>>       if (final->t11_t12 != 0)
>>           return;
>>
>> -    if (HAS_PCH_SPLIT(dev)) {
>> +    if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
>> +        pp_on_reg = BXT_PP_ON_DELAYS(0);
>> +        pp_off_reg = BXT_PP_OFF_DELAYS(0);
>> +    } else if (HAS_PCH_SPLIT(dev)) {
>>           pp_ctrl_reg = PCH_PP_CONTROL;
>>           pp_on_reg = PCH_PP_ON_DELAYS;
>>           pp_off_reg = PCH_PP_OFF_DELAYS;
>> @@ -5116,12 +5131,13 @@ intel_dp_init_panel_power_sequencer(struct
>> drm_device *dev,
>>
>>       /* Workaround: Need to write PP_CONTROL with the unlock key as
>>        * the very first thing. */
>> -    pp = ironlake_get_pp_control(intel_dp);
>> -    I915_WRITE(pp_ctrl_reg, pp);
>> +    pp_ctl = ironlake_get_pp_control(intel_dp);
>> +    I915_WRITE(pp_ctrl_reg, pp_ctl);
> Since you are not doing anything in ironlake_get_pp_control, you can
> move this write to !IS_BROXTON below
>
Agreed.
>>
>>       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);
>>
>>       /* Pull timing values out of registers */
>>       cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
>> @@ -5136,7 +5152,11 @@ 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) >>
>> +    if (IS_BROXTON(dev))
>> +        cur.t11_t12 = (((pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
>> +            BXT_POWER_CYCLE_DELAY_SHIFT) - 1) * 1000;
> Why a -1 here for broxton and not for other platforms?
According to the BSpec, "The value should be programmed to (desired 
delay / 100 milliseconds) + 1". This -1 is to make up for that. This is 
mentioned for other platforms as well. So maybe we should make the 
change there as well ??
> Also, since 0 is also a valid value and cur.t11_t12 is u16, this -1 will
> result in undefined value.
Agreed. Making the change.

- Vandana
>
> Regards,
> Sonika
>> +    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",
>> @@ -5194,13 +5214,23 @@
>> 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_PCH_SPLIT(dev)) {
>> +    if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
>> +        pp_on_reg = BXT_PP_ON_DELAYS(0);
>> +        pp_off_reg = BXT_PP_OFF_DELAYS(0);
>> +
>> +    } else if (HAS_PCH_SPLIT(dev)) {
>>           pp_on_reg = PCH_PP_ON_DELAYS;
>>           pp_off_reg = PCH_PP_OFF_DELAYS;
>>           pp_div_reg = PCH_PP_DIVISOR;
>> @@ -5226,9 +5256,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. */
>> @@ -5245,11 +5282,16 @@
>> 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) :
>>                 I915_READ(pp_div_reg));
>>   }
>>
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
  2015-06-17  9:22   ` Kannan, Vandana
@ 2015-06-18  5:30     ` Vandana Kannan
  2015-06-18  6:08       ` Jindal, Sonika
  0 siblings, 1 reply; 10+ messages in thread
From: Vandana Kannan @ 2015-06-18  5:30 UTC (permalink / raw)
  To: intel-gfx

Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
registers for BXT.
BXT does not have PP_DIV register. Making changes to handle this.
Second set of PPS registers have been defined but will be used when VBT
provides a selection between the 2 sets of registers.

v2:
[Jani] Added 2nd set of PPS registers and the macro
Jani's review comments
	- remove reference in i915_suspend.c
	- Use BXT PP macro
Squashing all PPS related patches into one.

v3: Jani's review comments addressed
	- Use pp_ctl instead of pp
	- ironlake_get_pp_control() is not required for BXT
	- correct the use of && in the print statement
	- drop the shift in the print statement

v4: Jani's comments
	- modify ironlake_get_pp_control() - dont set unlock key for bxt

v5: Sonika's comments addressed
	- check alignment
	- move pp_ctrl_reg write (after ironlake_get_pp_control())
	to !IS_BROXTON case.
	- check before subtracting 1 for t11_t12

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
 drivers/gpu/drm/i915/intel_dp.c | 82 ++++++++++++++++++++++++++++++++---------
 2 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0b979ad..27eb49e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6391,6 +6391,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)
@@ -6419,6 +6421,17 @@ enum skl_disp_power_wells {
 #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
 #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
 
+/* BXT PPS changes - 2nd set of PPS registers */
+#define _BXT_PP_STATUS2 	0xc7300
+#define _BXT_PP_CONTROL2 	0xc7304
+#define _BXT_PP_ON_DELAYS2	0xc7308
+#define _BXT_PP_OFF_DELAYS2	0xc730c
+
+#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
+#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
+#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
+#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
+
 #define PCH_DP_B		0xe4100
 #define PCH_DPB_AUX_CH_CTL	0xe4110
 #define PCH_DPB_AUX_CH_DATA1	0xe4114
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f52eef1..c2145e7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_BROXTON(dev))
+		return BXT_PP_CONTROL(0);
+	else if (HAS_PCH_SPLIT(dev))
 		return PCH_PP_CONTROL;
 	else
 		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
@@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_BROXTON(dev))
+		return BXT_PP_STATUS(0);
+	else if (HAS_PCH_SPLIT(dev))
 		return PCH_PP_STATUS;
 	else
 		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
@@ -1702,8 +1706,10 @@ static  u32 ironlake_get_pp_control(struct intel_dp *intel_dp)
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
 	control = I915_READ(_pp_ctrl_reg(intel_dp));
-	control &= ~PANEL_UNLOCK_MASK;
-	control |= PANEL_UNLOCK_REGS;
+	if (!IS_BROXTON(dev)) {
+		control &= ~PANEL_UNLOCK_MASK;
+		control |= PANEL_UNLOCK_REGS;
+	}
 	return control;
 }
 
@@ -5092,8 +5098,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;
+	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -5101,7 +5107,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	if (final->t11_t12 != 0)
 		return;
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
+		pp_on_reg = BXT_PP_ON_DELAYS(0);
+		pp_off_reg = BXT_PP_OFF_DELAYS(0);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		pp_ctrl_reg = PCH_PP_CONTROL;
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
@@ -5117,12 +5132,14 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 
 	/* Workaround: Need to write PP_CONTROL with the unlock key as
 	 * the very first thing. */
-	pp = ironlake_get_pp_control(intel_dp);
-	I915_WRITE(pp_ctrl_reg, pp);
+	pp_ctl = ironlake_get_pp_control(intel_dp);
 
 	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)) {
+		I915_WRITE(pp_ctrl_reg, pp_ctl);
+		pp_div = I915_READ(pp_div_reg);
+	}
 
 	/* Pull timing values out of registers */
 	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
@@ -5137,8 +5154,17 @@ 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) >>
+	if (IS_BROXTON(dev)) {
+		u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
+			BXT_POWER_CYCLE_DELAY_SHIFT;
+		if (tmp > 0)
+			cur.t11_t12 = (tmp - 1) * 1000;
+		else
+			cur.t11_t12 = 0;
+	} 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);
@@ -5195,13 +5221,23 @@ 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_PCH_SPLIT(dev)) {
+	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
+		pp_on_reg = BXT_PP_ON_DELAYS(0);
+		pp_off_reg = BXT_PP_OFF_DELAYS(0);
+
+	} else if (HAS_PCH_SPLIT(dev)) {
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
 		pp_div_reg = PCH_PP_DIVISOR;
@@ -5227,9 +5263,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. */
@@ -5246,11 +5289,16 @@ 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) :
 		      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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
  2015-06-18  5:30     ` [PATCH v5] " Vandana Kannan
@ 2015-06-18  6:08       ` Jindal, Sonika
  2015-06-18  6:48         ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Jindal, Sonika @ 2015-06-18  6:08 UTC (permalink / raw)
  To: Kannan, Vandana, intel-gfx@lists.freedesktop.org

Looks good to me.
Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>

> -----Original Message-----
> From: Kannan, Vandana
> Sent: Thursday, June 18, 2015 11:01 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Jindal, Sonika; Kannan, Vandana
> Subject: [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
> 
> Changes for BXT - added a IS_BROXTON check to use the macro related to
> PPS registers for BXT.
> BXT does not have PP_DIV register. Making changes to handle this.
> Second set of PPS registers have been defined but will be used when VBT
> provides a selection between the 2 sets of registers.
> 
> v2:
> [Jani] Added 2nd set of PPS registers and the macro Jani's review comments
> 	- remove reference in i915_suspend.c
> 	- Use BXT PP macro
> Squashing all PPS related patches into one.
> 
> v3: Jani's review comments addressed
> 	- Use pp_ctl instead of pp
> 	- ironlake_get_pp_control() is not required for BXT
> 	- correct the use of && in the print statement
> 	- drop the shift in the print statement
> 
> v4: Jani's comments
> 	- modify ironlake_get_pp_control() - dont set unlock key for bxt
> 
> v5: Sonika's comments addressed
> 	- check alignment
> 	- move pp_ctrl_reg write (after ironlake_get_pp_control())
> 	to !IS_BROXTON case.
> 	- check before subtracting 1 for t11_t12
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
> drivers/gpu/drm/i915/intel_dp.c | 82
> ++++++++++++++++++++++++++++++++---------
>  2 files changed, 78 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 0b979ad..27eb49e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6391,6 +6391,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)
> @@ -6419,6 +6421,17 @@ enum skl_disp_power_wells {
>  #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
>  #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
> 
> +/* BXT PPS changes - 2nd set of PPS registers */
> +#define _BXT_PP_STATUS2 	0xc7300
> +#define _BXT_PP_CONTROL2 	0xc7304
> +#define _BXT_PP_ON_DELAYS2	0xc7308
> +#define _BXT_PP_OFF_DELAYS2	0xc730c
> +
> +#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
> +#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL :
> _BXT_PP_CONTROL2)
> +#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS :
> _BXT_PP_ON_DELAYS2)
> +#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS :
> _BXT_PP_OFF_DELAYS2)
> +
>  #define PCH_DP_B		0xe4100
>  #define PCH_DPB_AUX_CH_CTL	0xe4110
>  #define PCH_DPB_AUX_CH_DATA1	0xe4114
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index f52eef1..c2145e7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> 
> -	if (HAS_PCH_SPLIT(dev))
> +	if (IS_BROXTON(dev))
> +		return BXT_PP_CONTROL(0);
> +	else if (HAS_PCH_SPLIT(dev))
>  		return PCH_PP_CONTROL;
>  	else
>  		return
> VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)  {
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> 
> -	if (HAS_PCH_SPLIT(dev))
> +	if (IS_BROXTON(dev))
> +		return BXT_PP_STATUS(0);
> +	else if (HAS_PCH_SPLIT(dev))
>  		return PCH_PP_STATUS;
>  	else
>  		return
> VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -1702,8 +1706,10 @@ static  u32 ironlake_get_pp_control(struct
> intel_dp *intel_dp)
>  	lockdep_assert_held(&dev_priv->pps_mutex);
> 
>  	control = I915_READ(_pp_ctrl_reg(intel_dp));
> -	control &= ~PANEL_UNLOCK_MASK;
> -	control |= PANEL_UNLOCK_REGS;
> +	if (!IS_BROXTON(dev)) {
> +		control &= ~PANEL_UNLOCK_MASK;
> +		control |= PANEL_UNLOCK_REGS;
> +	}
>  	return control;
>  }
> 
> @@ -5092,8 +5098,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;
> +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
> 
>  	lockdep_assert_held(&dev_priv->pps_mutex);
> 
> @@ -5101,7 +5107,16 @@ intel_dp_init_panel_power_sequencer(struct
> drm_device *dev,
>  	if (final->t11_t12 != 0)
>  		return;
> 
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> +	} else if (HAS_PCH_SPLIT(dev)) {
>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5117,12 +5132,14 @@ intel_dp_init_panel_power_sequencer(struct
> drm_device *dev,
> 
>  	/* Workaround: Need to write PP_CONTROL with the unlock key as
>  	 * the very first thing. */
> -	pp = ironlake_get_pp_control(intel_dp);
> -	I915_WRITE(pp_ctrl_reg, pp);
> +	pp_ctl = ironlake_get_pp_control(intel_dp);
> 
>  	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)) {
> +		I915_WRITE(pp_ctrl_reg, pp_ctl);
> +		pp_div = I915_READ(pp_div_reg);
> +	}
> 
>  	/* Pull timing values out of registers */
>  	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >> @@ -
> 5137,8 +5154,17 @@ 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) >>
> +	if (IS_BROXTON(dev)) {
> +		u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
> +			BXT_POWER_CYCLE_DELAY_SHIFT;
> +		if (tmp > 0)
> +			cur.t11_t12 = (tmp - 1) * 1000;
> +		else
> +			cur.t11_t12 = 0;
> +	} 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); @@ -5195,13
> +5221,23 @@ 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_PCH_SPLIT(dev)) {
> +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> +
> +	} else if (HAS_PCH_SPLIT(dev)) {
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>  		pp_div_reg = PCH_PP_DIVISOR;
> @@ -5227,9 +5263,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. */
> @@ -5246,11 +5289,16 @@
> 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) :
>  		      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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
  2015-06-18  6:08       ` Jindal, Sonika
@ 2015-06-18  6:48         ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2015-06-18  6:48 UTC (permalink / raw)
  To: Jindal, Sonika; +Cc: intel-gfx@lists.freedesktop.org

On Thu, Jun 18, 2015 at 06:08:46AM +0000, Jindal, Sonika wrote:
> Looks good to me.
> Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
> 
> > -----Original Message-----
> > From: Kannan, Vandana
> > Sent: Thursday, June 18, 2015 11:01 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Jindal, Sonika; Kannan, Vandana
> > Subject: [PATCH v5] drm/i915/bxt: eDP Panel Power sequencing
> > 
> > Changes for BXT - added a IS_BROXTON check to use the macro related to
> > PPS registers for BXT.
> > BXT does not have PP_DIV register. Making changes to handle this.
> > Second set of PPS registers have been defined but will be used when VBT
> > provides a selection between the 2 sets of registers.
> > 
> > v2:
> > [Jani] Added 2nd set of PPS registers and the macro Jani's review comments
> > 	- remove reference in i915_suspend.c
> > 	- Use BXT PP macro
> > Squashing all PPS related patches into one.
> > 
> > v3: Jani's review comments addressed
> > 	- Use pp_ctl instead of pp
> > 	- ironlake_get_pp_control() is not required for BXT
> > 	- correct the use of && in the print statement
> > 	- drop the shift in the print statement
> > 
> > v4: Jani's comments
> > 	- modify ironlake_get_pp_control() - dont set unlock key for bxt
> > 
> > v5: Sonika's comments addressed
> > 	- check alignment
> > 	- move pp_ctrl_reg write (after ironlake_get_pp_control())
> > 	to !IS_BROXTON case.
> > 	- check before subtracting 1 for t11_t12
> > 
> > Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> > Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>

When resending patches please always include reviewers from previous
rounds in the Cc: list of the patch. Otherwise it's very likely that
they'll miss the patch in the overall mail flood on intel-gfx. Adding
Jani&Sonika.

Queued for -next (assuming Jani's ok too), thanks for the patch.
-Daniel
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 13 +++++++
> > drivers/gpu/drm/i915/intel_dp.c | 82
> > ++++++++++++++++++++++++++++++++---------
> >  2 files changed, 78 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 0b979ad..27eb49e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6391,6 +6391,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)
> > @@ -6419,6 +6421,17 @@ enum skl_disp_power_wells {
> >  #define  PANEL_POWER_CYCLE_DELAY_MASK	(0x1f)
> >  #define  PANEL_POWER_CYCLE_DELAY_SHIFT	0
> > 
> > +/* BXT PPS changes - 2nd set of PPS registers */
> > +#define _BXT_PP_STATUS2 	0xc7300
> > +#define _BXT_PP_CONTROL2 	0xc7304
> > +#define _BXT_PP_ON_DELAYS2	0xc7308
> > +#define _BXT_PP_OFF_DELAYS2	0xc730c
> > +
> > +#define BXT_PP_STATUS(n)	((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
> > +#define BXT_PP_CONTROL(n)	((!n) ? PCH_PP_CONTROL :
> > _BXT_PP_CONTROL2)
> > +#define BXT_PP_ON_DELAYS(n)	((!n) ? PCH_PP_ON_DELAYS :
> > _BXT_PP_ON_DELAYS2)
> > +#define BXT_PP_OFF_DELAYS(n)	((!n) ? PCH_PP_OFF_DELAYS :
> > _BXT_PP_OFF_DELAYS2)
> > +
> >  #define PCH_DP_B		0xe4100
> >  #define PCH_DPB_AUX_CH_CTL	0xe4110
> >  #define PCH_DPB_AUX_CH_DATA1	0xe4114
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index f52eef1..c2145e7 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)  {
> >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > 
> > -	if (HAS_PCH_SPLIT(dev))
> > +	if (IS_BROXTON(dev))
> > +		return BXT_PP_CONTROL(0);
> > +	else if (HAS_PCH_SPLIT(dev))
> >  		return PCH_PP_CONTROL;
> >  	else
> >  		return
> > VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> > @@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)  {
> >  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > 
> > -	if (HAS_PCH_SPLIT(dev))
> > +	if (IS_BROXTON(dev))
> > +		return BXT_PP_STATUS(0);
> > +	else if (HAS_PCH_SPLIT(dev))
> >  		return PCH_PP_STATUS;
> >  	else
> >  		return
> > VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> > @@ -1702,8 +1706,10 @@ static  u32 ironlake_get_pp_control(struct
> > intel_dp *intel_dp)
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> > 
> >  	control = I915_READ(_pp_ctrl_reg(intel_dp));
> > -	control &= ~PANEL_UNLOCK_MASK;
> > -	control |= PANEL_UNLOCK_REGS;
> > +	if (!IS_BROXTON(dev)) {
> > +		control &= ~PANEL_UNLOCK_MASK;
> > +		control |= PANEL_UNLOCK_REGS;
> > +	}
> >  	return control;
> >  }
> > 
> > @@ -5092,8 +5098,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;
> > +	int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
> > 
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> > 
> > @@ -5101,7 +5107,16 @@ intel_dp_init_panel_power_sequencer(struct
> > drm_device *dev,
> >  	if (final->t11_t12 != 0)
> >  		return;
> > 
> > -	if (HAS_PCH_SPLIT(dev)) {
> > +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> > +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> > +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > +	} else if (HAS_PCH_SPLIT(dev)) {
> >  		pp_ctrl_reg = PCH_PP_CONTROL;
> >  		pp_on_reg = PCH_PP_ON_DELAYS;
> >  		pp_off_reg = PCH_PP_OFF_DELAYS;
> > @@ -5117,12 +5132,14 @@ intel_dp_init_panel_power_sequencer(struct
> > drm_device *dev,
> > 
> >  	/* Workaround: Need to write PP_CONTROL with the unlock key as
> >  	 * the very first thing. */
> > -	pp = ironlake_get_pp_control(intel_dp);
> > -	I915_WRITE(pp_ctrl_reg, pp);
> > +	pp_ctl = ironlake_get_pp_control(intel_dp);
> > 
> >  	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)) {
> > +		I915_WRITE(pp_ctrl_reg, pp_ctl);
> > +		pp_div = I915_READ(pp_div_reg);
> > +	}
> > 
> >  	/* Pull timing values out of registers */
> >  	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >> @@ -
> > 5137,8 +5154,17 @@ 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) >>
> > +	if (IS_BROXTON(dev)) {
> > +		u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
> > +			BXT_POWER_CYCLE_DELAY_SHIFT;
> > +		if (tmp > 0)
> > +			cur.t11_t12 = (tmp - 1) * 1000;
> > +		else
> > +			cur.t11_t12 = 0;
> > +	} 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); @@ -5195,13
> > +5221,23 @@ 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_PCH_SPLIT(dev)) {
> > +	if (IS_BROXTON(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 = BXT_PP_CONTROL(0);
> > +		pp_on_reg = BXT_PP_ON_DELAYS(0);
> > +		pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > +
> > +	} else if (HAS_PCH_SPLIT(dev)) {
> >  		pp_on_reg = PCH_PP_ON_DELAYS;
> >  		pp_off_reg = PCH_PP_OFF_DELAYS;
> >  		pp_div_reg = PCH_PP_DIVISOR;
> > @@ -5227,9 +5263,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. */
> > @@ -5246,11 +5289,16 @@
> > 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) :
> >  		      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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-06-18  6:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-12 10:27 [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing Vandana Kannan
2015-06-17  9:15 ` Jindal, Sonika
2015-06-17  9:22   ` Kannan, Vandana
2015-06-18  5:30     ` [PATCH v5] " Vandana Kannan
2015-06-18  6:08       ` Jindal, Sonika
2015-06-18  6:48         ` Daniel Vetter
  -- strict thread matches above, loose matches on Subject: below --
2015-05-11 14:34 [PATCH v3] " Kannan, Vandana
2015-05-13  9:43 ` [PATCH v4] " Vandana Kannan
2015-05-13  9:22   ` Kannan, Vandana
2015-06-03 12:01     ` Kannan, Vandana
2015-05-14  9:13   ` shuang.he

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox