Re: [PATCH v5 06/12] drm/i915/dip: Store and use AS SDP transmission line from crtc state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Spurious state mismatch warnings for dip.emp_as_sdp_tl due to unconditional hardware readout of stale register values.
- [Medium] Unconditional fastset check for `dip.emp_as_sdp_tl` breaks seamless Link Refresh Rate (LRR) updates, forcing heavy full modesets.
--

commit fdc12bb4795c43a7d04cbec4746e152e2a99a6d8
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/dip: Store and use AS SDP transmission line from crtc state

This commit moves the computation of the Adaptive Sync SDP transmission
line from programming time to the crtc state computation phase. It stores
the value in the crtc state to be used when programming the EMP_AS_SDP_TL
register, preparing for programmable transmission lines for other DP SDPs.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
[ ... ]
> @@ -4235,6 +4236,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>  	intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA);
>  	intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC);
>  	intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC);
> +	intel_dip_sdp_transmission_line_get_config(pipe_config);

[Severity: Medium]
Could this unconditional hardware readout capture stale values and cause
spurious state mismatch warnings?

intel_dip_sdp_transmission_line_get_config() unconditionally reads the
EMP_AS_SDP_TL hardware register. If the encoder is HDMI, or if VRR was
disabled, the computed software state leaves dip.emp_as_sdp_tl = 0.

However, when VRR is disabled, intel_vrr_set_transcoder_timings() in
drivers/gpu/drm/i915/display/intel_vrr.c returns early and never clears
EMP_AS_SDP_TL. This leaves a stale non-zero value in the hardware register
from a previous DP modeset.

Could this mismatch cause intel_pipe_config_compare() to fail, incorrectly
forcing an unnecessary full modeset instead of a fastset?

> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
> --- a/drivers/gpu/drm/i915/display/intel_dip.c
> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
[ ... ]
> +void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
> +{
> +	crtc_state->dip.emp_as_sdp_tl = intel_dip_get_as_sdp_transmission_line(crtc_state);
> +}

[Severity: Medium]
Does computing and checking this transmission line state break seamless
Link Refresh Rate (LRR) updates?

LRR is designed to perform seamless refresh rate transitions without screen
blanking. The intel_pipe_config_compare() function in intel_display.c
ignores vrr.vsync_start differences during a fastset because it is
explicitly guarded by an if (!fastset) condition.

Because dip.emp_as_sdp_tl is computed directly from vrr.vsync_start, and
its corresponding state check (PIPE_CONF_CHECK_I) runs outside the
!fastset block, an LRR transition that naturally alters the vtotal and
vsync_start will produce a mismatch.

Will this erroneously fail the fastset check and force a heavy full modeset
instead of a seamless transition?

> +
> +void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
> +{
> +	crtc_state->dip.emp_as_sdp_tl = intel_dip_read_emp_as_sdp_tl(crtc_state);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908062224.2897987-1-ankit.k.nautiyal@intel.com?part=6