All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT
@ 2015-04-30  7:37 Vandana Kannan
  2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Vandana Kannan @ 2015-04-30  7:37 UTC (permalink / raw)
  To: intel-gfx

Changes based on future platform readiness patches related to
HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
---
 drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
 drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index cf67f82..e91d637 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
 		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
 
 	/* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(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);
@@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
 
 	/* Panel power sequencer */
-	if (HAS_PCH_SPLIT(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);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 937ba31..68e10c1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -559,7 +559,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
 		return PCH_PP_CONTROL;
 	else
 		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
@@ -569,7 +569,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
 		return PCH_PP_STATUS;
 	else
 		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
@@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
 	if (final->t11_t12 != 0)
 		return;
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(dev)) {
 		pp_ctrl_reg = PCH_PP_CONTROL;
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
@@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (!HAS_GMCH_DISPLAY(dev)) {
 		pp_on_reg = PCH_PP_ON_DELAYS;
 		pp_off_reg = PCH_PP_OFF_DELAYS;
 		pp_div_reg = PCH_PP_DIVISOR;
-- 
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] 25+ messages in thread

* [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes
  2015-04-30  7:37 [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Vandana Kannan
@ 2015-04-30  7:37 ` Vandana Kannan
  2015-04-30  8:57   ` Jani Nikula
  2015-04-30  9:27   ` David Weinehall
  2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 25+ messages in thread
From: Vandana Kannan @ 2015-04-30  7:37 UTC (permalink / raw)
  To: intel-gfx

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);
 		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));
 }
 
-- 
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] 25+ messages in thread

* [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set
  2015-04-30  7:37 [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Vandana Kannan
  2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
@ 2015-04-30  7:37 ` Vandana Kannan
  2015-04-30  8:43   ` Jani Nikula
                     ` (2 more replies)
  2015-04-30  8:50 ` [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Jani Nikula
  2015-05-06 10:08 ` Daniel Vetter
  3 siblings, 3 replies; 25+ messages in thread
From: Vandana Kannan @ 2015-04-30  7:37 UTC (permalink / raw)
  To: intel-gfx

Second set of PPS registers have been defined but will be used when VBT
provides a selection between the 2 sets of registers.

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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 580f5cb..199a1747 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6345,6 +6345,12 @@ 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 PCH_DP_B		0xe4100
 #define PCH_DPB_AUX_CH_CTL	0xe4110
 #define PCH_DPB_AUX_CH_DATA1	0xe4114
-- 
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] 25+ messages in thread

* Re: [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set
  2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
@ 2015-04-30  8:43   ` Jani Nikula
  2015-04-30 11:15   ` Imre Deak
  2015-05-01 13:30   ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set shuang.he
  2 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2015-04-30  8:43 UTC (permalink / raw)
  To: Vandana Kannan, intel-gfx

On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> Second set of PPS registers have been defined but will be used when VBT
> provides a selection between the 2 sets of registers.
>
> 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 | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 580f5cb..199a1747 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6345,6 +6345,12 @@ 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 PCH_DP_B		0xe4100
>  #define PCH_DPB_AUX_CH_CTL	0xe4110
>  #define PCH_DPB_AUX_CH_DATA1	0xe4114

How about doing this patch first, with something like:

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 435c372d001e..a3af3526cb4f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6343,6 +6343,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

And you could use BXT_PP_* from the start. I believe this will add
clarity to the usage and pinpoint where you'll need to touch the code to
enable the 2nd power sequencer.


BR,
Jani.


> -- 
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT
  2015-04-30  7:37 [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Vandana Kannan
  2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
  2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
@ 2015-04-30  8:50 ` Jani Nikula
  2015-05-06 10:08 ` Daniel Vetter
  3 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2015-04-30  8:50 UTC (permalink / raw)
  To: Vandana Kannan, intel-gfx

On Thu, 30 Apr 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> Changes based on future platform readiness patches related to
> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
>
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index cf67f82..e91d637 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(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);
> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
>  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(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);

I don't think you should touch i915_suspend.c at all. We're trying to
get rid of this blind register save/restore, and make everything work in
the encoder/connector code. Do note that we already skip this for
vlv/chv power sequencer registers.

> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 937ba31..68e10c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -559,7 +559,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_CONTROL;
>  	else
>  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -569,7 +569,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_STATUS;
>  	else
>  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	if (final->t11_t12 != 0)
>  		return;
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>  		pp_div_reg = PCH_PP_DIVISOR;

For the rest, I'd like you to do what I suggested in my reply to patch
3, i.e. add a new if (IS_BROXTON(dev)) at the top of each of these, with
BXT_PP_WHATEVER(0) as the register. Later on, we can add code to choose
the power sequencer, a bit similar to what vlv/chv do now (except there
it's based on pipe).

BR,
Jani.


> -- 
> 2.0.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes
  2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
@ 2015-04-30  8:57   ` Jani Nikula
  2015-04-30  9:27   ` David Weinehall
  1 sibling, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2015-04-30  8:57 UTC (permalink / raw)
  To: Vandana Kannan, intel-gfx

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes
  2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
  2015-04-30  8:57   ` Jani Nikula
@ 2015-04-30  9:27   ` David Weinehall
  1 sibling, 0 replies; 25+ messages in thread
From: David Weinehall @ 2015-04-30  9:27 UTC (permalink / raw)
  To: Vandana Kannan; +Cc: intel-gfx

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set
  2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
  2015-04-30  8:43   ` Jani Nikula
@ 2015-04-30 11:15   ` Imre Deak
  2015-04-30 11:23     ` Jani Nikula
  2015-05-01 13:30   ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set shuang.he
  2 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2015-04-30 11:15 UTC (permalink / raw)
  To: Vandana Kannan; +Cc: intel-gfx

On to, 2015-04-30 at 13:07 +0530, Vandana Kannan wrote:
> Second set of PPS registers have been defined but will be used when VBT
> provides a selection between the 2 sets of registers.
> 
> 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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 580f5cb..199a1747 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6345,6 +6345,12 @@ 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
> +

Can we add these in the (future) patch which takes them into use?

>  #define PCH_DP_B		0xe4100
>  #define PCH_DPB_AUX_CH_CTL	0xe4110
>  #define PCH_DPB_AUX_CH_DATA1	0xe4114


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set
  2015-04-30 11:15   ` Imre Deak
@ 2015-04-30 11:23     ` Jani Nikula
  2015-05-04  6:24       ` Kannan, Vandana
  0 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2015-04-30 11:23 UTC (permalink / raw)
  To: imre.deak, Vandana Kannan; +Cc: intel-gfx

On Thu, 30 Apr 2015, Imre Deak <imre.deak@intel.com> wrote:
> On to, 2015-04-30 at 13:07 +0530, Vandana Kannan wrote:
>> Second set of PPS registers have been defined but will be used when VBT
>> provides a selection between the 2 sets of registers.
>> 
>> 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 | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 580f5cb..199a1747 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6345,6 +6345,12 @@ 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
>> +
>
> Can we add these in the (future) patch which takes them into use?

See my review comment, we should use them off the bat.

BR,
Jani.

>
>>  #define PCH_DP_B		0xe4100
>>  #define PCH_DPB_AUX_CH_CTL	0xe4110
>>  #define PCH_DPB_AUX_CH_DATA1	0xe4114
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set
  2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
  2015-04-30  8:43   ` Jani Nikula
  2015-04-30 11:15   ` Imre Deak
@ 2015-05-01 13:30   ` shuang.he
  2 siblings, 0 replies; 25+ messages in thread
From: shuang.he @ 2015-05-01 13:30 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: 6296
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  264/264              264/264
BYT                 -4              227/227              223/227
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@drm_vma_limiter_cached      FAIL(4)PASS(3)      FAIL(1)NO_RESULT(1)
*BYT  igt@gem_dummy_reloc_loop@render      FAIL(1)PASS(15)      TIMEOUT(1)PASS(1)
 BYT  igt@gem_pipe_control_store_loop@fresh-buffer      FAIL(1)TIMEOUT(7)PASS(9)      TIMEOUT(1)PASS(1)
*BYT  igt@gem_tiled_pread      FAIL(1)PASS(4)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+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] 25+ messages in thread

* Re: [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set
  2015-04-30 11:23     ` Jani Nikula
@ 2015-05-04  6:24       ` Kannan, Vandana
  2015-05-04  7:06         ` [PATCH] drm/i915: eDP Panel Power sequencing Vandana Kannan
  0 siblings, 1 reply; 25+ messages in thread
From: Kannan, Vandana @ 2015-05-04  6:24 UTC (permalink / raw)
  To: Jani Nikula, imre.deak; +Cc: intel-gfx



On 4/30/2015 4:53 PM, Jani Nikula wrote:
> On Thu, 30 Apr 2015, Imre Deak <imre.deak@intel.com> wrote:
>> On to, 2015-04-30 at 13:07 +0530, Vandana Kannan wrote:
>>> Second set of PPS registers have been defined but will be used when VBT
>>> provides a selection between the 2 sets of registers.
>>>
>>> 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 | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 580f5cb..199a1747 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -6345,6 +6345,12 @@ 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
>>> +
>>
>> Can we add these in the (future) patch which takes them into use?
>
> See my review comment, we should use them off the bat.
>
> BR,
> Jani.
>
Agree with all your review comments.
I've made the changes and will be resending the patch today.

- Vandana
>>
>>>   #define PCH_DP_B		0xe4100
>>>   #define PCH_DPB_AUX_CH_CTL	0xe4110
>>>   #define PCH_DPB_AUX_CH_DATA1	0xe4114
>>
>>
>> _______________________________________________
>> 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] 25+ messages in thread

* [PATCH] drm/i915: eDP Panel Power sequencing
  2015-05-04  6:24       ` Kannan, Vandana
@ 2015-05-04  7:06         ` Vandana Kannan
  2015-05-04 11:12           ` shuang.he
  2015-05-06 15:05           ` Jani Nikula
  0 siblings, 2 replies; 25+ messages in thread
From: Vandana Kannan @ 2015-05-04  7:06 UTC (permalink / raw)
  To: intel-gfx

Changes based on future platform readiness patches related to
HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT.
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.

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 | 69 +++++++++++++++++++++++++++++++++--------
 2 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..e8d93ea 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 937ba31..fd91006 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));
@@ -4954,8 +4958,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);
 
@@ -4963,7 +4967,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;
@@ -4984,7 +4997,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,7 +5015,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",
@@ -5057,13 +5077,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;
@@ -5089,9 +5119,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 +5145,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

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

* Re: [PATCH] drm/i915: eDP Panel Power sequencing
  2015-05-04  7:06         ` [PATCH] drm/i915: eDP Panel Power sequencing Vandana Kannan
@ 2015-05-04 11:12           ` shuang.he
  2015-05-06 15:05           ` Jani Nikula
  1 sibling, 0 replies; 25+ messages in thread
From: shuang.he @ 2015-05-04 11:12 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: 6309
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                                  286/286              286/286
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
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] 25+ messages in thread

* Re: [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT
  2015-04-30  7:37 [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Vandana Kannan
                   ` (2 preceding siblings ...)
  2015-04-30  8:50 ` [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Jani Nikula
@ 2015-05-06 10:08 ` Daniel Vetter
  2015-05-06 10:12   ` Jani Nikula
  3 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2015-05-06 10:08 UTC (permalink / raw)
  To: Vandana Kannan; +Cc: intel-gfx

On Thu, Apr 30, 2015 at 01:07:33PM +0530, Vandana Kannan wrote:
> Changes based on future platform readiness patches related to
> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
> 
> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>

A least for edp we really need to get rid of the save/restore code in
i915_suspend.c and instead have it all tied together with the other pp
code in intel_dp.c. This series seems like a good opportunity to do just
that.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index cf67f82..e91d637 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(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);
> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
>  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>  
>  	/* Panel power sequencer */
> -	if (HAS_PCH_SPLIT(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);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 937ba31..68e10c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -559,7 +559,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_CONTROL;
>  	else
>  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
> @@ -569,7 +569,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
>  		return PCH_PP_STATUS;
>  	else
>  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>  	if (final->t11_t12 != 0)
>  		return;
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_ctrl_reg = PCH_PP_CONTROL;
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
> @@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (!HAS_GMCH_DISPLAY(dev)) {
>  		pp_on_reg = PCH_PP_ON_DELAYS;
>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>  		pp_div_reg = PCH_PP_DIVISOR;
> -- 
> 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] 25+ messages in thread

* Re: [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT
  2015-05-06 10:08 ` Daniel Vetter
@ 2015-05-06 10:12   ` Jani Nikula
  0 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2015-05-06 10:12 UTC (permalink / raw)
  To: Daniel Vetter, Vandana Kannan; +Cc: intel-gfx

On Wed, 06 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Apr 30, 2015 at 01:07:33PM +0530, Vandana Kannan wrote:
>> Changes based on future platform readiness patches related to
>> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT
>> 
>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
>
> A least for edp we really need to get rid of the save/restore code in
> i915_suspend.c and instead have it all tied together with the other pp
> code in intel_dp.c. This series seems like a good opportunity to do just
> that.

The next version of the patch already drops the blind save/restore in
i915_suspend.c and looks much better.

BR,
Jani.

> -Daniel
>
>> ---
>>  drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>>  drivers/gpu/drm/i915/intel_dp.c     | 8 ++++----
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
>> index cf67f82..e91d637 100644
>> --- a/drivers/gpu/drm/i915/i915_suspend.c
>> +++ b/drivers/gpu/drm/i915/i915_suspend.c
>> @@ -44,7 +44,7 @@ static void i915_save_display(struct drm_device *dev)
>>  		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
>>  
>>  	/* Panel power sequencer */
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(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);
>> @@ -79,7 +79,7 @@ static void i915_restore_display(struct drm_device *dev)
>>  		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
>>  
>>  	/* Panel power sequencer */
>> -	if (HAS_PCH_SPLIT(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);
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 937ba31..68e10c1 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -559,7 +559,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
>>  		return PCH_PP_CONTROL;
>>  	else
>>  		return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
>> @@ -569,7 +569,7 @@ 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 (!HAS_GMCH_DISPLAY(dev))
>>  		return PCH_PP_STATUS;
>>  	else
>>  		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>> @@ -4963,7 +4963,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>>  	if (final->t11_t12 != 0)
>>  		return;
>>  
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(dev)) {
>>  		pp_ctrl_reg = PCH_PP_CONTROL;
>>  		pp_on_reg = PCH_PP_ON_DELAYS;
>>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>> @@ -5063,7 +5063,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
>>  
>>  	lockdep_assert_held(&dev_priv->pps_mutex);
>>  
>> -	if (HAS_PCH_SPLIT(dev)) {
>> +	if (!HAS_GMCH_DISPLAY(dev)) {
>>  		pp_on_reg = PCH_PP_ON_DELAYS;
>>  		pp_off_reg = PCH_PP_OFF_DELAYS;
>>  		pp_div_reg = PCH_PP_DIVISOR;
>> -- 
>> 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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: eDP Panel Power sequencing
  2015-05-04  7:06         ` [PATCH] drm/i915: eDP Panel Power sequencing Vandana Kannan
  2015-05-04 11:12           ` shuang.he
@ 2015-05-06 15:05           ` Jani Nikula
  2015-05-07  4:13             ` Kannan, Vandana
  2015-05-07  7:31             ` [PATCH v3] drm/i915/bxt: " Vandana Kannan
  1 sibling, 2 replies; 25+ messages in thread
From: Jani Nikula @ 2015-05-06 15:05 UTC (permalink / raw)
  To: Vandana Kannan, intel-gfx

On Mon, 04 May 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
> Changes based on future platform readiness patches related to
> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT.

This needs an update to reflect the patch.

> 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.
>
> 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 | 69 +++++++++++++++++++++++++++++++++--------
>  2 files changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 36805b6..e8d93ea 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 937ba31..fd91006 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));
> @@ -4954,8 +4958,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;

Note that pp is actually used for pp_ctl. You can rename that if you
want.

>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> @@ -4963,7 +4967,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;
> @@ -4984,7 +4997,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);

You already have pp_ctl in pp at this point. It is read using
ironlake_get_pp_control, which also needs an update for bxt, as AFAICT
panel unlock is no longer needed and MBZ.

>  
>  	/* Pull timing values out of registers */
>  	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
> @@ -4999,7 +5015,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",
> @@ -5057,13 +5077,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;
> @@ -5089,9 +5119,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 +5145,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)

Should be & not &&. Not sure if the shift is needed really.

BR,
Jani.

> +		       >> BXT_POWER_CYCLE_DELAY_SHIFT) :
>  		      I915_READ(pp_div_reg));
>  }
>  
> -- 
> 2.0.1
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: eDP Panel Power sequencing
  2015-05-06 15:05           ` Jani Nikula
@ 2015-05-07  4:13             ` Kannan, Vandana
  2015-05-07  7:31             ` [PATCH v3] drm/i915/bxt: " Vandana Kannan
  1 sibling, 0 replies; 25+ messages in thread
From: Kannan, Vandana @ 2015-05-07  4:13 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx



On 5/6/2015 8:35 PM, Jani Nikula wrote:
> On Mon, 04 May 2015, Vandana Kannan <vandana.kannan@intel.com> wrote:
>> Changes based on future platform readiness patches related to
>> HAS_PCH_SPLIT(). Use HAS_GMCH_DISPLAY() instead of HAS_PCH_SPLIT.
>
> This needs an update to reflect the patch.
>
>> 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.
>>
>> 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 | 69 +++++++++++++++++++++++++++++++++--------
>>   2 files changed, 69 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 36805b6..e8d93ea 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 937ba31..fd91006 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));
>> @@ -4954,8 +4958,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;
>
> Note that pp is actually used for pp_ctl. You can rename that if you
> want.
>
Ok.
>>
>>   	lockdep_assert_held(&dev_priv->pps_mutex);
>>
>> @@ -4963,7 +4967,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;
>> @@ -4984,7 +4997,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);
>
> You already have pp_ctl in pp at this point. It is read using
> ironlake_get_pp_control, which also needs an update for bxt, as AFAICT
> panel unlock is no longer needed and MBZ.
>
Ok.
>>
>>   	/* Pull timing values out of registers */
>>   	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
>> @@ -4999,7 +5015,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",
>> @@ -5057,13 +5077,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;
>> @@ -5089,9 +5119,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 +5145,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)
>
> Should be & not &&. Not sure if the shift is needed really.
Oops !!
Thanks..
>
> BR,
> Jani.
>
>> +		       >> 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

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

* [PATCH v3] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-06 15:05           ` Jani Nikula
  2015-05-07  4:13             ` Kannan, Vandana
@ 2015-05-07  7:31             ` Vandana Kannan
  2015-05-07  7:37               ` Jani Nikula
  2015-05-08  5:47               ` [PATCH v3] " shuang.he
  1 sibling, 2 replies; 25+ messages in thread
From: Vandana Kannan @ 2015-05-07  7:31 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

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 | 74 ++++++++++++++++++++++++++++++++---------
 2 files changed, 72 insertions(+), 15 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..3082224 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));
@@ -4969,8 +4973,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 +4982,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 +5007,17 @@ 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);
+	if (!IS_BROXTON(dev)) {
+		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);
+	else
+		pp_ctl = I915_READ(pp_ctrl_reg);
 
 	/* Pull timing values out of registers */
 	cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
@@ -5014,7 +5032,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 +5094,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 +5136,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 +5162,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] 25+ messages in thread

* Re: [PATCH v3] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-07  7:31             ` [PATCH v3] drm/i915/bxt: " Vandana Kannan
@ 2015-05-07  7:37               ` Jani Nikula
  2015-05-11 14:34                 ` Kannan, Vandana
  2015-05-08  5:47               ` [PATCH v3] " shuang.he
  1 sibling, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2015-05-07  7:37 UTC (permalink / raw)
  To: Vandana Kannan, intel-gfx

On Thu, 07 May 2015, Vandana Kannan <vandana.kannan@intel.com> 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
>
> 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 | 74 ++++++++++++++++++++++++++++++++---------
>  2 files changed, 72 insertions(+), 15 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..3082224 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));
> @@ -4969,8 +4973,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 +4982,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 +5007,17 @@ 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);
> +	if (!IS_BROXTON(dev)) {
> +		pp_ctl = ironlake_get_pp_control(intel_dp);
> +		I915_WRITE(pp_ctrl_reg, pp_ctl);
> +	}

I tried to say you should update the ironlake_get_pp_control function
too. It gets called in a number of places, also for bxt, it adds the
unlock key (0xabcd << 16) to the result and the call sites write that
back to pp ctl. And we shouldn't do that since those bits must be zero
on bxt.

With that function fixed, you can read pp ctl once using that, for all
platforms, and only have the !is_broxton branch below for reading pp div
since you'll already have pp ctl.

---

It is practically impossible to express everything clearly and
exhaustively except by effectively rewriting the patch. But that would
defeat the purpose of you writing the patch and me reviewing... so
please do try to think about all the implications of the review
comments. In the end, we all want to get the review done in as few
rounds as possible.

Thanks,
Jani.


>  
>  	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) >>
> @@ -5014,7 +5032,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 +5094,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 +5136,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 +5162,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
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-07  7:31             ` [PATCH v3] drm/i915/bxt: " Vandana Kannan
  2015-05-07  7:37               ` Jani Nikula
@ 2015-05-08  5:47               ` shuang.he
  1 sibling, 0 replies; 25+ messages in thread
From: shuang.he @ 2015-05-08  5:47 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: 6341
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  316/316              316/316
IVB                                  342/342              342/342
BYT                                  286/286              286/286
BDW                                  323/323              323/323
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
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] 25+ messages in thread

* Re: [PATCH v3] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-07  7:37               ` Jani Nikula
@ 2015-05-11 14:34                 ` Kannan, Vandana
  2015-05-13  9:43                   ` [PATCH v4] " Vandana Kannan
  0 siblings, 1 reply; 25+ messages in thread
From: Kannan, Vandana @ 2015-05-11 14:34 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx



On 5/7/2015 1:07 PM, Jani Nikula wrote:
> On Thu, 07 May 2015, Vandana Kannan <vandana.kannan@intel.com> 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
>>

[...]

>>   	/* 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);
>> +	if (!IS_BROXTON(dev)) {
>> +		pp_ctl = ironlake_get_pp_control(intel_dp);
>> +		I915_WRITE(pp_ctrl_reg, pp_ctl);
>> +	}
>
> I tried to say you should update the ironlake_get_pp_control function
> too. It gets called in a number of places, also for bxt, it adds the
> unlock key (0xabcd << 16) to the result and the call sites write that
> back to pp ctl. And we shouldn't do that since those bits must be zero
> on bxt.
>
> With that function fixed, you can read pp ctl once using that, for all
> platforms, and only have the !is_broxton branch below for reading pp div
> since you'll already have pp ctl.
>
> ---
>
> It is practically impossible to express everything clearly and
> exhaustively except by effectively rewriting the patch. But that would
> defeat the purpose of you writing the patch and me reviewing... so
> please do try to think about all the implications of the review
> comments. In the end, we all want to get the review done in as few
> rounds as possible.
>
Apologies.. Guess I was in a hurry the other day. Will take care in this 
patch and in future.

Thanks,
Vandana

> Thanks,
> Jani.
>
>
>>
>>   	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) >>
>> @@ -5014,7 +5032,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 +5094,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);
>>

[...]

>>
>> --
>> 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] 25+ 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-06-03 12:01                       ` Kannan, Vandana
  2015-05-14  9:13                     ` shuang.he
  1 sibling, 1 reply; 25+ 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] 25+ messages in thread

* [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
  2015-05-11 14:34                 ` 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ messages in thread
From: Kannan, Vandana @ 2015-06-03 12:01 UTC (permalink / raw)
  To: intel-gfx

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] 25+ messages in thread

end of thread, other threads:[~2015-06-03 12:01 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-30  7:37 [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Vandana Kannan
2015-04-30  7:37 ` [PATCH 2/3] drm/i915: eDP Panel Power sequencing PP_DIV register changes Vandana Kannan
2015-04-30  8:57   ` Jani Nikula
2015-04-30  9:27   ` David Weinehall
2015-04-30  7:37 ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set Vandana Kannan
2015-04-30  8:43   ` Jani Nikula
2015-04-30 11:15   ` Imre Deak
2015-04-30 11:23     ` Jani Nikula
2015-05-04  6:24       ` Kannan, Vandana
2015-05-04  7:06         ` [PATCH] drm/i915: eDP Panel Power sequencing Vandana Kannan
2015-05-04 11:12           ` shuang.he
2015-05-06 15:05           ` Jani Nikula
2015-05-07  4:13             ` Kannan, Vandana
2015-05-07  7:31             ` [PATCH v3] drm/i915/bxt: " Vandana Kannan
2015-05-07  7:37               ` Jani Nikula
2015-05-11 14:34                 ` 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
2015-05-08  5:47               ` [PATCH v3] " shuang.he
2015-05-01 13:30   ` [PATCH 3/3] drm/i915: eDP Panel Power sequencing add PPS reg set shuang.he
2015-04-30  8:50 ` [PATCH 1/3] drm/i915: eDP Panel Power sequencing modify use of HAS_PCH_SPLIT Jani Nikula
2015-05-06 10:08 ` Daniel Vetter
2015-05-06 10:12   ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.