Re: [Intel-gfx] [PATCH v2 08/11] drm/i915: Introduce crtc_state->enhanced_framing

[Luca Coelho <luca@coelho.fi>]

On Wed, 2023-05-03 at 14:36 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Track DP enhanced framing properly in the crtc state instead
> of relying just on the cached DPCD everywhere, and hook it
> up into the state check and dump.
> 
> v2: Actually set enhanced_framing in .compute_config()
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/g4x_dp.c                 | 10 ++++++++--
>  drivers/gpu/drm/i915/display/intel_crt.c              |  2 ++
>  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c  |  5 +++--
>  drivers/gpu/drm/i915/display/intel_ddi.c              | 11 +++++++++--
>  drivers/gpu/drm/i915/display/intel_display.c          |  1 +
>  drivers/gpu/drm/i915/display/intel_display_types.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_dp.c               |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c |  2 +-
>  8 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 920d570f7594..534546ea7d0b 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -141,7 +141,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>  
>  		intel_de_rmw(dev_priv, TRANS_DP_CTL(crtc->pipe),
>  			     TRANS_DP_ENH_FRAMING,
> -			     drm_dp_enhanced_frame_cap(intel_dp->dpcd) ?
> +			     pipe_config->enhanced_framing ?
>  			     TRANS_DP_ENH_FRAMING : 0);
>  	} else {
>  		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
> @@ -153,7 +153,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>  			intel_dp->DP |= DP_SYNC_VS_HIGH;
>  		intel_dp->DP |= DP_LINK_TRAIN_OFF;
>  
> -		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
> +		if (pipe_config->enhanced_framing)
>  			intel_dp->DP |= DP_ENHANCED_FRAMING;
>  
>  		if (IS_CHERRYVIEW(dev_priv))
> @@ -351,6 +351,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  		u32 trans_dp = intel_de_read(dev_priv,
>  					     TRANS_DP_CTL(crtc->pipe));
>  
> +		if (trans_dp & TRANS_DP_ENH_FRAMING)
> +			pipe_config->enhanced_framing = true;
> +
>  		if (trans_dp & TRANS_DP_HSYNC_ACTIVE_HIGH)
>  			flags |= DRM_MODE_FLAG_PHSYNC;
>  		else
> @@ -361,6 +364,9 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  		else
>  			flags |= DRM_MODE_FLAG_NVSYNC;
>  	} else {
> +		if (tmp & DP_ENHANCED_FRAMING)
> +			pipe_config->enhanced_framing = true;
> +
>  		if (tmp & DP_SYNC_HS_HIGH)
>  			flags |= DRM_MODE_FLAG_PHSYNC;
>  		else
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 13519f78cf9f..52af64aa9953 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -449,6 +449,8 @@ static int hsw_crt_compute_config(struct intel_encoder *encoder,
>  	/* FDI must always be 2.7 GHz */
>  	pipe_config->port_clock = 135000 * 2;
>  
> +	pipe_config->enhanced_framing = true;
> +

Just curious, why are you setting it to true by default here?

Otherwise, the changes look reasonable:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.