Re: [PATCH 08/14] drm/i915/dp: Add AS SDP support for PR with link ON

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Fri, 2026-02-27 at 06:37 +0200, Ville Syrjälä wrote:
> On Thu, Feb 26, 2026 at 06:10:34PM +0200, Ville Syrjälä wrote:
> > On Mon, Feb 23, 2026 at 07:14:25PM +0530, Ankit Nautiyal wrote:
> > > Adaptive Sync SDP (AS SDP) is also used for Panel Replay (PR).
> > > For PR with Link ON, the AS SDP version V1 is used. When using
> > > AS SDP Version 1, AS SDP payload and parity bytes are all set to
> > > zeroes.
> > 
> > I don't think we should need to transmit any AS SDP in link ON
> > mode.
> 
> Hmm, maybe we do. While the DP spec is is extremely confusing here
> (mostly just talking about the ALPM+AS SDP combo and ignoring the
> link ON case), I think what it might be hinting at is that the sink
> tells us whether it needs the AS SDP, via
> ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR.

This sounds reasonable. It seems I have been reading only the mention
"Source device may choose to disable Adaptive-Sync SDP transmission
during a PR Active state." There is also a note saying in case of eDP
this should be 0 always.

Maybe instead of just checking Panel Replay we could add a helper
checking this bit and use that as a enabler for AS SDP? Unless we are
enabling AS SDP always when supported.

BR,
Jouni Högander

> 
> > 
> > > 
> > > Bspec: 75639
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 17 ++++++++++++++++-
> > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index d08db4938638..5952db6197cf 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -3115,7 +3115,8 @@ static bool intel_dp_needs_as_sdp(struct
> > > intel_dp *intel_dp,
> > >  				  struct intel_crtc_state
> > > *crtc_state)
> > >  {
> > >  	return crtc_state->vrr.enable ||
> > > -	       crtc_state->cmrr.enable;
> > > +	       crtc_state->cmrr.enable ||
> > > +	       crtc_state->has_panel_replay;
> > >  }
> > >  
> > >  static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
> > > @@ -3137,6 +3138,16 @@ static void intel_dp_compute_as_sdp(struct
> > > intel_dp *intel_dp,
> > >  	as_sdp->length = 0x9;
> > >  	as_sdp->duration_incr_ms = 0;
> > >  	as_sdp->version = 0x2;
> > > +
> > > +	if (crtc_state->vrr.enable)
> > > +		as_sdp->version = 0x2;
> > 
> > I think we rather need some versions checks or something to
> > make sure the device actually supports v2. VRR itself shouldn't
> > need v2 since the v1 was originally added exactly for VRR+PCON
> > use cases. PR+ALPM definitely does need v2, but not sure anything
> > else actually needs it.
> > 
> > > +	else
> > > +		as_sdp->version = 0x1;
> > > +
> > > +	/* No payload data bytes for Version 1 */
> > > +	if (as_sdp->version == 0x1)
> > > +		return;
> > > +
> > >  	as_sdp->vtotal = intel_vrr_vmin_vtotal(crtc_state);
> > >  
> > >  	if (crtc_state->cmrr.enable) {
> > > @@ -5005,6 +5016,10 @@ static ssize_t intel_dp_as_sdp_pack(const
> > > struct drm_dp_as_sdp *as_sdp,
> > >  	sdp->sdp_header.HB2 = as_sdp->version;
> > >  	sdp->sdp_header.HB3 = as_sdp->length;
> > >  
> > > +	/* No Payload Data bytes for Version 1 */
> > > +	if (as_sdp->version == 0x1)
> > > +		return length;
> > > +
> > >  	/* Fill AS (Adaptive Sync) SDP Payload */
> > >  	sdp->db[0] = as_sdp->mode;
> > >  	sdp->db[1] = as_sdp->vtotal & 0xFF;
> > > -- 
> > > 2.45.2
> > 
> > -- 
> > Ville Syrjälä
> > Intel
>