Re: [PATCH 1/6] drm/i915/cursor: Check joiner cursor commit status

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

On Wed, Apr 22, 2026 at 01:07:17PM +0530, Nemesa Garg wrote:
> In joiner mode, secondary cursor commits may still be running
> even when the primary cursor commit is done.
> Check secondary cursor commit status before taking the fast path.
> If any secondary commit is still pending, fallback to slow path.
> 
> Assisted-by: Claude:claude-sonnet-4.6
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c | 29 +++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index 18d1014de361..609915c798ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -13,6 +13,7 @@
>  #include <drm/drm_vblank.h>
>  
>  #include "intel_atomic.h"
> +#include "intel_crtc.h"
>  #include "intel_cursor.h"
>  #include "intel_cursor_regs.h"
>  #include "intel_de.h"
> @@ -797,6 +798,30 @@ void intel_cursor_unpin_work(struct kthread_work *base)
>  	intel_plane_destroy_state(&plane->base, &plane_state->uapi);
>  }
>  
> +static bool
> +intel_cursor_joiner_commits_idle(struct intel_display *display,
> +				 const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *secondary_crtc;
> +	u8 secondary_pipes = intel_crtc_joiner_secondary_pipes(crtc_state);
> +
> +	if (!secondary_pipes)
> +		return true;
> +
> +	for_each_intel_crtc_in_pipe_mask(display->drm, secondary_crtc, secondary_pipes) {

You want to just iterate over intel_crtc_joined_pipe_mask() everywhere.
That way the primary and secondary pipes all just get handled in exactly
the same way.

AFAICS you've completely skipped over proper locking in this series.

> +		struct intel_plane *secondary_plane =
> +					intel_crtc_get_plane(secondary_crtc, PLANE_CURSOR);
> +		struct intel_plane_state *secondary_plane_state =
> +					to_intel_plane_state(secondary_plane->base.state);
> +
> +		if (secondary_plane_state->uapi.commit &&
> +		    !try_wait_for_completion(&secondary_plane_state->uapi.commit->hw_done))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  static int
>  intel_legacy_cursor_update(struct drm_plane *_plane,
>  			   struct drm_crtc *_crtc,
> @@ -843,6 +868,10 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
>  	    !try_wait_for_completion(&old_plane_state->uapi.commit->hw_done))
>  		goto slow;
>  
> +	/* For joiner case also do the same thing as above */
> +	if (!intel_cursor_joiner_commits_idle(display, crtc_state))
> +		goto slow;
> +
>  	/*
>  	 * If any parameters change that may affect watermarks,
>  	 * take the slowpath. Only changing fb or position should be
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel