Re: [PATCH 09/11] drm/i915/dp: Split AS SDP computation between compute_config and compute_config_late

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

On Mon, May 18, 2026 at 09:24:59AM +0530, Ankit Nautiyal wrote:
> Currently we enable AS SDP only when VRR is enabled. As we start using
> AS SDP for other features, this becomes a problem. The AS SDP
> configuration can change dynamically based on VRR, CMRR, PR, ALPM, etc.
> Since these features may be enabled or disabled after the initial
> configuration, the AS SDP parameters need to be computed later in the
> pipeline.
> 
> However, not all of the AS SDP logic can be moved to the late stage:
> the VRR guardband optimization depends on knowing early whether AS SDP
> can be used. Without this, we would end up accounting for AS SDP on all
> platforms that support it, even for panels that do not support AS SDP.
> Therefore we set the infoframe enable bit for AS SDP during
> compute_config(), before the guardband is computed.
> 
> To handle these constraints, split the AS SDP programming into two
> phases:
> 
>  - intel_dp_compute_as_sdp()
>    Runs during compute_config().
>    Sets only the infoframe enable bit so that the guardband logic can
>    account for AS SDP requirements.
> 
>  - intel_dp_as_sdp_compute_config_late()
>    Runs during compute_config_late().
>    Computes all remaining AS SDP fields based on the features that need
>    it.
> 
> The late-stage computation is called from
> intel_dp_sdp_compute_config_late(), before computing the minimum guardband
> for SDPs.
> 
> This is a preparatory change. A subsequent patches will compute PR related
> AS SDP fields and enable AS SDP not only for VRR but for other features
> as well.

I don't think we actually need this. Based on what I see in the spec it
should be perfectly fine to always provide the coasting vtotal whenever
the sink supports panel replay.

Also I don't think we support the "suspend AS SDP during PR active" mode
yet, so for the moment the sink should never even use the coasting
vtotal value we provide.

> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 66 ++++++++++++++++---------
>  1 file changed, 43 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8d0d04f306a7..c1c6f394eb0b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3187,10 +3187,6 @@ static bool intel_dp_needs_as_sdp(struct intel_dp *intel_dp,
>  static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
>  				    struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_dp_as_sdp *as_sdp = &crtc_state->infoframes.as_sdp;
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> -
>  	/*
>  	 * #FIXME: SDP/infoframe updates aren’t truly atomic, and with the new
>  	 * cdclk->tc clock crossing we may transiently send a corrupted packet
> @@ -3199,23 +3195,13 @@ static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
>  	if (!intel_dp_needs_as_sdp(intel_dp, crtc_state))
>  		return;
>  
> +	/*
> +	 * Only set the infoframes.enable flag here. The remaining AS SDP fields
> +	 * are programmed in the compute_config_late() phase. We need this flag
> +	 * early so that the VRR guardband calculation can properly account for
> +	 * AS SDP requirements.
> +	 */
>  	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
> -
> -	as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC;
> -	as_sdp->length = 0x9;
> -	as_sdp->duration_incr_ms = 0;
> -	as_sdp->revision = 0x2;
> -	as_sdp->vtotal = intel_vrr_vmin_vtotal(crtc_state);
> -
> -	if (crtc_state->cmrr.enable) {
> -		as_sdp->mode = DP_AS_SDP_FAVT_TRR_REACHED;
> -		as_sdp->target_rr = drm_mode_vrefresh(adjusted_mode);
> -		as_sdp->target_rr_divider = true;
> -	} else if (crtc_state->vrr.enable) {
> -		as_sdp->mode = DP_AS_SDP_AVT_DYNAMIC_VTOTAL;
> -	} else {
> -		as_sdp->mode = DP_AS_SDP_AVT_FIXED_VTOTAL;
> -	}
>  }
>  
>  static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
> @@ -7459,11 +7445,45 @@ void intel_dp_mst_resume(struct intel_display *display)
>  }
>  
>  static
> -int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
> +void intel_dp_as_sdp_compute_config_late(struct intel_dp *intel_dp,
> +					 struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_dp_as_sdp *as_sdp = &crtc_state->infoframes.as_sdp;
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->hw.adjusted_mode;
> +
> +	if ((crtc_state->infoframes.enable &
> +	    intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC)) == 0)
> +		return;
> +
> +	as_sdp->sdp_type = DP_SDP_ADAPTIVE_SYNC;
> +	as_sdp->length = 0x9;
> +	as_sdp->duration_incr_ms = 0;
> +	as_sdp->revision = 0x2;
> +	as_sdp->vtotal = intel_vrr_vmin_vtotal(crtc_state);
> +
> +	if (crtc_state->cmrr.enable) {
> +		as_sdp->mode = DP_AS_SDP_FAVT_TRR_REACHED;
> +		as_sdp->target_rr = drm_mode_vrefresh(adjusted_mode);
> +		as_sdp->target_rr_divider = true;
> +	} else if (crtc_state->vrr.enable) {
> +		as_sdp->mode = DP_AS_SDP_AVT_DYNAMIC_VTOTAL;
> +	} else {
> +		as_sdp->mode = DP_AS_SDP_AVT_FIXED_VTOTAL;
> +	}
> +}
> +
> +static
> +int intel_dp_sdp_compute_config_late(struct intel_dp *intel_dp,
> +				     struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	int guardband = intel_crtc_vblank_length(crtc_state);
> -	int min_sdp_guardband = intel_dp_sdp_min_guardband(crtc_state, false);
> +	int min_sdp_guardband;
> +
> +	intel_dp_as_sdp_compute_config_late(intel_dp, crtc_state);
> +
> +	min_sdp_guardband = intel_dp_sdp_min_guardband(crtc_state, false);
>  
>  	if (guardband < min_sdp_guardband) {
>  		drm_dbg_kms(display->drm, "guardband %d < min sdp guardband %d\n",
> @@ -7483,7 +7503,7 @@ int intel_dp_compute_config_late(struct intel_encoder *encoder,
>  
>  	intel_psr_compute_config_late(intel_dp, crtc_state);
>  
> -	ret = intel_dp_sdp_compute_config_late(crtc_state);
> +	ret = intel_dp_sdp_compute_config_late(intel_dp, crtc_state);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel