Re: [PATCH v2] drm/i915/dp: Only send AS SDP when VRR/CMRR is enabled or PR !async

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

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/20260923052937.22817-1-xizheTang2005@163.com
> 
> 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: stable@vger.kernel.org # 7.2.x
> Signed-off-by: Xizhe Tang <xizheTang2005@163.com>
> Tested-by: Xizhe Tang <xizheTang2005@163.com> # 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