On 7/6/2026 5:26 PM, Nemesa Garg wrote:
In joiner mode the fast path cursor update must handle
secondary pipes. Iterate over all joined pipes uniformly
to duplicate plane state, run check_plane(), pin the
framebuffer and on success swap in the new plane state
for each secondary cursor.
Track every successfully prepared pipe in a joined[] array
of struct intel_cursor_pipe so that later frontbuffer, unpin
and error-cleanup paths treat primary and secondaries uniformly,
and ensures the primary's pinned framebuffer is released if a
secondary fails partway through.
v2: Use intel_crtc_joined_pipe_mask(). [Ville]
Add locking mechanism. [Ville]
v3: Drop the per-pipe fastpath mutex array. [sashiko]
v4: Fold parallel arrays into struct intel_cursor_pipe joined[],
unify primary/secondary in a single loop, and use bare
check_plane(). [Chaitanya]
Assisted-by: Claude:claude-sonnet-4.6
Signed-off-by: Nemesa Garg <[email protected]>
---
drivers/gpu/drm/i915/display/intel_cursor.c | 125 ++++++++++++++------
1 file changed, 90 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
b/drivers/gpu/drm/i915/display/intel_cursor.c
index 632f7a0a5a63..fad5d5302b36 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -864,6 +864,14 @@ intel_cursor_fastpath_update_plane_state(struct
intel_plane_state *plane_state,
intel_plane_copy_uapi_to_hw_state(NULL, plane_state, plane_state,
hw_crtc);
}
+struct intel_cursor_pipe {
+ struct intel_plane *plane;
+ struct intel_crtc *crtc;
+ struct intel_crtc_state *crtc_state;
+ struct intel_plane_state *old_plane_state;
+ struct intel_plane_state *new_plane_state;
+};
+
nit: struct intel_cursor_joiner_state
static int
intel_legacy_cursor_update(struct drm_plane *_plane,
struct drm_crtc *_crtc,
@@ -879,11 +887,13 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
struct intel_display *display = to_intel_display(plane);
struct intel_plane_state *old_plane_state =
to_intel_plane_state(plane->base.state);
- struct intel_plane_state *new_plane_state;
+ struct intel_plane_state *new_plane_state = NULL;
struct intel_crtc_state *crtc_state =
to_intel_crtc_state(crtc->base.state);
- struct intel_crtc_state *new_crtc_state;
struct intel_vblank_evade_ctx evade;
+ struct intel_cursor_pipe joined[I915_MAX_PIPES] = {};
nit: joined_pipe_state
+ struct intel_crtc *pipe_crtc;
+ int num_pipes = 0;
int ret;
/*
@@ -929,38 +939,74 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
if (!intel_cursor_joiner_commits_idle(display, crtc_state))
goto slow;
- new_plane_state = to_intel_plane_state(intel_plane_duplicate_state(&plane->base));
- if (!new_plane_state)
- return -ENOMEM;
+ /*
+ * Iterate over all joined pipes (primary and secondary) uniformly.
+ * The joined pipe mask includes both the primary pipe and all
+ * secondary joiner pipes, allowing us to handle them all the same way.
+ */
+ for_each_intel_crtc_in_pipe_mask(display, pipe_crtc,
+
intel_crtc_joined_pipe_mask(crtc_state)) {
+ struct intel_cursor_pipe *j = &joined[num_pipes];
+
+ j->plane = intel_crtc_get_plane(pipe_crtc, PLANE_CURSOR);
+ j->crtc = pipe_crtc;
+ j->crtc_state = to_intel_crtc_state(pipe_crtc->base.state);
+ j->old_plane_state = to_intel_plane_state(j->plane->base.state);
+ j->new_plane_state =
+
to_intel_plane_state(intel_plane_duplicate_state(&j->plane->base));
+
+ if (!j->new_plane_state) {
+ ret = -ENOMEM;
+ goto out_free;
+ }
+
+ /* Joiner secondary: uapi.crtc points at the primary uapi crtc.
*/
+ j->new_plane_state->uapi.crtc = &crtc->base;
+
This copies the secondary pipe's uapi.crtc to the primary's crtc. So the
first time any atomic commit touches this plane by its own pipe after a
fast path joiner cursor update, it will still point to the primary's
crtc and results into a atomic check fail. See plane_switching_crtc().
As far as I see, nothing else in the driver treats a secondary's plane
state that way.
This should perhaps be
j->new_plane_state->uapi.crtc = &pipe_crtc->base;
+ intel_cursor_fastpath_update_plane_state(j->new_plane_state, fb,
+ pipe_crtc,
+ crtc_x, crtc_y,
+ crtc_w, crtc_h,
+ src_x, src_y,
+ src_w, src_h);
+
+ ret = j->plane->check_plane(j->crtc_state, j->new_plane_state);
+ if (ret) {
+ intel_plane_destroy_state(&j->plane->base,
+ &j->new_plane_state->uapi);
+ goto out_free;
+ }
+
+ ret = intel_plane_pin_fb(j->new_plane_state,
j->old_plane_state);
+ if (ret) {
+ intel_plane_destroy_state(&j->plane->base,
+ &j->new_plane_state->uapi);
+ goto out_free;
+ }
- new_crtc_state = to_intel_crtc_state(intel_crtc_duplicate_state(&crtc->base));
- if (!new_crtc_state) {
- ret = -ENOMEM;
- goto out_free;
+ num_pipes++;
}
- intel_cursor_fastpath_update_plane_state(new_plane_state, fb,
- crtc,
- crtc_x, crtc_y, crtc_w, crtc_h,
- src_x, src_y, src_w, src_h);
+ new_plane_state = joined[0].new_plane_state;
+
intel_frontbuffer_flush(to_intel_frontbuffer(joined[0].new_plane_state->hw.fb),
+ ORIGIN_CURSOR_UPDATE);
- ret = intel_plane_atomic_check_with_state(crtc_state, new_crtc_state,
- old_plane_state,
new_plane_state);
- if (ret)
- goto out_free;
+ for (int i = 0; i < num_pipes; i++)
+
intel_frontbuffer_track(to_intel_frontbuffer(joined[i].old_plane_state->hw.fb),
+
to_intel_frontbuffer(joined[i].new_plane_state->hw.fb),
+ joined[i].plane->frontbuffer_bit);
- ret = intel_plane_pin_fb(new_plane_state, old_plane_state);
- if (ret)
- goto out_free;
+ for (int i = 0; i < num_pipes; i++) {
+ struct intel_crtc_state *cs =
+ to_intel_crtc_state(joined[i].crtc->base.state);
Isn't this already collected in joined[].crtc_state?
- intel_frontbuffer_flush(to_intel_frontbuffer(new_plane_state->hw.fb),
- ORIGIN_CURSOR_UPDATE);
- intel_frontbuffer_track(to_intel_frontbuffer(old_plane_state->hw.fb),
- to_intel_frontbuffer(new_plane_state->hw.fb),
- plane->frontbuffer_bit);
+ joined[i].plane->base.state = &joined[i].new_plane_state->uapi;
- /* Swap plane state */
- plane->base.state = &new_plane_state->uapi;
+ if (joined[i].new_plane_state->uapi.visible)
+ cs->active_planes |= BIT(PLANE_CURSOR);
+ else
+ cs->active_planes &= ~BIT(PLANE_CURSOR);
+ }
/*
* We cannot swap crtc_state as it may be in use by an atomic commit or
@@ -972,7 +1018,6 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
* planes atomically. If the cursor was part of the atomic update then
* we would have taken the slowpath.
*/
- crtc_state->active_planes = new_crtc_state->active_planes;
intel_vblank_evade_init(crtc_state, crtc_state, &evade);
@@ -1005,6 +1050,10 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
intel_psr_unlock(crtc_state);
+ /*
+ * Schedule or immediately unpin old framebuffers.
+ * Protect against concurrent access.
+ */
if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) {
drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base,
intel_cursor_unpin_work);
@@ -1013,18 +1062,24 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
drm_crtc_accurate_vblank_count(&crtc->base) + 1,
false);
- old_plane_state = NULL;
+ joined[0].old_plane_state = NULL;
} else {
intel_plane_unpin_fb(old_plane_state);
}
out_free:
- if (new_crtc_state)
- intel_crtc_destroy_state(&crtc->base, &new_crtc_state->uapi);
- if (ret)
- intel_plane_destroy_state(&plane->base, &new_plane_state->uapi);
- else if (old_plane_state)
- intel_plane_destroy_state(&plane->base, &old_plane_state->uapi);
+ if (ret) {
+ for (int i = 0; i < num_pipes; i++) {
+ intel_plane_unpin_fb(joined[i].new_plane_state);
+ intel_plane_destroy_state(&joined[i].plane->base,
+
&joined[i].new_plane_state->uapi);
+ }
+ } else {
+ for (int i = 0; i < num_pipes; i++)
+ if (joined[i].old_plane_state)
+
intel_plane_destroy_state(&joined[i].plane->base,
+
&joined[i].old_plane_state->uapi);
+ }
return ret;
slow:
