Re: [PATCH] media: rzg2l-cru: serialize state transitions with qlock

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hello Shuhao Fu

On Tue, Apr 21, 2026 at 02:03:07PM +0800, Shuhao Fu wrote:
> struct rzg2l_cru_dev.state is documented as protected by qlock, and the
> IRQ path already reads and updates it under that lock. However,
> rzg2l_cru_stop_streaming() writes STOPPING and
> rzg2l_cru_start_streaming_vq() writes STARTING without taking qlock.
>
> That lets process-context stream control race with rzg2l_cru_irq().
> If the IRQ handler misses a concurrent STOPPING update, it can continue
> normal frame completion and slot refill after streamoff has begun. A
> similar race around STARTING can make the IRQ path observe the wrong
> phase during startup synchronization.
>
> Fix both state transitions by serializing the writes with qlock, while
> still keeping rzg2l_cru_set_stream() outside the locked region.

Thanks, but I would rather complete my series that removes 'state'
completely:
https://patchwork.linuxtv.org/project/linux-media/list/?series=23674

Thanks
  j

>
> Fixes: 07fc05bd0a79 ("media: platform: Add Renesas RZ/G2L CRU driver")
> Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
> ---
>  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 162e2ace693184..434754fd155a8e 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -560,7 +560,11 @@ pipe_line_stop:
>
>  static void rzg2l_cru_stop_streaming(struct rzg2l_cru_dev *cru)
>  {
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&cru->qlock, flags);
>  	cru->state = RZG2L_CRU_DMA_STOPPING;
> +	spin_unlock_irqrestore(&cru->qlock, flags);
>
>  	rzg2l_cru_set_stream(cru, 0);
>  }
> @@ -749,6 +753,7 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>  static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
>  {
>  	struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
> +	unsigned long flags;
>  	int ret;
>
>  	ret = pm_runtime_resume_and_get(cru->dev);
> @@ -791,7 +796,9 @@ static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count
>  		goto out;
>  	}
>
> +	spin_lock_irqsave(&cru->qlock, flags);
>  	cru->state = RZG2L_CRU_DMA_STARTING;
> +	spin_unlock_irqrestore(&cru->qlock, flags);
>  	dev_dbg(cru->dev, "Starting to capture\n");
>  	return 0;
>
> --
> 2.25.1
>