Re: [PATCH 4/6] drm/i915/cursor: Program secondary cursor planes

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 7/6/2026 5:26 PM, Nemesa Garg wrote:
> Iterate over all joined pipes when arming/disabling the cursor plane so
> secondary pipes are updated together with the primary. The pin, check
> and state duplication for secondary pipes was already prepared in the
> previous commit; this converts the update_arm pass to a loop over the
> joined[] array.
> 
> Because the whole loop runs inside a single primary vblank-evade,
> sample intel_crtc_get_vblank_counter() per pipe around each arm and
> emit a drm_err() if the counter ticks during that pipe's own
> programming.
> 
> v2: Check primary and secondary pipe together. [Ville]
> v3: Use struct intel_cursor_pipe. [Ville]
> v4: Add per-pipe vblank straddle detection around the arm loop. [Chaitanya]
> 
> Assisted-by: Claude:claude-sonnet-4.6
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_cursor.c | 26 +++++++++++++++++----
>   1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
> index fad5d5302b36..65e1fa60a606 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -1039,11 +1039,27 @@ 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);
> -	} else {
> -		intel_plane_disable_arm(NULL, plane, crtc_state);
> +	for (int i = 0; i < num_pipes; i++) {
> +		u32 start_vbl_count = intel_crtc_get_vblank_counter(joined[i].crtc);
> +		u32 end_vbl_count;
> +
> +		if (joined[i].new_plane_state->uapi.visible) {
> +			intel_plane_update_noarm(NULL, joined[i].plane,
> +						 joined[i].crtc_state,
> +						 joined[i].new_plane_state);
> +			intel_plane_update_arm(NULL, joined[i].plane,
> +					       joined[i].crtc_state,
> +					       joined[i].new_plane_state);
> +		} else {
> +			intel_plane_disable_arm(NULL, joined[i].plane, joined[i].crtc_state);
> +		}
> +
> +		end_vbl_count = intel_crtc_get_vblank_counter(joined[i].crtc);
> +		if (start_vbl_count != end_vbl_count)
> +			drm_err(display->drm,
> +				"Atomic update failure on pipe %c (start=%u end=%u)\n",
> +				pipe_name(joined[i].crtc->pipe),
> +				start_vbl_count, end_vbl_count);

The straddling logic should be outside the for loop. We want to make 
sure all the updates went in the same frame. Not that write to each pipe 
is done within the same frame.

Assuming that joiner pipes are always in sync, sampling the frame count 
only from the primary pipe should be enough. Please call this out in a 
comment - this and also that the evasion logic also depends on the 
primary pipe.

Also no log printing when IRQs are disabled. It extends the period IRQs 
are off and will make a straddle, if any, even worse.

>   	}
>   
>   	local_irq_enable();