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

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

On Mon, Mar 30, 2026 at 09:36:55AM +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 patch will always enable AS
> SDP when the source and sink support it.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 67 ++++++++++++++++---------
>  1 file changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 343bb2a86675..7da3dee226a0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3142,30 +3142,17 @@ static bool intel_dp_can_use_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;
> -
>  	if (!intel_dp_can_use_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,
> @@ -7370,11 +7357,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;
> +	}

Nothing in here right now depends on anything but
intel_vrr_compute_config(). Was there something specific you will
need to consult here that is computed later?

> +}
> +
> +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",
> @@ -7394,7 +7415,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