Re: [PATCH v7 11/14] drm/i915/dip: Enable Common SDP Transmission line

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

On 9/15/2026 10:58 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/i915/dip: Missing CMN_SDP_TL programming for DP MST slave transcoders
> --
>
> commit de3a67cbeba4a6e70cc767d1dce1950454f58dfa
> Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>
> drm/i915/dip: Enable Common SDP Transmission line
>
> This commit enables programming of the common SDP transmission line on
> supported platforms. It computes and programs the base transmission line
> and stagger values during modeset. It also disables the feature on pipe
> disable.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index dacb4b7588a02..3cdb06e81130b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2737,6 +2737,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_dip_cmn_sdp_transmission_line_enable(crtc_state);
>> +
> [Severity: Medium]
> Will placing this transcoder-specific initialization in mtl_ddi_pre_enable_dp()
> cause it to be skipped for subsequent MST streams?
>
> In DP MST configurations, mst_stream_pre_enable() invokes the primary encoder's
> pre_enable hook (mtl_ddi_pre_enable_dp()) solely for the first active MST
> stream.
>
> Because intel_dip_cmn_sdp_transmission_line_enable() is called here, it
> appears it will be skipped for all subsequent streams. This would leave the
> CMN_SDP_TL registers uninitialized for slave transcoders, potentially causing
> broken or mistimed SDP transmission on secondary MST monitors.
>
>>   	/* 7.a 128b/132b SST. */
>>   	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
>>   		/* VCPID 1, start slot 0 for 128b/132b, tu slots */
> [ ... ]
>> @@ -3124,6 +3126,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
>>   			     DP_TP_CTL_ENABLE, 0);
>>   	}
>>   
>> +	intel_dip_cmn_sdp_transmission_line_disable(crtc_state);
> [Severity: Medium]
> Does placing this disable routine in intel_ddi_buf_disable() prevent it from
> executing for slave transcoders during MST teardown?
>
> Similar to the enable path, intel_ddi_buf_disable() is only called for the
> last active stream during teardown, which means the CMN_SDP_TL disable routine
> would be skipped for slave streams.


For both comments. MST + VRR is not yet supported.

Regards,

Ankit

>
>>   	intel_ddi_disable_fec(encoder, crtc_state);
>>   
>>   	if (DISPLAY_VER(display) < 14)