On 8/27/2026 10:12 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_pipe_state[] array
of struct intel_cursor_joiner_state 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.

Convert the primary-only vblank unpin block into a loop over the
joined_pipe_state[] array so each pipe's old cursor framebuffer is scheduled for
unpin (or unpinned inline when unchanged) independently. All unpin
work is armed on the primary crtc's vblank (&crtc->base), consistent
with the single primary vblank evasion used above.

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_joiner_state 
joined_pipe_state[].
     unify primary/secondary in a single loop.
     use bare check_plane(). [Chaitanya]
v5: Fix secondary uapi.crtc to each pipe's own crtc.
     Rename intel_cursor_pipe to intel_cursor_joiner_state and
     joined[] to joined_pipe_state[]. [Chaitanya]
v6: Mirror primary uapi into secondary hw state, don't set uapi.crtc. [sashiko]
v7: Squash patch 5 with this patch. [Chaitanya]

Thank you for squashing the patches. One small nit, "patch 5" won't mean much when this series is merged, use the following instead.

v7: Squash the per-pipe cursor unpin scheduling into this patch

The change can be made while merging no need to resend.

==
Chaitanya


Assisted-by: Claude:claude-sonnet-4.6
Signed-off-by: Nemesa Garg <[email protected]>
Reviewed-by: Chaitanya Kumar Borah <[email protected]>
---
  drivers/gpu/drm/i915/display/intel_cursor.c | 181 +++++++++++++-------
  1 file changed, 120 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c 
b/drivers/gpu/drm/i915/display/intel_cursor.c
index b57b446b9788..e4a7d4d2915a 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -847,6 +847,7 @@ intel_cursor_joiner_commits_idle(struct intel_display 
*display,
static void
  intel_cursor_fastpath_update_plane_state(struct intel_plane_state 
*plane_state,
+                                        const struct intel_plane_state 
*from_plane_state,
                                         struct drm_framebuffer *fb,
                                         struct intel_crtc *hw_crtc,
                                         int crtc_x, int crtc_y,
@@ -854,20 +855,34 @@ intel_cursor_fastpath_update_plane_state(struct 
intel_plane_state *plane_state,
                                         u32 src_x, u32 src_y,
                                         u32 src_w, u32 src_h)
  {
-       drm_atomic_set_fb_for_plane(&plane_state->uapi, fb);
-
-       plane_state->uapi.src_x = src_x;
-       plane_state->uapi.src_y = src_y;
-       plane_state->uapi.src_w = src_w;
-       plane_state->uapi.src_h = src_h;
-       plane_state->uapi.crtc_x = crtc_x;
-       plane_state->uapi.crtc_y = crtc_y;
-       plane_state->uapi.crtc_w = crtc_w;
-       plane_state->uapi.crtc_h = crtc_h;
-
-       intel_plane_copy_uapi_to_hw_state(NULL, plane_state, plane_state, 
hw_crtc);
+       /*
+        * Only the primary owns its uapi state; a secondary mirrors it, so
+        * its uapi.crtc/fb stay NULL and hw.crtc comes from hw_crtc.
+        */
+       if (plane_state == from_plane_state) {
+               drm_atomic_set_fb_for_plane(&plane_state->uapi, fb);
+
+               plane_state->uapi.src_x = src_x;
+               plane_state->uapi.src_y = src_y;
+               plane_state->uapi.src_w = src_w;
+               plane_state->uapi.src_h = src_h;
+               plane_state->uapi.crtc_x = crtc_x;
+               plane_state->uapi.crtc_y = crtc_y;
+               plane_state->uapi.crtc_w = crtc_w;
+               plane_state->uapi.crtc_h = crtc_h;
+       }
+
+       intel_plane_copy_uapi_to_hw_state(NULL, plane_state, from_plane_state, 
hw_crtc);
  }
+struct intel_cursor_joiner_state {
+       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;
+};
+
  static int
  intel_legacy_cursor_update(struct drm_plane *_plane,
                           struct drm_crtc *_crtc,
@@ -883,11 +898,12 @@ 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_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_joiner_state joined_pipe_state[I915_MAX_PIPES] = {};
+       struct intel_crtc *pipe_crtc;
+       int num_pipes = 0;
        int ret;
/*
@@ -933,38 +949,68 @@ 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;
-
-       new_crtc_state = 
to_intel_crtc_state(intel_crtc_duplicate_state(&crtc->base));
-       if (!new_crtc_state) {
-               ret = -ENOMEM;
-               goto out_free;
-       }
+       /*
+        * 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_joiner_state *j = 
&joined_pipe_state[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;
+               }
- 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);
+               intel_cursor_fastpath_update_plane_state(j->new_plane_state,
+                                                        
joined_pipe_state[0].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_atomic_check_with_state(crtc_state, new_crtc_state,
-                                                 old_plane_state, 
new_plane_state);
-       if (ret)
-               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;
+               }
- ret = intel_plane_pin_fb(new_plane_state, old_plane_state);
-       if (ret)
-               goto out_free;
+               num_pipes++;
+       }
- intel_frontbuffer_flush(to_intel_frontbuffer(new_plane_state->hw.fb),
+       
intel_frontbuffer_flush(to_intel_frontbuffer(joined_pipe_state[0].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);
- /* Swap plane state */
-       plane->base.state = &new_plane_state->uapi;
+       for (int i = 0; i < num_pipes; i++)
+               
intel_frontbuffer_track(to_intel_frontbuffer(joined_pipe_state[i].old_plane_state->hw.fb),
+                                       
to_intel_frontbuffer(joined_pipe_state[i].new_plane_state->hw.fb),
+                                       
joined_pipe_state[i].plane->frontbuffer_bit);
+
+       for (int i = 0; i < num_pipes; i++) {
+               joined_pipe_state[i].plane->base.state = 
&joined_pipe_state[i].new_plane_state->uapi;
+
+               if (joined_pipe_state[i].new_plane_state->uapi.visible)
+                       joined_pipe_state[i].crtc_state->active_planes |= 
BIT(PLANE_CURSOR);
+               else
+                       joined_pipe_state[i].crtc_state->active_planes &= 
~BIT(PLANE_CURSOR);
+       }
/*
         * We cannot swap crtc_state as it may be in use by an atomic commit or
@@ -976,7 +1022,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); @@ -998,9 +1043,11 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
                local_irq_disable();
        }
- if (new_plane_state->uapi.visible) {
-               intel_plane_update_noarm(NULL, plane, crtc_state, 
new_plane_state);
-               intel_plane_update_arm(NULL, plane, crtc_state, 
new_plane_state);
+       if (joined_pipe_state[0].new_plane_state->uapi.visible) {
+               intel_plane_update_noarm(NULL, plane, crtc_state,
+                                        joined_pipe_state[0].new_plane_state);
+               intel_plane_update_arm(NULL, plane, crtc_state,
+                                      joined_pipe_state[0].new_plane_state);
        } else {
                intel_plane_disable_arm(NULL, plane, crtc_state);
        }
@@ -1009,26 +1056,38 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
intel_psr_unlock(crtc_state); - 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);
-
-               drm_vblank_work_schedule(&old_plane_state->unpin_work,
-                                        
drm_crtc_accurate_vblank_count(&crtc->base) + 1,
-                                        false);
-
-               old_plane_state = NULL;
-       } else {
-               intel_plane_unpin_fb(old_plane_state);
+       /*
+        * Schedule or immediately unpin old framebuffers.
+        * Protect against concurrent access.
+        */
+       for (int i = 0; i < num_pipes; i++) {
+               struct intel_plane_state *old = 
joined_pipe_state[i].old_plane_state;
+
+               if (old->ggtt_vma != 
joined_pipe_state[i].new_plane_state->ggtt_vma) {
+                       drm_vblank_work_init(&old->unpin_work, &crtc->base,
+                                            intel_cursor_unpin_work);
+                       drm_vblank_work_schedule(&old->unpin_work,
+                                                
drm_crtc_accurate_vblank_count(&crtc->base) + 1,
+                                                false);
+                       joined_pipe_state[i].old_plane_state = NULL;
+               } else {
+                       intel_plane_unpin_fb(old);
+               }
        }
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_pipe_state[i].new_plane_state);
+                       
intel_plane_destroy_state(&joined_pipe_state[i].plane->base,
+                                                 
&joined_pipe_state[i].new_plane_state->uapi);
+               }
+       } else {
+               for (int i = 0; i < num_pipes; i++)
+                       if (joined_pipe_state[i].old_plane_state)
+                               
intel_plane_destroy_state(&joined_pipe_state[i].plane->base,
+                                                         
&joined_pipe_state[i].old_plane_state->uapi);
+       }
        return ret;
slow:

Reply via email to