On Thu, Sep 24, 2026 at 03:51:59AM +0800, Xizhe Tang wrote:
> A Panther Lake eDP panel that advertises VRR in EDID but runs at a fixed
> refresh rate has received an Adaptive-Sync SDP since commit 6a1712052859
> ("drm/i915/dp: Enable AS SDP whenever VRR is possible or PR !async").
> On this panel the first modeset at boot is vertically streaked.
>
> intel_vrr_possible() is only crtc_state->vrr.flipline != 0. Fixed-refresh
> timings program flipline too:
>
> intel_vrr_compute_fixed_rr_timings():
> /* For fixed rr, vmin = vmax = flipline */
> crtc_state->vrr.flipline = crtc_state->vrr.vmin;
>
> intel_vrr_compute_config() takes that path when VRR is not actually
> enabled (uapi.vrr_enabled is false, or vmin == vmax). Then
> intel_dp_needs_as_sdp() is true with `vrr: no, fixed rr: yes`, and
> intel_dp_compute_as_sdp() programs DP_AS_SDP_AVT_FIXED_VTOTAL.
>
> Gate the terminal condition on the states that consume the SDP:
> crtc_state->vrr.enable (VRR) or crtc_state->cmrr.enable (CMRR / FAVT).
> Leave the Panel Replay aux-less-ALPM early-return from the same commit
> unchanged.
>
> CMRR is still hard-disabled (is_cmrr_frac_required() has "|| true"), so
> cmrr.enable stays false today and the OR is a no-op versus v1 at fixed
> refresh. intel_vrr_compute_cmrr_timings() sets cmrr.enable without
> vrr.enable; the OR keeps the FAVT branch reachable when CMRR is re-enabled.
>
> This is a no-op while VRR is actually active. It does not fix Adaptive
> Sync = Always corruption, nor the non-atomic SDP update named by the
> #FIXME above intel_dp_compute_as_sdp(). Trailer is Link:, not Closes:.
>
> Tested on LENOVO 21VG (PTL eDP, 8086:b080), v7.2.6-200.fc44.x86_64,
> rebuilding only xe.ko with this hunk:
>
> Adaptive Sync = Never (Tested-by): vrr: no, fixed rr: yes,
> infoframes enabled: 0x6 (no BIT(3)), zero Adaptive-Sync SDP, panel
> clean. This boot: six s2idle suspend/resume cycles, all clean.
>
> Adaptive Sync = Always (not Tested-by): vrr: yes, vmin 2016 / vmax 8064,
> infoframes enabled: 0xe, Adaptive-Sync SDP still sent. Panel
> appearance on Always is not claimed.
>
> CMRR / FAVT: not tested.
>
> On the same panel, Adaptive Sync = Never, first modeset, drm.debug=0xe:
>
> 7.1.13 (clean): infoframes enabled: 0x4 (VSC only)
> 7.2.4 (streaked): infoframes enabled: 0xc (VSC + AS SDP,
> operation mode 1 = DP_AS_SDP_AVT_FIXED_VTOTAL)
>
> Later dumps of those boots are 0x6 vs 0xe; each non-zero bad mask is
> good | BIT(3).
>
> Changes in v2:
> - OR crtc_state->cmrr.enable so CMRR still gets AS SDP (v1 review).
> At fixed refresh v2 matches v1.
> v1: https://lore.kernel.org/r/[email protected]
>
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9252
> Fixes: 6a1712052859 ("drm/i915/dp: Enable AS SDP whenever VRR is possible or
> PR !async")
> Cc: [email protected] # 7.2.x
> Signed-off-by: Xizhe Tang <[email protected]>
> Tested-by: Xizhe Tang <[email protected]> # v7.2.6, PTL eDP, Adaptive
> Sync=Never
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3115,8 +3115,9 @@ static bool intel_dp_needs_as_sdp(struct intel_dp
> *intel_dp,
> if (intel_psr_needs_alpm_aux_less(intel_dp, crtc_state) &&
> !intel_psr_pr_async_video_timing_supported(intel_dp))
> return true;
>
> - return intel_vrr_possible(crtc_state);
> + return crtc_state->vrr.enable ||
> + crtc_state->cmrr.enable;
The real problem is that intel_vrr_possible() no longer does what
it says on the tin. I think we have three different things
intel_vrr_possible() gets used for currently:
- intel_dp_needs_as_sdp() actually wants to know whether variable VRR
timings are possible or not, and it wants to know that without
actually looking at uapi.vrr_enabled in order to avoid changes to
the guardband when uapi.vrr_enabled changes
- _intel_psr_min_set_context_latency() might want to know whether we
could end up using the VRR timing generator or not. Not 100% sure
about this one though
- everything in intel_vrr.c just wants to know whether we should program
the VRR timing generator registers or not. These are the only places
where the current intel_vrr_possible() actually looks correct, albeit
with a bad function name
> }
>
> static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
> --
> 2.47.0
--
Ville Syrjälä
Intel
