Re: [PATCH] drm/i915/display: Ignore disabled planes in async flip checks

Re: [PATCH] drm/i915/display: Ignore disabled planes in async flip checks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/i915/display: reachable drm_WARN_ON() in
  intel_async_flip_check_hw() with NV12 Y-plane
--

commit b042ec62b54473b2633218f68a54b0d394afbc1e
Author: Erwin Springer <aot.lerwin@gmail.com>

drm/i915/display: Ignore disabled planes in async flip checks

This commit skips planes that have neither an old nor a new framebuffer
in intel_async_flip_check_uapi() and intel_async_flip_check_hw() so that
unchanged disabled planes included in an atomic commit do not cause async
flips to be rejected.

> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f79caeb5a82ec..defa55da52a74 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6173,6 +6173,14 @@ static int intel_async_flip_check_uapi(struct intel_atomic_state *state,
>  		if (plane->pipe != crtc->pipe)
>  			continue;
>  
> +		/*
> +		 * Userspace may add planes it doesn't change to the commit.
> +		 * A plane that is disabled and stays disabled takes no part
> +		 * in the async flip.
> +		 */
> +		if (!old_plane_state->uapi.fb && !new_plane_state->uapi.fb)
> +			continue;
> +
>  		/*
>  		 * TODO: Async flip is only supported through the page flip IOCTL
>  		 * as of now. So support currently added for primary plane only.
> @@ -6238,6 +6246,10 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
>  		if (plane->pipe != crtc->pipe)
>  			continue;
>  
> +		/* Disabled planes that stay disabled take no part in the flip. */
> +		if (!old_plane_state->hw.fb && !new_plane_state->hw.fb)
> +			continue;
> +
>  		/*
>  		 * Only async flip capable planes should be in the state
>  		 * if we're really about to ask the hardware to perform

[Severity: Medium]
Can this trigger drm_WARN_ON(display->drm, new_crtc_state->do_async_flip
&& !plane->async_flip) in intel_async_flip_check_hw() when a non-primary
plane is scanning out a planar YUV framebuffer (such as NV12 on ICL+)?

When a plane scans out a planar format, icl_check_nv12_planes() links it
to an unused SDR Y-plane via link_nv12_planes().  That Y-plane remains
disabled in UAPI (uapi.fb is NULL), but has a non-NULL hw.fb and
planar_linked_plane set.

If userspace submits atomic async flips on the primary plane while
including all UAPI-disabled planes on the pipe,
intel_async_flip_check_uapi() now skips the Y-plane because its old and
new uapi.fb are NULL.  Later in intel_atomic_check_planes(),
intel_joiner_add_affected_planes() sees the Y-plane's planar_linked_plane
and pulls the UV plane into the atomic state, and icl_check_nv12_planes()
re-links them with non-NULL hw.fb.  Once new_crtc_state->do_async_flip is
set on the following async flip, both planes bypass the !hw.fb check in
intel_async_flip_check_hw() and hit the drm_WARN_ON() because
plane->async_flip is NULL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-i915-async-planes-v1-1-d13498d0b6be@gmail.com?part=1