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

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

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).

> +
>  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

-- 
Ville Syrjälä
Intel