Re: [RFC PATCH] drm/i915/display: Prevent NV12 Y-plane allocation from stealing userspace planes

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

On Wed, Feb 11, 2026 at 04:48:52PM -0800, Khaled Almahallawy wrote:
> The driver automatically allocates a Y-plane (4A/5A) when userspace
> configures an NV12 surface. The allocation loop doesn't check if a
> candidate plane is already configured by userspace in the same atomic
> commit, causing conflict as observed in this i915_display_info:
> 
>   [PLANE:124:plane 4A]: type=OVL
>       uapi: [FB:566] AB24 little-endian (0x34324241),0x0,1920x1280, visible=visible
>       planar: Linked to [PLANE:34:plane 1A] as a Y plane
>       hw: [FB:564] NV12 little-endian (0x3231564e),0x0,1920x1080, visible=yes
> 
> Plane 4A's uapi state shows userspace's AB24 framebuffer, but the hw
> state shows it was reprogrammed with the NV12 Y-plane.
> 
> Example triggered by experiment with IGT test to commit NV12 + multiple
> AB24 planes:
> 
>   === Testing with NV12 primary + 3 ABGR8888 overlays ===
>     Plane 0 (Primary): NV12 1920x1080 at (0, 0)
>     Plane 1 (Overlay 0): ABGR8888 1920x1280 (fullscreen) at (0, 0)
>     Plane 2 (Overlay 1): ABGR8888 1920x1280 (fullscreen) at (0, 0)
>     Plane 3 (Overlay 2): ABGR8888 1920x1280 (fullscreen) at (0, 0)
>     TEST_ONLY passed, committing...
>     Atomic commit SUCCEEDED
> 
> The bug triggers a kernel WARNING in unlink_nv12_plane():
>   WARNING: drivers/gpu/drm/i915/display/intel_plane.c:1521
>   drm_WARN_ON(plane_state->uapi.visible)
>

I think the actual bug is that we unlink the nv12 planes after
plane_atomic_check(). unlink_nv12_plane() will then clobber
some things in the crtc state that was set up by 
plane_atomic_check().

So we perhaps want something like this:

diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
index 3dc2ed52147f..98d0255b8b18 100644
--- a/drivers/gpu/drm/i915/display/intel_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_plane.c
@@ -441,6 +441,8 @@ void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
 {
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 
+	unlink_nv12_plane(crtc_state, plane_state);
+
 	crtc_state->active_planes &= ~BIT(plane->id);
 	crtc_state->scaled_planes &= ~BIT(plane->id);
 	crtc_state->nv12_planes &= ~BIT(plane->id);
@@ -1513,6 +1515,9 @@ static void unlink_nv12_plane(struct intel_crtc_state *crtc_state,
 	struct intel_display *display = to_intel_display(plane_state);
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 
+	if (!plane_state->planar_linked_plane)
+		return;
+
 	plane_state->planar_linked_plane = NULL;
 
 	if (!plane_state->is_y_plane)
@@ -1550,8 +1555,7 @@ static int icl_check_nv12_planes(struct intel_atomic_state *state,
 		if (plane->pipe != crtc->pipe)
 			continue;
 
-		if (plane_state->planar_linked_plane)
-			unlink_nv12_plane(crtc_state, plane_state);
+		unlink_nv12_plane(crtc_state, plane_state);
 	}
 
 	if (!crtc_state->nv12_planes)

With that we could perhaps even drop the second unlink_nv12_plane()
call, but haven't really thought through the details...

> Fix by checking uapi.fb before allocating a Y-plane. If set, userspace
> configured this plane, so skip to the next candidate. This enables
> graceful fallback (4A busy -> try 5A) rather than the current
> behavior that steals planes from userspace.

I do have a patch in some branch that changes the Y plane
selection to use 'enabled_planes' instead of 'active_planes'
which is equivalent to this. It is perhaps the slightly more
logical approach but it could result some specific usage
scenarios losing NV12 scanout capability. IIRC I also had
some unsolved issue with that approach, which is why I never
even sent out the patch.

> IGT test and kernel fix generated with assistance from Claude Sonnet 4.5
> through an iterative process. The following is a summary of the prompts
> used:
> 
> IGT test generation prompt:
> Need an IGT test to:
> 1. Reproduce the NV12 + multiple AB24 plane allocation conflict
> 2. Work across different GPU vendors (not Intel-specific)
> 3. Discover hardware limits through iteration (not hardcoded)
> 4. Test atomic commit behavior with mixed formats
> 5. Validate driver properly rejects invalid configurations
> 6. Help debug plane allocation issues (interactive inspection)
> 
> Kernel fix debug process:
> 1. Explained NV12 UV->Y plane linking mechanism (link_nv12_planes)
> 2. Traced Y-plane selection algorithm and hardware constraints
> 3. Analyzed i915_display_info output showing uapi vs hw state mismatch
> 4. Triggered kernel WARNING in unlink_nv12_plane() confirming the bug
> 5. Traced kernel logs through atomic commit sequence
> 6. Identified root cause: Y-plane allocation checks uapi.crtc, but that's
>    set later during plane validation. uapi.fb is set earlier during state
>    setup, making it the correct indicator of userspace configuration
> 7. Evaluated uapi.fb vs uapi.visible for detection timing
> 8. Initially suggested rejecting commit with -EINVAL, but decided graceful
>    fallback with continue is better - allows trying alternate Y-planes
>    (4A busy -> 5A) instead of failing entire atomic commit
> 9. Validated fix prevents plane stealing while allowing alternate Y-plane
> 
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_plane.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_plane.c b/drivers/gpu/drm/i915/display/intel_plane.c
> index 3dc2ed52147f..57d1a9cd226e 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane.c
> @@ -1578,6 +1578,10 @@ static int icl_check_nv12_planes(struct intel_atomic_state *state,
>  			if (IS_ERR(y_plane_state))
>  				return PTR_ERR(y_plane_state);
>  
> +			/* Reject if this Y-plane is being configured by userspace */
> +			if (y_plane_state->uapi.fb)
> +				continue;
> +
>  			break;
>  		}
>  
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel