Re: [PATCH] drm/i915/display: Update the CMN_SDP_TL in fastset path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915: missing stagger values in cmn_sdp_changed() during fastset
--

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

drm/i915/display: Update the CMN_SDP_TL in fastset path

Commit bfa597238073 ("drm/i915/dip: Enable Common SDP Transmission line")
enabled the CMN_SDP_TL for the modeset path, but missed to add it to the
fastset path. This commit updates the CMN_SDP_TL register during fastsets
if it has changed, similar to EMP_AS_SDP_TL.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e3e0d2f8faefd..01604c4e8615c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
[ ... ]
> @@ -999,6 +1000,12 @@ static bool cmrr_params_changed(const struct intel_crtc_state *old_crtc_state,
>  		old_crtc_state->vrr.cmrr.cmrr_n != new_crtc_state->vrr.cmrr.cmrr_n;
>  }
>  
> +static bool cmn_sdp_changed(const struct intel_crtc_state *old_crtc_state,
> +			    const struct intel_crtc_state *new_crtc_state)
> +{
> +	return old_crtc_state->dip.cmn_sdp_tl != new_crtc_state->dip.cmn_sdp_tl;
> +}
> +

[Severity: Medium]
Should cmn_sdp_changed() in drivers/gpu/drm/i915/display/intel_display.c also
check other transmission line fields like gmp_sdp_tl, pps_sdp_tl, and
vsc_ext_sdp_tl?

intel_dip_cmn_sdp_transmission_line_enable() programs the CMN_SDP_TL_STGR_CTL
register using these stagger values. During a fastset taking over from BIOS,
if the BIOS programmed non-default stagger values but left the base cmn_sdp_tl
identical to the driver's default, cmn_sdp_changed() will return false.

Since intel_pipe_config_compare() ignores differences in these stagger fields,
intel_dip_cmn_sdp_transmission_line_enable() would never be called to update
CMN_SDP_TL_STGR_CTL, leaving the hardware operating with the inherited BIOS
values.

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