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

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

On Fri, Oct 30, 2026 at 05:29:24PM +0000, sashiko-bot@kernel.org wrote:
> 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?

PTL+ is actually where this changed. I misread the check in
intel_ddi_enable_fec(). And that also explains why I was still seeing
the timing generator dependency on LNL. After further testing, on PTL
the dependency is truly gone.

> 
> 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

-- 
Ville Syrjälä
Intel