Re: [PATCH 09/10] drm/i915: Fix FEC wait with port sync on pre-LNL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915/ddi: LNL vs PTL version mismatch in FEC status wait comment and check
--

commit c0a68d9ab06674ed868f492b35dc93f1dc05b1bf
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm/i915: Fix FEC wait with port sync on pre-LNL

This commit defers waiting for FEC status in intel_ddi_enable() until after
the timing generators have started, and adds a helper to wait for FEC status
across all synced transcoders when port sync is enabled.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 9939fa43e8f73..2e7f278a1b6c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c

[ ... ]

> @@ -3623,6 +3654,16 @@ static void intel_ddi_enable(struct intel_atomic_state *state,
>  	else
>  		intel_crtc_vblank_on(state, crtc_state);
>  
> +	/*
> +	 * On pre-LNL FEC_DECODE_EN will only be sent when the timing
> +	 * generator starts. Also note that FEC_DECODE_EN cannot be
> +	 * re-sent without restarting the timing generator.
> +	 */
> +	if (is_trans_port_sync_mode(crtc_state))
> +		trans_port_sync_wait_for_fec_status(state, encoder, crtc_state, true);
> +	else
> +		intel_ddi_wait_for_fec_status(encoder, crtc_state, true);

[Severity: Medium]
Should the comment here (and the commit message) refer to pre-PTL and PTL+
rather than pre-LNL and LNL+, or should the version check in
intel_ddi_enable_fec() be updated?

The commit message and the comment in intel_ddi_enable() state that on LNL+
FEC_DECODE_EN is sent immediately when enabling FEC in DP_TP_CTL, and that
LNL+ therefore checks FEC status immediately in intel_ddi_enable_fec() and
then a second time here in intel_ddi_enable().

However, intel_ddi_enable_fec() skips the immediate check and retry loop on
DISPLAY_VER < 30:

drivers/gpu/drm/i915/display/intel_ddi.c:intel_ddi_enable_fec() {
	...
	intel_de_rmw(display, dp_tp_ctl_reg(encoder, crtc_state),
		     0, DP_TP_CTL_FEC_ENABLE);

	if (DISPLAY_VER(display) < 30)
		return;

	ret = intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
	...
}

Since LNL is DISPLAY_VER 20 and PTL is DISPLAY_VER 30, LNL returns early in
intel_ddi_enable_fec() without waiting for FEC status, and port sync is also
enabled up through DISPLAY_VER 20 in intel_ddi_port_sync_transcoders():

drivers/gpu/drm/i915/display/intel_ddi.c:intel_ddi_port_sync_transcoders() {
	/*
	 * We don't enable port sync on BDW due to missing w/as and
	 * due to not having adjusted the modeset sequence appropriately.
	 */
	if (!IS_DISPLAY_VER(display, 9, 20))
		return 0;
	...
}

Is intel_ddi_enable_fec() supposed to check DISPLAY_VER(display) < 20 for
LNL+, or was the description intended to say pre-PTL / PTL+?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030171327.29829-1-ville.syrjala@linux.intel.com?part=9