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

Ville Syrjälä ville.syrjala at linux.intel.com
Wed Oct 3 13:59:12 UTC 2018


On Wed, Oct 03, 2018 at 03:37:04PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel


More information about the Intel-gfx mailing list