Re: [PATCH 5/5] drm/i915/dp: Enable Common SDP Transmission line

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 3/6/2026 5:26 PM, Ville Syrjälä wrote:
> On Thu, Mar 05, 2026 at 09:31:18AM +0530, Ankit Nautiyal wrote:
>> Enable the Common SDP Transmission line for the SDP packets.
>> We get the clear picture about the SDPs and guardband only in
>> intel_dp_sdp_compute_config_late() therefore we must configure the
>> crtc_state members related to Common SDP Transmission line at this
>> point.
>>
>> Currently, the stagger values are set as per the default policy of the
>> Hardware. This can be optimized later if we come up with a specific
>> driver policy to sequence the SDPs better.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c |  3 ++
>>   drivers/gpu/drm/i915/display/intel_dp.c  | 64 ++++++++++++++++++++++++
>>   2 files changed, 67 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index bdbd89600bee..e1e95f18f0ed 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2726,6 +2726,8 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>>   	/* 6.o Configure and enable FEC if needed */
>>   	intel_ddi_enable_fec(encoder, crtc_state);
>>   
>> +	intel_dp_cmn_sdp_transmission_line_enable(crtc_state);
>> +
>>   	/* 7.a 128b/132b SST. */
>>   	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
>>   		/* VCPID 1, start slot 0 for 128b/132b, tu slots */
>> @@ -3113,6 +3115,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
>>   			     DP_TP_CTL_ENABLE, 0);
>>   	}
>>   
>> +	intel_dp_cmn_sdp_transmission_line_disable(crtc_state);
>>   	intel_ddi_disable_fec(encoder, crtc_state);
>>   
>>   	if (DISPLAY_VER(display) < 14)
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 59a8fa5e5ba9..87d73d0239bc 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -7189,6 +7189,68 @@ void intel_dp_mst_resume(struct intel_display *display)
>>   	}
>>   }
>>   
>> +static
>> +void intel_dp_cmn_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_display *display = to_intel_display(crtc_state);
>> +	bool as_sdp, gmp_sdp, pps_sdp, vsc_sdp, vsc_ext_sdp;
>> +
>> +	if (!HAS_CMN_SDP_TL(display))
>> +		return;
>> +
>> +	as_sdp = crtc_state->infoframes.enable &
>> +		 intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
>> +
>> +	gmp_sdp = crtc_state->infoframes.enable &
>> +		  intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA);
>> +
>> +	pps_sdp = crtc_state->infoframes.enable &
>> +		  intel_hdmi_infoframe_enable(DP_SDP_PPS);
>> +
>> +	vsc_sdp = crtc_state->infoframes.enable &
>> +		  intel_hdmi_infoframe_enable(DP_SDP_VSC);
>> +
>> +	vsc_ext_sdp = crtc_state->infoframes.enable &
>> +		      (intel_hdmi_infoframe_enable(DP_SDP_VSC_EXT_VESA) |
>> +		       intel_hdmi_infoframe_enable(DP_SDP_VSC_EXT_CEA));
>> +
>> +	if (!gmp_sdp && !pps_sdp && !vsc_sdp && !vsc_ext_sdp)
>> +		return;
> I don't see why we wouldn't just always program this stuff.


Hmm..Yeah you are right, we can always program this stuff. By not 
enabling, the transcoder will simply be ready to use its own logic.

I will drop this check and the variables to track the SDPs. Later at 
some point when we want to optimize the staggering, then we might need 
something like these.


Regards,

Ankit

>
>> +
>> +	crtc_state->cmn_sdp_tl.enable = true;
>> +
>> +	/*
>> +	 * When AS SDP is enabled :
>> +	 *  - The common SDP Transmission Line matches the EMP SDP Transmission Line.
>> +	 *
>> +	 * When AS SDP is disabled:
>> +	 *  - Bspec mentions the positions as lines of delayed vblank.
>> +	 *  - Guardband = 1st line of delayed vblank
>> +	 *  - Common SDP Transmission line is set to 2nd line of delayed vblank.
>> +	 */
>> +
>> +	if (as_sdp)
>> +		crtc_state->cmn_sdp_tl.transmission_line =
>> +			intel_dp_emp_as_sdp_tl(crtc_state);
>> +	else
>> +		crtc_state->cmn_sdp_tl.transmission_line =
>> +			crtc_state->vrr.guardband - 1;
>> +
>> +	/*
>> +	 * Currently we are programming the default stagger values, but these
>> +	 * can be optimized if required, based on number of SDPs enabled.
>> +	 *
>> +	 * Default values of the Transmission lines for SDPs other than AS SDP:
>> +	 * VSC : CMN SDP Transmission line
>> +	 * GMP : CMN SDP Transmission line
>> +	 * PPS : CMN SDP Transmission line + 1
>> +	 * VSC_EXT: CMN SDP Transmission line + 2
>> +	 */
>> +	crtc_state->cmn_sdp_tl.gmp_stagger = GMP_STAGGER_DEFAULT;
>> +	crtc_state->cmn_sdp_tl.pps_stagger = PPS_STAGGER_DEFAULT;
>> +	crtc_state->cmn_sdp_tl.vsc_ext_stagger = VSC_EXT_STAGGER_DEFAULT;
>> +}
>> +
>>   static
>>   int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
>>   {
>> @@ -7202,6 +7264,8 @@ int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
>>   		return -EINVAL;
>>   	}
>>   
>> +	intel_dp_cmn_sdp_tl_compute_config_late(crtc_state);
>> +
>>   	return 0;
>>   }
>>   
>> -- 
>> 2.45.2