Re: [PATCH 02/10] drm/vblank: Add a 2-stage version of drm_crtc_arm_vblank_event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Insufficient memory barrier for ordering MMIO hardware programming against vblank event arming
--

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

drm/vblank: Add a 2-stage version of drm_crtc_arm_vblank_event

This patch introduces drm_crtc_prepare_arm_vblank_event() and
drm_crtc_arm_prepared_vblank_event() to allow arming vblank events in two
stages. This avoids taking vblank-related spinlocks with interrupts disabled
during hardware programming, which is useful for PREEMPT_RT kernels.

> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f90fb2d13e423..b52e0215312f8 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
[ ... ]
> +void drm_crtc_arm_prepared_vblank_event(struct drm_pending_vblank_event *e)
> +{
> +	WARN_ON(!e->postponed);
> +
> +	/* Ensure previous writes are visible */
> +	smp_wmb();
> +
> +	/* remove the flag to be processed as a normal event */
> +	WRITE_ONCE(e->postponed, false);
> +}

[Severity: Medium]
Is smp_wmb() sufficient to order prior MMIO hardware programming against this
memory write to e->postponed?

If the intent is to order the prior hardware programming (MMIO writes) against
setting e->postponed to false, smp_wmb() is strictly a CPU-to-CPU barrier for
normal memory. On weakly-ordered architectures (like ARM64), the CPU can
reorder the normal memory write (postponed = false) before the MMIO writes are
issued to the bus, potentially causing the vblank interrupt handler to process
the event before the hardware programming is complete.

Does this need a full wmb() to correctly order I/O against memory?

Conversely, if it were intended for CPU memory ordering, the reader in
drm_handle_vblank_events() lacks a matching smp_rmb(). Since all other CPU
memory fields are protected by event_lock, there shouldn't be lockless CPU
memory writes to order here.

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