Re: [PATCH 02/12] drm/i915/vrr: Add helper to readback EMP_AS_SDP_TL

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

On Mon, Apr 13, 2026 at 03:24:30PM +0530, Nautiyal, Ankit K wrote:
> 
> On 4/13/2026 2:55 PM, Ville Syrjälä wrote:
> > On Mon, Apr 13, 2026 at 09:23:39AM +0530, Ankit Nautiyal wrote:
> >> EMP_AS_SDP_TL is used to program both DP Adaptive Sync SDP and HDMI
> >> Video Timing EMP for VRR operation. Add a helper to read back the
> >> programmed transmission line from hardware so VRR code can populate
> >> the corresponding CRTC state fields during get_config.
> >>
> >> This provides a common read-back path for VRR packet transmission
> >> line state.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_vrr.c | 13 +++++++++++++
> >>   drivers/gpu/drm/i915/display/intel_vrr.h |  1 +
> >>   2 files changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> >> index 1fed597439b0..abdae7f1f8a8 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> >> @@ -1218,3 +1218,16 @@ int intel_vrr_dcb_vmax_vblank_start_final(const struct intel_crtc_state *crtc_st
> >>   
> >>   	return intel_vrr_vblank_start(crtc_state, VRR_DCB_VMAX(tmp) + 1);
> >>   }
> >> +
> >> +u16 intel_vrr_read_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
> >> +{
> >> +	struct intel_display *display = to_intel_display(crtc_state);
> >> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >> +	u32 val;
> >> +
> >> +	if (!HAS_EMP_AS_SDP_TL(display))
> >> +		return 0;
> >> +
> >> +	val = intel_de_read(display, EMP_AS_SDP_TL(display, cpu_transcoder));
> >> +	return REG_FIELD_GET(EMP_AS_SDP_DB_TL_MASK, val);
> > This stuff really doesn't seem to belong in intel_vrr.c. We probably need
> > some kind of infoframe/SDP file where all the relevant stuff lives. Long
> > ago I did attempt to extract all the infoframe stuff from intel_hdmi.c
> > into intel_dip.c (or something like that), but there were still far too
> > many HDMI specifics in the result for my liking.
> >
> > So I think what's really needed is an effort to distill the core of the
> > video DIP implementation (really just the low level buffer read/write
> > stuff, and I suspect now also this transmission line stuff) into a new
> > file. You could perhaps introduce that new file here, and then we'll
> > try to get to extracting the DIP buffer stuff later.
> Alright. I can start with a new file for SDP transmission line stuff.
> 
> Since we are on the topic, I had a query:
> The hsw_infoframe_enable() switch mixes HDMI packet types and DP SDP 
> types in a single namespace.
> The types currently used don't collide, but I think these might collide 
> at some point, (if not already).
> Do we need to change this? OR these are guaranteed to be non-overlapping?

We should perhaps introduce a DIP type enum and have the higher level
code convert the HDMI/DP specific types into that.


-- 
Ville Syrjälä
Intel