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 <[email protected]>
> ---
> 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
