Re: [Intel-gfx] [PATCH 1/5] drm/i915: Setup DPLL/DPLLMD for DSI too on VLV/CHV

[Ville Syrjälä]

On Fri, Apr 15, 2016 at 04:09:31PM +0300, Jani Nikula wrote:
> On Tue, 12 Apr 2016, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> >
> > Set up DPLL and DPLL_MD even when driving DSI output on VLV/CHV. While
> > the DPLL isn't used to provide the clock we still need the refclock, and
> > it appears that the pixel repeat factor also has an effect on DSI
> > output. So set up eveyrhing in DPLL and DPLL_MD as we would do for
> > DP/HDMI/VGA, but don't actually enable the DPLL or configure the
> > dividers via DPIO.
> >
> > Signed-off-by: Ville Syrjälä 
> 
> Reviewed-by: Jani Nikula 
> 
> I quickly glanced over 2-5 too, my earlier r-b on them still stand.

Thanks. Series pushed to dinq.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 120 
> > +--
> >  drivers/gpu/drm/i915/intel_dsi.c |  28 ++--
> >  2 files changed, 80 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 607dc41bcc68..7c74a930f45d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1530,45 +1530,47 @@ static void assert_pch_ports_disabled(struct 
> > drm_i915_private *dev_priv,
> > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
> >  }
> >  
> > +static void _vlv_enable_pll(struct intel_crtc *crtc,
> > +   const struct intel_crtc_state *pipe_config)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +   enum pipe pipe = crtc->pipe;
> > +
> > +   I915_WRITE(DPLL(pipe), pipe_config->dpll_hw_state.dpll);
> > +   POSTING_READ(DPLL(pipe));
> > +   udelay(150);
> > +
> > +   if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == 
> > DPLL_LOCK_VLV), 1))
> > +   DRM_ERROR("DPLL %d failed to lock\n", pipe);
> > +}
> > +
> >  static void vlv_enable_pll(struct intel_crtc *crtc,
> >const struct intel_crtc_state *pipe_config)
> >  {
> > -   struct drm_device *dev = crtc->base.dev;
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum pipe pipe = crtc->pipe;
> > -   i915_reg_t reg = DPLL(pipe);
> > -   u32 dpll = pipe_config->dpll_hw_state.dpll;
> >  
> > assert_pipe_disabled(dev_priv, pipe);
> >  
> > /* PLL is protected by panel, make sure we can write it */
> > assert_panel_unlocked(dev_priv, pipe);
> >  
> > -   I915_WRITE(reg, dpll);
> > -   POSTING_READ(reg);
> > -   udelay(150);
> > -
> > -   if (wait_for(((I915_READ(reg) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> > -   DRM_ERROR("DPLL %d failed to lock\n", pipe);
> > +   if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)
> > +   _vlv_enable_pll(crtc, pipe_config);
> >  
> > I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
> > POSTING_READ(DPLL_MD(pipe));
> >  }
> >  
> > -static void chv_enable_pll(struct intel_crtc *crtc,
> > -  const struct intel_crtc_state *pipe_config)
> > +
> > +static void _chv_enable_pll(struct intel_crtc *crtc,
> > +   const struct intel_crtc_state *pipe_config)
> >  {
> > -   struct drm_device *dev = crtc->base.dev;
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > enum pipe pipe = crtc->pipe;
> > enum dpio_channel port = vlv_pipe_to_channel(pipe);
> > u32 tmp;
> >  
> > -   assert_pipe_disabled(dev_priv, pipe);
> > -
> > -   /* PLL is protected by panel, make sure we can write it */
> > -   assert_panel_unlocked(dev_priv, pipe);
> > -
> > mutex_lock(_priv->sb_lock);
> >  
> > /* Enable back the 10bit clock to display controller */
> > @@ -1589,6 +1591,21 @@ static void chv_enable_pll(struct intel_crtc *crtc,
> > /* Check PLL is locked */
> > if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == 
> > DPLL_LOCK_VLV), 1))
> > DRM_ERROR("PLL %d failed to lock\n", pipe);
> > +}
> > +
> > +static void chv_enable_pll(struct intel_crtc *crtc,
> > +  const struct intel_crtc_state *pipe_config)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +   enum pipe pipe = crtc->pipe;
> > +
> > +   assert_pipe_disabled(dev_priv, pipe);
> > +
> > +   /* PLL is protected by panel, make sure we can write it */
> > +   assert_panel_unlocked(dev_priv, pipe);
> > +
> > +   if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)
> > +   _chv_enable_pll(crtc, pipe_config);
> >  
> > if (pipe != PIPE_A) {
> > /*
> > @@ -6073,14 +6090,12 @@ static void valleyview_crtc_enable(struct drm_crtc 
> > *crtc)
> > if (encoder->pre_pll_enable)
> > encoder->pre_pll_enable(encoder);
> >  
> > -   if 

Re: [Intel-gfx] [PATCH 1/5] drm/i915: Setup DPLL/DPLLMD for DSI too on VLV/CHV

[Jani Nikula]

On Tue, 12 Apr 2016, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
>
> Set up DPLL and DPLL_MD even when driving DSI output on VLV/CHV. While
> the DPLL isn't used to provide the clock we still need the refclock, and
> it appears that the pixel repeat factor also has an effect on DSI
> output. So set up eveyrhing in DPLL and DPLL_MD as we would do for
> DP/HDMI/VGA, but don't actually enable the DPLL or configure the
> dividers via DPIO.
>
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Jani Nikula 

I quickly glanced over 2-5 too, my earlier r-b on them still stand.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 120 
> +--
>  drivers/gpu/drm/i915/intel_dsi.c |  28 ++--
>  2 files changed, 80 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 607dc41bcc68..7c74a930f45d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1530,45 +1530,47 @@ static void assert_pch_ports_disabled(struct 
> drm_i915_private *dev_priv,
>   assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
>  }
>  
> +static void _vlv_enable_pll(struct intel_crtc *crtc,
> + const struct intel_crtc_state *pipe_config)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> +
> + I915_WRITE(DPLL(pipe), pipe_config->dpll_hw_state.dpll);
> + POSTING_READ(DPLL(pipe));
> + udelay(150);
> +
> + if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == 
> DPLL_LOCK_VLV), 1))
> + DRM_ERROR("DPLL %d failed to lock\n", pipe);
> +}
> +
>  static void vlv_enable_pll(struct intel_crtc *crtc,
>  const struct intel_crtc_state *pipe_config)
>  {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>   enum pipe pipe = crtc->pipe;
> - i915_reg_t reg = DPLL(pipe);
> - u32 dpll = pipe_config->dpll_hw_state.dpll;
>  
>   assert_pipe_disabled(dev_priv, pipe);
>  
>   /* PLL is protected by panel, make sure we can write it */
>   assert_panel_unlocked(dev_priv, pipe);
>  
> - I915_WRITE(reg, dpll);
> - POSTING_READ(reg);
> - udelay(150);
> -
> - if (wait_for(((I915_READ(reg) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> - DRM_ERROR("DPLL %d failed to lock\n", pipe);
> + if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)
> + _vlv_enable_pll(crtc, pipe_config);
>  
>   I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
>   POSTING_READ(DPLL_MD(pipe));
>  }
>  
> -static void chv_enable_pll(struct intel_crtc *crtc,
> -const struct intel_crtc_state *pipe_config)
> +
> +static void _chv_enable_pll(struct intel_crtc *crtc,
> + const struct intel_crtc_state *pipe_config)
>  {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>   enum pipe pipe = crtc->pipe;
>   enum dpio_channel port = vlv_pipe_to_channel(pipe);
>   u32 tmp;
>  
> - assert_pipe_disabled(dev_priv, pipe);
> -
> - /* PLL is protected by panel, make sure we can write it */
> - assert_panel_unlocked(dev_priv, pipe);
> -
>   mutex_lock(_priv->sb_lock);
>  
>   /* Enable back the 10bit clock to display controller */
> @@ -1589,6 +1591,21 @@ static void chv_enable_pll(struct intel_crtc *crtc,
>   /* Check PLL is locked */
>   if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == 
> DPLL_LOCK_VLV), 1))
>   DRM_ERROR("PLL %d failed to lock\n", pipe);
> +}
> +
> +static void chv_enable_pll(struct intel_crtc *crtc,
> +const struct intel_crtc_state *pipe_config)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> +
> + assert_pipe_disabled(dev_priv, pipe);
> +
> + /* PLL is protected by panel, make sure we can write it */
> + assert_panel_unlocked(dev_priv, pipe);
> +
> + if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)
> + _chv_enable_pll(crtc, pipe_config);
>  
>   if (pipe != PIPE_A) {
>   /*
> @@ -6073,14 +6090,12 @@ static void valleyview_crtc_enable(struct drm_crtc 
> *crtc)
>   if (encoder->pre_pll_enable)
>   encoder->pre_pll_enable(encoder);
>  
> - if (!intel_crtc->config->has_dsi_encoder) {
> - if (IS_CHERRYVIEW(dev)) {
> - chv_prepare_pll(intel_crtc, intel_crtc->config);
> - chv_enable_pll(intel_crtc, intel_crtc->config);
> - } else 

[Intel-gfx] [PATCH 1/5] drm/i915: Setup DPLL/DPLLMD for DSI too on VLV/CHV

[ville . syrjala]

From: Ville Syrjälä 

Set up DPLL and DPLL_MD even when driving DSI output on VLV/CHV. While
the DPLL isn't used to provide the clock we still need the refclock, and
it appears that the pixel repeat factor also has an effect on DSI
output. So set up eveyrhing in DPLL and DPLL_MD as we would do for
DP/HDMI/VGA, but don't actually enable the DPLL or configure the
dividers via DPIO.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 120 +--
 drivers/gpu/drm/i915/intel_dsi.c |  28 ++--
 2 files changed, 80 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 607dc41bcc68..7c74a930f45d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1530,45 +1530,47 @@ static void assert_pch_ports_disabled(struct 
drm_i915_private *dev_priv,
assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
 }
 
+static void _vlv_enable_pll(struct intel_crtc *crtc,
+   const struct intel_crtc_state *pipe_config)
+{
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   enum pipe pipe = crtc->pipe;
+
+   I915_WRITE(DPLL(pipe), pipe_config->dpll_hw_state.dpll);
+   POSTING_READ(DPLL(pipe));
+   udelay(150);
+
+   if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == 
DPLL_LOCK_VLV), 1))
+   DRM_ERROR("DPLL %d failed to lock\n", pipe);
+}
+
 static void vlv_enable_pll(struct intel_crtc *crtc,
   const struct intel_crtc_state *pipe_config)
 {
-   struct drm_device *dev = crtc->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum pipe pipe = crtc->pipe;
-   i915_reg_t reg = DPLL(pipe);
-   u32 dpll = pipe_config->dpll_hw_state.dpll;
 
assert_pipe_disabled(dev_priv, pipe);
 
/* PLL is protected by panel, make sure we can write it */
assert_panel_unlocked(dev_priv, pipe);
 
-   I915_WRITE(reg, dpll);
-   POSTING_READ(reg);
-   udelay(150);
-
-   if (wait_for(((I915_READ(reg) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
-   DRM_ERROR("DPLL %d failed to lock\n", pipe);
+   if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)
+   _vlv_enable_pll(crtc, pipe_config);
 
I915_WRITE(DPLL_MD(pipe), pipe_config->dpll_hw_state.dpll_md);
POSTING_READ(DPLL_MD(pipe));
 }
 
-static void chv_enable_pll(struct intel_crtc *crtc,
-  const struct intel_crtc_state *pipe_config)
+
+static void _chv_enable_pll(struct intel_crtc *crtc,
+   const struct intel_crtc_state *pipe_config)
 {
-   struct drm_device *dev = crtc->base.dev;
-   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
enum pipe pipe = crtc->pipe;
enum dpio_channel port = vlv_pipe_to_channel(pipe);
u32 tmp;
 
-   assert_pipe_disabled(dev_priv, pipe);
-
-   /* PLL is protected by panel, make sure we can write it */
-   assert_panel_unlocked(dev_priv, pipe);
-
mutex_lock(_priv->sb_lock);
 
/* Enable back the 10bit clock to display controller */
@@ -1589,6 +1591,21 @@ static void chv_enable_pll(struct intel_crtc *crtc,
/* Check PLL is locked */
if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == 
DPLL_LOCK_VLV), 1))
DRM_ERROR("PLL %d failed to lock\n", pipe);
+}
+
+static void chv_enable_pll(struct intel_crtc *crtc,
+  const struct intel_crtc_state *pipe_config)
+{
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   enum pipe pipe = crtc->pipe;
+
+   assert_pipe_disabled(dev_priv, pipe);
+
+   /* PLL is protected by panel, make sure we can write it */
+   assert_panel_unlocked(dev_priv, pipe);
+
+   if (pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)
+   _chv_enable_pll(crtc, pipe_config);
 
if (pipe != PIPE_A) {
/*
@@ -6073,14 +6090,12 @@ static void valleyview_crtc_enable(struct drm_crtc 
*crtc)
if (encoder->pre_pll_enable)
encoder->pre_pll_enable(encoder);
 
-   if (!intel_crtc->config->has_dsi_encoder) {
-   if (IS_CHERRYVIEW(dev)) {
-   chv_prepare_pll(intel_crtc, intel_crtc->config);
-   chv_enable_pll(intel_crtc, intel_crtc->config);
-   } else {
-   vlv_prepare_pll(intel_crtc, intel_crtc->config);
-   vlv_enable_pll(intel_crtc, intel_crtc->config);
-   }
+   if (IS_CHERRYVIEW(dev)) {
+   chv_prepare_pll(intel_crtc, intel_crtc->config);
+