On 5/22/2026 8:54 PM, Ville Syrjälä wrote:
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.


Hmm ok, I will drop this change for now.

Regards,

Ankit


Signed-off-by: Ankit Nautiyal <[email protected]>
---
  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

Reply via email to