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 Wed, Mar 11, 2026 at 05:24:22PM +0530, Nautiyal, Ankit K wrote:
> 
> On 3/10/2026 2:38 PM, Ville Syrjälä wrote:
> > On Fri, Mar 06, 2026 at 02:02:09PM +0200, 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).
> > I think instead of tracking the low level stagger values directly, what
> > we want to track is just the transmission line itself for each type of
> > SDP. That seems like a form that is easier to use elsewhere in the code.
> 
> Agreed storing TL will be easier, stagger values can be derived from that.
> 
> So we set the transmission line for the packets only if we plan to send 
> them right?

I don't see why we wouldn't just always set them. And if we set one we
have to anyway set all of them since they're all derived from that one
pair of registers.

> 
> 
> >
> > And to accommodate VRR I suppose we should use the "transmission line
> > is specified relative to the end of vblank/vtotal" convention for all
> > of these.
> 
> Makes sense.
> 
> 
> Regards,
> 
> Ankit
> 
> >

-- 
Ville Syrjälä
Intel