[PATCH 00/13] drm/i915: First cleanup pass to get rid of more crtc->config users.

[PATCH 00/13] drm/i915: First cleanup pass to get rid of more crtc->config users.

[Maarten Lankhorst]

There are a lot of places where we use crtc->config, but shouldn't and easy
ways around it are available. In many cases, either the function itself, or
the calling function has a pointer to the correct state, so use it when
available.

This is not a full cleanup yet, want to send it in parts. :)

Maarten Lankhorst (13):
  drm/i915: Remove dereferences of crtc->config in set_pipeconf/misc
    functions.
  drm/i915: Make panel fitter functions take state
  drm/i915: Make intel_set_pipe_timings/src_size take a pointer to
    crtc_state
  drm/i915: Use crtc_state in ironlake_enable_pch_transcoder
  drm/i915: Make skl_detach_scalers take crtc_state
  drm/i915: Make pll functions take crtc_state
  drm/i915: Make ironlake_pch_transcoder_set_timings take crtc_state
  drm/i915: Make shared dpll functions take crtc_state
  drm/i915: Get rid of crtc->config from icl_pll_to_ddi_pll_sel
  drm/i915: Use crtc->state in intel_fbdev_init_bios
  drm/i915: Get rid of crtc->config dereference in intel_dp_retrain_link
  drm/i915: Get rid of crtc->config in chv_data_lane_soft_reset
  drm/i915: Get rid of intel_crtc->config in crtc_enable/disable
    functions

 drivers/gpu/drm/i915/intel_ddi.c      |  15 +-
 drivers/gpu/drm/i915/intel_display.c  | 335 +++++++++++++-------------
 drivers/gpu/drm/i915/intel_dp.c       |   4 +-
 drivers/gpu/drm/i915/intel_dpio_phy.c |   4 +-
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  31 +--
 drivers/gpu/drm/i915/intel_dpll_mgr.h |   6 +-
 drivers/gpu/drm/i915/intel_fbdev.c    |   8 +-
 7 files changed, 198 insertions(+), 205 deletions(-)

-- 
2.19.0

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

[PATCH 01/13] drm/i915: Remove dereferences of crtc->config in set_pipeconf/misc functions.

[Maarten Lankhorst]

One more user of crtc->config down. :)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++--------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 36434c5359b1..264feed76c08 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -141,15 +141,15 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 static int intel_framebuffer_init(struct intel_framebuffer *ifb,
 				  struct drm_i915_gem_object *obj,
 				  struct drm_mode_fb_cmd2 *mode_cmd);
-static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
+static void i9xx_set_pipeconf(struct intel_crtc_state *crtc_state);
 static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
 static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc);
 static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 					 struct intel_link_m_n *m_n,
 					 struct intel_link_m_n *m2_n2);
-static void ironlake_set_pipeconf(struct drm_crtc *crtc);
-static void haswell_set_pipeconf(struct drm_crtc *crtc);
-static void haswell_set_pipemisc(struct drm_crtc *crtc);
+static void ironlake_set_pipeconf(struct intel_crtc_state *crtc_state);
+static void haswell_set_pipeconf(struct intel_crtc_state *crtc_state);
+static void haswell_set_pipemisc(struct intel_crtc_state *crtc_state);
 static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
@@ -5604,7 +5604,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 				     &intel_crtc->config->fdi_m_n, NULL);
 	}
 
-	ironlake_set_pipeconf(crtc);
+	ironlake_set_pipeconf(pipe_config);
 
 	intel_crtc->active = true;
 
@@ -5737,9 +5737,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	}
 
 	if (!transcoder_is_dsi(cpu_transcoder))
-		haswell_set_pipeconf(crtc);
+		haswell_set_pipeconf(pipe_config);
 
-	haswell_set_pipemisc(crtc);
+	haswell_set_pipemisc(pipe_config);
 
 	intel_color_set_csc(&pipe_config->base);
 
@@ -6073,7 +6073,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 		I915_WRITE(CHV_CANVAS(pipe), 0);
 	}
 
-	i9xx_set_pipeconf(intel_crtc);
+	i9xx_set_pipeconf(pipe_config);
 
 	intel_color_set_csc(&pipe_config->base);
 
@@ -6138,7 +6138,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_set_pipe_timings(intel_crtc);
 	intel_set_pipe_src_size(intel_crtc);
 
-	i9xx_set_pipeconf(intel_crtc);
+	i9xx_set_pipeconf(pipe_config);
 
 	intel_crtc->active = true;
 
@@ -7480,8 +7480,9 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 	drm_mode_set_name(mode);
 }
 
-static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
+static void i9xx_set_pipeconf(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	uint32_t pipeconf;
 
@@ -7491,18 +7492,18 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 	if (IS_I830(dev_priv))
 		pipeconf |= I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE;
 
-	if (intel_crtc->config->double_wide)
+	if (crtc_state->double_wide)
 		pipeconf |= PIPECONF_DOUBLE_WIDE;
 
 	/* only g4x and later have fancy bpc/dither controls */
 	if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
 	    IS_CHERRYVIEW(dev_priv)) {
 		/* Bspec claims that we can't use dithering for 30bpp pipes. */
-		if (intel_crtc->config->dither && intel_crtc->config->pipe_bpp != 30)
+		if (crtc_state->dither && crtc_state->pipe_bpp != 30)
 			pipeconf |= PIPECONF_DITHER_EN |
 				    PIPECONF_DITHER_TYPE_SP;
 
-		switch (intel_crtc->config->pipe_bpp) {
+		switch (crtc_state->pipe_bpp) {
 		case 18:
 			pipeconf |= PIPECONF_6BPC;
 			break;
@@ -7518,9 +7519,9 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 		}
 	}
 
-	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	if (crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		if (INTEL_GEN(dev_priv) < 4 ||
-		    intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
+		    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO))
 			pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
 		else
 			pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT;
@@ -7528,7 +7529,7 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 		pipeconf |= PIPECONF_PROGRESSIVE;
 
 	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
-	     intel_crtc->config->limited_color_range)
+	     crtc_state->limited_color_range)
 		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
 
 	I915_WRITE(PIPECONF(intel_crtc->pipe), pipeconf);
@@ -8431,16 +8432,16 @@ void intel_init_pch_refclk(struct drm_i915_private *dev_priv)
 		lpt_init_pch_refclk(dev_priv);
 }
 
-static void ironlake_set_pipeconf(struct drm_crtc *crtc)
+static void ironlake_set_pipeconf(struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	int pipe = intel_crtc->pipe;
 	uint32_t val;
 
 	val = 0;
 
-	switch (intel_crtc->config->pipe_bpp) {
+	switch (crtc_state->pipe_bpp) {
 	case 18:
 		val |= PIPECONF_6BPC;
 		break;
@@ -8458,32 +8459,32 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 		BUG();
 	}
 
-	if (intel_crtc->config->dither)
+	if (crtc_state->dither)
 		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
 
-	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
 		val |= PIPECONF_INTERLACED_ILK;
 	else
 		val |= PIPECONF_PROGRESSIVE;
 
-	if (intel_crtc->config->limited_color_range)
+	if (crtc_state->limited_color_range)
 		val |= PIPECONF_COLOR_RANGE_SELECT;
 
 	I915_WRITE(PIPECONF(pipe), val);
 	POSTING_READ(PIPECONF(pipe));
 }
 
-static void haswell_set_pipeconf(struct drm_crtc *crtc)
+static void haswell_set_pipeconf(struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 val = 0;
 
-	if (IS_HASWELL(dev_priv) && intel_crtc->config->dither)
+	if (IS_HASWELL(dev_priv) && crtc_state->dither)
 		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
 
-	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
 		val |= PIPECONF_INTERLACED_ILK;
 	else
 		val |= PIPECONF_PROGRESSIVE;
@@ -8492,16 +8493,15 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
 	POSTING_READ(PIPECONF(cpu_transcoder));
 }
 
-static void haswell_set_pipemisc(struct drm_crtc *crtc)
+static void haswell_set_pipemisc(struct intel_crtc_state *crtc_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_crtc_state *config = intel_crtc->config;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 
 	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
 		u32 val = 0;
 
-		switch (intel_crtc->config->pipe_bpp) {
+		switch (crtc_state->pipe_bpp) {
 		case 18:
 			val |= PIPEMISC_DITHER_6_BPC;
 			break;
@@ -8519,10 +8519,10 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 			BUG();
 		}
 
-		if (intel_crtc->config->dither)
+		if (crtc_state->dither)
 			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
 
-		if (config->ycbcr420) {
+		if (crtc_state->ycbcr420) {
 			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
 				PIPEMISC_YUV420_ENABLE |
 				PIPEMISC_YUV420_MODE_FULL_BLEND;
-- 
2.19.0

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

Re: [PATCH 01/13] drm/i915: Remove dereferences of crtc->config in set_pipeconf/misc functions.

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:03PM +0200, Maarten Lankhorst wrote:
> One more user of crtc->config down. :)
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++--------------
>  1 file changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 36434c5359b1..264feed76c08 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -141,15 +141,15 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
>  static int intel_framebuffer_init(struct intel_framebuffer *ifb,
>  				  struct drm_i915_gem_object *obj,
>  				  struct drm_mode_fb_cmd2 *mode_cmd);
> -static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc);
> +static void i9xx_set_pipeconf(struct intel_crtc_state *crtc_state);

Group this with the others?

I think the state can be const for all of these.

>  static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
>  static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc);
>  static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>  					 struct intel_link_m_n *m_n,
>  					 struct intel_link_m_n *m2_n2);
> -static void ironlake_set_pipeconf(struct drm_crtc *crtc);
> -static void haswell_set_pipeconf(struct drm_crtc *crtc);
> -static void haswell_set_pipemisc(struct drm_crtc *crtc);
> +static void ironlake_set_pipeconf(struct intel_crtc_state *crtc_state);
> +static void haswell_set_pipeconf(struct intel_crtc_state *crtc_state);
> +static void haswell_set_pipemisc(struct intel_crtc_state *crtc_state);
>  static void vlv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config);
>  static void chv_prepare_pll(struct intel_crtc *crtc,
> @@ -5604,7 +5604,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  				     &intel_crtc->config->fdi_m_n, NULL);
>  	}
>  
> -	ironlake_set_pipeconf(crtc);
> +	ironlake_set_pipeconf(pipe_config);
>  
>  	intel_crtc->active = true;
>  
> @@ -5737,9 +5737,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	}
>  
>  	if (!transcoder_is_dsi(cpu_transcoder))
> -		haswell_set_pipeconf(crtc);
> +		haswell_set_pipeconf(pipe_config);
>  
> -	haswell_set_pipemisc(crtc);
> +	haswell_set_pipemisc(pipe_config);
>  
>  	intel_color_set_csc(&pipe_config->base);
>  
> @@ -6073,7 +6073,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  		I915_WRITE(CHV_CANVAS(pipe), 0);
>  	}
>  
> -	i9xx_set_pipeconf(intel_crtc);
> +	i9xx_set_pipeconf(pipe_config);
>  
>  	intel_color_set_csc(&pipe_config->base);
>  
> @@ -6138,7 +6138,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_set_pipe_timings(intel_crtc);
>  	intel_set_pipe_src_size(intel_crtc);
>  
> -	i9xx_set_pipeconf(intel_crtc);
> +	i9xx_set_pipeconf(pipe_config);
>  
>  	intel_crtc->active = true;
>  
> @@ -7480,8 +7480,9 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  	drm_mode_set_name(mode);
>  }
>  
> -static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
> +static void i9xx_set_pipeconf(struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);

s/intel_crtc/crtc/ while at it?

same in the other functions.

>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	uint32_t pipeconf;
>  
> @@ -7491,18 +7492,18 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  	if (IS_I830(dev_priv))
>  		pipeconf |= I915_READ(PIPECONF(intel_crtc->pipe)) & PIPECONF_ENABLE;
>  
> -	if (intel_crtc->config->double_wide)
> +	if (crtc_state->double_wide)
>  		pipeconf |= PIPECONF_DOUBLE_WIDE;
>  
>  	/* only g4x and later have fancy bpc/dither controls */
>  	if (IS_G4X(dev_priv) || IS_VALLEYVIEW(dev_priv) ||
>  	    IS_CHERRYVIEW(dev_priv)) {
>  		/* Bspec claims that we can't use dithering for 30bpp pipes. */
> -		if (intel_crtc->config->dither && intel_crtc->config->pipe_bpp != 30)
> +		if (crtc_state->dither && crtc_state->pipe_bpp != 30)
>  			pipeconf |= PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
>  
> -		switch (intel_crtc->config->pipe_bpp) {
> +		switch (crtc_state->pipe_bpp) {
>  		case 18:
>  			pipeconf |= PIPECONF_6BPC;
>  			break;
> @@ -7518,9 +7519,9 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  		}
>  	}
>  
> -	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
> +	if (crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
>  		if (INTEL_GEN(dev_priv) < 4 ||
> -		    intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
> +		    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO))
>  			pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
>  		else
>  			pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT;
> @@ -7528,7 +7529,7 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  		pipeconf |= PIPECONF_PROGRESSIVE;
>  
>  	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
> -	     intel_crtc->config->limited_color_range)
> +	     crtc_state->limited_color_range)
>  		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
>  
>  	I915_WRITE(PIPECONF(intel_crtc->pipe), pipeconf);
> @@ -8431,16 +8432,16 @@ void intel_init_pch_refclk(struct drm_i915_private *dev_priv)
>  		lpt_init_pch_refclk(dev_priv);
>  }
>  
> -static void ironlake_set_pipeconf(struct drm_crtc *crtc)
> +static void ironlake_set_pipeconf(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	int pipe = intel_crtc->pipe;

s/int/enum pipe/ while at it?

Apart from those lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	uint32_t val;
>  
>  	val = 0;
>  
> -	switch (intel_crtc->config->pipe_bpp) {
> +	switch (crtc_state->pipe_bpp) {
>  	case 18:
>  		val |= PIPECONF_6BPC;
>  		break;
> @@ -8458,32 +8459,32 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc)
>  		BUG();
>  	}
>  
> -	if (intel_crtc->config->dither)
> +	if (crtc_state->dither)
>  		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>  
> -	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		val |= PIPECONF_INTERLACED_ILK;
>  	else
>  		val |= PIPECONF_PROGRESSIVE;
>  
> -	if (intel_crtc->config->limited_color_range)
> +	if (crtc_state->limited_color_range)
>  		val |= PIPECONF_COLOR_RANGE_SELECT;
>  
>  	I915_WRITE(PIPECONF(pipe), val);
>  	POSTING_READ(PIPECONF(pipe));
>  }
>  
> -static void haswell_set_pipeconf(struct drm_crtc *crtc)
> +static void haswell_set_pipeconf(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 val = 0;
>  
> -	if (IS_HASWELL(dev_priv) && intel_crtc->config->dither)
> +	if (IS_HASWELL(dev_priv) && crtc_state->dither)
>  		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>  
> -	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		val |= PIPECONF_INTERLACED_ILK;
>  	else
>  		val |= PIPECONF_PROGRESSIVE;
> @@ -8492,16 +8493,15 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  	POSTING_READ(PIPECONF(cpu_transcoder));
>  }
>  
> -static void haswell_set_pipemisc(struct drm_crtc *crtc)
> +static void haswell_set_pipemisc(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_crtc_state *config = intel_crtc->config;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  
>  	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
>  		u32 val = 0;
>  
> -		switch (intel_crtc->config->pipe_bpp) {
> +		switch (crtc_state->pipe_bpp) {
>  		case 18:
>  			val |= PIPEMISC_DITHER_6_BPC;
>  			break;
> @@ -8519,10 +8519,10 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  			BUG();
>  		}
>  
> -		if (intel_crtc->config->dither)
> +		if (crtc_state->dither)
>  			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
>  
> -		if (config->ycbcr420) {
> +		if (crtc_state->ycbcr420) {
>  			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV |
>  				PIPEMISC_YUV420_ENABLE |
>  				PIPEMISC_YUV420_MODE_FULL_BLEND;
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 02/13] drm/i915: Make panel fitter functions take state

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 91 ++++++++++++++--------------
 1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 264feed76c08..701caab4e382 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -158,9 +158,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void intel_crtc_init_scalers(struct intel_crtc *crtc,
 				    struct intel_crtc_state *crtc_state);
-static void skylake_pfit_enable(struct intel_crtc *crtc);
-static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
-static void ironlake_pfit_enable(struct intel_crtc *crtc);
+static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state);
+static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state);
+static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state);
 static void intel_modeset_setup_hw_state(struct drm_device *dev,
 					 struct drm_modeset_acquire_ctx *ctx);
 static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
@@ -3919,12 +3919,12 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
 		skl_detach_scalers(crtc);
 
 		if (new_crtc_state->pch_pfit.enabled)
-			skylake_pfit_enable(crtc);
+			skylake_pfit_enable(new_crtc_state);
 	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		if (new_crtc_state->pch_pfit.enabled)
-			ironlake_pfit_enable(crtc);
+			ironlake_pfit_enable(new_crtc_state);
 		else if (old_crtc_state->pch_pfit.enabled)
-			ironlake_pfit_disable(crtc, true);
+			ironlake_pfit_disable(old_crtc_state);
 	}
 }
 
@@ -5041,19 +5041,19 @@ static void skylake_scaler_disable(struct intel_crtc *crtc)
 		skl_detach_scaler(crtc, i);
 }
 
-static void skylake_pfit_enable(struct intel_crtc *crtc)
+static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe = crtc->pipe;
-	struct intel_crtc_scaler_state *scaler_state =
-		&crtc->config->scaler_state;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	const struct intel_crtc_scaler_state *scaler_state =
+		&crtc_state->scaler_state;
 
-	if (crtc->config->pch_pfit.enabled) {
+	if (crtc_state->pch_pfit.enabled) {
 		u16 uv_rgb_hphase, uv_rgb_vphase;
 		int id;
 
-		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))
+		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
 			return;
 
 		uv_rgb_hphase = skl_scaler_calc_phase(1, false);
@@ -5066,18 +5066,18 @@ static void skylake_pfit_enable(struct intel_crtc *crtc)
 			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
 		I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),
 			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
-		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config->pch_pfit.pos);
-		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config->pch_pfit.size);
+		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc_state->pch_pfit.pos);
+		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc_state->pch_pfit.size);
 	}
 }
 
-static void ironlake_pfit_enable(struct intel_crtc *crtc)
+static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int pipe = crtc->pipe;
 
-	if (crtc->config->pch_pfit.enabled) {
+	if (crtc_state->pch_pfit.enabled) {
 		/* Force use of hard-coded filter coefficients
 		 * as some pre-programmed values are broken,
 		 * e.g. x201.
@@ -5087,8 +5087,8 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
 						 PF_PIPE_SEL_IVB(pipe));
 		else
 			I915_WRITE(PF_CTL(pipe), PF_ENABLE | PF_FILTER_MED_3x3);
-		I915_WRITE(PF_WIN_POS(pipe), crtc->config->pch_pfit.pos);
-		I915_WRITE(PF_WIN_SZ(pipe), crtc->config->pch_pfit.size);
+		I915_WRITE(PF_WIN_POS(pipe), crtc_state->pch_pfit.pos);
+		I915_WRITE(PF_WIN_SZ(pipe), crtc_state->pch_pfit.size);
 	}
 }
 
@@ -5620,7 +5620,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 		assert_fdi_rx_disabled(dev_priv, pipe);
 	}
 
-	ironlake_pfit_enable(intel_crtc);
+	ironlake_pfit_enable(pipe_config);
 
 	/*
 	 * On ILK+ LUT must be loaded before the pipe is running but with
@@ -5752,9 +5752,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
 
 	if (INTEL_GEN(dev_priv) >= 9)
-		skylake_pfit_enable(intel_crtc);
+		skylake_pfit_enable(pipe_config);
 	else
-		ironlake_pfit_enable(intel_crtc);
+		ironlake_pfit_enable(pipe_config);
 
 	/*
 	 * On ILK+ LUT must be loaded before the pipe is running but with
@@ -5812,15 +5812,15 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	}
 }
 
-static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
+static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe = crtc->pipe;
+	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
 
 	/* To avoid upsetting the power well on haswell only disable the pfit if
 	 * it's in use. The hw state code will make sure we get this right. */
-	if (force || crtc->config->pch_pfit.enabled) {
+	if (old_crtc_state->pch_pfit.enabled) {
 		I915_WRITE(PF_CTL(pipe), 0);
 		I915_WRITE(PF_WIN_POS(pipe), 0);
 		I915_WRITE(PF_WIN_SZ(pipe), 0);
@@ -5851,7 +5851,7 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	intel_disable_pipe(old_crtc_state);
 
-	ironlake_pfit_disable(intel_crtc, false);
+	ironlake_pfit_disable(old_crtc_state);
 
 	if (intel_crtc->config->has_pch_encoder)
 		ironlake_fdi_disable(crtc);
@@ -5912,7 +5912,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	if (INTEL_GEN(dev_priv) >= 9)
 		skylake_scaler_disable(intel_crtc);
 	else
-		ironlake_pfit_disable(intel_crtc, false);
+		ironlake_pfit_disable(old_crtc_state);
 
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
@@ -5920,13 +5920,12 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 		icl_unmap_plls_to_ports(crtc, old_crtc_state, old_state);
 }
 
-static void i9xx_pfit_enable(struct intel_crtc *crtc)
+static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc_state *pipe_config = crtc->config;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	if (!pipe_config->gmch_pfit.control)
+	if (!crtc_state->gmch_pfit.control)
 		return;
 
 	/*
@@ -5936,8 +5935,8 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 	WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
 	assert_pipe_disabled(dev_priv, crtc->pipe);
 
-	I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
-	I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
+	I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios);
+	I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control);
 
 	/* Border color in case we don't scale up to the full screen. Black by
 	 * default, change to something else for debugging. */
@@ -6093,7 +6092,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
-	i9xx_pfit_enable(intel_crtc);
+	i9xx_pfit_enable(pipe_config);
 
 	intel_color_load_luts(&pipe_config->base);
 
@@ -6149,7 +6148,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	i9xx_enable_pll(intel_crtc, pipe_config);
 
-	i9xx_pfit_enable(intel_crtc);
+	i9xx_pfit_enable(pipe_config);
 
 	intel_color_load_luts(&pipe_config->base);
 
@@ -6166,12 +6165,12 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_encoders_enable(crtc, pipe_config, old_state);
 }
 
-static void i9xx_pfit_disable(struct intel_crtc *crtc)
+static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	if (!crtc->config->gmch_pfit.control)
+	if (!old_crtc_state->gmch_pfit.control)
 		return;
 
 	assert_pipe_disabled(dev_priv, crtc->pipe);
@@ -6204,7 +6203,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	intel_disable_pipe(old_crtc_state);
 
-	i9xx_pfit_disable(intel_crtc);
+	i9xx_pfit_disable(old_crtc_state);
 
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
-- 
2.19.0

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

Re: [PATCH 02/13] drm/i915: Make panel fitter functions take state

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:04PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 91 ++++++++++++++--------------
>  1 file changed, 45 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 264feed76c08..701caab4e382 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -158,9 +158,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void intel_crtc_init_scalers(struct intel_crtc *crtc,
>  				    struct intel_crtc_state *crtc_state);
> -static void skylake_pfit_enable(struct intel_crtc *crtc);
> -static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> -static void ironlake_pfit_enable(struct intel_crtc *crtc);
> +static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state);
> +static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state);
> +static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state);
>  static void intel_modeset_setup_hw_state(struct drm_device *dev,
>  					 struct drm_modeset_acquire_ctx *ctx);
>  static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
> @@ -3919,12 +3919,12 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
>  		skl_detach_scalers(crtc);
>  
>  		if (new_crtc_state->pch_pfit.enabled)
> -			skylake_pfit_enable(crtc);
> +			skylake_pfit_enable(new_crtc_state);
>  	} else if (HAS_PCH_SPLIT(dev_priv)) {
>  		if (new_crtc_state->pch_pfit.enabled)
> -			ironlake_pfit_enable(crtc);
> +			ironlake_pfit_enable(new_crtc_state);
>  		else if (old_crtc_state->pch_pfit.enabled)
> -			ironlake_pfit_disable(crtc, true);
> +			ironlake_pfit_disable(old_crtc_state);
>  	}
>  }
>  
> @@ -5041,19 +5041,19 @@ static void skylake_scaler_disable(struct intel_crtc *crtc)
>  		skl_detach_scaler(crtc, i);
>  }
>  
> -static void skylake_pfit_enable(struct intel_crtc *crtc)
> +static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int pipe = crtc->pipe;
> -	struct intel_crtc_scaler_state *scaler_state =
> -		&crtc->config->scaler_state;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> +	const struct intel_crtc_scaler_state *scaler_state =
> +		&crtc_state->scaler_state;
>  
> -	if (crtc->config->pch_pfit.enabled) {
> +	if (crtc_state->pch_pfit.enabled) {
>  		u16 uv_rgb_hphase, uv_rgb_vphase;
>  		int id;
>  
> -		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))
> +		if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
>  			return;
>  
>  		uv_rgb_hphase = skl_scaler_calc_phase(1, false);
> @@ -5066,18 +5066,18 @@ static void skylake_pfit_enable(struct intel_crtc *crtc)
>  			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
>  		I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),
>  			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
> -		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config->pch_pfit.pos);
> -		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config->pch_pfit.size);
> +		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc_state->pch_pfit.pos);
> +		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc_state->pch_pfit.size);
>  	}
>  }
>  
> -static void ironlake_pfit_enable(struct intel_crtc *crtc)
> +static void ironlake_pfit_enable(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	int pipe = crtc->pipe;
>  
> -	if (crtc->config->pch_pfit.enabled) {
> +	if (crtc_state->pch_pfit.enabled) {
>  		/* Force use of hard-coded filter coefficients
>  		 * as some pre-programmed values are broken,
>  		 * e.g. x201.
> @@ -5087,8 +5087,8 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
>  						 PF_PIPE_SEL_IVB(pipe));
>  		else
>  			I915_WRITE(PF_CTL(pipe), PF_ENABLE | PF_FILTER_MED_3x3);
> -		I915_WRITE(PF_WIN_POS(pipe), crtc->config->pch_pfit.pos);
> -		I915_WRITE(PF_WIN_SZ(pipe), crtc->config->pch_pfit.size);
> +		I915_WRITE(PF_WIN_POS(pipe), crtc_state->pch_pfit.pos);
> +		I915_WRITE(PF_WIN_SZ(pipe), crtc_state->pch_pfit.size);
>  	}
>  }
>  
> @@ -5620,7 +5620,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  		assert_fdi_rx_disabled(dev_priv, pipe);
>  	}
>  
> -	ironlake_pfit_enable(intel_crtc);
> +	ironlake_pfit_enable(pipe_config);
>  
>  	/*
>  	 * On ILK+ LUT must be loaded before the pipe is running but with
> @@ -5752,9 +5752,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
>  
>  	if (INTEL_GEN(dev_priv) >= 9)
> -		skylake_pfit_enable(intel_crtc);
> +		skylake_pfit_enable(pipe_config);
>  	else
> -		ironlake_pfit_enable(intel_crtc);
> +		ironlake_pfit_enable(pipe_config);
>  
>  	/*
>  	 * On ILK+ LUT must be loaded before the pipe is running but with
> @@ -5812,15 +5812,15 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	}
>  }
>  
> -static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> +static void ironlake_pfit_disable(const struct intel_crtc_state *old_crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int pipe = crtc->pipe;
> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
>  
>  	/* To avoid upsetting the power well on haswell only disable the pfit if
>  	 * it's in use. The hw state code will make sure we get this right. */
> -	if (force || crtc->config->pch_pfit.enabled) {
> +	if (old_crtc_state->pch_pfit.enabled) {

I think the removal of 'force' deserves a mention in the commit message
(which was missing entirely I guess?). I do agree that 'force' is not
needed.

So with commit message added
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I have a feeling many of these lack a commit message. It's probably
going to become repititive considering the topic, but I think we still
want something there to justify the changes (probably could just
copy paste the same thing to each patch).

>  		I915_WRITE(PF_CTL(pipe), 0);
>  		I915_WRITE(PF_WIN_POS(pipe), 0);
>  		I915_WRITE(PF_WIN_SZ(pipe), 0);
> @@ -5851,7 +5851,7 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  
>  	intel_disable_pipe(old_crtc_state);
>  
> -	ironlake_pfit_disable(intel_crtc, false);
> +	ironlake_pfit_disable(old_crtc_state);
>  
>  	if (intel_crtc->config->has_pch_encoder)
>  		ironlake_fdi_disable(crtc);
> @@ -5912,7 +5912,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  	if (INTEL_GEN(dev_priv) >= 9)
>  		skylake_scaler_disable(intel_crtc);
>  	else
> -		ironlake_pfit_disable(intel_crtc, false);
> +		ironlake_pfit_disable(old_crtc_state);
>  
>  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>  
> @@ -5920,13 +5920,12 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  		icl_unmap_plls_to_ports(crtc, old_crtc_state, old_state);
>  }
>  
> -static void i9xx_pfit_enable(struct intel_crtc *crtc)
> +static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc_state *pipe_config = crtc->config;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  
> -	if (!pipe_config->gmch_pfit.control)
> +	if (!crtc_state->gmch_pfit.control)
>  		return;
>  
>  	/*
> @@ -5936,8 +5935,8 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
>  	WARN_ON(I915_READ(PFIT_CONTROL) & PFIT_ENABLE);
>  	assert_pipe_disabled(dev_priv, crtc->pipe);
>  
> -	I915_WRITE(PFIT_PGM_RATIOS, pipe_config->gmch_pfit.pgm_ratios);
> -	I915_WRITE(PFIT_CONTROL, pipe_config->gmch_pfit.control);
> +	I915_WRITE(PFIT_PGM_RATIOS, crtc_state->gmch_pfit.pgm_ratios);
> +	I915_WRITE(PFIT_CONTROL, crtc_state->gmch_pfit.control);
>  
>  	/* Border color in case we don't scale up to the full screen. Black by
>  	 * default, change to something else for debugging. */
> @@ -6093,7 +6092,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
> -	i9xx_pfit_enable(intel_crtc);
> +	i9xx_pfit_enable(pipe_config);
>  
>  	intel_color_load_luts(&pipe_config->base);
>  
> @@ -6149,7 +6148,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	i9xx_enable_pll(intel_crtc, pipe_config);
>  
> -	i9xx_pfit_enable(intel_crtc);
> +	i9xx_pfit_enable(pipe_config);
>  
>  	intel_color_load_luts(&pipe_config->base);
>  
> @@ -6166,12 +6165,12 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_encoders_enable(crtc, pipe_config, old_state);
>  }
>  
> -static void i9xx_pfit_disable(struct intel_crtc *crtc)
> +static void i9xx_pfit_disable(const struct intel_crtc_state *old_crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  
> -	if (!crtc->config->gmch_pfit.control)
> +	if (!old_crtc_state->gmch_pfit.control)
>  		return;
>  
>  	assert_pipe_disabled(dev_priv, crtc->pipe);
> @@ -6204,7 +6203,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  
>  	intel_disable_pipe(old_crtc_state);
>  
> -	i9xx_pfit_disable(intel_crtc);
> +	i9xx_pfit_disable(old_crtc_state);
>  
>  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 03/13] drm/i915: Make intel_set_pipe_timings/src_size take a pointer to crtc_state

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++--------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 701caab4e382..ca5e4d72d476 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -142,8 +142,8 @@ static int intel_framebuffer_init(struct intel_framebuffer *ifb,
 				  struct drm_i915_gem_object *obj,
 				  struct drm_mode_fb_cmd2 *mode_cmd);
 static void i9xx_set_pipeconf(struct intel_crtc_state *crtc_state);
-static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
-static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc);
+static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state);
+static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
 static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 					 struct intel_link_m_n *m_n,
 					 struct intel_link_m_n *m2_n2);
@@ -5596,8 +5596,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
-	intel_set_pipe_timings(intel_crtc);
-	intel_set_pipe_src_size(intel_crtc);
+	intel_set_pipe_timings(pipe_config);
+	intel_set_pipe_src_size(pipe_config);
 
 	if (intel_crtc->config->has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
@@ -5721,9 +5721,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	if (!transcoder_is_dsi(cpu_transcoder))
-		intel_set_pipe_timings(intel_crtc);
+		intel_set_pipe_timings(pipe_config);
 
-	intel_set_pipe_src_size(intel_crtc);
+	intel_set_pipe_src_size(pipe_config);
 
 	if (cpu_transcoder != TRANSCODER_EDP &&
 	    !transcoder_is_dsi(cpu_transcoder)) {
@@ -6062,12 +6062,10 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
-	intel_set_pipe_timings(intel_crtc);
-	intel_set_pipe_src_size(intel_crtc);
+	intel_set_pipe_timings(pipe_config);
+	intel_set_pipe_src_size(pipe_config);
 
 	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) {
-		struct drm_i915_private *dev_priv = to_i915(dev);
-
 		I915_WRITE(CHV_BLEND(pipe), CHV_BLEND_LEGACY);
 		I915_WRITE(CHV_CANVAS(pipe), 0);
 	}
@@ -6134,8 +6132,8 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
-	intel_set_pipe_timings(intel_crtc);
-	intel_set_pipe_src_size(intel_crtc);
+	intel_set_pipe_timings(pipe_config);
+	intel_set_pipe_src_size(pipe_config);
 
 	i9xx_set_pipeconf(pipe_config);
 
@@ -7331,12 +7329,13 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
 	crtc_state->dpll_hw_state.dpll = dpll;
 }
 
-static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
+static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	enum pipe pipe = intel_crtc->pipe;
-	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
-	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
 	uint32_t crtc_vtotal, crtc_vblank_end;
 	int vsyncshift = 0;
 
@@ -7350,7 +7349,7 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 		crtc_vtotal -= 1;
 		crtc_vblank_end -= 1;
 
-		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
+		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO))
 			vsyncshift = (adjusted_mode->crtc_htotal - 1) / 2;
 		else
 			vsyncshift = adjusted_mode->crtc_hsync_start -
@@ -7392,18 +7391,18 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 
 }
 
-static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc)
+static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	enum pipe pipe = intel_crtc->pipe;
 
 	/* pipesrc controls the size that is scaled from, which should
 	 * always be the user's requested size.
 	 */
 	I915_WRITE(PIPESRC(pipe),
-		   ((intel_crtc->config->pipe_src_w - 1) << 16) |
-		   (intel_crtc->config->pipe_src_h - 1));
+		   ((crtc_state->pipe_src_w - 1) << 16) |
+		   (crtc_state->pipe_src_h - 1));
 }
 
 static void intel_get_pipe_timings(struct intel_crtc *crtc,
-- 
2.19.0

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

Re: [PATCH 03/13] drm/i915: Make intel_set_pipe_timings/src_size take a pointer to crtc_state

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:05PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++--------------
>  1 file changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 701caab4e382..ca5e4d72d476 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -142,8 +142,8 @@ static int intel_framebuffer_init(struct intel_framebuffer *ifb,
>  				  struct drm_i915_gem_object *obj,
>  				  struct drm_mode_fb_cmd2 *mode_cmd);
>  static void i9xx_set_pipeconf(struct intel_crtc_state *crtc_state);
> -static void intel_set_pipe_timings(struct intel_crtc *intel_crtc);
> -static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc);
> +static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state);
> +static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
>  static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>  					 struct intel_link_m_n *m_n,
>  					 struct intel_link_m_n *m2_n2);
> @@ -5596,8 +5596,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
> -	intel_set_pipe_timings(intel_crtc);
> -	intel_set_pipe_src_size(intel_crtc);
> +	intel_set_pipe_timings(pipe_config);
> +	intel_set_pipe_src_size(pipe_config);
>  
>  	if (intel_crtc->config->has_pch_encoder) {
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
> @@ -5721,9 +5721,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
>  	if (!transcoder_is_dsi(cpu_transcoder))
> -		intel_set_pipe_timings(intel_crtc);
> +		intel_set_pipe_timings(pipe_config);
>  
> -	intel_set_pipe_src_size(intel_crtc);
> +	intel_set_pipe_src_size(pipe_config);
>  
>  	if (cpu_transcoder != TRANSCODER_EDP &&
>  	    !transcoder_is_dsi(cpu_transcoder)) {
> @@ -6062,12 +6062,10 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
> -	intel_set_pipe_timings(intel_crtc);
> -	intel_set_pipe_src_size(intel_crtc);
> +	intel_set_pipe_timings(pipe_config);
> +	intel_set_pipe_src_size(pipe_config);
>  
>  	if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) {
> -		struct drm_i915_private *dev_priv = to_i915(dev);
> -
>  		I915_WRITE(CHV_BLEND(pipe), CHV_BLEND_LEGACY);
>  		I915_WRITE(CHV_CANVAS(pipe), 0);
>  	}
> @@ -6134,8 +6132,8 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
> -	intel_set_pipe_timings(intel_crtc);
> -	intel_set_pipe_src_size(intel_crtc);
> +	intel_set_pipe_timings(pipe_config);
> +	intel_set_pipe_src_size(pipe_config);
>  
>  	i9xx_set_pipeconf(pipe_config);
>  
> @@ -7331,12 +7329,13 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
>  	crtc_state->dpll_hw_state.dpll = dpll;
>  }
>  
> -static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> +static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);

More easy s/intel_crtc/crtc/ candidates in this patch.

And missing a commit message. Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	enum pipe pipe = intel_crtc->pipe;
> -	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> -	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>  	uint32_t crtc_vtotal, crtc_vblank_end;
>  	int vsyncshift = 0;
>  
> @@ -7350,7 +7349,7 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  		crtc_vtotal -= 1;
>  		crtc_vblank_end -= 1;
>  
> -		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
> +		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO))
>  			vsyncshift = (adjusted_mode->crtc_htotal - 1) / 2;
>  		else
>  			vsyncshift = adjusted_mode->crtc_hsync_start -
> @@ -7392,18 +7391,18 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  
>  }
>  
> -static void intel_set_pipe_src_size(struct intel_crtc *intel_crtc)
> +static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	enum pipe pipe = intel_crtc->pipe;
>  
>  	/* pipesrc controls the size that is scaled from, which should
>  	 * always be the user's requested size.
>  	 */
>  	I915_WRITE(PIPESRC(pipe),
> -		   ((intel_crtc->config->pipe_src_w - 1) << 16) |
> -		   (intel_crtc->config->pipe_src_h - 1));
> +		   ((crtc_state->pipe_src_w - 1) << 16) |
> +		   (crtc_state->pipe_src_h - 1));
>  }
>  
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 04/13] drm/i915: Use crtc_state in ironlake_enable_pch_transcoder

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ca5e4d72d476..91574abafb65 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1666,16 +1666,16 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 		     I915_READ(dpll_reg) & port_mask, expected_mask);
 }
 
-static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
-					   enum pipe pipe)
+static void ironlake_enable_pch_transcoder(const struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
-								pipe);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	enum pipe pipe = intel_crtc->pipe;
 	i915_reg_t reg;
 	uint32_t val, pipeconf_val;
 
 	/* Make sure PCH DPLL is enabled */
-	assert_shared_dpll_enabled(dev_priv, intel_crtc->config->shared_dpll);
+	assert_shared_dpll_enabled(dev_priv, crtc_state->shared_dpll);
 
 	/* FDI must be feeding us bits for PCH ports */
 	assert_fdi_tx_enabled(dev_priv, pipe);
@@ -1701,7 +1701,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 		 * here for both 8bpc and 12bpc.
 		 */
 		val &= ~PIPECONF_BPC_MASK;
-		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_HDMI))
+		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 			val |= PIPECONF_8BPC;
 		else
 			val |= pipeconf_val & PIPECONF_BPC_MASK;
@@ -1710,7 +1710,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 	val &= ~TRANS_INTERLACE_MASK;
 	if ((pipeconf_val & PIPECONF_INTERLACE_MASK) == PIPECONF_INTERLACED_ILK)
 		if (HAS_PCH_IBX(dev_priv) &&
-		    intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
+		    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO))
 			val |= TRANS_LEGACY_INTERLACED_ILK;
 		else
 			val |= TRANS_INTERLACED;
@@ -4784,7 +4784,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
 		I915_WRITE(reg, temp);
 	}
 
-	ironlake_enable_pch_transcoder(dev_priv, pipe);
+	ironlake_enable_pch_transcoder(crtc_state);
 }
 
 static void lpt_pch_enable(const struct intel_atomic_state *state,
-- 
2.19.0

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

Re: [PATCH 04/13] drm/i915: Use crtc_state in ironlake_enable_pch_transcoder

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:06PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ca5e4d72d476..91574abafb65 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1666,16 +1666,16 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>  		     I915_READ(dpll_reg) & port_mask, expected_mask);
>  }
>  
> -static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> -					   enum pipe pipe)
> +static void ironlake_enable_pch_transcoder(const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
> -								pipe);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);

s/intel_crtc/crtc/ looks easy here as well, And commit msg is awol.

Otherwise
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	enum pipe pipe = intel_crtc->pipe;
>  	i915_reg_t reg;
>  	uint32_t val, pipeconf_val;
>  
>  	/* Make sure PCH DPLL is enabled */
> -	assert_shared_dpll_enabled(dev_priv, intel_crtc->config->shared_dpll);
> +	assert_shared_dpll_enabled(dev_priv, crtc_state->shared_dpll);
>  
>  	/* FDI must be feeding us bits for PCH ports */
>  	assert_fdi_tx_enabled(dev_priv, pipe);
> @@ -1701,7 +1701,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
>  		 * here for both 8bpc and 12bpc.
>  		 */
>  		val &= ~PIPECONF_BPC_MASK;
> -		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_HDMI))
> +		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  			val |= PIPECONF_8BPC;
>  		else
>  			val |= pipeconf_val & PIPECONF_BPC_MASK;
> @@ -1710,7 +1710,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
>  	val &= ~TRANS_INTERLACE_MASK;
>  	if ((pipeconf_val & PIPECONF_INTERLACE_MASK) == PIPECONF_INTERLACED_ILK)
>  		if (HAS_PCH_IBX(dev_priv) &&
> -		    intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
> +		    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO))
>  			val |= TRANS_LEGACY_INTERLACED_ILK;
>  		else
>  			val |= TRANS_INTERLACED;
> @@ -4784,7 +4784,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
>  		I915_WRITE(reg, temp);
>  	}
>  
> -	ironlake_enable_pch_transcoder(dev_priv, pipe);
> +	ironlake_enable_pch_transcoder(crtc_state);
>  }
>  
>  static void lpt_pch_enable(const struct intel_atomic_state *state,
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 05/13] drm/i915: Make skl_detach_scalers take crtc_state

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 91574abafb65..56784ae26369 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3502,13 +3502,13 @@ static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
 /*
  * This function detaches (aka. unbinds) unused scalers in hardware
  */
-static void skl_detach_scalers(struct intel_crtc *intel_crtc)
+static void skl_detach_scalers(const struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc_scaler_state *scaler_state;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct intel_crtc_scaler_state *scaler_state =
+		&crtc_state->scaler_state;
 	int i;
 
-	scaler_state = &intel_crtc->config->scaler_state;
-
 	/* loop through and disable scalers that aren't in use */
 	for (i = 0; i < intel_crtc->num_scalers; i++) {
 		if (!scaler_state->scalers[i].in_use)
@@ -3916,7 +3916,7 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
 
 	/* on skylake this is done by detaching scalers */
 	if (INTEL_GEN(dev_priv) >= 9) {
-		skl_detach_scalers(crtc);
+		skl_detach_scalers(new_crtc_state);
 
 		if (new_crtc_state->pch_pfit.enabled)
 			skylake_pfit_enable(new_crtc_state);
@@ -13335,7 +13335,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 	if (intel_cstate->update_pipe)
 		intel_update_pipe_config(old_intel_cstate, intel_cstate);
 	else if (INTEL_GEN(dev_priv) >= 9)
-		skl_detach_scalers(intel_crtc);
+		skl_detach_scalers(intel_cstate);
 
 out:
 	if (dev_priv->display.atomic_update_watermarks)
-- 
2.19.0

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

Re: [PATCH 05/13] drm/i915: Make skl_detach_scalers take crtc_state

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:07PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 91574abafb65..56784ae26369 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3502,13 +3502,13 @@ static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
>  /*
>   * This function detaches (aka. unbinds) unused scalers in hardware
>   */
> -static void skl_detach_scalers(struct intel_crtc *intel_crtc)
> +static void skl_detach_scalers(const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc_scaler_state *scaler_state;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);

s/intel_crtc/crtc/ looks easy enough to sneak in again.
Commit msg missing.

Otherwise
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	const struct intel_crtc_scaler_state *scaler_state =
> +		&crtc_state->scaler_state;
>  	int i;
>  
> -	scaler_state = &intel_crtc->config->scaler_state;
> -
>  	/* loop through and disable scalers that aren't in use */
>  	for (i = 0; i < intel_crtc->num_scalers; i++) {
>  		if (!scaler_state->scalers[i].in_use)
> @@ -3916,7 +3916,7 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
>  
>  	/* on skylake this is done by detaching scalers */
>  	if (INTEL_GEN(dev_priv) >= 9) {
> -		skl_detach_scalers(crtc);
> +		skl_detach_scalers(new_crtc_state);
>  
>  		if (new_crtc_state->pch_pfit.enabled)
>  			skylake_pfit_enable(new_crtc_state);
> @@ -13335,7 +13335,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>  	if (intel_cstate->update_pipe)
>  		intel_update_pipe_config(old_intel_cstate, intel_cstate);
>  	else if (INTEL_GEN(dev_priv) >= 9)
> -		skl_detach_scalers(intel_crtc);
> +		skl_detach_scalers(intel_cstate);
>  
>  out:
>  	if (dev_priv->display.atomic_update_watermarks)
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 06/13] drm/i915: Make pll functions take crtc_state

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 56784ae26369..36a8d5fb8881 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1565,14 +1565,15 @@ static void i9xx_enable_pll(struct intel_crtc *crtc,
 	}
 }
 
-static void i9xx_disable_pll(struct intel_crtc *crtc)
+static void i9xx_disable_pll(const struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 
 	/* Disable DVO 2x clock on both PLLs if necessary */
 	if (IS_I830(dev_priv) &&
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO) &&
+	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO) &&
 	    !intel_num_dvo_pipes(dev_priv)) {
 		I915_WRITE(DPLL(PIPE_B),
 			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
@@ -4323,10 +4324,10 @@ static void ivb_manual_fdi_link_train(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("FDI train done.\n");
 }
 
-static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
+static void ironlake_fdi_pll_enable(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
 	int pipe = intel_crtc->pipe;
 	i915_reg_t reg;
 	u32 temp;
@@ -4335,7 +4336,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
 	temp &= ~(FDI_DP_PORT_WIDTH_MASK | (0x7 << 16));
-	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config->fdi_lanes);
+	temp |= FDI_DP_PORT_WIDTH(crtc_state->fdi_lanes);
 	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
 	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
 
@@ -5614,7 +5615,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 		/* Note: FDI PLL enabling _must_ be done before we enable the
 		 * cpu pipes, hence this is separate from all the other fdi/pch
 		 * enabling. */
-		ironlake_fdi_pll_enable(intel_crtc);
+		ironlake_fdi_pll_enable(pipe_config);
 	} else {
 		assert_fdi_tx_disabled(dev_priv, pipe);
 		assert_fdi_rx_disabled(dev_priv, pipe);
@@ -6211,7 +6212,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
 		else if (IS_VALLEYVIEW(dev_priv))
 			vlv_disable_pll(dev_priv, pipe);
 		else
-			i9xx_disable_pll(intel_crtc);
+			i9xx_disable_pll(old_crtc_state);
 	}
 
 	intel_encoders_post_pll_disable(crtc, old_crtc_state, old_state);
-- 
2.19.0

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

Re: [PATCH 06/13] drm/i915: Make pll functions take crtc_state

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:08PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 56784ae26369..36a8d5fb8881 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1565,14 +1565,15 @@ static void i9xx_enable_pll(struct intel_crtc *crtc,
>  	}
>  }
>  
> -static void i9xx_disable_pll(struct intel_crtc *crtc)
> +static void i9xx_disable_pll(const struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
>  
>  	/* Disable DVO 2x clock on both PLLs if necessary */
>  	if (IS_I830(dev_priv) &&
> -	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO) &&
> +	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO) &&
>  	    !intel_num_dvo_pipes(dev_priv)) {
>  		I915_WRITE(DPLL(PIPE_B),
>  			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
> @@ -4323,10 +4324,10 @@ static void ivb_manual_fdi_link_train(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("FDI train done.\n");
>  }

Looks like you missed i9xx_set_pll_dividers() here. I think I saw it in
a later patch though, but maybe move it here?

commit msg...

Otherwise
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
> -static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
> +static void ironlake_fdi_pll_enable(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
>  	int pipe = intel_crtc->pipe;
>  	i915_reg_t reg;
>  	u32 temp;
> @@ -4335,7 +4336,7 @@ static void ironlake_fdi_pll_enable(struct intel_crtc *intel_crtc)
>  	reg = FDI_RX_CTL(pipe);
>  	temp = I915_READ(reg);
>  	temp &= ~(FDI_DP_PORT_WIDTH_MASK | (0x7 << 16));
> -	temp |= FDI_DP_PORT_WIDTH(intel_crtc->config->fdi_lanes);
> +	temp |= FDI_DP_PORT_WIDTH(crtc_state->fdi_lanes);
>  	temp |= (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) << 11;
>  	I915_WRITE(reg, temp | FDI_RX_PLL_ENABLE);
>  
> @@ -5614,7 +5615,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  		/* Note: FDI PLL enabling _must_ be done before we enable the
>  		 * cpu pipes, hence this is separate from all the other fdi/pch
>  		 * enabling. */
> -		ironlake_fdi_pll_enable(intel_crtc);
> +		ironlake_fdi_pll_enable(pipe_config);
>  	} else {
>  		assert_fdi_tx_disabled(dev_priv, pipe);
>  		assert_fdi_rx_disabled(dev_priv, pipe);
> @@ -6211,7 +6212,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  		else if (IS_VALLEYVIEW(dev_priv))
>  			vlv_disable_pll(dev_priv, pipe);
>  		else
> -			i9xx_disable_pll(intel_crtc);
> +			i9xx_disable_pll(old_crtc_state);
>  	}
>  
>  	intel_encoders_post_pll_disable(crtc, old_crtc_state, old_state);
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 07/13] drm/i915: Make ironlake_pch_transcoder_set_timings take crtc_state

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 36a8d5fb8881..57c81e8e44be 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4599,12 +4599,12 @@ int lpt_get_iclkip(struct drm_i915_private *dev_priv)
 				 desired_divisor << auxdiv);
 }
 
-static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
+static void ironlake_pch_transcoder_set_timings(const struct intel_crtc_state *crtc_state,
 						enum pipe pch_transcoder)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
 	I915_WRITE(PCH_TRANS_HTOTAL(pch_transcoder),
 		   I915_READ(HTOTAL(cpu_transcoder)));
@@ -4753,7 +4753,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
 
 	/* set transcoder timing, panel must allow it */
 	assert_panel_unlocked(dev_priv, pipe);
-	ironlake_pch_transcoder_set_timings(crtc, pipe);
+	ironlake_pch_transcoder_set_timings(crtc_state, pipe);
 
 	intel_fdi_normal_train(crtc);
 
@@ -4800,7 +4800,7 @@ static void lpt_pch_enable(const struct intel_atomic_state *state,
 	lpt_program_iclkip(crtc);
 
 	/* Set transcoder timing. */
-	ironlake_pch_transcoder_set_timings(crtc, PIPE_A);
+	ironlake_pch_transcoder_set_timings(crtc_state, PIPE_A);
 
 	lpt_enable_pch_transcoder(dev_priv, cpu_transcoder);
 }
-- 
2.19.0

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

Re: [PATCH 07/13] drm/i915: Make ironlake_pch_transcoder_set_timings take crtc_state

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:09PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

commit msg

Otherwise
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 36a8d5fb8881..57c81e8e44be 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4599,12 +4599,12 @@ int lpt_get_iclkip(struct drm_i915_private *dev_priv)
>  				 desired_divisor << auxdiv);
>  }
>  
> -static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
> +static void ironlake_pch_transcoder_set_timings(const struct intel_crtc_state *crtc_state,
>  						enum pipe pch_transcoder)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
>  	I915_WRITE(PCH_TRANS_HTOTAL(pch_transcoder),
>  		   I915_READ(HTOTAL(cpu_transcoder)));
> @@ -4753,7 +4753,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
>  
>  	/* set transcoder timing, panel must allow it */
>  	assert_panel_unlocked(dev_priv, pipe);
> -	ironlake_pch_transcoder_set_timings(crtc, pipe);
> +	ironlake_pch_transcoder_set_timings(crtc_state, pipe);
>  
>  	intel_fdi_normal_train(crtc);
>  
> @@ -4800,7 +4800,7 @@ static void lpt_pch_enable(const struct intel_atomic_state *state,
>  	lpt_program_iclkip(crtc);
>  
>  	/* Set transcoder timing. */
> -	ironlake_pch_transcoder_set_timings(crtc, PIPE_A);
> +	ironlake_pch_transcoder_set_timings(crtc_state, PIPE_A);
>  
>  	lpt_enable_pch_transcoder(dev_priv, cpu_transcoder);
>  }
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 08/13] drm/i915: Make shared dpll functions take crtc_state

[Maarten Lankhorst]

Do not rely on crtc->config any more. Remove the assertion from
ibx_pch_dpll_disable, because we the dpll state tracking should
already handle this case correctly.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 14 ++++++------
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 31 +++++++++++----------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  6 +++---
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 57c81e8e44be..22f556b824f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4749,7 +4749,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
 	 * Note that enable_shared_dpll tries to do the right thing, but
 	 * get_shared_dpll unconditionally resets the pll - we need that to have
 	 * the right LVDS enable sequence. */
-	intel_enable_shared_dpll(crtc);
+	intel_enable_shared_dpll(crtc_state);
 
 	/* set transcoder timing, panel must allow it */
 	assert_panel_unlocked(dev_priv, pipe);
@@ -5591,8 +5591,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
 	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
 
-	if (intel_crtc->config->has_pch_encoder)
-		intel_prepare_shared_dpll(intel_crtc);
+	if (pipe_config->has_pch_encoder)
+		intel_prepare_shared_dpll(pipe_config);
 
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
@@ -5710,8 +5710,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
-	if (intel_crtc->config->shared_dpll)
-		intel_enable_shared_dpll(intel_crtc);
+	if (pipe_config->shared_dpll)
+		intel_enable_shared_dpll(pipe_config);
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_map_plls_to_ports(crtc, pipe_config, old_state);
@@ -6286,7 +6286,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 
 	intel_fbc_disable(intel_crtc);
 	intel_update_watermarks(intel_crtc);
-	intel_disable_shared_dpll(intel_crtc);
+	intel_disable_shared_dpll(crtc_state);
 
 	domains = intel_crtc->enabled_power_domains;
 	for_each_power_domain(domain, domains)
@@ -12735,7 +12735,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 			dev_priv->display.crtc_disable(old_intel_crtc_state, state);
 			intel_crtc->active = false;
 			intel_fbc_disable(intel_crtc);
-			intel_disable_shared_dpll(intel_crtc);
+			intel_disable_shared_dpll(old_intel_crtc_state);
 
 			/*
 			 * Underruns don't always raise
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index e6cac9225536..54662c974c0b 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -131,13 +131,13 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
  * This calls the PLL's prepare hook if it has one and if the PLL is not
  * already enabled. The prepare hook is platform specific.
  */
-void intel_prepare_shared_dpll(struct intel_crtc *crtc)
+void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 
-	if (WARN_ON(pll == NULL))
+	if (!WARN_ON(!pll))
 		return;
 
 	mutex_lock(&dev_priv->dpll_lock);
@@ -158,11 +158,11 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc)
  *
  * Enable the shared DPLL used by @crtc.
  */
-void intel_enable_shared_dpll(struct intel_crtc *crtc)
+void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
 	unsigned int old_mask;
 
@@ -203,10 +203,11 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
  *
  * Disable the shared DPLL used by @crtc.
  */
-void intel_disable_shared_dpll(struct intel_crtc *crtc)
+void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
+	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
 
 	/* PCH only available on ILK+ */
@@ -409,14 +410,6 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
 				 struct intel_shared_dpll *pll)
 {
 	const enum intel_dpll_id id = pll->info->id;
-	struct drm_device *dev = &dev_priv->drm;
-	struct intel_crtc *crtc;
-
-	/* Make sure no transcoder isn't still depending on us. */
-	for_each_intel_crtc(dev, crtc) {
-		if (crtc->config->shared_dpll == pll)
-			assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
-	}
 
 	I915_WRITE(PCH_DPLL(id), 0);
 	POSTING_READ(PCH_DPLL(id));
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index bf0de8a4dc63..9c033236f2ba 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -334,9 +334,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
 void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
 			       struct intel_crtc *crtc,
 			       struct drm_atomic_state *state);
-void intel_prepare_shared_dpll(struct intel_crtc *crtc);
-void intel_enable_shared_dpll(struct intel_crtc *crtc);
-void intel_disable_shared_dpll(struct intel_crtc *crtc);
+void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
+void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
+void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
 void intel_shared_dpll_swap_state(struct drm_atomic_state *state);
 void intel_shared_dpll_init(struct drm_device *dev);
 
-- 
2.19.0

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

Re: [PATCH 08/13] drm/i915: Make shared dpll functions take crtc_state

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:10PM +0200, Maarten Lankhorst wrote:
> Do not rely on crtc->config any more. Remove the assertion from
> ibx_pch_dpll_disable, because we the dpll state tracking should
> already handle this case correctly.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c  | 14 ++++++------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 31 +++++++++++----------------
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  6 +++---
>  3 files changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 57c81e8e44be..22f556b824f9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4749,7 +4749,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
>  	 * Note that enable_shared_dpll tries to do the right thing, but
>  	 * get_shared_dpll unconditionally resets the pll - we need that to have
>  	 * the right LVDS enable sequence. */
> -	intel_enable_shared_dpll(crtc);
> +	intel_enable_shared_dpll(crtc_state);
>  
>  	/* set transcoder timing, panel must allow it */
>  	assert_panel_unlocked(dev_priv, pipe);
> @@ -5591,8 +5591,8 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
>  	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>  
> -	if (intel_crtc->config->has_pch_encoder)
> -		intel_prepare_shared_dpll(intel_crtc);
> +	if (pipe_config->has_pch_encoder)
> +		intel_prepare_shared_dpll(pipe_config);
>  
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
> @@ -5710,8 +5710,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
> -	if (intel_crtc->config->shared_dpll)
> -		intel_enable_shared_dpll(intel_crtc);
> +	if (pipe_config->shared_dpll)
> +		intel_enable_shared_dpll(pipe_config);
>  
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		icl_map_plls_to_ports(crtc, pipe_config, old_state);
> @@ -6286,7 +6286,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
>  
>  	intel_fbc_disable(intel_crtc);
>  	intel_update_watermarks(intel_crtc);
> -	intel_disable_shared_dpll(intel_crtc);
> +	intel_disable_shared_dpll(crtc_state);
>  
>  	domains = intel_crtc->enabled_power_domains;
>  	for_each_power_domain(domain, domains)
> @@ -12735,7 +12735,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  			dev_priv->display.crtc_disable(old_intel_crtc_state, state);
>  			intel_crtc->active = false;
>  			intel_fbc_disable(intel_crtc);
> -			intel_disable_shared_dpll(intel_crtc);
> +			intel_disable_shared_dpll(old_intel_crtc_state);
>  
>  			/*
>  			 * Underruns don't always raise
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e6cac9225536..54662c974c0b 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -131,13 +131,13 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>   * This calls the PLL's prepare hook if it has one and if the PLL is not
>   * already enabled. The prepare hook is platform specific.
>   */
> -void intel_prepare_shared_dpll(struct intel_crtc *crtc)
> +void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  
> -	if (WARN_ON(pll == NULL))
> +	if (!WARN_ON(!pll))
>  		return;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
> @@ -158,11 +158,11 @@ void intel_prepare_shared_dpll(struct intel_crtc *crtc)
>   *
>   * Enable the shared DPLL used by @crtc.
>   */
> -void intel_enable_shared_dpll(struct intel_crtc *crtc)
> +void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
>  	unsigned int old_mask;
>  
> @@ -203,10 +203,11 @@ void intel_enable_shared_dpll(struct intel_crtc *crtc)
>   *
>   * Disable the shared DPLL used by @crtc.
>   */
> -void intel_disable_shared_dpll(struct intel_crtc *crtc)
> +void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	struct intel_shared_dpll *pll = crtc->config->shared_dpll;
> +	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  	unsigned int crtc_mask = drm_crtc_mask(&crtc->base);
>  
>  	/* PCH only available on ILK+ */
> @@ -409,14 +410,6 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
>  				 struct intel_shared_dpll *pll)
>  {
>  	const enum intel_dpll_id id = pll->info->id;
> -	struct drm_device *dev = &dev_priv->drm;
> -	struct intel_crtc *crtc;
> -
> -	/* Make sure no transcoder isn't still depending on us. */
> -	for_each_intel_crtc(dev, crtc) {
> -		if (crtc->config->shared_dpll == pll)
> -			assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
> -	}

Yeah I guess this can go. If we really wanted to keep it we should
probably make it not depend on any software state and just read out
everything from the hw.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	I915_WRITE(PCH_DPLL(id), 0);
>  	POSTING_READ(PCH_DPLL(id));
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index bf0de8a4dc63..9c033236f2ba 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -334,9 +334,9 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
>  void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
>  			       struct intel_crtc *crtc,
>  			       struct drm_atomic_state *state);
> -void intel_prepare_shared_dpll(struct intel_crtc *crtc);
> -void intel_enable_shared_dpll(struct intel_crtc *crtc);
> -void intel_disable_shared_dpll(struct intel_crtc *crtc);
> +void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
> +void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
> +void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);
>  void intel_shared_dpll_swap_state(struct drm_atomic_state *state);
>  void intel_shared_dpll_init(struct drm_device *dev);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 09/13] drm/i915: Get rid of crtc->config from icl_pll_to_ddi_pll_sel

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b6594948b617..9e82281b4fdf 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1060,10 +1060,10 @@ static uint32_t hsw_pll_to_ddi_pll_sel(const struct intel_shared_dpll *pll)
 }
 
 static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder,
-				       const struct intel_shared_dpll *pll)
+				       const struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
-	int clock = crtc->config->port_clock;
+	const struct intel_shared_dpll *pll = crtc_state->shared_dpll;
+	int clock = crtc_state->port_clock;
 	const enum intel_dpll_id id = pll->info->id;
 
 	switch (id) {
@@ -2798,11 +2798,12 @@ void icl_unmap_plls_to_ports(struct drm_crtc *crtc,
 }
 
 static void intel_ddi_clk_select(struct intel_encoder *encoder,
-				 const struct intel_shared_dpll *pll)
+				 const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = encoder->port;
 	uint32_t val;
+	const struct intel_shared_dpll *pll = crtc_state->shared_dpll;
 
 	if (WARN_ON(!pll))
 		return;
@@ -2812,7 +2813,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 	if (IS_ICELAKE(dev_priv)) {
 		if (port >= PORT_C)
 			I915_WRITE(DDI_CLK_SEL(port),
-				   icl_pll_to_ddi_pll_sel(encoder, pll));
+				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
 	} else if (IS_CANNONLAKE(dev_priv)) {
 		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
 		val = I915_READ(DPCLKA_CFGCR0);
@@ -2886,7 +2887,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	intel_edp_panel_on(intel_dp);
 
-	intel_ddi_clk_select(encoder, crtc_state->shared_dpll);
+	intel_ddi_clk_select(encoder, crtc_state);
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
@@ -2928,7 +2929,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
-	intel_ddi_clk_select(encoder, crtc_state->shared_dpll);
+	intel_ddi_clk_select(encoder, crtc_state);
 
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
-- 
2.19.0

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

Re: [PATCH 09/13] drm/i915: Get rid of crtc->config from icl_pll_to_ddi_pll_sel

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:11PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

With a bit if commit msg added
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b6594948b617..9e82281b4fdf 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1060,10 +1060,10 @@ static uint32_t hsw_pll_to_ddi_pll_sel(const struct intel_shared_dpll *pll)
>  }
>  
>  static uint32_t icl_pll_to_ddi_pll_sel(struct intel_encoder *encoder,
> -				       const struct intel_shared_dpll *pll)
> +				       const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> -	int clock = crtc->config->port_clock;
> +	const struct intel_shared_dpll *pll = crtc_state->shared_dpll;
> +	int clock = crtc_state->port_clock;
>  	const enum intel_dpll_id id = pll->info->id;
>  
>  	switch (id) {
> @@ -2798,11 +2798,12 @@ void icl_unmap_plls_to_ports(struct drm_crtc *crtc,
>  }
>  
>  static void intel_ddi_clk_select(struct intel_encoder *encoder,
> -				 const struct intel_shared_dpll *pll)
> +				 const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = encoder->port;
>  	uint32_t val;
> +	const struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  
>  	if (WARN_ON(!pll))
>  		return;
> @@ -2812,7 +2813,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  	if (IS_ICELAKE(dev_priv)) {
>  		if (port >= PORT_C)
>  			I915_WRITE(DDI_CLK_SEL(port),
> -				   icl_pll_to_ddi_pll_sel(encoder, pll));
> +				   icl_pll_to_ddi_pll_sel(encoder, crtc_state));
>  	} else if (IS_CANNONLAKE(dev_priv)) {
>  		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
>  		val = I915_READ(DPCLKA_CFGCR0);
> @@ -2886,7 +2887,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	intel_edp_panel_on(intel_dp);
>  
> -	intel_ddi_clk_select(encoder, crtc_state->shared_dpll);
> +	intel_ddi_clk_select(encoder, crtc_state);
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> @@ -2928,7 +2929,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  
>  	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
> -	intel_ddi_clk_select(encoder, crtc_state->shared_dpll);
> +	intel_ddi_clk_select(encoder, crtc_state);
>  
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 10/13] drm/i915: Use crtc->state in intel_fbdev_init_bios

[Maarten Lankhorst]

fbdev init shouldn't race with userspace since it's called from
intel_modeset_init, so it's safe to dereference crtc->state and
assume nothing changed yet.

At least not more harmful than crtc->config.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index f99332972b7a..2480c7d6edee 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -593,7 +593,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 		 * pipe.  Note we need to use the selected fb's pitch and bpp
 		 * rather than the current pipe's, since they differ.
 		 */
-		cur_size = intel_crtc->config->base.adjusted_mode.crtc_hdisplay;
+		cur_size = crtc->state->adjusted_mode.crtc_hdisplay;
 		cur_size = cur_size * fb->base.format->cpp[0];
 		if (fb->base.pitches[0] < cur_size) {
 			DRM_DEBUG_KMS("fb not wide enough for plane %c (%d vs %d)\n",
@@ -603,13 +603,13 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 			break;
 		}
 
-		cur_size = intel_crtc->config->base.adjusted_mode.crtc_vdisplay;
+		cur_size = crtc->state->adjusted_mode.crtc_vdisplay;
 		cur_size = intel_fb_align_height(&fb->base, 0, cur_size);
 		cur_size *= fb->base.pitches[0];
 		DRM_DEBUG_KMS("pipe %c area: %dx%d, bpp: %d, size: %d\n",
 			      pipe_name(intel_crtc->pipe),
-			      intel_crtc->config->base.adjusted_mode.crtc_hdisplay,
-			      intel_crtc->config->base.adjusted_mode.crtc_vdisplay,
+			      crtc->state->adjusted_mode.crtc_hdisplay,
+			      crtc->state->adjusted_mode.crtc_vdisplay,
 			      fb->base.format->cpp[0] * 8,
 			      cur_size);
 
-- 
2.19.0

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

Re: [PATCH 10/13] drm/i915: Use crtc->state in intel_fbdev_init_bios

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:12PM +0200, Maarten Lankhorst wrote:
> fbdev init shouldn't race with userspace since it's called from
> intel_modeset_init, so it's safe to dereference crtc->state and
> assume nothing changed yet.
> 
> At least not more harmful than crtc->config.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index f99332972b7a..2480c7d6edee 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -593,7 +593,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  		 * pipe.  Note we need to use the selected fb's pitch and bpp
>  		 * rather than the current pipe's, since they differ.
>  		 */
> -		cur_size = intel_crtc->config->base.adjusted_mode.crtc_hdisplay;
> +		cur_size = crtc->state->adjusted_mode.crtc_hdisplay;

Yeah, we're alreday looking at crtc->state->active above, so this should
equally safe.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		cur_size = cur_size * fb->base.format->cpp[0];
>  		if (fb->base.pitches[0] < cur_size) {
>  			DRM_DEBUG_KMS("fb not wide enough for plane %c (%d vs %d)\n",
> @@ -603,13 +603,13 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  			break;
>  		}
>  
> -		cur_size = intel_crtc->config->base.adjusted_mode.crtc_vdisplay;
> +		cur_size = crtc->state->adjusted_mode.crtc_vdisplay;
>  		cur_size = intel_fb_align_height(&fb->base, 0, cur_size);
>  		cur_size *= fb->base.pitches[0];
>  		DRM_DEBUG_KMS("pipe %c area: %dx%d, bpp: %d, size: %d\n",
>  			      pipe_name(intel_crtc->pipe),
> -			      intel_crtc->config->base.adjusted_mode.crtc_hdisplay,
> -			      intel_crtc->config->base.adjusted_mode.crtc_vdisplay,
> +			      crtc->state->adjusted_mode.crtc_hdisplay,
> +			      crtc->state->adjusted_mode.crtc_vdisplay,
>  			      fb->base.format->cpp[0] * 8,
>  			      cur_size);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 11/13] drm/i915: Get rid of crtc->config dereference in intel_dp_retrain_link

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 15a981ef5966..d9cc52b2ba79 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4417,7 +4417,7 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 
 	/* Suppress underruns caused by re-training */
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
-	if (crtc->config->has_pch_encoder)
+	if (crtc_state->has_pch_encoder)
 		intel_set_pch_fifo_underrun_reporting(dev_priv,
 						      intel_crtc_pch_transcoder(crtc), false);
 
@@ -4428,7 +4428,7 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	intel_wait_for_vblank(dev_priv, crtc->pipe);
 
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
-	if (crtc->config->has_pch_encoder)
+	if (crtc_state->has_pch_encoder)
 		intel_set_pch_fifo_underrun_reporting(dev_priv,
 						      intel_crtc_pch_transcoder(crtc), true);
 
-- 
2.19.0

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

Re: [PATCH 11/13] drm/i915: Get rid of crtc->config dereference in intel_dp_retrain_link

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:13PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 15a981ef5966..d9cc52b2ba79 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4417,7 +4417,7 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
>  
>  	/* Suppress underruns caused by re-training */
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
> -	if (crtc->config->has_pch_encoder)
> +	if (crtc_state->has_pch_encoder)

Oh yes, we have a state now. Which means all the remaining ->config
usages in link training code should also be nukeable now? Mostly
down to some plumbing work perhaps?

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		intel_set_pch_fifo_underrun_reporting(dev_priv,
>  						      intel_crtc_pch_transcoder(crtc), false);
>  
> @@ -4428,7 +4428,7 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
>  	intel_wait_for_vblank(dev_priv, crtc->pipe);
>  
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
> -	if (crtc->config->has_pch_encoder)
> +	if (crtc_state->has_pch_encoder)
>  		intel_set_pch_fifo_underrun_reporting(dev_priv,
>  						      intel_crtc_pch_transcoder(crtc), true);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 12/13] drm/i915: Get rid of crtc->config in chv_data_lane_soft_reset

[Maarten Lankhorst]

Fixing chv_set_phy_signal_level() still requires too many levels of
indirection to pass crtc_state along.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpio_phy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
index 00b3ab656b06..3c7f10d17658 100644
--- a/drivers/gpu/drm/i915/intel_dpio_phy.c
+++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
@@ -748,7 +748,7 @@ void chv_data_lane_soft_reset(struct intel_encoder *encoder,
 		val |= DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET;
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
 
-	if (crtc->config->lane_count > 2) {
+	if (crtc_state->lane_count > 2) {
 		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
 		if (reset)
 			val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
@@ -765,7 +765,7 @@ void chv_data_lane_soft_reset(struct intel_encoder *encoder,
 		val |= DPIO_PCS_CLK_SOFT_RESET;
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
 
-	if (crtc->config->lane_count > 2) {
+	if (crtc_state->lane_count > 2) {
 		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
 		val |= CHV_PCS_REQ_SOFTRESET_EN;
 		if (reset)
-- 
2.19.0

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

Re: [PATCH 12/13] drm/i915: Get rid of crtc->config in chv_data_lane_soft_reset

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:14PM +0200, Maarten Lankhorst wrote:
> Fixing chv_set_phy_signal_level() still requires too many levels of
> indirection to pass crtc_state along.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpio_phy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c
> index 00b3ab656b06..3c7f10d17658 100644
> --- a/drivers/gpu/drm/i915/intel_dpio_phy.c
> +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c
> @@ -748,7 +748,7 @@ void chv_data_lane_soft_reset(struct intel_encoder *encoder,
>  		val |= DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET;
>  	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW0(ch), val);
>  
> -	if (crtc->config->lane_count > 2) {
> +	if (crtc_state->lane_count > 2) {
>  		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW0(ch));
>  		if (reset)
>  			val &= ~(DPIO_PCS_TX_LANE2_RESET | DPIO_PCS_TX_LANE1_RESET);
> @@ -765,7 +765,7 @@ void chv_data_lane_soft_reset(struct intel_encoder *encoder,
>  		val |= DPIO_PCS_CLK_SOFT_RESET;
>  	vlv_dpio_write(dev_priv, pipe, VLV_PCS01_DW1(ch), val);
>  
> -	if (crtc->config->lane_count > 2) {
> +	if (crtc_state->lane_count > 2) {

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		val = vlv_dpio_read(dev_priv, pipe, VLV_PCS23_DW1(ch));
>  		val |= CHV_PCS_REQ_SOFTRESET_EN;
>  		if (reset)
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 13/13] drm/i915: Get rid of intel_crtc->config in crtc_enable/disable functions

[Maarten Lankhorst]

These functions already have a pointer to the correct state,
so use it instead of crtc->config.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++--------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 22f556b824f9..01ad6bd3b7de 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5594,15 +5594,15 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (pipe_config->has_pch_encoder)
 		intel_prepare_shared_dpll(pipe_config);
 
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+	if (intel_crtc_has_dp_encoder(pipe_config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	intel_set_pipe_timings(pipe_config);
 	intel_set_pipe_src_size(pipe_config);
 
-	if (intel_crtc->config->has_pch_encoder) {
+	if (pipe_config->has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
-				     &intel_crtc->config->fdi_m_n, NULL);
+				     &pipe_config->fdi_m_n, NULL);
 	}
 
 	ironlake_set_pipeconf(pipe_config);
@@ -5611,7 +5611,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
-	if (intel_crtc->config->has_pch_encoder) {
+	if (pipe_config->has_pch_encoder) {
 		/* Note: FDI PLL enabling _must_ be done before we enable the
 		 * cpu pipes, hence this is separate from all the other fdi/pch
 		 * enabling. */
@@ -5630,10 +5630,10 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_color_load_luts(&pipe_config->base);
 
 	if (dev_priv->display.initial_watermarks != NULL)
-		dev_priv->display.initial_watermarks(old_intel_state, intel_crtc->config);
+		dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
 	intel_enable_pipe(pipe_config);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (pipe_config->has_pch_encoder)
 		ironlake_pch_enable(old_intel_state, pipe_config);
 
 	assert_vblank_disabled(crtc);
@@ -5650,7 +5650,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	 * some interlaced HDMI modes. Let's do the double wait always
 	 * in case there are more corner cases we don't know about.
 	 */
-	if (intel_crtc->config->has_pch_encoder) {
+	if (pipe_config->has_pch_encoder) {
 		intel_wait_for_vblank(dev_priv, pipe);
 		intel_wait_for_vblank(dev_priv, pipe);
 	}
@@ -5699,7 +5699,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe, hsw_workaround_pipe;
-	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	struct intel_atomic_state *old_intel_state =
 		to_intel_atomic_state(old_state);
 	bool psl_clkgate_wa;
@@ -5718,7 +5718,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+	if (intel_crtc_has_dp_encoder(pipe_config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	if (!transcoder_is_dsi(cpu_transcoder))
@@ -5729,12 +5729,12 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (cpu_transcoder != TRANSCODER_EDP &&
 	    !transcoder_is_dsi(cpu_transcoder)) {
 		I915_WRITE(PIPE_MULT(cpu_transcoder),
-			   intel_crtc->config->pixel_multiplier - 1);
+			   pipe_config->pixel_multiplier - 1);
 	}
 
-	if (intel_crtc->config->has_pch_encoder) {
+	if (pipe_config->has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
-				     &intel_crtc->config->fdi_m_n, NULL);
+				     &pipe_config->fdi_m_n, NULL);
 	}
 
 	if (!transcoder_is_dsi(cpu_transcoder))
@@ -5748,7 +5748,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
 	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
-			 intel_crtc->config->pch_pfit.enabled;
+			 pipe_config->pch_pfit.enabled;
 	if (psl_clkgate_wa)
 		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
 
@@ -5788,10 +5788,10 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_enable_pipe(pipe_config);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (pipe_config->has_pch_encoder)
 		lpt_pch_enable(old_intel_state, pipe_config);
 
-	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP_MST))
+	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
 		intel_ddi_set_vc_payload_alloc(pipe_config, true);
 
 	assert_vblank_disabled(crtc);
@@ -5854,12 +5854,12 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	ironlake_pfit_disable(old_crtc_state);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (old_crtc_state->has_pch_encoder)
 		ironlake_fdi_disable(crtc);
 
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
-	if (intel_crtc->config->has_pch_encoder) {
+	if (old_crtc_state->has_pch_encoder) {
 		ironlake_disable_pch_transcoder(dev_priv, pipe);
 
 		if (HAS_PCH_CPT(dev_priv)) {
@@ -6060,7 +6060,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (WARN_ON(intel_crtc->active))
 		return;
 
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+	if (intel_crtc_has_dp_encoder(pipe_config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	intel_set_pipe_timings(pipe_config);
@@ -6082,11 +6082,11 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
 	if (IS_CHERRYVIEW(dev_priv)) {
-		chv_prepare_pll(intel_crtc, intel_crtc->config);
-		chv_enable_pll(intel_crtc, intel_crtc->config);
+		chv_prepare_pll(intel_crtc, pipe_config);
+		chv_enable_pll(intel_crtc, pipe_config);
 	} else {
-		vlv_prepare_pll(intel_crtc, intel_crtc->config);
-		vlv_enable_pll(intel_crtc, intel_crtc->config);
+		vlv_prepare_pll(intel_crtc, pipe_config);
+		vlv_enable_pll(intel_crtc, pipe_config);
 	}
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
@@ -6105,13 +6105,13 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_encoders_enable(crtc, pipe_config, old_state);
 }
 
-static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
+static void i9xx_set_pll_dividers(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc_state *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	I915_WRITE(FP0(crtc->pipe), crtc->config->dpll_hw_state.fp0);
-	I915_WRITE(FP1(crtc->pipe), crtc->config->dpll_hw_state.fp1);
+	I915_WRITE(FP0(crtc->pipe), crtc_state->dpll_hw_state.fp0);
+	I915_WRITE(FP1(crtc->pipe), crtc_state->dpll_hw_state.fp1);
 }
 
 static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
@@ -6130,7 +6130,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	i9xx_set_pll_dividers(intel_crtc);
 
-	if (intel_crtc_has_dp_encoder(intel_crtc->config))
+	if (intel_crtc_has_dp_encoder(pipe_config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	intel_set_pipe_timings(pipe_config);
@@ -6153,7 +6153,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	if (dev_priv->display.initial_watermarks != NULL)
 		dev_priv->display.initial_watermarks(old_intel_state,
-						     intel_crtc->config);
+						     pipe_config);
 	else
 		intel_update_watermarks(intel_crtc);
 	intel_enable_pipe(pipe_config);
@@ -6206,7 +6206,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
 
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
 
-	if (!intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI)) {
+	if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI)) {
 		if (IS_CHERRYVIEW(dev_priv))
 			chv_disable_pll(dev_priv, pipe);
 		else if (IS_VALLEYVIEW(dev_priv))
-- 
2.19.0

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

Re: [PATCH 13/13] drm/i915: Get rid of intel_crtc->config in crtc_enable/disable functions

[Ville Syrjälä]

On Wed, Oct 03, 2018 at 03:37:15PM +0200, Maarten Lankhorst wrote:
> These functions already have a pointer to the correct state,
> so use it instead of crtc->config.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++--------------
>  1 file changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 22f556b824f9..01ad6bd3b7de 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5594,15 +5594,15 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (pipe_config->has_pch_encoder)
>  		intel_prepare_shared_dpll(pipe_config);
>  
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(pipe_config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
>  	intel_set_pipe_timings(pipe_config);
>  	intel_set_pipe_src_size(pipe_config);
>  
> -	if (intel_crtc->config->has_pch_encoder) {
> +	if (pipe_config->has_pch_encoder) {
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
> -				     &intel_crtc->config->fdi_m_n, NULL);
> +				     &pipe_config->fdi_m_n, NULL);
>  	}
>  
>  	ironlake_set_pipeconf(pipe_config);
> @@ -5611,7 +5611,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
> -	if (intel_crtc->config->has_pch_encoder) {
> +	if (pipe_config->has_pch_encoder) {
>  		/* Note: FDI PLL enabling _must_ be done before we enable the
>  		 * cpu pipes, hence this is separate from all the other fdi/pch
>  		 * enabling. */
> @@ -5630,10 +5630,10 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_color_load_luts(&pipe_config->base);
>  
>  	if (dev_priv->display.initial_watermarks != NULL)
> -		dev_priv->display.initial_watermarks(old_intel_state, intel_crtc->config);
> +		dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
>  	intel_enable_pipe(pipe_config);
>  
> -	if (intel_crtc->config->has_pch_encoder)
> +	if (pipe_config->has_pch_encoder)
>  		ironlake_pch_enable(old_intel_state, pipe_config);
>  
>  	assert_vblank_disabled(crtc);
> @@ -5650,7 +5650,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	 * some interlaced HDMI modes. Let's do the double wait always
>  	 * in case there are more corner cases we don't know about.
>  	 */
> -	if (intel_crtc->config->has_pch_encoder) {
> +	if (pipe_config->has_pch_encoder) {
>  		intel_wait_for_vblank(dev_priv, pipe);
>  		intel_wait_for_vblank(dev_priv, pipe);
>  	}
> @@ -5699,7 +5699,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe, hsw_workaround_pipe;
> -	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>  	struct intel_atomic_state *old_intel_state =
>  		to_intel_atomic_state(old_state);
>  	bool psl_clkgate_wa;
> @@ -5718,7 +5718,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(pipe_config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
>  	if (!transcoder_is_dsi(cpu_transcoder))
> @@ -5729,12 +5729,12 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (cpu_transcoder != TRANSCODER_EDP &&
>  	    !transcoder_is_dsi(cpu_transcoder)) {
>  		I915_WRITE(PIPE_MULT(cpu_transcoder),
> -			   intel_crtc->config->pixel_multiplier - 1);
> +			   pipe_config->pixel_multiplier - 1);
>  	}
>  
> -	if (intel_crtc->config->has_pch_encoder) {
> +	if (pipe_config->has_pch_encoder) {
>  		intel_cpu_transcoder_set_m_n(intel_crtc,
> -				     &intel_crtc->config->fdi_m_n, NULL);
> +				     &pipe_config->fdi_m_n, NULL);
>  	}
>  
>  	if (!transcoder_is_dsi(cpu_transcoder))
> @@ -5748,7 +5748,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
>  	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> -			 intel_crtc->config->pch_pfit.enabled;
> +			 pipe_config->pch_pfit.enabled;
>  	if (psl_clkgate_wa)
>  		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
>  
> @@ -5788,10 +5788,10 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (!transcoder_is_dsi(cpu_transcoder))
>  		intel_enable_pipe(pipe_config);
>  
> -	if (intel_crtc->config->has_pch_encoder)
> +	if (pipe_config->has_pch_encoder)
>  		lpt_pch_enable(old_intel_state, pipe_config);
>  
> -	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP_MST))
> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
>  		intel_ddi_set_vc_payload_alloc(pipe_config, true);
>  
>  	assert_vblank_disabled(crtc);
> @@ -5854,12 +5854,12 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  
>  	ironlake_pfit_disable(old_crtc_state);
>  
> -	if (intel_crtc->config->has_pch_encoder)
> +	if (old_crtc_state->has_pch_encoder)
>  		ironlake_fdi_disable(crtc);
>  
>  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>  
> -	if (intel_crtc->config->has_pch_encoder) {
> +	if (old_crtc_state->has_pch_encoder) {
>  		ironlake_disable_pch_transcoder(dev_priv, pipe);
>  
>  		if (HAS_PCH_CPT(dev_priv)) {
> @@ -6060,7 +6060,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (WARN_ON(intel_crtc->active))
>  		return;
>  
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(pipe_config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
>  	intel_set_pipe_timings(pipe_config);
> @@ -6082,11 +6082,11 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
>  	if (IS_CHERRYVIEW(dev_priv)) {
> -		chv_prepare_pll(intel_crtc, intel_crtc->config);
> -		chv_enable_pll(intel_crtc, intel_crtc->config);
> +		chv_prepare_pll(intel_crtc, pipe_config);
> +		chv_enable_pll(intel_crtc, pipe_config);
>  	} else {
> -		vlv_prepare_pll(intel_crtc, intel_crtc->config);
> -		vlv_enable_pll(intel_crtc, intel_crtc->config);
> +		vlv_prepare_pll(intel_crtc, pipe_config);
> +		vlv_enable_pll(intel_crtc, pipe_config);
>  	}
>  
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
> @@ -6105,13 +6105,13 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_encoders_enable(crtc, pipe_config, old_state);
>  }
>  
> -static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
> +static void i9xx_set_pll_dividers(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct intel_crtc_state *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  
> -	I915_WRITE(FP0(crtc->pipe), crtc->config->dpll_hw_state.fp0);
> -	I915_WRITE(FP1(crtc->pipe), crtc->config->dpll_hw_state.fp1);
> +	I915_WRITE(FP0(crtc->pipe), crtc_state->dpll_hw_state.fp0);
> +	I915_WRITE(FP1(crtc->pipe), crtc_state->dpll_hw_state.fp1);
>  }
>  
>  static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
> @@ -6130,7 +6130,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	i9xx_set_pll_dividers(intel_crtc);

Missed the caller. But maybe you'll take my suggestion and move the
i9xx_set_pll_dividerschange () into the pll patch anyway.

Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

And the compiler should catch what I didn't ;)

>  
> -	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> +	if (intel_crtc_has_dp_encoder(pipe_config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
>  	intel_set_pipe_timings(pipe_config);
> @@ -6153,7 +6153,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	if (dev_priv->display.initial_watermarks != NULL)
>  		dev_priv->display.initial_watermarks(old_intel_state,
> -						     intel_crtc->config);
> +						     pipe_config);
>  	else
>  		intel_update_watermarks(intel_crtc);
>  	intel_enable_pipe(pipe_config);
> @@ -6206,7 +6206,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  
>  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>  
> -	if (!intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI)) {
> +	if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI)) {
>  		if (IS_CHERRYVIEW(dev_priv))
>  			chv_disable_pll(dev_priv, pipe);
>  		else if (IS_VALLEYVIEW(dev_priv))
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

✗ Fi.CI.BAT: failure for drm/i915: First cleanup pass to get rid of more crtc->config users.

[Patchwork]

== Series Details ==

Series: drm/i915: First cleanup pass to get rid of more crtc->config users.
URL   : https://patchwork.freedesktop.org/series/50506/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/intel_display.o
In file included from ./include/linux/list.h:9:0,
                 from ./include/linux/dmi.h:5,
                 from drivers/gpu/drm/i915/intel_display.c:27:
drivers/gpu/drm/i915/intel_display.c: In function ‘i9xx_set_pll_dividers’:
./include/linux/kernel.h:995:41: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
 #define container_of(ptr, type, member) ({    \
                                         ^
drivers/gpu/drm/i915/intel_drv.h:1003:26: note: in expansion of macro ‘container_of’
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
                          ^~~~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:6110:34: note: in expansion of macro ‘to_intel_crtc’
  struct intel_crtc_state *crtc = to_intel_crtc(crtc_state->base.crtc);
                                  ^~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:6111:56: error: ‘struct drm_crtc_state’ has no member named ‘dev’
  struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
                                                        ^
In file included from drivers/gpu/drm/i915/intel_drv.h:33:0,
                 from drivers/gpu/drm/i915/intel_display.c:36:
drivers/gpu/drm/i915/intel_display.c:6113:21: error: ‘struct intel_crtc_state’ has no member named ‘pipe’
  I915_WRITE(FP0(crtc->pipe), crtc_state->dpll_hw_state.fp0);
                     ^
drivers/gpu/drm/i915/i915_drv.h:3617:76: note: in definition of macro ‘I915_WRITE’
 #define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), true)
                                                                            ^~~
drivers/gpu/drm/i915/i915_reg.h:161:32: note: in expansion of macro ‘_MMIO’
 #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
                                ^~~~~
drivers/gpu/drm/i915/i915_reg.h:160:27: note: in expansion of macro ‘_PICK_EVEN’
 #define _PIPE(pipe, a, b) _PICK_EVEN(pipe, a, b)
                           ^~~~~~~~~~
drivers/gpu/drm/i915/i915_reg.h:161:38: note: in expansion of macro ‘_PIPE’
 #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
                                      ^~~~~
drivers/gpu/drm/i915/i915_reg.h:3315:19: note: in expansion of macro ‘_MMIO_PIPE’
 #define FP0(pipe) _MMIO_PIPE(pipe, _FPA0, _FPB0)
                   ^~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:6113:13: note: in expansion of macro ‘FP0’
  I915_WRITE(FP0(crtc->pipe), crtc_state->dpll_hw_state.fp0);
             ^~~
drivers/gpu/drm/i915/intel_display.c:6114:21: error: ‘struct intel_crtc_state’ has no member named ‘pipe’
  I915_WRITE(FP1(crtc->pipe), crtc_state->dpll_hw_state.fp1);
                     ^
drivers/gpu/drm/i915/i915_drv.h:3617:76: note: in definition of macro ‘I915_WRITE’
 #define I915_WRITE(reg, val) dev_priv->uncore.funcs.mmio_writel(dev_priv, (reg), (val), true)
                                                                            ^~~
drivers/gpu/drm/i915/i915_reg.h:161:32: note: in expansion of macro ‘_MMIO’
 #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
                                ^~~~~
drivers/gpu/drm/i915/i915_reg.h:160:27: note: in expansion of macro ‘_PICK_EVEN’
 #define _PIPE(pipe, a, b) _PICK_EVEN(pipe, a, b)
                           ^~~~~~~~~~
drivers/gpu/drm/i915/i915_reg.h:161:38: note: in expansion of macro ‘_PIPE’
 #define _MMIO_PIPE(pipe, a, b) _MMIO(_PIPE(pipe, a, b))
                                      ^~~~~
drivers/gpu/drm/i915/i915_reg.h:3316:19: note: in expansion of macro ‘_MMIO_PIPE’
 #define FP1(pipe) _MMIO_PIPE(pipe, _FPA1, _FPB1)
                   ^~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:6114:13: note: in expansion of macro ‘FP1’
  I915_WRITE(FP1(crtc->pipe), crtc_state->dpll_hw_state.fp1);
             ^~~
drivers/gpu/drm/i915/intel_display.c: In function ‘i9xx_crtc_enable’:
drivers/gpu/drm/i915/intel_display.c:6131:24: error: passing argument 1 of ‘i9xx_set_pll_dividers’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  i9xx_set_pll_dividers(intel_crtc);
                        ^~~~~~~~~~
drivers/gpu/drm/i915/intel_display.c:6108:13: note: expected ‘const struct intel_crtc_state *’ but argument is of type ‘struct intel_crtc *’
 static void i9xx_set_pll_dividers(const struct intel_crtc_state *crtc_state)
             ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:305: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:546: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1050: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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