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

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

On Thu, May 25, 2023 at 12:51:28PM +0300, Luca Coelho wrote:
> 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?

We always want to use enhanced framing with FDI. Setting it here
and doing the readout allows us to also state check it also for FDI.

> 
> Otherwise, the changes look reasonable:
> 
> Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
> 
> --
> Cheers,
> Luca.

-- 
Ville Syrjälä
Intel