Re: [PATCH v2 4/8] drm/i915: Pass pipe_config to pch_enable() functions

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Mar 01, 2017 at 04:13:14PM +0200, Ander Conselvan de Oliveira wrote:
> Using crtc->config directly is being removed in favor of passing a
> pipe_config. Follow the trend and pass pipe_config to pch_enable()
> functions.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 45415f0..aea302b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4456,7 +4456,8 @@ intel_trans_dp_port_sel(struct intel_crtc *crtc)
>   *   - DP transcoding bits
>   *   - transcoder
>   */
> -static void ironlake_pch_enable(struct intel_crtc *crtc)
> +static void ironlake_pch_enable(struct intel_crtc *crtc,
> +				struct intel_crtc_state *pipe_config)

const

Also s/pipe_config/crtc_state/ perhaps? IMO we should try to start
unifying our naming convention to reduce the confusion a bit.

You could also pass just the state since you can dig up the crtc from
there.

>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -4484,7 +4485,7 @@ static void ironlake_pch_enable(struct intel_crtc *crtc)
>  		temp = I915_READ(PCH_DPLL_SEL);
>  		temp |= TRANS_DPLL_ENABLE(pipe);
>  		sel = TRANS_DPLLB_SEL(pipe);
> -		if (crtc->config->shared_dpll ==
> +		if (pipe_config->shared_dpll ==
>  		    intel_get_shared_dpll_by_id(dev_priv, DPLL_ID_PCH_PLL_B))
>  			temp |= sel;
>  		else
> @@ -4509,9 +4510,9 @@ static void ironlake_pch_enable(struct intel_crtc *crtc)
>  
>  	/* For PCH DP, enable TRANS_DP_CTL */
>  	if (HAS_PCH_CPT(dev_priv) &&
> -	    intel_crtc_has_dp_encoder(crtc->config)) {
> +	    intel_crtc_has_dp_encoder(pipe_config)) {
>  		const struct drm_display_mode *adjusted_mode =
> -			&crtc->config->base.adjusted_mode;
> +			&pipe_config->base.adjusted_mode;
>  		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) >> 5;
>  		i915_reg_t reg = TRANS_DP_CTL(pipe);
>  		temp = I915_READ(reg);
> @@ -4546,10 +4547,11 @@ static void ironlake_pch_enable(struct intel_crtc *crtc)
>  	ironlake_enable_pch_transcoder(dev_priv, pipe);
>  }
>  
> -static void lpt_pch_enable(struct intel_crtc *crtc)
> +static void lpt_pch_enable(struct intel_crtc *crtc,
> +			   struct intel_crtc_state *pipe_config)

ditto

Anyway looks correct, so w/ or w/o the extra ideas this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>  
>  	assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A);
>  
> @@ -5283,7 +5285,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_enable_pipe(intel_crtc);
>  
>  	if (intel_crtc->config->has_pch_encoder)
> -		ironlake_pch_enable(intel_crtc);
> +		ironlake_pch_enable(intel_crtc, pipe_config);
>  
>  	assert_vblank_disabled(crtc);
>  	drm_crtc_vblank_on(crtc);
> @@ -5393,7 +5395,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  		intel_enable_pipe(intel_crtc);
>  
>  	if (intel_crtc->config->has_pch_encoder)
> -		lpt_pch_enable(intel_crtc);
> +		lpt_pch_enable(intel_crtc, pipe_config);
>  
>  	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP_MST))
>  		intel_ddi_set_vc_payload_alloc(crtc, true);
> -- 
> 2.9.3

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