Re: [PATCH 3/5] drm/i915/dp: Add helper for AS SDP TL and fix documentation

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

On 3/6/2026 5:32 PM, Ville Syrjälä wrote:
> On Thu, Mar 05, 2026 at 09:31:16AM +0530, Ankit Nautiyal wrote:
>> Add a helper, intel_dp_emp_as_sdp_tl(), to compute the EMP_AS_SDP_TL
>> value used when programming the double‑buffering point and transmission
>> line for VRR packets.
>> Also improve the documentation: the AS SDP transmission line corresponds
>> to the T1 position, which maps to the start of the Vsync pulse.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c  | 9 +++++++++
>>   drivers/gpu/drm/i915/display/intel_dp.h  | 1 +
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 4 ++--
>>   3 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 86390553800d..9204a813639a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -7288,6 +7288,15 @@ bool intel_dp_joiner_candidate_valid(struct intel_connector *connector,
>>   	return true;
>>   }
>>   
>> +int intel_dp_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
>> +{
>> +	/*
>> +	 * EMP_AS_SDP_TL defines the T1 position : The default AS SDP position
>> +	 * that corresponds to the start of the Vsync pulse.
>> +	 */
>> +	return crtc_state->vrr.vsync_start;
>> +}
> Other parts of the code (eg. ALPM) still just directly use the
> adjusted_mode timings to calculate the same stuff. So this doesn't
> really seem to help us.
>
> Feels like all of our abstractions around this SDP transmission line
> stuff are way too low level, and thus the same information is
> calculated in different ways in different parts of the code. There
> should be a single place that defines the transmission line(s),
> and everyone should just consult that stuff (regardless of whether
> the platform uses implicit transmission lines, EMP_AS_SDP_TL, or
> the new stuff).

Yeah currently AS SDP itself is not used properly for different features.

In the series [1], I am trying to address that. Perhaps that will help 
making things better for the EMP_AS_SDP_TL. (I am yet to send the new 
version for this)

One more thing to note is that if AS SDP is sent,  EMP_AS_SDP_TL is the 
point where VSC SDP is also sent. I am not sure if we need to do 
something in existing code for it yet.


[1] https://patchwork.freedesktop.org/series/161977/


Regards,

Ankit

>
>> +
>>   void intel_dp_cmn_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
>>   {
>>   	struct intel_display *display = to_intel_display(crtc_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>> index 24df234a43d3..abb2fcdea352 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -237,6 +237,7 @@ bool intel_dp_joiner_candidate_valid(struct intel_connector *connector,
>>   	for ((__num_joined_pipes) = 1; (__num_joined_pipes) <= (I915_MAX_PIPES); (__num_joined_pipes)++) \
>>   		for_each_if(intel_dp_joiner_candidate_valid(__connector, (__mode)->hdisplay, __num_joined_pipes))
>>   
>> +int intel_dp_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
>>   void intel_dp_cmn_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state);
>>   
>>   #endif /* __INTEL_DP_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 00ca76dbdd6c..2b4e4e55d008 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -642,12 +642,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>>   	 * double buffering point and transmission line for VRR packets for
>>   	 * HDMI2.1/DP/eDP/DP->HDMI2.1 PCON.
>>   	 * Since currently we support VRR only for DP/eDP, so this is programmed
>> -	 * to for Adaptive Sync SDP to Vsync start.
>> +	 * only for Adaptive Sync SDP.
>>   	 */
>>   	if (DISPLAY_VERx100(display) == 1401 || DISPLAY_VER(display) >= 20)
>>   		intel_de_write(display,
>>   			       EMP_AS_SDP_TL(display, cpu_transcoder),
>> -			       EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));
>> +			       EMP_AS_SDP_DB_TL(intel_dp_emp_as_sdp_tl(crtc_state)));
>>   }
>>   
>>   void
>> -- 
>> 2.45.2