Re: [PATCH v8 13/27] drm/i915/display: Use intel_de_write_fw in intel_pipe_fastset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missed conversions to `_fw` variants in nested and adjacent functions called during the atomic vblank evasion window.
--

commit 1b8145ae8a083acd96357095f72f68d606b4e071
Author: Maarten Lankhorst <dev@lankhorst.se>

drm/i915/display: Use intel_de_write_fw in intel_pipe_fastset

This commit converts several register writes in functions like 
intel_set_pipe_src_size() and intel_set_transcoder_timings_lrr() 
to use the _fw non-sleeping variants. This prevents a "sleeping 
function called from invalid context" BUG during atomic evasion 
on PREEMPT_RT.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e2e4b00a8fa99..2e3ade44f28ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c

[ ... ]

> @@ -2839,9 +2839,9 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc
>  	 * The double buffer latch point for TRANS_VTOTAL
>  	 * is the transcoder's undelayed vblank.
>  	 */
> -	intel_de_write(display, TRANS_VTOTAL(display, cpu_transcoder),
> -		       VACTIVE(crtc_vdisplay - 1) |
> -		       VTOTAL(crtc_vtotal - 1));
> +	intel_de_write_fw(display, TRANS_VTOTAL(display, cpu_transcoder),
> +			  VACTIVE(crtc_vdisplay - 1) |
> +			  VTOTAL(crtc_vtotal - 1));
>  
>  	intel_vrr_set_fixed_rr_timings(crtc_state);
>  	intel_vrr_transcoder_enable(crtc_state);

[Severity: High]
This is a pre-existing issue, but it seems the fix for PREEMPT_RT might be 
incomplete.

intel_set_transcoder_timings_lrr() unconditionally calls 
intel_vrr_transcoder_enable(), which in turn calls 
intel_vrr_set_transcoder_timings(). That nested function still uses 
intel_de_write(), intel_de_read(), and intel_de_rmw() rather than the 
_fw variants:

drivers/gpu/drm/i915/display/intel_vrr.c:intel_vrr_set_transcoder_timings() {
	...
	drm_WARN_ON(display->drm,
		    !(intel_de_read(display, TRANS_DDI_FUNC_CTL(display, cpu_transcoder)) & TRANS_DDI_FUNC_ENABLE));
	...
	intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), trans_vrr_ctl(crtc_state));
	...
}

Because these non-fw accessors acquire locks that are sleepable on PREEMPT_RT,
will they still cause the same panic during the atomic vblank evasion window?

Additionally, commit_pipe_post_planes() runs in this same atomic window and 
calls intel_vrr_enable(). This eventually executes multiple sleepable 
intel_de_write() calls:

drivers/gpu/drm/i915/display/intel_vrr.c:intel_vrr_enable_dc_balancing() {
	...
	intel_de_write(display, TRANS_VRR_DCB_ADJ_VMAX_CFG(cpu_transcoder),
		       VRR_DCB_ADJ_VMAX(crtc_state->vrr.vmax - 1));
	...
}

drivers/gpu/drm/i915/display/intel_vrr.c:intel_vrr_set_vrr_timings() {
	...
	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
		       intel_vrr_hw_vmin(crtc_state) - 1);
	...
}

Do those accesses also need to be converted to the _fw variants to fully 
prevent the BUG?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615201846.307297-1-dev@lankhorst.se?part=13