[PATCH 0/9] drm/i915: VLV/CHV DPLL workarounds and cleanups

[PATCH 0/9] drm/i915: VLV/CHV DPLL workarounds and cleanups

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

While trawling the w/a database I spotted a workaround we didn't
have related to CHV DPLL pixel multiuplier setting. So I set forth
to implement it, and while doing that I ended up cleaning up the
VLV/CHV DPLL handling a bit. This also touched the DSI code a bit
and while testing that I noticed some problems with out DSI PLL
handling so I included a fix for those.

Ville Syrjälä (9):
  drm/i915: Keep GMCH DPLL VGA mode always disabled
  drm/i915: Apply OCD to VLV/CHV DPLL defines
  drm/i915: Simplify CHV pipe A power well code
  drm/i915: Refactor VLV display power well init/deinit
  drm/i915: Clear out DPLL state from pipe config in DSI get config
  drm/i915: Move DPLL ref/cri/VGA mode frobbing to the disp2d well
    enable
  drm/i915: Make {vlv,chv}_{disable,update}_pll() more similar
  drm/i915: Implement WaPixelRepeatModeFixForC0:chv
  drm/i915: Disable DSI PLL before reconfiguring it

 drivers/gpu/drm/i915/i915_drv.h         |   7 ++
 drivers/gpu/drm/i915/i915_reg.h         |   9 ++-
 drivers/gpu/drm/i915/intel_display.c    |  99 +++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dsi.c        |  16 ++---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 110 +++++++++++++++-----------------
 5 files changed, 124 insertions(+), 117 deletions(-)

-- 
2.3.6

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

[PATCH 1/9] drm/i915: Keep GMCH DPLL VGA mode always disabled

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We disable the DPLL VGA mode when enabling the DPLL, but we enaable it
again when disabling the DPLL. Having VGA mode enabled even in unused
DPLLs can cause problems for CHV, so it seems wiser to always keep it
disabled. And let's just do that on all GMCH platforms to keep things
as similar as possible between them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c    | 8 +++++---
 drivers/gpu/drm/i915/intel_dsi.c        | 2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++----
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d624f58..dd3b649 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1774,13 +1774,13 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
 	/* Make sure the pipe isn't still relying on us */
 	assert_pipe_disabled(dev_priv, pipe);
 
-	I915_WRITE(DPLL(pipe), 0);
+	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
 	POSTING_READ(DPLL(pipe));
 }
 
 static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
-	u32 val = 0;
+	u32 val;
 
 	/* Make sure the pipe isn't still relying on us */
 	assert_pipe_disabled(dev_priv, pipe);
@@ -1789,6 +1789,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * Leave integrated clock source and reference clock enabled for pipe B.
 	 * The latter is needed for VGA hotplug / manual detection.
 	 */
+	val = DPLL_VGA_MODE_DIS;
 	if (pipe == PIPE_B)
 		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), val);
@@ -1805,7 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	assert_pipe_disabled(dev_priv, pipe);
 
 	/* Set PLL en = 0 */
-	val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
+	val = DPLL_SSC_REF_CLOCK_CHV |
+		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 	if (pipe != PIPE_A)
 		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
 	I915_WRITE(DPLL(pipe), val);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3f29385..e087360 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -427,7 +427,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	/* update the hw state for DPLL */
 	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
-		DPLL_REFA_CLK_ENABLE_VLV;
+		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1a45385..f0e6f49 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -882,7 +882,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	 * display and the reference clock for VGA
 	 * hotplug / manual detection.
 	 */
-	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
 		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 
@@ -933,13 +933,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	 */
 	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
 		phy = DPIO_PHY0;
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
 			   DPLL_REFA_CLK_ENABLE_VLV);
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
 			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	} else {
 		phy = DPIO_PHY1;
-		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
+		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
 			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	}
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
-- 
2.3.6

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

Re: [PATCH 1/9] drm/i915: Keep GMCH DPLL VGA mode always disabled

[Sivakumar Thulasimani]

On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We disable the DPLL VGA mode when enabling the DPLL, but we enaable it
> again when disabling the DPLL. Having VGA mode enabled even in unused
> DPLLs can cause problems for CHV, so it seems wiser to always keep it
> disabled. And let's just do that on all GMCH platforms to keep things
> as similar as possible between them.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c    | 8 +++++---
>   drivers/gpu/drm/i915/intel_dsi.c        | 2 +-
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++----
>   3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d624f58..dd3b649 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1774,13 +1774,13 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
>   	/* Make sure the pipe isn't still relying on us */
>   	assert_pipe_disabled(dev_priv, pipe);
>   
> -	I915_WRITE(DPLL(pipe), 0);
> +	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
>   	POSTING_READ(DPLL(pipe));
>   }
>   
>   static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   {
> -	u32 val = 0;
> +	u32 val;
>   
>   	/* Make sure the pipe isn't still relying on us */
>   	assert_pipe_disabled(dev_priv, pipe);
> @@ -1789,6 +1789,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   	 * Leave integrated clock source and reference clock enabled for pipe B.
>   	 * The latter is needed for VGA hotplug / manual detection.
>   	 */
> +	val = DPLL_VGA_MODE_DIS;
>   	if (pipe == PIPE_B)
>   		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
>   	I915_WRITE(DPLL(pipe), val);
> @@ -1805,7 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   	assert_pipe_disabled(dev_priv, pipe);
>   
>   	/* Set PLL en = 0 */
> -	val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
> +	val = DPLL_SSC_REF_CLOCK_CHV |
> +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>   	if (pipe != PIPE_A)
>   		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
>   	I915_WRITE(DPLL(pipe), val);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3f29385..e087360 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -427,7 +427,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>   
>   	/* update the hw state for DPLL */
>   	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> -		DPLL_REFA_CLK_ENABLE_VLV;
> +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>   
>   	tmp = I915_READ(DSPCLK_GATE_D);
>   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1a45385..f0e6f49 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -882,7 +882,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	 * display and the reference clock for VGA
>   	 * hotplug / manual detection.
>   	 */
> -	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
>   		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
>   
> @@ -933,13 +933,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	 */
>   	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
>   		phy = DPIO_PHY0;
> -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
>   			   DPLL_REFA_CLK_ENABLE_VLV);
> -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
>   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	} else {
>   		phy = DPIO_PHY1;
> -		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
> +		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
>   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	}
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
since we keep DPLL_VGA_MODE_DIS always set, even during disable is this 
needed explicitly again here ?  other than this i am fine with this.
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

-- 
regards,
Sivakumar

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

Re: [PATCH 1/9] drm/i915: Keep GMCH DPLL VGA mode always disabled

[Ville Syrjälä]

On Mon, Jun 29, 2015 at 07:46:18PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We disable the DPLL VGA mode when enabling the DPLL, but we enaable it
> > again when disabling the DPLL. Having VGA mode enabled even in unused
> > DPLLs can cause problems for CHV, so it seems wiser to always keep it
> > disabled. And let's just do that on all GMCH platforms to keep things
> > as similar as possible between them.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c    | 8 +++++---
> >   drivers/gpu/drm/i915/intel_dsi.c        | 2 +-
> >   drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++----
> >   3 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d624f58..dd3b649 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1774,13 +1774,13 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
> >   	/* Make sure the pipe isn't still relying on us */
> >   	assert_pipe_disabled(dev_priv, pipe);
> >   
> > -	I915_WRITE(DPLL(pipe), 0);
> > +	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
> >   	POSTING_READ(DPLL(pipe));
> >   }
> >   
> >   static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >   {
> > -	u32 val = 0;
> > +	u32 val;
> >   
> >   	/* Make sure the pipe isn't still relying on us */
> >   	assert_pipe_disabled(dev_priv, pipe);
> > @@ -1789,6 +1789,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >   	 * Leave integrated clock source and reference clock enabled for pipe B.
> >   	 * The latter is needed for VGA hotplug / manual detection.
> >   	 */
> > +	val = DPLL_VGA_MODE_DIS;
> >   	if (pipe == PIPE_B)
> >   		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
> >   	I915_WRITE(DPLL(pipe), val);
> > @@ -1805,7 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >   	assert_pipe_disabled(dev_priv, pipe);
> >   
> >   	/* Set PLL en = 0 */
> > -	val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
> > +	val = DPLL_SSC_REF_CLOCK_CHV |
> > +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >   	if (pipe != PIPE_A)
> >   		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> >   	I915_WRITE(DPLL(pipe), val);
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 3f29385..e087360 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -427,7 +427,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >   
> >   	/* update the hw state for DPLL */
> >   	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> > -		DPLL_REFA_CLK_ENABLE_VLV;
> > +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >   
> >   	tmp = I915_READ(DSPCLK_GATE_D);
> >   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 1a45385..f0e6f49 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -882,7 +882,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> >   	 * display and the reference clock for VGA
> >   	 * hotplug / manual detection.
> >   	 */
> > -	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> > +	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >   		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> >   
> > @@ -933,13 +933,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> >   	 */
> >   	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> >   		phy = DPIO_PHY0;
> > -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> > +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >   			   DPLL_REFA_CLK_ENABLE_VLV);
> > -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> > +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >   	} else {
> >   		phy = DPIO_PHY1;
> > -		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
> > +		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
> >   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >   	}
> >   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> since we keep DPLL_VGA_MODE_DIS always set, even during disable is this 
> needed explicitly again here ?  other than this i am fine with this.
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

Yes we still need this because during driver init the BIOS may have
left it enabled, and the register loses state when the disp2d/pipe-a
power well goes down and the poweron default has VGA mode enabled.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/9] drm/i915: Apply OCD to VLV/CHV DPLL defines

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Drop the spurious 'A' from the VLV/CHV ref clock enable define,
and add the "REF" to the VLV ref clock selection bit. Also
s/CLOCK/CLK/ for extra consistency.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  6 +++---
 drivers/gpu/drm/i915/intel_display.c    | 14 +++++++-------
 drivers/gpu/drm/i915/intel_dsi.c        |  6 +++---
 drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++----
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fa6780f..f08f729 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2127,7 +2127,7 @@ enum skl_disp_power_wells {
 #define   DPLL_DVO_2X_MODE		(1 << 30)
 #define   DPLL_EXT_BUFFER_ENABLE_VLV	(1 << 30)
 #define   DPLL_SYNCLOCK_ENABLE		(1 << 29)
-#define   DPLL_REFA_CLK_ENABLE_VLV	(1 << 29)
+#define   DPLL_REF_CLK_ENABLE_VLV	(1 << 29)
 #define   DPLL_VGA_MODE_DIS		(1 << 28)
 #define   DPLLB_MODE_DAC_SERIAL		(1 << 26) /* i915 */
 #define   DPLLB_MODE_LVDS		(2 << 26) /* i915 */
@@ -2141,8 +2141,8 @@ enum skl_disp_power_wells {
 #define   DPLL_FPA01_P1_POST_DIV_MASK_PINEVIEW	0x00ff8000 /* Pineview */
 #define   DPLL_LOCK_VLV			(1<<15)
 #define   DPLL_INTEGRATED_CRI_CLK_VLV	(1<<14)
-#define   DPLL_INTEGRATED_CLOCK_VLV	(1<<13)
-#define   DPLL_SSC_REF_CLOCK_CHV	(1<<13)
+#define   DPLL_INTEGRATED_REF_CLK_VLV	(1<<13)
+#define   DPLL_SSC_REF_CLK_CHV		(1<<13)
 #define   DPLL_PORTC_READY_MASK		(0xf << 4)
 #define   DPLL_PORTB_READY_MASK		(0xf)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dd3b649..0870532 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1791,7 +1791,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 */
 	val = DPLL_VGA_MODE_DIS;
 	if (pipe == PIPE_B)
-		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
+		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REF_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), val);
 	POSTING_READ(DPLL(pipe));
 
@@ -1806,8 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	assert_pipe_disabled(dev_priv, pipe);
 
 	/* Set PLL en = 0 */
-	val = DPLL_SSC_REF_CLOCK_CHV |
-		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
+	val = DPLL_SSC_REF_CLK_CHV |
+		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 	if (pipe != PIPE_A)
 		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
 	I915_WRITE(DPLL(pipe), val);
@@ -7198,8 +7198,8 @@ static void vlv_update_pll(struct intel_crtc *crtc,
 	 * clock for pipe B, since VGA hotplug / manual detection depends
 	 * on it.
 	 */
-	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV |
-		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV;
+	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REF_CLK_ENABLE_VLV |
+		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_REF_CLK_VLV;
 	/* We should never disable this, set it here for state tracking */
 	if (crtc->pipe == PIPE_B)
 		dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
@@ -7305,8 +7305,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 static void chv_update_pll(struct intel_crtc *crtc,
 			   struct intel_crtc_state *pipe_config)
 {
-	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
-		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
+	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV |
+		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
 		DPLL_VCO_ENABLE;
 	if (crtc->pipe != PIPE_A)
 		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index e087360..36e2148 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -422,12 +422,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 	/* Disable DPOunit clock gating, can stall pipe
 	 * and we need DPLL REFA always enabled */
 	tmp = I915_READ(DPLL(pipe));
-	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
+	tmp |= DPLL_REF_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), tmp);
 
 	/* update the hw state for DPLL */
-	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
-		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
+	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
+		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index f0e6f49..932d963 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -883,7 +883,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	 * hotplug / manual detection.
 	 */
 	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
-		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
+		   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 
 	vlv_set_power_well(dev_priv, power_well, true);
@@ -934,13 +934,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
 		phy = DPIO_PHY0;
 		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
-			   DPLL_REFA_CLK_ENABLE_VLV);
+			   DPLL_REF_CLK_ENABLE_VLV);
 		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
-			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
+			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	} else {
 		phy = DPIO_PHY1;
 		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
-			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
+			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	}
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 	vlv_set_power_well(dev_priv, power_well, true);
-- 
2.3.6

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

Re: [PATCH 2/9] drm/i915: Apply OCD to VLV/CHV DPLL defines

[Sivakumar Thulasimani]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Drop the spurious 'A' from the VLV/CHV ref clock enable define,
> and add the "REF" to the VLV ref clock selection bit. Also
> s/CLOCK/CLK/ for extra consistency.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h         |  6 +++---
>   drivers/gpu/drm/i915/intel_display.c    | 14 +++++++-------
>   drivers/gpu/drm/i915/intel_dsi.c        |  6 +++---
>   drivers/gpu/drm/i915/intel_runtime_pm.c |  8 ++++----
>   4 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fa6780f..f08f729 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2127,7 +2127,7 @@ enum skl_disp_power_wells {
>   #define   DPLL_DVO_2X_MODE		(1 << 30)
>   #define   DPLL_EXT_BUFFER_ENABLE_VLV	(1 << 30)
>   #define   DPLL_SYNCLOCK_ENABLE		(1 << 29)
> -#define   DPLL_REFA_CLK_ENABLE_VLV	(1 << 29)
> +#define   DPLL_REF_CLK_ENABLE_VLV	(1 << 29)
>   #define   DPLL_VGA_MODE_DIS		(1 << 28)
>   #define   DPLLB_MODE_DAC_SERIAL		(1 << 26) /* i915 */
>   #define   DPLLB_MODE_LVDS		(2 << 26) /* i915 */
> @@ -2141,8 +2141,8 @@ enum skl_disp_power_wells {
>   #define   DPLL_FPA01_P1_POST_DIV_MASK_PINEVIEW	0x00ff8000 /* Pineview */
>   #define   DPLL_LOCK_VLV			(1<<15)
>   #define   DPLL_INTEGRATED_CRI_CLK_VLV	(1<<14)
> -#define   DPLL_INTEGRATED_CLOCK_VLV	(1<<13)
> -#define   DPLL_SSC_REF_CLOCK_CHV	(1<<13)
> +#define   DPLL_INTEGRATED_REF_CLK_VLV	(1<<13)
> +#define   DPLL_SSC_REF_CLK_CHV		(1<<13)
>   #define   DPLL_PORTC_READY_MASK		(0xf << 4)
>   #define   DPLL_PORTB_READY_MASK		(0xf)
>   
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dd3b649..0870532 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1791,7 +1791,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   	 */
>   	val = DPLL_VGA_MODE_DIS;
>   	if (pipe == PIPE_B)
> -		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
> +		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REF_CLK_ENABLE_VLV;
>   	I915_WRITE(DPLL(pipe), val);
>   	POSTING_READ(DPLL(pipe));
>   
> @@ -1806,8 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   	assert_pipe_disabled(dev_priv, pipe);
>   
>   	/* Set PLL en = 0 */
> -	val = DPLL_SSC_REF_CLOCK_CHV |
> -		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> +	val = DPLL_SSC_REF_CLK_CHV |
> +		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>   	if (pipe != PIPE_A)
>   		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
>   	I915_WRITE(DPLL(pipe), val);
> @@ -7198,8 +7198,8 @@ static void vlv_update_pll(struct intel_crtc *crtc,
>   	 * clock for pipe B, since VGA hotplug / manual detection depends
>   	 * on it.
>   	 */
> -	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV |
> -		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV;
> +	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REF_CLK_ENABLE_VLV |
> +		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_REF_CLK_VLV;
>   	/* We should never disable this, set it here for state tracking */
>   	if (crtc->pipe == PIPE_B)
>   		dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> @@ -7305,8 +7305,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>   static void chv_update_pll(struct intel_crtc *crtc,
>   			   struct intel_crtc_state *pipe_config)
>   {
> -	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
> -		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
> +	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV |
> +		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
>   		DPLL_VCO_ENABLE;
>   	if (crtc->pipe != PIPE_A)
>   		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index e087360..36e2148 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -422,12 +422,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>   	/* Disable DPOunit clock gating, can stall pipe
>   	 * and we need DPLL REFA always enabled */
>   	tmp = I915_READ(DPLL(pipe));
> -	tmp |= DPLL_REFA_CLK_ENABLE_VLV;
> +	tmp |= DPLL_REF_CLK_ENABLE_VLV;
>   	I915_WRITE(DPLL(pipe), tmp);
>   
>   	/* update the hw state for DPLL */
> -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> -		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> +	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> +		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>   
>   	tmp = I915_READ(DSPCLK_GATE_D);
>   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index f0e6f49..932d963 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -883,7 +883,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	 * hotplug / manual detection.
>   	 */
>   	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> -		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> +		   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
>   
>   	vlv_set_power_well(dev_priv, power_well, true);
> @@ -934,13 +934,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
>   		phy = DPIO_PHY0;
>   		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> -			   DPLL_REFA_CLK_ENABLE_VLV);
> +			   DPLL_REF_CLK_ENABLE_VLV);
>   		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> -			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> +			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	} else {
>   		phy = DPIO_PHY1;
>   		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
> -			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> +			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	}
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
>   	vlv_set_power_well(dev_priv, power_well, true);

-- 
regards,
Sivakumar

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

[PATCH 3/9] drm/i915: Simplify CHV pipe A power well code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The pipe A power well is the "disp2d" well on CHV and pipe B and C wells
don't even exist. Thereforce we can remove the checks for pipe A vs.
others and just assume it's always pipe A.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 47 ++++++++++++++-------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 932d963..1bd947a 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1042,53 +1042,46 @@ out:
 static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
 					struct i915_power_well *power_well)
 {
+	WARN_ON_ONCE(power_well->data != PIPE_A);
+
 	chv_set_pipe_power_well(dev_priv, power_well, power_well->count > 0);
 }
 
 static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
 				       struct i915_power_well *power_well)
 {
-	WARN_ON_ONCE(power_well->data != PIPE_A &&
-		     power_well->data != PIPE_B &&
-		     power_well->data != PIPE_C);
+	WARN_ON_ONCE(power_well->data != PIPE_A);
 
 	chv_set_pipe_power_well(dev_priv, power_well, true);
 
-	if (power_well->data == PIPE_A) {
-		spin_lock_irq(&dev_priv->irq_lock);
-		valleyview_enable_display_irqs(dev_priv);
-		spin_unlock_irq(&dev_priv->irq_lock);
+	spin_lock_irq(&dev_priv->irq_lock);
+	valleyview_enable_display_irqs(dev_priv);
+	spin_unlock_irq(&dev_priv->irq_lock);
 
-		/*
-		 * During driver initialization/resume we can avoid restoring the
-		 * part of the HW/SW state that will be inited anyway explicitly.
-		 */
-		if (dev_priv->power_domains.initializing)
-			return;
+	/*
+	 * During driver initialization/resume we can avoid restoring the
+	 * part of the HW/SW state that will be inited anyway explicitly.
+	 */
+	if (dev_priv->power_domains.initializing)
+		return;
 
-		intel_hpd_init(dev_priv);
+	intel_hpd_init(dev_priv);
 
-		i915_redisable_vga_power_on(dev_priv->dev);
-	}
+	i915_redisable_vga_power_on(dev_priv->dev);
 }
 
 static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
 					struct i915_power_well *power_well)
 {
-	WARN_ON_ONCE(power_well->data != PIPE_A &&
-		     power_well->data != PIPE_B &&
-		     power_well->data != PIPE_C);
-
-	if (power_well->data == PIPE_A) {
-		spin_lock_irq(&dev_priv->irq_lock);
-		valleyview_disable_display_irqs(dev_priv);
-		spin_unlock_irq(&dev_priv->irq_lock);
-	}
+	WARN_ON_ONCE(power_well->data != PIPE_A);
+
+	spin_lock_irq(&dev_priv->irq_lock);
+	valleyview_disable_display_irqs(dev_priv);
+	spin_unlock_irq(&dev_priv->irq_lock);
 
 	chv_set_pipe_power_well(dev_priv, power_well, false);
 
-	if (power_well->data == PIPE_A)
-		vlv_power_sequencer_reset(dev_priv);
+	vlv_power_sequencer_reset(dev_priv);
 }
 
 /**
-- 
2.3.6

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

Re: [PATCH 3/9] drm/i915: Simplify CHV pipe A power well code

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 3354 bytes --]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>


On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The pipe A power well is the "disp2d" well on CHV and pipe B and C wells
> don't even exist. Thereforce we can remove the checks for pipe A vs.
> others and just assume it's always pipe A.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 47 ++++++++++++++-------------------
>   1 file changed, 20 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 932d963..1bd947a 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1042,53 +1042,46 @@ out:
>   static void chv_pipe_power_well_sync_hw(struct drm_i915_private *dev_priv,
>   					struct i915_power_well *power_well)
>   {
> +	WARN_ON_ONCE(power_well->data != PIPE_A);
> +
>   	chv_set_pipe_power_well(dev_priv, power_well, power_well->count > 0);
>   }
>   
>   static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
>   				       struct i915_power_well *power_well)
>   {
> -	WARN_ON_ONCE(power_well->data != PIPE_A &&
> -		     power_well->data != PIPE_B &&
> -		     power_well->data != PIPE_C);
> +	WARN_ON_ONCE(power_well->data != PIPE_A);
>   
>   	chv_set_pipe_power_well(dev_priv, power_well, true);
>   
> -	if (power_well->data == PIPE_A) {
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		valleyview_enable_display_irqs(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	valleyview_enable_display_irqs(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>   
> -		/*
> -		 * During driver initialization/resume we can avoid restoring the
> -		 * part of the HW/SW state that will be inited anyway explicitly.
> -		 */
> -		if (dev_priv->power_domains.initializing)
> -			return;
> +	/*
> +	 * During driver initialization/resume we can avoid restoring the
> +	 * part of the HW/SW state that will be inited anyway explicitly.
> +	 */
> +	if (dev_priv->power_domains.initializing)
> +		return;
>   
> -		intel_hpd_init(dev_priv);
> +	intel_hpd_init(dev_priv);
>   
> -		i915_redisable_vga_power_on(dev_priv->dev);
> -	}
> +	i915_redisable_vga_power_on(dev_priv->dev);
>   }
>   
>   static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
>   					struct i915_power_well *power_well)
>   {
> -	WARN_ON_ONCE(power_well->data != PIPE_A &&
> -		     power_well->data != PIPE_B &&
> -		     power_well->data != PIPE_C);
> -
> -	if (power_well->data == PIPE_A) {
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		valleyview_disable_display_irqs(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> -	}
> +	WARN_ON_ONCE(power_well->data != PIPE_A);
> +
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	valleyview_disable_display_irqs(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>   
>   	chv_set_pipe_power_well(dev_priv, power_well, false);
>   
> -	if (power_well->data == PIPE_A)
> -		vlv_power_sequencer_reset(dev_priv);
> +	vlv_power_sequencer_reset(dev_priv);
>   }
>   
>   /**

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 4227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 4/9] drm/i915: Refactor VLV display power well init/deinit

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We do the exact same steps around the disp2d/pipe A power well
enable/disable on VLV and CHV. Refactor the shared code into
some helpers.

Note that this means we now call vlv_power_sequencer_reset() before
turning off the power well, whereas before we did it after. That
doesn't matter though since vlv_power_sequencer_reset() just resets
the power sequencer software tracking and doesn't touch the hardware
at all.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 52 +++++++++++++++------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1bd947a..6393b76 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -835,12 +835,8 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
 	return enabled;
 }
 
-static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
-					  struct i915_power_well *power_well)
+static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 {
-	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
-
-	vlv_set_power_well(dev_priv, power_well, true);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	valleyview_enable_display_irqs(dev_priv);
@@ -858,18 +854,33 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
 	i915_redisable_vga_power_on(dev_priv->dev);
 }
 
+static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->irq_lock);
+	valleyview_disable_display_irqs(dev_priv);
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	vlv_power_sequencer_reset(dev_priv);
+}
+
+static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
+					  struct i915_power_well *power_well)
+{
+	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
+
+	vlv_set_power_well(dev_priv, power_well, true);
+
+	vlv_display_power_well_init(dev_priv);
+}
+
 static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
 					   struct i915_power_well *power_well)
 {
 	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	valleyview_disable_display_irqs(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+	vlv_display_power_well_deinit(dev_priv);
 
 	vlv_set_power_well(dev_priv, power_well, false);
-
-	vlv_power_sequencer_reset(dev_priv);
 }
 
 static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
@@ -1054,20 +1065,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
 
 	chv_set_pipe_power_well(dev_priv, power_well, true);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	valleyview_enable_display_irqs(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
-
-	/*
-	 * During driver initialization/resume we can avoid restoring the
-	 * part of the HW/SW state that will be inited anyway explicitly.
-	 */
-	if (dev_priv->power_domains.initializing)
-		return;
-
-	intel_hpd_init(dev_priv);
-
-	i915_redisable_vga_power_on(dev_priv->dev);
+	vlv_display_power_well_init(dev_priv);
 }
 
 static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
@@ -1075,13 +1073,9 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->data != PIPE_A);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	valleyview_disable_display_irqs(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+	vlv_display_power_well_deinit(dev_priv);
 
 	chv_set_pipe_power_well(dev_priv, power_well, false);
-
-	vlv_power_sequencer_reset(dev_priv);
 }
 
 /**
-- 
2.3.6

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

Re: [PATCH 4/9] drm/i915: Refactor VLV display power well init/deinit

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 4254 bytes --]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>


On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We do the exact same steps around the disp2d/pipe A power well
> enable/disable on VLV and CHV. Refactor the shared code into
> some helpers.
>
> Note that this means we now call vlv_power_sequencer_reset() before
> turning off the power well, whereas before we did it after. That
> doesn't matter though since vlv_power_sequencer_reset() just resets
> the power sequencer software tracking and doesn't touch the hardware
> at all.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 52 +++++++++++++++------------------
>   1 file changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1bd947a..6393b76 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -835,12 +835,8 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
>   	return enabled;
>   }
>   
> -static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> -					  struct i915_power_well *power_well)
> +static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>   {
> -	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
> -
> -	vlv_set_power_well(dev_priv, power_well, true);
>   
>   	spin_lock_irq(&dev_priv->irq_lock);
>   	valleyview_enable_display_irqs(dev_priv);
> @@ -858,18 +854,33 @@ static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
>   	i915_redisable_vga_power_on(dev_priv->dev);
>   }
>   
> +static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> +{
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	valleyview_disable_display_irqs(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +
> +	vlv_power_sequencer_reset(dev_priv);
> +}
> +
> +static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
> +
> +	vlv_set_power_well(dev_priv, power_well, true);
> +
> +	vlv_display_power_well_init(dev_priv);
> +}
> +
>   static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
>   					   struct i915_power_well *power_well)
>   {
>   	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DISP2D);
>   
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	valleyview_disable_display_irqs(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> +	vlv_display_power_well_deinit(dev_priv);
>   
>   	vlv_set_power_well(dev_priv, power_well, false);
> -
> -	vlv_power_sequencer_reset(dev_priv);
>   }
>   
>   static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> @@ -1054,20 +1065,7 @@ static void chv_pipe_power_well_enable(struct drm_i915_private *dev_priv,
>   
>   	chv_set_pipe_power_well(dev_priv, power_well, true);
>   
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	valleyview_enable_display_irqs(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> -	/*
> -	 * During driver initialization/resume we can avoid restoring the
> -	 * part of the HW/SW state that will be inited anyway explicitly.
> -	 */
> -	if (dev_priv->power_domains.initializing)
> -		return;
> -
> -	intel_hpd_init(dev_priv);
> -
> -	i915_redisable_vga_power_on(dev_priv->dev);
> +	vlv_display_power_well_init(dev_priv);
>   }
>   
>   static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -1075,13 +1073,9 @@ static void chv_pipe_power_well_disable(struct drm_i915_private *dev_priv,
>   {
>   	WARN_ON_ONCE(power_well->data != PIPE_A);
>   
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	valleyview_disable_display_irqs(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> +	vlv_display_power_well_deinit(dev_priv);
>   
>   	chv_set_pipe_power_well(dev_priv, power_well, false);
> -
> -	vlv_power_sequencer_reset(dev_priv);
>   }
>   
>   /**

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 5018 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
from the pipe_config in intel_dsi_get_config(). This avoids spurious
state checker warnings. We already did it this way for DPLL_MD, but do
it for DPLL too.

Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
are enabled however. Supposedly they have some meaning to DSI too.
We now keep the ref clocks always enabled while the disp2d well is
enabled.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 36e2148..92bb252 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	/* Disable DPOunit clock gating, can stall pipe
 	 * and we need DPLL REFA always enabled */
-	tmp = I915_READ(DPLL(pipe));
-	tmp |= DPLL_REF_CLK_ENABLE_VLV;
-	I915_WRITE(DPLL(pipe), tmp);
-
-	/* update the hw state for DPLL */
-	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
-		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
-
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
 	I915_WRITE(DSPCLK_GATE_D, tmp);
 
+	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
+
 	/* put device in ready state */
 	intel_dsi_device_ready(encoder);
 
@@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	DRM_DEBUG_KMS("\n");
 
 	/*
-	 * DPLL_MD is not used in case of DSI, reading will get some default value
-	 * set dpll_md = 0
+	 * DPLL is not used in case of DSI, reading will getsome default value.
+	 * Clear the state to keep the state checker happy.
 	 */
+	pipe_config->dpll_hw_state.dpll = 0;
 	pipe_config->dpll_hw_state.dpll_md = 0;
 
 	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
-- 
2.3.6

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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Daniel Vetter]

On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> from the pipe_config in intel_dsi_get_config(). This avoids spurious
> state checker warnings. We already did it this way for DPLL_MD, but do
> it for DPLL too.
> 
> Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> are enabled however. Supposedly they have some meaning to DSI too.
> We now keep the ref clocks always enabled while the disp2d well is
> enabled.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 36e2148..92bb252 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  
>  	/* Disable DPOunit clock gating, can stall pipe
>  	 * and we need DPLL REFA always enabled */
> -	tmp = I915_READ(DPLL(pipe));
> -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> -	I915_WRITE(DPLL(pipe), tmp);
> -
> -	/* update the hw state for DPLL */
> -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> -
>  	tmp = I915_READ(DSPCLK_GATE_D);
>  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>  	I915_WRITE(DSPCLK_GATE_D, tmp);
>  
> +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> +
>  	/* put device in ready state */
>  	intel_dsi_device_ready(encoder);
>  
> @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>  	DRM_DEBUG_KMS("\n");
>  
>  	/*
> -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> -	 * set dpll_md = 0
> +	 * DPLL is not used in case of DSI, reading will getsome default value.
> +	 * Clear the state to keep the state checker happy.
>  	 */
> +	pipe_config->dpll_hw_state.dpll = 0;
>  	pipe_config->dpll_hw_state.dpll_md = 0;

State configs are supposed to be kzallocated. Needing this indicates a
pretty serious bug - I'd vote to instead also ditch the dpll_md line and
fix the offender.
-Daniel

>  
>  	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> -- 
> 2.3.6
> 
> _______________________________________________
> 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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Ville Syrjälä]

On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> > from the pipe_config in intel_dsi_get_config(). This avoids spurious
> > state checker warnings. We already did it this way for DPLL_MD, but do
> > it for DPLL too.
> > 
> > Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> > are enabled however. Supposedly they have some meaning to DSI too.
> > We now keep the ref clocks always enabled while the disp2d well is
> > enabled.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> >  1 file changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 36e2148..92bb252 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >  
> >  	/* Disable DPOunit clock gating, can stall pipe
> >  	 * and we need DPLL REFA always enabled */
> > -	tmp = I915_READ(DPLL(pipe));
> > -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> > -	I915_WRITE(DPLL(pipe), tmp);
> > -
> > -	/* update the hw state for DPLL */
> > -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> > -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> > -
> >  	tmp = I915_READ(DSPCLK_GATE_D);
> >  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> >  	I915_WRITE(DSPCLK_GATE_D, tmp);
> >  
> > +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> > +
> >  	/* put device in ready state */
> >  	intel_dsi_device_ready(encoder);
> >  
> > @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >  	DRM_DEBUG_KMS("\n");
> >  
> >  	/*
> > -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> > -	 * set dpll_md = 0
> > +	 * DPLL is not used in case of DSI, reading will getsome default value.
> > +	 * Clear the state to keep the state checker happy.
> >  	 */
> > +	pipe_config->dpll_hw_state.dpll = 0;
> >  	pipe_config->dpll_hw_state.dpll_md = 0;
> 
> State configs are supposed to be kzallocated. Needing this indicates a
> pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> fix the offender.

There is no offender really. We read out the DPLL state before we know
which ports are active and hence can't tell at that point if the
information is really relevant.

The alternative would be to explicitly program the DPLL to a specific
state for DSI. That is, we could just call {vlv,chv}_disable_pll()
from  crtc_enable when the pipe is driving a DSI port), and have
intel_dsi_compute_config() fill in an identical state.

> -Daniel
> 
> >  
> >  	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> > -- 
> > 2.3.6
> > 
> > _______________________________________________
> > 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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Ville Syrjälä]

On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
> On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> > On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> > > from the pipe_config in intel_dsi_get_config(). This avoids spurious
> > > state checker warnings. We already did it this way for DPLL_MD, but do
> > > it for DPLL too.
> > > 
> > > Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> > > are enabled however. Supposedly they have some meaning to DSI too.
> > > We now keep the ref clocks always enabled while the disp2d well is
> > > enabled.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> > >  1 file changed, 5 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > > index 36e2148..92bb252 100644
> > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> > >  
> > >  	/* Disable DPOunit clock gating, can stall pipe
> > >  	 * and we need DPLL REFA always enabled */
> > > -	tmp = I915_READ(DPLL(pipe));
> > > -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> > > -	I915_WRITE(DPLL(pipe), tmp);
> > > -
> > > -	/* update the hw state for DPLL */
> > > -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> > > -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> > > -
> > >  	tmp = I915_READ(DSPCLK_GATE_D);
> > >  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > >  	I915_WRITE(DSPCLK_GATE_D, tmp);
> > >  
> > > +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> > > +
> > >  	/* put device in ready state */
> > >  	intel_dsi_device_ready(encoder);
> > >  
> > > @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> > >  	DRM_DEBUG_KMS("\n");
> > >  
> > >  	/*
> > > -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> > > -	 * set dpll_md = 0
> > > +	 * DPLL is not used in case of DSI, reading will getsome default value.
> > > +	 * Clear the state to keep the state checker happy.
> > >  	 */
> > > +	pipe_config->dpll_hw_state.dpll = 0;
> > >  	pipe_config->dpll_hw_state.dpll_md = 0;
> > 
> > State configs are supposed to be kzallocated. Needing this indicates a
> > pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> > fix the offender.
> 
> There is no offender really. We read out the DPLL state before we know
> which ports are active and hence can't tell at that point if the
> information is really relevant.
> 
> The alternative would be to explicitly program the DPLL to a specific
> state for DSI. That is, we could just call {vlv,chv}_disable_pll()
> from  crtc_enable when the pipe is driving a DSI port), and have
> intel_dsi_compute_config() fill in an identical state.

Or I suppose we could move clock readout to the encoder
.get_config() entirely.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Daniel Vetter]

On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
> On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
> > On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> > > On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> > > > from the pipe_config in intel_dsi_get_config(). This avoids spurious
> > > > state checker warnings. We already did it this way for DPLL_MD, but do
> > > > it for DPLL too.
> > > > 
> > > > Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> > > > are enabled however. Supposedly they have some meaning to DSI too.
> > > > We now keep the ref clocks always enabled while the disp2d well is
> > > > enabled.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> > > >  1 file changed, 5 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > > > index 36e2148..92bb252 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> > > >  
> > > >  	/* Disable DPOunit clock gating, can stall pipe
> > > >  	 * and we need DPLL REFA always enabled */
> > > > -	tmp = I915_READ(DPLL(pipe));
> > > > -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> > > > -	I915_WRITE(DPLL(pipe), tmp);
> > > > -
> > > > -	/* update the hw state for DPLL */
> > > > -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> > > > -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> > > > -
> > > >  	tmp = I915_READ(DSPCLK_GATE_D);
> > > >  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > > >  	I915_WRITE(DSPCLK_GATE_D, tmp);
> > > >  
> > > > +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> > > > +
> > > >  	/* put device in ready state */
> > > >  	intel_dsi_device_ready(encoder);
> > > >  
> > > > @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> > > >  	DRM_DEBUG_KMS("\n");
> > > >  
> > > >  	/*
> > > > -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> > > > -	 * set dpll_md = 0
> > > > +	 * DPLL is not used in case of DSI, reading will getsome default value.
> > > > +	 * Clear the state to keep the state checker happy.
> > > >  	 */
> > > > +	pipe_config->dpll_hw_state.dpll = 0;
> > > >  	pipe_config->dpll_hw_state.dpll_md = 0;
> > > 
> > > State configs are supposed to be kzallocated. Needing this indicates a
> > > pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> > > fix the offender.
> > 
> > There is no offender really. We read out the DPLL state before we know
> > which ports are active and hence can't tell at that point if the
> > information is really relevant.

So the bios leaves the DPLL enabled even when using a DSI port? Or do we
miss to check some routing bits in get_clock?

> > The alternative would be to explicitly program the DPLL to a specific
> > state for DSI. That is, we could just call {vlv,chv}_disable_pll()
> > from  crtc_enable when the pipe is driving a DSI port), and have
> > intel_dsi_compute_config() fill in an identical state.
> 
> Or I suppose we could move clock readout to the encoder
> .get_config() entirely.

Yeah either that or do a crtc sanitize to fixup the DPLL that's somehow
enabled but not needed. Either option is imo clearer than cleaning out
state that we've read.
-Daniel
-- 
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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Ville Syrjälä]

On Tue, Jun 30, 2015 at 12:13:37PM +0200, Daniel Vetter wrote:
> On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
> > On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
> > > On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> > > > On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> > > > > from the pipe_config in intel_dsi_get_config(). This avoids spurious
> > > > > state checker warnings. We already did it this way for DPLL_MD, but do
> > > > > it for DPLL too.
> > > > > 
> > > > > Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> > > > > are enabled however. Supposedly they have some meaning to DSI too.
> > > > > We now keep the ref clocks always enabled while the disp2d well is
> > > > > enabled.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> > > > >  1 file changed, 5 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > > > > index 36e2148..92bb252 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > > @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> > > > >  
> > > > >  	/* Disable DPOunit clock gating, can stall pipe
> > > > >  	 * and we need DPLL REFA always enabled */
> > > > > -	tmp = I915_READ(DPLL(pipe));
> > > > > -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> > > > > -	I915_WRITE(DPLL(pipe), tmp);
> > > > > -
> > > > > -	/* update the hw state for DPLL */
> > > > > -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> > > > > -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> > > > > -
> > > > >  	tmp = I915_READ(DSPCLK_GATE_D);
> > > > >  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > > > >  	I915_WRITE(DSPCLK_GATE_D, tmp);
> > > > >  
> > > > > +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> > > > > +
> > > > >  	/* put device in ready state */
> > > > >  	intel_dsi_device_ready(encoder);
> > > > >  
> > > > > @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> > > > >  	DRM_DEBUG_KMS("\n");
> > > > >  
> > > > >  	/*
> > > > > -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> > > > > -	 * set dpll_md = 0
> > > > > +	 * DPLL is not used in case of DSI, reading will getsome default value.
> > > > > +	 * Clear the state to keep the state checker happy.
> > > > >  	 */
> > > > > +	pipe_config->dpll_hw_state.dpll = 0;
> > > > >  	pipe_config->dpll_hw_state.dpll_md = 0;
> > > > 
> > > > State configs are supposed to be kzallocated. Needing this indicates a
> > > > pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> > > > fix the offender.
> > > 
> > > There is no offender really. We read out the DPLL state before we know
> > > which ports are active and hence can't tell at that point if the
> > > information is really relevant.
> 
> So the bios leaves the DPLL enabled even when using a DSI port? Or do we
> miss to check some routing bits in get_clock?

Not necessarily enabled, but there are other bits in there that could be
left in any state basically. The DSI port simply doesn't care.

> > > The alternative would be to explicitly program the DPLL to a specific
> > > state for DSI. That is, we could just call {vlv,chv}_disable_pll()
> > > from  crtc_enable when the pipe is driving a DSI port), and have
> > > intel_dsi_compute_config() fill in an identical state.
> > 
> > Or I suppose we could move clock readout to the encoder
> > .get_config() entirely.
> 
> Yeah either that or do a crtc sanitize to fixup the DPLL that's somehow
> enabled but not needed.

To do that properly we should just expose all the PLLs as "shared" PLLs.
I don't really want to add a VLV/CHV specific hack for that.

> Either option is imo clearer than cleaning out
> state that we've read.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Daniel Vetter]

On Tue, Jun 30, 2015 at 02:50:33PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 30, 2015 at 12:13:37PM +0200, Daniel Vetter wrote:
> > On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
> > > On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
> > > > On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> > > > > On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > 
> > > > > > VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> > > > > > from the pipe_config in intel_dsi_get_config(). This avoids spurious
> > > > > > state checker warnings. We already did it this way for DPLL_MD, but do
> > > > > > it for DPLL too.
> > > > > > 
> > > > > > Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> > > > > > are enabled however. Supposedly they have some meaning to DSI too.
> > > > > > We now keep the ref clocks always enabled while the disp2d well is
> > > > > > enabled.
> > > > > > 
> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> > > > > >  1 file changed, 5 insertions(+), 10 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > > > > > index 36e2148..92bb252 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > > > @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> > > > > >  
> > > > > >  	/* Disable DPOunit clock gating, can stall pipe
> > > > > >  	 * and we need DPLL REFA always enabled */
> > > > > > -	tmp = I915_READ(DPLL(pipe));
> > > > > > -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> > > > > > -	I915_WRITE(DPLL(pipe), tmp);
> > > > > > -
> > > > > > -	/* update the hw state for DPLL */
> > > > > > -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> > > > > > -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> > > > > > -
> > > > > >  	tmp = I915_READ(DSPCLK_GATE_D);
> > > > > >  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > > > > >  	I915_WRITE(DSPCLK_GATE_D, tmp);
> > > > > >  
> > > > > > +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> > > > > > +
> > > > > >  	/* put device in ready state */
> > > > > >  	intel_dsi_device_ready(encoder);
> > > > > >  
> > > > > > @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> > > > > >  	DRM_DEBUG_KMS("\n");
> > > > > >  
> > > > > >  	/*
> > > > > > -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> > > > > > -	 * set dpll_md = 0
> > > > > > +	 * DPLL is not used in case of DSI, reading will getsome default value.
> > > > > > +	 * Clear the state to keep the state checker happy.
> > > > > >  	 */
> > > > > > +	pipe_config->dpll_hw_state.dpll = 0;
> > > > > >  	pipe_config->dpll_hw_state.dpll_md = 0;
> > > > > 
> > > > > State configs are supposed to be kzallocated. Needing this indicates a
> > > > > pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> > > > > fix the offender.
> > > > 
> > > > There is no offender really. We read out the DPLL state before we know
> > > > which ports are active and hence can't tell at that point if the
> > > > information is really relevant.
> > 
> > So the bios leaves the DPLL enabled even when using a DSI port? Or do we
> > miss to check some routing bits in get_clock?
> 
> Not necessarily enabled, but there are other bits in there that could be
> left in any state basically. The DSI port simply doesn't care.

If the enable bit is what's gating things here then we should just forgo
reading out any dpll register state if that's not set. Looking at the
vlv/chv state readout code that seems to be the trouble - there's nothing
guarding the register reads into the pipe_config at all. Didn't matter
pre-vlv without dsi since enable pipe should imply enabled dpll, but
obviously won't work correctly with dsi. Can you please spin such a patch
and remove the hacks here from dsi_get_config?
-Daniel
-- 
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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Sivakumar Thulasimani]

On 7/1/2015 6:12 PM, Daniel Vetter wrote:
> On Tue, Jun 30, 2015 at 02:50:33PM +0300, Ville Syrjälä wrote:
>> On Tue, Jun 30, 2015 at 12:13:37PM +0200, Daniel Vetter wrote:
>>> On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
>>>> On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
>>>>> On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
>>>>>> On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>
>>>>>>> VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
>>>>>>> from the pipe_config in intel_dsi_get_config(). This avoids spurious
>>>>>>> state checker warnings. We already did it this way for DPLL_MD, but do
>>>>>>> it for DPLL too.
>>>>>>>
>>>>>>> Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
>>>>>>> are enabled however. Supposedly they have some meaning to DSI too.
>>>>>>> We now keep the ref clocks always enabled while the disp2d well is
>>>>>>> enabled.
>>>>>>>
>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>> ---
>>>>>>>   drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
>>>>>>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>>>>>> index 36e2148..92bb252 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>>>>>> @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>>>>>>>   
>>>>>>>   	/* Disable DPOunit clock gating, can stall pipe
>>>>>>>   	 * and we need DPLL REFA always enabled */
>>>>>>> -	tmp = I915_READ(DPLL(pipe));
>>>>>>> -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
>>>>>>> -	I915_WRITE(DPLL(pipe), tmp);
>>>>>>> -
>>>>>>> -	/* update the hw state for DPLL */
>>>>>>> -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
>>>>>>> -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>>>>>>> -
>>>>>>>   	tmp = I915_READ(DSPCLK_GATE_D);
>>>>>>>   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>>>>>>>   	I915_WRITE(DSPCLK_GATE_D, tmp);
>>>>>>>   
>>>>>>> +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
>>>>>>> +
>>>>>>>   	/* put device in ready state */
>>>>>>>   	intel_dsi_device_ready(encoder);
>>>>>>>   
>>>>>>> @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>>>>>>>   	DRM_DEBUG_KMS("\n");
>>>>>>>   
>>>>>>>   	/*
>>>>>>> -	 * DPLL_MD is not used in case of DSI, reading will get some default value
>>>>>>> -	 * set dpll_md = 0
>>>>>>> +	 * DPLL is not used in case of DSI, reading will getsome default value.
>>>>>>> +	 * Clear the state to keep the state checker happy.
>>>>>>>   	 */
>>>>>>> +	pipe_config->dpll_hw_state.dpll = 0;
>>>>>>>   	pipe_config->dpll_hw_state.dpll_md = 0;
>>>>>> State configs are supposed to be kzallocated. Needing this indicates a
>>>>>> pretty serious bug - I'd vote to instead also ditch the dpll_md line and
>>>>>> fix the offender.
>>>>> There is no offender really. We read out the DPLL state before we know
>>>>> which ports are active and hence can't tell at that point if the
>>>>> information is really relevant.
>>> So the bios leaves the DPLL enabled even when using a DSI port? Or do we
>>> miss to check some routing bits in get_clock?
>> Not necessarily enabled, but there are other bits in there that could be
>> left in any state basically. The DSI port simply doesn't care.
> If the enable bit is what's gating things here then we should just forgo
> reading out any dpll register state if that's not set. Looking at the
> vlv/chv state readout code that seems to be the trouble - there's nothing
> guarding the register reads into the pipe_config at all. Didn't matter
> pre-vlv without dsi since enable pipe should imply enabled dpll, but
> obviously won't work correctly with dsi. Can you please spin such a patch
> and remove the hacks here from dsi_get_config?
> -Daniel
Not sure i understand the point of contention here, just noticed this 
after i gave my RB :)
so my justification on why this is proper is that DSI is not supposed to 
touch DPLL register
any place we access dpll_hw_state.dpll is under !is_dsi check so that 
ensures that we dont
program DPLL register for dsi panel. it was wrong to have originally 
modfied DPLL register
inside intel_dsi_pre_enable so removal is fine. setting it to zero in 
intel_dsi_get_config
is of no impact since any place we write back the contents of 
dpll_hw_state is past the
!is_dsi check is never consumed by anyone as long as the CRTC uses DSI.

-- 
regards,
Sivakumar

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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Daniel Vetter]

On Fri, Jul 10, 2015 at 05:37:07PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 7/1/2015 6:12 PM, Daniel Vetter wrote:
> >On Tue, Jun 30, 2015 at 02:50:33PM +0300, Ville Syrjälä wrote:
> >>On Tue, Jun 30, 2015 at 12:13:37PM +0200, Daniel Vetter wrote:
> >>>On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
> >>>>On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
> >>>>>On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> >>>>>>On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> >>>>>>>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>>
> >>>>>>>VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> >>>>>>>from the pipe_config in intel_dsi_get_config(). This avoids spurious
> >>>>>>>state checker warnings. We already did it this way for DPLL_MD, but do
> >>>>>>>it for DPLL too.
> >>>>>>>
> >>>>>>>Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> >>>>>>>are enabled however. Supposedly they have some meaning to DSI too.
> >>>>>>>We now keep the ref clocks always enabled while the disp2d well is
> >>>>>>>enabled.
> >>>>>>>
> >>>>>>>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>>---
> >>>>>>>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> >>>>>>>  1 file changed, 5 insertions(+), 10 deletions(-)
> >>>>>>>
> >>>>>>>diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >>>>>>>index 36e2148..92bb252 100644
> >>>>>>>--- a/drivers/gpu/drm/i915/intel_dsi.c
> >>>>>>>+++ b/drivers/gpu/drm/i915/intel_dsi.c
> >>>>>>>@@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >>>>>>>  	/* Disable DPOunit clock gating, can stall pipe
> >>>>>>>  	 * and we need DPLL REFA always enabled */
> >>>>>>>-	tmp = I915_READ(DPLL(pipe));
> >>>>>>>-	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> >>>>>>>-	I915_WRITE(DPLL(pipe), tmp);
> >>>>>>>-
> >>>>>>>-	/* update the hw state for DPLL */
> >>>>>>>-	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> >>>>>>>-		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >>>>>>>-
> >>>>>>>  	tmp = I915_READ(DSPCLK_GATE_D);
> >>>>>>>  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> >>>>>>>  	I915_WRITE(DSPCLK_GATE_D, tmp);
> >>>>>>>+	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> >>>>>>>+
> >>>>>>>  	/* put device in ready state */
> >>>>>>>  	intel_dsi_device_ready(encoder);
> >>>>>>>@@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >>>>>>>  	DRM_DEBUG_KMS("\n");
> >>>>>>>  	/*
> >>>>>>>-	 * DPLL_MD is not used in case of DSI, reading will get some default value
> >>>>>>>-	 * set dpll_md = 0
> >>>>>>>+	 * DPLL is not used in case of DSI, reading will getsome default value.
> >>>>>>>+	 * Clear the state to keep the state checker happy.
> >>>>>>>  	 */
> >>>>>>>+	pipe_config->dpll_hw_state.dpll = 0;
> >>>>>>>  	pipe_config->dpll_hw_state.dpll_md = 0;
> >>>>>>State configs are supposed to be kzallocated. Needing this indicates a
> >>>>>>pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> >>>>>>fix the offender.
> >>>>>There is no offender really. We read out the DPLL state before we know
> >>>>>which ports are active and hence can't tell at that point if the
> >>>>>information is really relevant.
> >>>So the bios leaves the DPLL enabled even when using a DSI port? Or do we
> >>>miss to check some routing bits in get_clock?
> >>Not necessarily enabled, but there are other bits in there that could be
> >>left in any state basically. The DSI port simply doesn't care.
> >If the enable bit is what's gating things here then we should just forgo
> >reading out any dpll register state if that's not set. Looking at the
> >vlv/chv state readout code that seems to be the trouble - there's nothing
> >guarding the register reads into the pipe_config at all. Didn't matter
> >pre-vlv without dsi since enable pipe should imply enabled dpll, but
> >obviously won't work correctly with dsi. Can you please spin such a patch
> >and remove the hacks here from dsi_get_config?
> >-Daniel
> Not sure i understand the point of contention here, just noticed this after
> i gave my RB :)
> so my justification on why this is proper is that DSI is not supposed to
> touch DPLL register
> any place we access dpll_hw_state.dpll is under !is_dsi check so that
> ensures that we dont
> program DPLL register for dsi panel. it was wrong to have originally modfied
> DPLL register
> inside intel_dsi_pre_enable so removal is fine. setting it to zero in
> intel_dsi_get_config
> is of no impact since any place we write back the contents of dpll_hw_state
> is past the
> !is_dsi check is never consumed by anyone as long as the CRTC uses DSI.

Ok, let's try a patch. Does the below work y/n? I've merged the patches up
to this one to dinq meanwhile, but I'd really like to close this first. If
it works I can rebase this patch here myself. Diff below is based on
-nightly with the above patch, so if you want to test on top of it you
also have to remove the dpll = 0; line too ofc.

Thanks, Daniel

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a7482ab140e1..c770655f5612 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8079,6 +8079,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 
 	i9xx_get_pfit_config(crtc, pipe_config);
 
+	tmp = I915_READ(DPLL(crtc->pipe));
+	if (!(tmp & DPLL_VCO_ENABLE))
+		return true;
+
 	if (INTEL_INFO(dev)->gen >= 4) {
 		tmp = I915_READ(DPLL_MD(crtc->pipe));
 		pipe_config->pixel_multiplier =
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index f4438eb5b458..116b06632cb0 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -621,12 +621,6 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
-	/*
-	 * DPLL_MD is not used in case of DSI, reading will get some default value
-	 * set dpll_md = 0
-	 */
-	pipe_config->dpll_hw_state.dpll_md = 0;
-
 	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
 	if (!pclk)
 		return;
-- 
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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Sivakumar Thulasimani]

On 7/13/2015 2:21 PM, Daniel Vetter wrote:
> On Fri, Jul 10, 2015 at 05:37:07PM +0530, Sivakumar Thulasimani wrote:
>>
>> On 7/1/2015 6:12 PM, Daniel Vetter wrote:
>>> On Tue, Jun 30, 2015 at 02:50:33PM +0300, Ville Syrjälä wrote:
>>>> On Tue, Jun 30, 2015 at 12:13:37PM +0200, Daniel Vetter wrote:
>>>>> On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
>>>>>> On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
>>>>>>> On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
>>>>>>>> On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
>>>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>>>
>>>>>>>>> VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
>>>>>>>> >from the pipe_config in intel_dsi_get_config(). This avoids spurious
>>>>>>>>> state checker warnings. We already did it this way for DPLL_MD, but do
>>>>>>>>> it for DPLL too.
>>>>>>>>>
>>>>>>>>> Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
>>>>>>>>> are enabled however. Supposedly they have some meaning to DSI too.
>>>>>>>>> We now keep the ref clocks always enabled while the disp2d well is
>>>>>>>>> enabled.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>>> ---
>>>>>>>>>   drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
>>>>>>>>>   1 file changed, 5 insertions(+), 10 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>>>>>>>> index 36e2148..92bb252 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>>>>>>>> @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>>>>>>>>>   	/* Disable DPOunit clock gating, can stall pipe
>>>>>>>>>   	 * and we need DPLL REFA always enabled */
>>>>>>>>> -	tmp = I915_READ(DPLL(pipe));
>>>>>>>>> -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
>>>>>>>>> -	I915_WRITE(DPLL(pipe), tmp);
>>>>>>>>> -
>>>>>>>>> -	/* update the hw state for DPLL */
>>>>>>>>> -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
>>>>>>>>> -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>>>>>>>>> -
>>>>>>>>>   	tmp = I915_READ(DSPCLK_GATE_D);
>>>>>>>>>   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>>>>>>>>>   	I915_WRITE(DSPCLK_GATE_D, tmp);
>>>>>>>>> +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
>>>>>>>>> +
>>>>>>>>>   	/* put device in ready state */
>>>>>>>>>   	intel_dsi_device_ready(encoder);
>>>>>>>>> @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>>>>>>>>>   	DRM_DEBUG_KMS("\n");
>>>>>>>>>   	/*
>>>>>>>>> -	 * DPLL_MD is not used in case of DSI, reading will get some default value
>>>>>>>>> -	 * set dpll_md = 0
>>>>>>>>> +	 * DPLL is not used in case of DSI, reading will getsome default value.
>>>>>>>>> +	 * Clear the state to keep the state checker happy.
>>>>>>>>>   	 */
>>>>>>>>> +	pipe_config->dpll_hw_state.dpll = 0;
>>>>>>>>>   	pipe_config->dpll_hw_state.dpll_md = 0;
>>>>>>>> State configs are supposed to be kzallocated. Needing this indicates a
>>>>>>>> pretty serious bug - I'd vote to instead also ditch the dpll_md line and
>>>>>>>> fix the offender.
>>>>>>> There is no offender really. We read out the DPLL state before we know
>>>>>>> which ports are active and hence can't tell at that point if the
>>>>>>> information is really relevant.
>>>>> So the bios leaves the DPLL enabled even when using a DSI port? Or do we
>>>>> miss to check some routing bits in get_clock?
>>>> Not necessarily enabled, but there are other bits in there that could be
>>>> left in any state basically. The DSI port simply doesn't care.
>>> If the enable bit is what's gating things here then we should just forgo
>>> reading out any dpll register state if that's not set. Looking at the
>>> vlv/chv state readout code that seems to be the trouble - there's nothing
>>> guarding the register reads into the pipe_config at all. Didn't matter
>>> pre-vlv without dsi since enable pipe should imply enabled dpll, but
>>> obviously won't work correctly with dsi. Can you please spin such a patch
>>> and remove the hacks here from dsi_get_config?
>>> -Daniel
>> Not sure i understand the point of contention here, just noticed this after
>> i gave my RB :)
>> so my justification on why this is proper is that DSI is not supposed to
>> touch DPLL register
>> any place we access dpll_hw_state.dpll is under !is_dsi check so that
>> ensures that we dont
>> program DPLL register for dsi panel. it was wrong to have originally modfied
>> DPLL register
>> inside intel_dsi_pre_enable so removal is fine. setting it to zero in
>> intel_dsi_get_config
>> is of no impact since any place we write back the contents of dpll_hw_state
>> is past the
>> !is_dsi check is never consumed by anyone as long as the CRTC uses DSI.
> Ok, let's try a patch. Does the below work y/n? I've merged the patches up
> to this one to dinq meanwhile, but I'd really like to close this first. If
> it works I can rebase this patch here myself. Diff below is based on
> -nightly with the above patch, so if you want to test on top of it you
> also have to remove the dpll = 0; line too ofc.
>
> Thanks, Daniel
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a7482ab140e1..c770655f5612 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8079,6 +8079,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>   
>   	i9xx_get_pfit_config(crtc, pipe_config);
>   
> +	tmp = I915_READ(DPLL(crtc->pipe));
> +	if (!(tmp & DPLL_VCO_ENABLE))
> +		return true;
> +
>   	if (INTEL_INFO(dev)->gen >= 4) {
>   		tmp = I915_READ(DPLL_MD(crtc->pipe));
>   		pipe_config->pixel_multiplier =
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index f4438eb5b458..116b06632cb0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -621,12 +621,6 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>   	u32 pclk;
>   	DRM_DEBUG_KMS("\n");
>   
> -	/*
> -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> -	 * set dpll_md = 0
> -	 */
> -	pipe_config->dpll_hw_state.dpll_md = 0;
> -
>   	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
>   	if (!pclk)
>   		return;
i am fine with this patch.

-- 
regards,
Sivakumar

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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Daniel Vetter]

On Mon, Jul 13, 2015 at 03:49:44PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 7/13/2015 2:21 PM, Daniel Vetter wrote:
> >On Fri, Jul 10, 2015 at 05:37:07PM +0530, Sivakumar Thulasimani wrote:
> >>
> >>On 7/1/2015 6:12 PM, Daniel Vetter wrote:
> >>>On Tue, Jun 30, 2015 at 02:50:33PM +0300, Ville Syrjälä wrote:
> >>>>On Tue, Jun 30, 2015 at 12:13:37PM +0200, Daniel Vetter wrote:
> >>>>>On Mon, Jun 29, 2015 at 08:08:27PM +0300, Ville Syrjälä wrote:
> >>>>>>On Mon, Jun 29, 2015 at 07:56:05PM +0300, Ville Syrjälä wrote:
> >>>>>>>On Mon, Jun 29, 2015 at 06:42:11PM +0200, Daniel Vetter wrote:
> >>>>>>>>On Mon, Jun 29, 2015 at 03:25:52PM +0300, ville.syrjala@linux.intel.com wrote:
> >>>>>>>>>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>>>>
> >>>>>>>>>VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> >>>>>>>>>from the pipe_config in intel_dsi_get_config(). This avoids spurious
> >>>>>>>>>state checker warnings. We already did it this way for DPLL_MD, but do
> >>>>>>>>>it for DPLL too.
> >>>>>>>>>
> >>>>>>>>>Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> >>>>>>>>>are enabled however. Supposedly they have some meaning to DSI too.
> >>>>>>>>>We now keep the ref clocks always enabled while the disp2d well is
> >>>>>>>>>enabled.
> >>>>>>>>>
> >>>>>>>>>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>>>>---
> >>>>>>>>>  drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
> >>>>>>>>>  1 file changed, 5 insertions(+), 10 deletions(-)
> >>>>>>>>>
> >>>>>>>>>diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >>>>>>>>>index 36e2148..92bb252 100644
> >>>>>>>>>--- a/drivers/gpu/drm/i915/intel_dsi.c
> >>>>>>>>>+++ b/drivers/gpu/drm/i915/intel_dsi.c
> >>>>>>>>>@@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >>>>>>>>>  	/* Disable DPOunit clock gating, can stall pipe
> >>>>>>>>>  	 * and we need DPLL REFA always enabled */
> >>>>>>>>>-	tmp = I915_READ(DPLL(pipe));
> >>>>>>>>>-	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> >>>>>>>>>-	I915_WRITE(DPLL(pipe), tmp);
> >>>>>>>>>-
> >>>>>>>>>-	/* update the hw state for DPLL */
> >>>>>>>>>-	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> >>>>>>>>>-		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >>>>>>>>>-
> >>>>>>>>>  	tmp = I915_READ(DSPCLK_GATE_D);
> >>>>>>>>>  	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> >>>>>>>>>  	I915_WRITE(DSPCLK_GATE_D, tmp);
> >>>>>>>>>+	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> >>>>>>>>>+
> >>>>>>>>>  	/* put device in ready state */
> >>>>>>>>>  	intel_dsi_device_ready(encoder);
> >>>>>>>>>@@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >>>>>>>>>  	DRM_DEBUG_KMS("\n");
> >>>>>>>>>  	/*
> >>>>>>>>>-	 * DPLL_MD is not used in case of DSI, reading will get some default value
> >>>>>>>>>-	 * set dpll_md = 0
> >>>>>>>>>+	 * DPLL is not used in case of DSI, reading will getsome default value.
> >>>>>>>>>+	 * Clear the state to keep the state checker happy.
> >>>>>>>>>  	 */
> >>>>>>>>>+	pipe_config->dpll_hw_state.dpll = 0;
> >>>>>>>>>  	pipe_config->dpll_hw_state.dpll_md = 0;
> >>>>>>>>State configs are supposed to be kzallocated. Needing this indicates a
> >>>>>>>>pretty serious bug - I'd vote to instead also ditch the dpll_md line and
> >>>>>>>>fix the offender.
> >>>>>>>There is no offender really. We read out the DPLL state before we know
> >>>>>>>which ports are active and hence can't tell at that point if the
> >>>>>>>information is really relevant.
> >>>>>So the bios leaves the DPLL enabled even when using a DSI port? Or do we
> >>>>>miss to check some routing bits in get_clock?
> >>>>Not necessarily enabled, but there are other bits in there that could be
> >>>>left in any state basically. The DSI port simply doesn't care.
> >>>If the enable bit is what's gating things here then we should just forgo
> >>>reading out any dpll register state if that's not set. Looking at the
> >>>vlv/chv state readout code that seems to be the trouble - there's nothing
> >>>guarding the register reads into the pipe_config at all. Didn't matter
> >>>pre-vlv without dsi since enable pipe should imply enabled dpll, but
> >>>obviously won't work correctly with dsi. Can you please spin such a patch
> >>>and remove the hacks here from dsi_get_config?
> >>>-Daniel
> >>Not sure i understand the point of contention here, just noticed this after
> >>i gave my RB :)
> >>so my justification on why this is proper is that DSI is not supposed to
> >>touch DPLL register
> >>any place we access dpll_hw_state.dpll is under !is_dsi check so that
> >>ensures that we dont
> >>program DPLL register for dsi panel. it was wrong to have originally modfied
> >>DPLL register
> >>inside intel_dsi_pre_enable so removal is fine. setting it to zero in
> >>intel_dsi_get_config
> >>is of no impact since any place we write back the contents of dpll_hw_state
> >>is past the
> >>!is_dsi check is never consumed by anyone as long as the CRTC uses DSI.
> >Ok, let's try a patch. Does the below work y/n? I've merged the patches up
> >to this one to dinq meanwhile, but I'd really like to close this first. If
> >it works I can rebase this patch here myself. Diff below is based on
> >-nightly with the above patch, so if you want to test on top of it you
> >also have to remove the dpll = 0; line too ofc.
> >
> >Thanks, Daniel
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index a7482ab140e1..c770655f5612 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -8079,6 +8079,10 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >  	i9xx_get_pfit_config(crtc, pipe_config);
> >+	tmp = I915_READ(DPLL(crtc->pipe));
> >+	if (!(tmp & DPLL_VCO_ENABLE))
> >+		return true;
> >+
> >  	if (INTEL_INFO(dev)->gen >= 4) {
> >  		tmp = I915_READ(DPLL_MD(crtc->pipe));
> >  		pipe_config->pixel_multiplier =
> >diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >index f4438eb5b458..116b06632cb0 100644
> >--- a/drivers/gpu/drm/i915/intel_dsi.c
> >+++ b/drivers/gpu/drm/i915/intel_dsi.c
> >@@ -621,12 +621,6 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >  	u32 pclk;
> >  	DRM_DEBUG_KMS("\n");
> >-	/*
> >-	 * DPLL_MD is not used in case of DSI, reading will get some default value
> >-	 * set dpll_md = 0
> >-	 */
> >-	pipe_config->dpll_hw_state.dpll_md = 0;
> >-
> >  	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);
> >  	if (!pclk)
> >  		return;
> i am fine with this patch.

Is that a tested-by/reviewed-by? I don't have a byt nor cht here, so this
is completely untested.
-Daniel
-- 
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

Re: [PATCH 5/9] drm/i915: Clear out DPLL state from pipe config in DSI get config

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 2381 bytes --]


Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>


On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> VLV/CHV don't use the DPLL with DSI, so just clear out the DPLL state
> from the pipe_config in intel_dsi_get_config(). This avoids spurious
> state checker warnings. We already did it this way for DPLL_MD, but do
> it for DPLL too.
>
> Toss in a WARN to intel_dsi_pre_enable() to make sure the ref clocks
> are enabled however. Supposedly they have some meaning to DSI too.
> We now keep the ref clocks always enabled while the disp2d well is
> enabled.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dsi.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 36e2148..92bb252 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -421,18 +421,12 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>   
>   	/* Disable DPOunit clock gating, can stall pipe
>   	 * and we need DPLL REFA always enabled */
> -	tmp = I915_READ(DPLL(pipe));
> -	tmp |= DPLL_REF_CLK_ENABLE_VLV;
> -	I915_WRITE(DPLL(pipe), tmp);
> -
> -	/* update the hw state for DPLL */
> -	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
> -		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> -
>   	tmp = I915_READ(DSPCLK_GATE_D);
>   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
>   	I915_WRITE(DSPCLK_GATE_D, tmp);
>   
> +	WARN_ON((I915_READ(DPLL(pipe)) & DPLL_REF_CLK_ENABLE_VLV) == 0);
> +
>   	/* put device in ready state */
>   	intel_dsi_device_ready(encoder);
>   
> @@ -635,9 +629,10 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>   	DRM_DEBUG_KMS("\n");
>   
>   	/*
> -	 * DPLL_MD is not used in case of DSI, reading will get some default value
> -	 * set dpll_md = 0
> +	 * DPLL is not used in case of DSI, reading will getsome default value.
> +	 * Clear the state to keep the state checker happy.
>   	 */
> +	pipe_config->dpll_hw_state.dpll = 0;
>   	pipe_config->dpll_hw_state.dpll_md = 0;
>   
>   	pclk = vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp);

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 3227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 6/9] drm/i915: Move DPLL ref/cri/VGA mode frobbing to the disp2d well enable

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Bunch of stuff needs the DPLL ref/cri clocks on both VLV and CHV,
and having VGA mode enabled causes some problems for CHV. So let's just
pull the code to configure those bits into the disp2d well enable hook.
With the DPLL disable code also fixed to leave those bits alone we
should now have a consistent DPLL state all the time even if the DPLL
is disabled.

This also neatly removes some duplicated code between the VLV and
CHV codepaths.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 45 ++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6393b76..2142ae6 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -837,6 +837,25 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
 
 static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 {
+	enum pipe pipe;
+
+	/*
+	 * Enable the CRI clock source so we can get at the
+	 * display and the reference clock for VGA
+	 * hotplug / manual detection. Supposedly DSI also
+	 * needs the ref clock up and running.
+	 *
+	 * CHV DPLL B/C have some issues if VGA mode is enabled.
+	 */
+	for_each_pipe(dev_priv->dev, pipe) {
+		u32 val = I915_READ(DPLL(pipe));
+
+		val |= DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
+		if (pipe != PIPE_A)
+			val |= DPLL_INTEGRATED_CRI_CLK_VLV;
+
+		I915_WRITE(DPLL(pipe), val);
+	}
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	valleyview_enable_display_irqs(dev_priv);
@@ -888,13 +907,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 {
 	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
 
-	/*
-	 * Enable the CRI clock source so we can get at the
-	 * display and the reference clock for VGA
-	 * hotplug / manual detection.
-	 */
-	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
-		   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
+	/* since ref/cri clock was enabled */
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 
 	vlv_set_power_well(dev_priv, power_well, true);
@@ -937,22 +950,12 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC &&
 		     power_well->data != PUNIT_POWER_WELL_DPIO_CMN_D);
 
-	/*
-	 * Enable the CRI clock source so we can get at the
-	 * display and the reference clock for VGA
-	 * hotplug / manual detection.
-	 */
-	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
+	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC)
 		phy = DPIO_PHY0;
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
-			   DPLL_REF_CLK_ENABLE_VLV);
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
-			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
-	} else {
+	else
 		phy = DPIO_PHY1;
-		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
-			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
-	}
+
+	/* since ref/cri clock was enabled */
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 	vlv_set_power_well(dev_priv, power_well, true);
 
-- 
2.3.6

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

Re: [PATCH 6/9] drm/i915: Move DPLL ref/cri/VGA mode frobbing to the disp2d well enable

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 3819 bytes --]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>



On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Bunch of stuff needs the DPLL ref/cri clocks on both VLV and CHV,
> and having VGA mode enabled causes some problems for CHV. So let's just
> pull the code to configure those bits into the disp2d well enable hook.
> With the DPLL disable code also fixed to leave those bits alone we
> should now have a consistent DPLL state all the time even if the DPLL
> is disabled.
>
> This also neatly removes some duplicated code between the VLV and
> CHV codepaths.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 45 ++++++++++++++++++---------------
>   1 file changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 6393b76..2142ae6 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -837,6 +837,25 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
>   
>   static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>   {
> +	enum pipe pipe;
> +
> +	/*
> +	 * Enable the CRI clock source so we can get at the
> +	 * display and the reference clock for VGA
> +	 * hotplug / manual detection. Supposedly DSI also
> +	 * needs the ref clock up and running.
> +	 *
> +	 * CHV DPLL B/C have some issues if VGA mode is enabled.
> +	 */
> +	for_each_pipe(dev_priv->dev, pipe) {
> +		u32 val = I915_READ(DPLL(pipe));
> +
> +		val |= DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> +		if (pipe != PIPE_A)
> +			val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> +
> +		I915_WRITE(DPLL(pipe), val);
> +	}
>   
>   	spin_lock_irq(&dev_priv->irq_lock);
>   	valleyview_enable_display_irqs(dev_priv);
> @@ -888,13 +907,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   {
>   	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
>   
> -	/*
> -	 * Enable the CRI clock source so we can get at the
> -	 * display and the reference clock for VGA
> -	 * hotplug / manual detection.
> -	 */
> -	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> -		   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> +	/* since ref/cri clock was enabled */
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
>   
>   	vlv_set_power_well(dev_priv, power_well, true);
> @@ -937,22 +950,12 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC &&
>   		     power_well->data != PUNIT_POWER_WELL_DPIO_CMN_D);
>   
> -	/*
> -	 * Enable the CRI clock source so we can get at the
> -	 * display and the reference clock for VGA
> -	 * hotplug / manual detection.
> -	 */
> -	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> +	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC)
>   		phy = DPIO_PHY0;
> -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> -			   DPLL_REF_CLK_ENABLE_VLV);
> -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> -			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> -	} else {
> +	else
>   		phy = DPIO_PHY1;
> -		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
> -			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> -	}
> +
> +	/* since ref/cri clock was enabled */
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
>   	vlv_set_power_well(dev_priv, power_well, true);
>   

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 4719 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 6/9] drm/i915: Move DPLL ref/cri/VGA mode frobbing to the disp2d well enable

[Daniel Vetter]

Queued for -next to resolve conflicts in another series, thanks for the patch.
-Daniel

On Fri, Jul 10, 2015 at 06:03:39PM +0530, Sivakumar Thulasimani wrote:
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> 
> 
> 
> On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Bunch of stuff needs the DPLL ref/cri clocks on both VLV and CHV,
> >and having VGA mode enabled causes some problems for CHV. So let's just
> >pull the code to configure those bits into the disp2d well enable hook.
> >With the DPLL disable code also fixed to leave those bits alone we
> >should now have a consistent DPLL state all the time even if the DPLL
> >is disabled.
> >
> >This also neatly removes some duplicated code between the VLV and
> >CHV codepaths.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 45 ++++++++++++++++++---------------
> >  1 file changed, 24 insertions(+), 21 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >index 6393b76..2142ae6 100644
> >--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >@@ -837,6 +837,25 @@ static bool vlv_power_well_enabled(struct drm_i915_private *dev_priv,
> >  static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
> >  {
> >+	enum pipe pipe;
> >+
> >+	/*
> >+	 * Enable the CRI clock source so we can get at the
> >+	 * display and the reference clock for VGA
> >+	 * hotplug / manual detection. Supposedly DSI also
> >+	 * needs the ref clock up and running.
> >+	 *
> >+	 * CHV DPLL B/C have some issues if VGA mode is enabled.
> >+	 */
> >+	for_each_pipe(dev_priv->dev, pipe) {
> >+		u32 val = I915_READ(DPLL(pipe));
> >+
> >+		val |= DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >+		if (pipe != PIPE_A)
> >+			val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> >+
> >+		I915_WRITE(DPLL(pipe), val);
> >+	}
> >  	spin_lock_irq(&dev_priv->irq_lock);
> >  	valleyview_enable_display_irqs(dev_priv);
> >@@ -888,13 +907,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> >  {
> >  	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC);
> >-	/*
> >-	 * Enable the CRI clock source so we can get at the
> >-	 * display and the reference clock for VGA
> >-	 * hotplug / manual detection.
> >-	 */
> >-	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >-		   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >+	/* since ref/cri clock was enabled */
> >  	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> >  	vlv_set_power_well(dev_priv, power_well, true);
> >@@ -937,22 +950,12 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> >  	WARN_ON_ONCE(power_well->data != PUNIT_POWER_WELL_DPIO_CMN_BC &&
> >  		     power_well->data != PUNIT_POWER_WELL_DPIO_CMN_D);
> >-	/*
> >-	 * Enable the CRI clock source so we can get at the
> >-	 * display and the reference clock for VGA
> >-	 * hotplug / manual detection.
> >-	 */
> >-	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> >+	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC)
> >  		phy = DPIO_PHY0;
> >-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >-			   DPLL_REF_CLK_ENABLE_VLV);
> >-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >-			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >-	} else {
> >+	else
> >  		phy = DPIO_PHY1;
> >-		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
> >-			   DPLL_REF_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >-	}
> >+
> >+	/* since ref/cri clock was enabled */
> >  	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> >  	vlv_set_power_well(dev_priv, power_well, true);
> 
> -- 
> regards,
> Sivakumar
> 

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

[PATCH 7/9] drm/i915: Make {vlv, chv}_{disable, update}_pll() more similar

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The VLV and CHV DPLL disable and update are almost identical in
how the DPLL/DPLL_MD registers need to be set up. But the code
looks more different than it really is. Try to bring them into
line.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 63 ++++++++++++++----------------------
 1 file changed, 25 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0870532..dec36a2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1785,16 +1785,13 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	/* Make sure the pipe isn't still relying on us */
 	assert_pipe_disabled(dev_priv, pipe);
 
-	/*
-	 * Leave integrated clock source and reference clock enabled for pipe B.
-	 * The latter is needed for VGA hotplug / manual detection.
-	 */
-	val = DPLL_VGA_MODE_DIS;
-	if (pipe == PIPE_B)
-		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REF_CLK_ENABLE_VLV;
+	val = DPLL_INTEGRATED_REF_CLK_VLV |
+		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
+	if (pipe != PIPE_A)
+		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
+
 	I915_WRITE(DPLL(pipe), val);
 	POSTING_READ(DPLL(pipe));
-
 }
 
 static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
@@ -1805,11 +1802,11 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	/* Make sure the pipe isn't still relying on us */
 	assert_pipe_disabled(dev_priv, pipe);
 
-	/* Set PLL en = 0 */
 	val = DPLL_SSC_REF_CLK_CHV |
 		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 	if (pipe != PIPE_A)
 		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
+
 	I915_WRITE(DPLL(pipe), val);
 	POSTING_READ(DPLL(pipe));
 
@@ -7191,24 +7188,27 @@ void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
 static void vlv_update_pll(struct intel_crtc *crtc,
 			   struct intel_crtc_state *pipe_config)
 {
-	u32 dpll, dpll_md;
+	pipe_config->dpll_hw_state.dpll = DPLL_INTEGRATED_REF_CLK_VLV |
+		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
+		DPLL_VCO_ENABLE | DPLL_EXT_BUFFER_ENABLE_VLV;
+	if (crtc->pipe != PIPE_A)
+		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
 
-	/*
-	 * Enable DPIO clock input. We should never disable the reference
-	 * clock for pipe B, since VGA hotplug / manual detection depends
-	 * on it.
-	 */
-	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REF_CLK_ENABLE_VLV |
-		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_REF_CLK_VLV;
-	/* We should never disable this, set it here for state tracking */
-	if (crtc->pipe == PIPE_B)
-		dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
-	dpll |= DPLL_VCO_ENABLE;
-	pipe_config->dpll_hw_state.dpll = dpll;
+	pipe_config->dpll_hw_state.dpll_md =
+		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
+}
+
+static void chv_update_pll(struct intel_crtc *crtc,
+			   struct intel_crtc_state *pipe_config)
+{
+	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV |
+		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
+		DPLL_VCO_ENABLE;
+	if (crtc->pipe != PIPE_A)
+		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
 
-	dpll_md = (pipe_config->pixel_multiplier - 1)
-		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
-	pipe_config->dpll_hw_state.dpll_md = dpll_md;
+	pipe_config->dpll_hw_state.dpll_md =
+		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
 }
 
 static void vlv_prepare_pll(struct intel_crtc *crtc,
@@ -7302,19 +7302,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 	mutex_unlock(&dev_priv->sb_lock);
 }
 
-static void chv_update_pll(struct intel_crtc *crtc,
-			   struct intel_crtc_state *pipe_config)
-{
-	pipe_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLK_CHV |
-		DPLL_REF_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
-		DPLL_VCO_ENABLE;
-	if (crtc->pipe != PIPE_A)
-		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
-
-	pipe_config->dpll_hw_state.dpll_md =
-		(pipe_config->pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
-}
-
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config)
 {
-- 
2.3.6

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

[PATCH 8/9] drm/i915: Implement WaPixelRepeatModeFixForC0:chv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

DPLL_MD(PIPE_C) is AWOL on CHV. Instead of fixing it someone added
chicken bits to propagate the pixel multiplier from DPLL_MD(PIPE_B)
to either pipe B or C. So do that to make pixel repeat work on pipes
B and C. Pipe A is fine without any tricks.

Fortunately the pixel repeat propagation appears to be a oneshot
operation, so once the value has been written we can clear the
chicken bits. So it is still possible to drive pipe B and C with
different pixel multipliers simultaneosly.

Looks like DPLL_VGA_MODE_DIS must also be set in DPLL(PIPE_B)
for this to work. But since we keep that bit always set in all
DPLLs there's no problem.

This of course means we can't reliably read out the pixel multiplier
for pipes B and C. That would make the state checker unhappy, so I
added shadow copies of those registers in to dev_priv. The other
option would have been to skip pixel multiplier, dpll_md an dotclock
checks entirely on CHV, but that feels like a serious loss of cross
checking, so just pretending that we have working DPLL MD registers
seemed better. Obviously with the shadow copies we can't detect if
the pixel multiplier was properly configured, nor can we take over
its state from the BIOS, but hopefully people won't have displays
that would be limitd to such crappy modes.

There is one strange flicker still remaining. It's visible on
pipe C/HDMID when HDMIB is enabled while driven by pipe B.
It doesn't occur if pipe A drives HDMIB, nor is there any glitch
on pipe B/HDMIB when port C/HDMID starts up. I don't have a board
with HDMIC so not sure if it happens there too. So I'm not sure
if it's somehow tied in with this strange linkage between pipe B
and C. Sadly I was unable to find an enable sequence that would
avoid the glitch, but at least it's not fatal ie. the output
recovers afterwards.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  7 +++++++
 drivers/gpu/drm/i915/i915_reg.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 37cc653..adaa656 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1851,7 +1851,14 @@ struct drm_i915_private {
 
 	u32 fdi_rx_config;
 
+	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
 	u32 chv_phy_control;
+	/*
+	 * Shadows for CHV DPLL_MD regs to keep the state
+	 * checker somewhat working in the presence hardware
+	 * crappiness (can't read out DPLL_MD for pipes B & C).
+	 */
+	u32 chv_dpll_md[I915_MAX_PIPES];
 
 	u32 suspend_count;
 	struct i915_suspend_saved_registers regfile;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f08f729..2361347 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4580,6 +4580,9 @@ enum skl_disp_power_wells {
 
 #define CBR1_VLV			(VLV_DISPLAY_BASE + 0x70400)
 #define  CBR_PND_DEADLINE_DISABLE	(1<<31)
+#define CBR4_VLV			(VLV_DISPLAY_BASE + 0x70450)
+#define  CBR_DPLLBMD_PIPE_C		(1<<29)
+#define  CBR_DPLLBMD_PIPE_B		(1<<18)
 
 /* FIFO watermark sizes etc */
 #define G4X_FIFO_LINE_SIZE	64
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dec36a2..b862307 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1667,9 +1667,27 @@ static void chv_enable_pll(struct intel_crtc *crtc,
 	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
 		DRM_ERROR("PLL %d failed to lock\n", pipe);
 
-	/* not sure when this should be written */
-	I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
-	POSTING_READ(DPLL_MD(pipe));
+	if (pipe != PIPE_A) {
+		/*
+		 * WaPixelRepeatModeFixForC0:chv
+		 *
+		 * DPLLCMD is AWOL. Use chicken bits to propagate
+		 * the value from DPLLBMD to either pipe B or C.
+		 */
+		I915_WRITE(CBR4_VLV, pipe == PIPE_B ? CBR_DPLLBMD_PIPE_B : CBR_DPLLBMD_PIPE_C);
+		I915_WRITE(DPLL_MD(PIPE_B), pipe_config->dpll_hw_state.dpll_md);
+		I915_WRITE(CBR4_VLV, 0);
+		dev_priv->chv_dpll_md[pipe] = pipe_config->dpll_hw_state.dpll_md;
+
+		/*
+		 * DPLLB VGA mode also seems to cause problems.
+		 * We should always have it disabled.
+		 */
+		WARN_ON((I915_READ(DPLL(PIPE_B)) & DPLL_VGA_MODE_DIS) == 0);
+	} else {
+		I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
+		POSTING_READ(DPLL_MD(pipe));
+	}
 }
 
 static int intel_num_dvo_pipes(struct drm_device *dev)
@@ -8065,7 +8083,11 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	i9xx_get_pfit_config(crtc, pipe_config);
 
 	if (INTEL_INFO(dev)->gen >= 4) {
-		tmp = I915_READ(DPLL_MD(crtc->pipe));
+		/* No way to read it out on pipes B and C */
+		if (IS_CHERRYVIEW(dev) && crtc->pipe != PIPE_A)
+			tmp = dev_priv->chv_dpll_md[crtc->pipe];
+		else
+			tmp = I915_READ(DPLL_MD(crtc->pipe));
 		pipe_config->pixel_multiplier =
 			((tmp & DPLL_MD_UDI_MULTIPLIER_MASK)
 			 >> DPLL_MD_UDI_MULTIPLIER_SHIFT) + 1;
-- 
2.3.6

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

Re: [PATCH 8/9] drm/i915: Implement WaPixelRepeatModeFixForC0:chv

[Sivakumar Thulasimani]

On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DPLL_MD(PIPE_C) is AWOL on CHV. Instead of fixing it someone added
> chicken bits to propagate the pixel multiplier from DPLL_MD(PIPE_B)
> to either pipe B or C. So do that to make pixel repeat work on pipes
> B and C. Pipe A is fine without any tricks.
>
> Fortunately the pixel repeat propagation appears to be a oneshot
> operation, so once the value has been written we can clear the
> chicken bits. So it is still possible to drive pipe B and C with
> different pixel multipliers simultaneosly.
>
> Looks like DPLL_VGA_MODE_DIS must also be set in DPLL(PIPE_B)
> for this to work. But since we keep that bit always set in all
> DPLLs there's no problem.
>
> This of course means we can't reliably read out the pixel multiplier
> for pipes B and C. That would make the state checker unhappy, so I
> added shadow copies of those registers in to dev_priv. The other
> option would have been to skip pixel multiplier, dpll_md an dotclock
> checks entirely on CHV, but that feels like a serious loss of cross
> checking, so just pretending that we have working DPLL MD registers
> seemed better. Obviously with the shadow copies we can't detect if
> the pixel multiplier was properly configured, nor can we take over
> its state from the BIOS, but hopefully people won't have displays
> that would be limitd to such crappy modes.
>
> There is one strange flicker still remaining. It's visible on
> pipe C/HDMID when HDMIB is enabled while driven by pipe B.
> It doesn't occur if pipe A drives HDMIB, nor is there any glitch
> on pipe B/HDMIB when port C/HDMID starts up. I don't have a board
> with HDMIC so not sure if it happens there too. So I'm not sure
> if it's somehow tied in with this strange linkage between pipe B
> and C. Sadly I was unable to find an enable sequence that would
> avoid the glitch, but at least it's not fatal ie. the output
> recovers afterwards.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  7 +++++++
>   drivers/gpu/drm/i915/i915_reg.h      |  3 +++
>   drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
>   3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 37cc653..adaa656 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1851,7 +1851,14 @@ struct drm_i915_private {
>   
>   	u32 fdi_rx_config;
>   
> +	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
>   	u32 chv_phy_control;
> +	/*
> +	 * Shadows for CHV DPLL_MD regs to keep the state
> +	 * checker somewhat working in the presence hardware
> +	 * crappiness (can't read out DPLL_MD for pipes B & C).
> +	 */
> +	u32 chv_dpll_md[I915_MAX_PIPES];
>   
>   	u32 suspend_count;
>   	struct i915_suspend_saved_registers regfile;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f08f729..2361347 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4580,6 +4580,9 @@ enum skl_disp_power_wells {
>   
>   #define CBR1_VLV			(VLV_DISPLAY_BASE + 0x70400)
>   #define  CBR_PND_DEADLINE_DISABLE	(1<<31)
> +#define CBR4_VLV			(VLV_DISPLAY_BASE + 0x70450)
> +#define  CBR_DPLLBMD_PIPE_C		(1<<29)
> +#define  CBR_DPLLBMD_PIPE_B		(1<<18)
>   
>   /* FIFO watermark sizes etc */
>   #define G4X_FIFO_LINE_SIZE	64
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dec36a2..b862307 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1667,9 +1667,27 @@ static void chv_enable_pll(struct intel_crtc *crtc,
>   	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
>   		DRM_ERROR("PLL %d failed to lock\n", pipe);
>   
> -	/* not sure when this should be written */
> -	I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> -	POSTING_READ(DPLL_MD(pipe));
> +	if (pipe != PIPE_A) {
> +		/*
> +		 * WaPixelRepeatModeFixForC0:chv
> +		 *
> +		 * DPLLCMD is AWOL. Use chicken bits to propagate
> +		 * the value from DPLLBMD to either pipe B or C.
> +		 */
> +		I915_WRITE(CBR4_VLV, pipe == PIPE_B ? CBR_DPLLBMD_PIPE_B : CBR_DPLLBMD_PIPE_C);
> +		I915_WRITE(DPLL_MD(PIPE_B), pipe_config->dpll_hw_state.dpll_md);
> +		I915_WRITE(CBR4_VLV, 0);
> +		dev_priv->chv_dpll_md[pipe] = pipe_config->dpll_hw_state.dpll_md;
> +
> +		/*
> +		 * DPLLB VGA mode also seems to cause problems.
> +		 * We should always have it disabled.
> +		 */
> +		WARN_ON((I915_READ(DPLL(PIPE_B)) & DPLL_VGA_MODE_DIS) == 0);
> +	} else {
> +		I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> +		POSTING_READ(DPLL_MD(pipe));
> +	}
>   }
>   
>   static int intel_num_dvo_pipes(struct drm_device *dev)
> @@ -8065,7 +8083,11 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>   	i9xx_get_pfit_config(crtc, pipe_config);
>   
>   	if (INTEL_INFO(dev)->gen >= 4) {
> -		tmp = I915_READ(DPLL_MD(crtc->pipe));
> +		/* No way to read it out on pipes B and C */
> +		if (IS_CHERRYVIEW(dev) && crtc->pipe != PIPE_A)
> +			tmp = dev_priv->chv_dpll_md[crtc->pipe];
> +		else
> +			tmp = I915_READ(DPLL_MD(crtc->pipe));
>   		pipe_config->pixel_multiplier =
>   			((tmp & DPLL_MD_UDI_MULTIPLIER_MASK)
>   			 >> DPLL_MD_UDI_MULTIPLIER_SHIFT) + 1;
correct me if my understanding is wrong, the only place we used to read 
pixel_multiplier was in
i9xx_get_pipe_config and we have replaced even that with dev_priv 
variables ? so the first time
we get pipe_config will it return 0 ?
i assume the right thing here is to read the CBR4_VLV and check if the 
current pipe is enabled,
if so return the value in DPLL_MD(PIPE_B) as required.
On a side note, we should calculate pixel_multiplier as part of 
compute_dpll instead of depending
on GOP/VBIOS programmed values.

-- 
regards,
Sivakumar

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

Re: [PATCH 8/9] drm/i915: Implement WaPixelRepeatModeFixForC0:chv

[Ville Syrjälä]

On Mon, Jul 13, 2015 at 11:44:44AM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DPLL_MD(PIPE_C) is AWOL on CHV. Instead of fixing it someone added
> > chicken bits to propagate the pixel multiplier from DPLL_MD(PIPE_B)
> > to either pipe B or C. So do that to make pixel repeat work on pipes
> > B and C. Pipe A is fine without any tricks.
> >
> > Fortunately the pixel repeat propagation appears to be a oneshot
> > operation, so once the value has been written we can clear the
> > chicken bits. So it is still possible to drive pipe B and C with
> > different pixel multipliers simultaneosly.
> >
> > Looks like DPLL_VGA_MODE_DIS must also be set in DPLL(PIPE_B)
> > for this to work. But since we keep that bit always set in all
> > DPLLs there's no problem.
> >
> > This of course means we can't reliably read out the pixel multiplier
> > for pipes B and C. That would make the state checker unhappy, so I
> > added shadow copies of those registers in to dev_priv. The other
> > option would have been to skip pixel multiplier, dpll_md an dotclock
> > checks entirely on CHV, but that feels like a serious loss of cross
> > checking, so just pretending that we have working DPLL MD registers
> > seemed better. Obviously with the shadow copies we can't detect if
> > the pixel multiplier was properly configured, nor can we take over
> > its state from the BIOS, but hopefully people won't have displays
> > that would be limitd to such crappy modes.
> >
> > There is one strange flicker still remaining. It's visible on
> > pipe C/HDMID when HDMIB is enabled while driven by pipe B.
> > It doesn't occur if pipe A drives HDMIB, nor is there any glitch
> > on pipe B/HDMIB when port C/HDMID starts up. I don't have a board
> > with HDMIC so not sure if it happens there too. So I'm not sure
> > if it's somehow tied in with this strange linkage between pipe B
> > and C. Sadly I was unable to find an enable sequence that would
> > avoid the glitch, but at least it's not fatal ie. the output
> > recovers afterwards.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h      |  7 +++++++
> >   drivers/gpu/drm/i915/i915_reg.h      |  3 +++
> >   drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++++----
> >   3 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 37cc653..adaa656 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1851,7 +1851,14 @@ struct drm_i915_private {
> >   
> >   	u32 fdi_rx_config;
> >   
> > +	/* Shadow for DISPLAY_PHY_CONTROL which can't be safely read */
> >   	u32 chv_phy_control;
> > +	/*
> > +	 * Shadows for CHV DPLL_MD regs to keep the state
> > +	 * checker somewhat working in the presence hardware
> > +	 * crappiness (can't read out DPLL_MD for pipes B & C).
> > +	 */
> > +	u32 chv_dpll_md[I915_MAX_PIPES];
> >   
> >   	u32 suspend_count;
> >   	struct i915_suspend_saved_registers regfile;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index f08f729..2361347 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4580,6 +4580,9 @@ enum skl_disp_power_wells {
> >   
> >   #define CBR1_VLV			(VLV_DISPLAY_BASE + 0x70400)
> >   #define  CBR_PND_DEADLINE_DISABLE	(1<<31)
> > +#define CBR4_VLV			(VLV_DISPLAY_BASE + 0x70450)
> > +#define  CBR_DPLLBMD_PIPE_C		(1<<29)
> > +#define  CBR_DPLLBMD_PIPE_B		(1<<18)
> >   
> >   /* FIFO watermark sizes etc */
> >   #define G4X_FIFO_LINE_SIZE	64
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index dec36a2..b862307 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1667,9 +1667,27 @@ static void chv_enable_pll(struct intel_crtc *crtc,
> >   	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> >   		DRM_ERROR("PLL %d failed to lock\n", pipe);
> >   
> > -	/* not sure when this should be written */
> > -	I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> > -	POSTING_READ(DPLL_MD(pipe));
> > +	if (pipe != PIPE_A) {
> > +		/*
> > +		 * WaPixelRepeatModeFixForC0:chv
> > +		 *
> > +		 * DPLLCMD is AWOL. Use chicken bits to propagate
> > +		 * the value from DPLLBMD to either pipe B or C.
> > +		 */
> > +		I915_WRITE(CBR4_VLV, pipe == PIPE_B ? CBR_DPLLBMD_PIPE_B : CBR_DPLLBMD_PIPE_C);
> > +		I915_WRITE(DPLL_MD(PIPE_B), pipe_config->dpll_hw_state.dpll_md);
> > +		I915_WRITE(CBR4_VLV, 0);
> > +		dev_priv->chv_dpll_md[pipe] = pipe_config->dpll_hw_state.dpll_md;
> > +
> > +		/*
> > +		 * DPLLB VGA mode also seems to cause problems.
> > +		 * We should always have it disabled.
> > +		 */
> > +		WARN_ON((I915_READ(DPLL(PIPE_B)) & DPLL_VGA_MODE_DIS) == 0);
> > +	} else {
> > +		I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> > +		POSTING_READ(DPLL_MD(pipe));
> > +	}
> >   }
> >   
> >   static int intel_num_dvo_pipes(struct drm_device *dev)
> > @@ -8065,7 +8083,11 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >   	i9xx_get_pfit_config(crtc, pipe_config);
> >   
> >   	if (INTEL_INFO(dev)->gen >= 4) {
> > -		tmp = I915_READ(DPLL_MD(crtc->pipe));
> > +		/* No way to read it out on pipes B and C */
> > +		if (IS_CHERRYVIEW(dev) && crtc->pipe != PIPE_A)
> > +			tmp = dev_priv->chv_dpll_md[crtc->pipe];
> > +		else
> > +			tmp = I915_READ(DPLL_MD(crtc->pipe));
> >   		pipe_config->pixel_multiplier =
> >   			((tmp & DPLL_MD_UDI_MULTIPLIER_MASK)
> >   			 >> DPLL_MD_UDI_MULTIPLIER_SHIFT) + 1;
> correct me if my understanding is wrong, the only place we used to read 
> pixel_multiplier was in
> i9xx_get_pipe_config and we have replaced even that with dev_priv 
> variables ? so the first time
> we get pipe_config will it return 0 ?

Yes. Which means 1x multiplier.

> i assume the right thing here is to read the CBR4_VLV and check if the 
> current pipe is enabled,
> if so return the value in DPLL_MD(PIPE_B) as required.

That won't work since the pixel multiplier update is a one-shot
operation. We migth be able to read the value back from one pipe by
looking at CBR4_VLV if the chicken bit for the pipe was left enabled
after the value was set, but there's no way to read back the value for
the second pipe. If both chicken bits are cleared in CBR4_VLV (which is
how I implemeted it) we can't read back the value for either pipe.

> On a side note, we should calculate pixel_multiplier as part of 
> compute_dpll instead of depending
> on GOP/VBIOS programmed values.

It's computed as part of encoder->compute_config().

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 9/9] drm/i915: Disable DSI PLL before reconfiguring it

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The BIOS maybe leave the DSI PLL enabled even if the port is disabled.
The PLL doesn't seem to like being reconfigured while it's enabled so
make sure it's disabled before doing that.

The better fix would be to expose all PLLs independently of their ports
so that we could disable any unused ones during the sanitize phase. But
this seems like an OK short term solution.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 92bb252..07c4bb3 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -907,6 +907,7 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
 
 	intel_dsi_prepare(encoder);
 
+	vlv_disable_dsi_pll(encoder);
 	vlv_enable_dsi_pll(encoder);
 }
 
-- 
2.3.6

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

Re: [PATCH 9/9] drm/i915: Disable DSI PLL before reconfiguring it

[Sivakumar Thulasimani]

[-- Attachment #1.1: Type: text/plain, Size: 1198 bytes --]

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>



On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The BIOS maybe leave the DSI PLL enabled even if the port is disabled.
> The PLL doesn't seem to like being reconfigured while it's enabled so
> make sure it's disabled before doing that.
>
> The better fix would be to expose all PLLs independently of their ports
> so that we could disable any unused ones during the sanitize phase. But
> this seems like an OK short term solution.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dsi.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 92bb252..07c4bb3 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -907,6 +907,7 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
>   
>   	intel_dsi_prepare(encoder);
>   
> +	vlv_disable_dsi_pll(encoder);
>   	vlv_enable_dsi_pll(encoder);
>   }
>   

-- 
regards,
Sivakumar


[-- Attachment #1.2: Type: text/html, Size: 2192 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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