On Wed, Oct 22, 2025 at 03:25:52PM +0300, Jouni Högander wrote:
> We started seeing "[drm] *ERROR* Timed out waiting PSR idle state" after
> taking optimized guardband into use. These are seen because VSC SDPs are
> sent on same line as AS SDPs when AS SDP is enabled. AS SDP is sent on line
> configured in EMP_AS_SDP_TL register. We are configuring
> crtc_state->vrr.vsync_start into that register.
>
> Fix this by ensuring AS SDP is sent on line which is within
> guardband. From the bspec:
>
> EMP_AS_SDP_TL < SCL + Guardband
>
> Bspec: 71197
>
> Fixes: 52ecd48b8d3f ("drm/i915/dp: Add helper to get min sdp guardband")
> Cc: Ankit Nautiyal <[email protected]>
> Signed-off-by: Jouni Högander <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index b0aeb6c2de86c..54b5e060be82a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7026,7 +7026,7 @@ int intel_dp_compute_config_late(struct intel_encoder
> *encoder,
> }
>
> static
> -int intel_dp_get_lines_for_sdp(u32 type)
> +int intel_dp_get_lines_for_sdp(const struct intel_crtc_state *crtc_state,
> u32 type)
> {
> switch (type) {
> case DP_SDP_VSC_EXT_VESA:
> @@ -7036,6 +7036,8 @@ int intel_dp_get_lines_for_sdp(u32 type)
> return 8;
> case DP_SDP_PPS:
> return 7;
> + case DP_SDP_ADAPTIVE_SYNC:
> + return crtc_state->vrr.vsync_start + 1;
Is the +1 actually needed? I get the impression the bspec page isn't
being very accurate with the '<' usage.
Hmm, there is an extra note in the EMP_AS_SDP_TL register:
"For DP/eDP, if there is a set context latency (SCL) window, then it
cannot be the first line of SCL
For DP/eDP, if there is no SCL window, then it cannot be the first line
of the Delayed V. Blank"
So I guess there might be a real reason for that extra line.
Though I'm pretty sure no one has even confirmed that we don't have any
off by one errors in EMP_AS_SDP_TL/etc. Should do that at some point...
> default:
> break;
> }
> @@ -7052,11 +7054,18 @@ int intel_dp_sdp_min_guardband(const struct
> intel_crtc_state *crtc_state,
> crtc_state->infoframes.enable &
> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA))
> sdp_guardband = max(sdp_guardband,
> -
> intel_dp_get_lines_for_sdp(HDMI_PACKET_TYPE_GAMUT_METADATA));
> + intel_dp_get_lines_for_sdp(crtc_state,
> +
> HDMI_PACKET_TYPE_GAMUT_METADATA));
>
> if (assume_all_enabled ||
> crtc_state->dsc.compression_enable)
> - sdp_guardband = max(sdp_guardband,
> intel_dp_get_lines_for_sdp(DP_SDP_PPS));
> + sdp_guardband = max(sdp_guardband,
> + intel_dp_get_lines_for_sdp(crtc_state,
> DP_SDP_PPS));
> +
> + if (assume_all_enabled ||
assume_all_enable && HAS_AS_SDP() ?
> + crtc_state->infoframes.enable &
> intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC))
> + sdp_guardband = max(sdp_guardband,
> + intel_dp_get_lines_for_sdp(crtc_state,
> DP_SDP_ADAPTIVE_SYNC));
>
> return sdp_guardband;
> }
> --
> 2.43.0
--
Ville Syrjälä
Intel
