[RFC v3 14/15] drm/i915/ddi: handle 128b/132b SST in intel_ddi_read_func_ctl()

[Imre Deak]

On Thu, Dec 19, 2024 at 03:24:22PM +0200, Jani Nikula wrote:
> On Wed, 18 Dec 2024, Imre Deak <imre.deak at intel.com> wrote:
> > From: Jani Nikula <jani.nikula at intel.com>
> >
> > We'll only ever get here in MST mode from MST stream encoders; the
> > primary encoder's ->get_config() won't be called when we've detected
> > it's MST.
> >
> > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> > [hack: fix enhanced framing hw readout/set master transcoder] (Imre)
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 34 ++++++++++++++++---
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  8 +++--
> >  .../drm/i915/display/intel_dp_link_training.c |  5 ++-
> >  3 files changed, 39 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 21dba87350856..14d8eb5f02ff1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -577,10 +577,18 @@ intel_ddi_transcoder_func_reg_val_get(struct intel_encoder *encoder,
> >  			temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
> >  		}
> >  	} else {
> > -		if (intel_dp_is_uhbr(crtc_state))
> > +		if (intel_dp_is_uhbr(crtc_state)) {
> > +			enum transcoder master;
> > +
> >  			temp |= TRANS_DDI_MODE_SELECT_FDI_OR_128B132B;
> > -		else
> > +
> > +			master = crtc_state->mst_master_transcoder;
> > +			drm_WARN_ON(&dev_priv->drm,
> > +				    master == INVALID_TRANSCODER);
> > +			temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
> > +		} else {
> >  			temp |= TRANS_DDI_MODE_SELECT_DP_SST;
> > +		}
> >  		temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
> >  	}
> >  
> > @@ -678,7 +686,8 @@ void intel_ddi_disable_transcoder_func(const struct intel_crtc_state *crtc_state
> >  			 TRANS_DDI_PORT_SYNC_MASTER_SELECT_MASK);
> >  
> >  	if (DISPLAY_VER(dev_priv) >= 12) {
> > -		if (!intel_dp_mst_is_master_trans(crtc_state)) {
> > +		if (!intel_dp_mst_is_master_trans(crtc_state) ||
> > +		    intel_dp_is_uhbr(crtc_state)) {
> 
> Hmm, what if this gets called from UHBR MST where this would also
> disable master transcoder?
> 
> I'll switch this to:
> 
> 	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> 
>         ...
> 
> 		if (!intel_dp_mst_is_master_trans(crtc_state) ||
> 		    (!is_mst && intel_dp_is_uhbr(crtc_state)) {

Yes, missed that case, the above change looks ok.

> 
> BR,
> Jani.
> 
> 
> 
> >  			ctl &= ~(TGL_TRANS_DDI_PORT_MASK |
> >  				 TRANS_DDI_MODE_SELECT_MASK);
> >  		}
> > @@ -4028,6 +4037,11 @@ static void intel_ddi_read_func_ctl_dp_sst(struct intel_encoder *encoder,
> >  	crtc_state->lane_count =
> >  		((ddi_func_ctl & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> >  
> > +	if (DISPLAY_VER(display) >= 12 &&
> > +	    (ddi_func_ctl & TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B)
> > +		crtc_state->mst_master_transcoder =
> > +			REG_FIELD_GET(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, ddi_func_ctl);
> > +
> >  	intel_cpu_transcoder_get_m1_n1(crtc, cpu_transcoder, &crtc_state->dp_m_n);
> >  	intel_cpu_transcoder_get_m2_n2(crtc, cpu_transcoder, &crtc_state->dp_m2_n2);
> >  
> > @@ -4122,9 +4136,19 @@ static void intel_ddi_read_func_ctl(struct intel_encoder *encoder,
> >  		intel_ddi_read_func_ctl_fdi(encoder, pipe_config, ddi_func_ctl);
> >  	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_SST) {
> >  		intel_ddi_read_func_ctl_dp_sst(encoder, pipe_config, ddi_func_ctl);
> > -	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST ||
> > -		   (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display))) {
> > +	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_DP_MST) {
> >  		intel_ddi_read_func_ctl_dp_mst(encoder, pipe_config, ddi_func_ctl);
> > +	} else if (ddi_mode == TRANS_DDI_MODE_SELECT_FDI_OR_128B132B && HAS_DP20(display)) {
> > +		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +
> > +		/*
> > +		 * If this is true, we know we're being called from mst stream
> > +		 * encoder's ->get_config().
> > +		 */
> > +		if (intel_dp->is_mst)
> > +			intel_ddi_read_func_ctl_dp_mst(encoder, pipe_config, ddi_func_ctl);
> > +		else
> > +			intel_ddi_read_func_ctl_dp_sst(encoder, pipe_config, ddi_func_ctl);
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index fba3af3382808..dc84979d32947 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -3148,8 +3148,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  	pipe_config->limited_color_range =
> >  		intel_dp_limited_color_range(pipe_config, conn_state);
> >  
> > -	pipe_config->enhanced_framing =
> > -		drm_dp_enhanced_frame_cap(intel_dp->dpcd);
> > +	if (intel_dp_is_uhbr(pipe_config)) {
> > +		pipe_config->mst_master_transcoder = pipe_config->cpu_transcoder;
> > +	} else {
> > +		pipe_config->enhanced_framing =
> > +			drm_dp_enhanced_frame_cap(intel_dp->dpcd);
> > +	}
> >  
> >  	if (pipe_config->dsc.compression_enable)
> >  		link_bpp_x16 = pipe_config->dsc.compressed_bpp_x16;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index 8b1977cfec503..7304220736b47 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -760,8 +760,11 @@ static void intel_dp_update_link_bw_set(struct intel_dp *intel_dp,
> >  					const struct intel_crtc_state *crtc_state,
> >  					u8 link_bw, u8 rate_select)
> >  {
> > +	bool enhanced_framing = crtc_state->enhanced_framing ||
> > +				intel_dp_is_uhbr(crtc_state);
> > +
> >  	intel_dp_link_training_set_bw(intel_dp, link_bw, rate_select, crtc_state->lane_count,
> > -				      crtc_state->enhanced_framing);
> > +				      enhanced_framing);
> >  }
> >  
> >  /*
> 
> -- 
> Jani Nikula, Intel