Re: [PATCH 11/14] media: rz2gl-cru: Return pending buffers in order

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Jacopo,
Thanks for your patch.

On Fri, Mar 27, 2026 at 06:10:16PM +0100, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> 
> Buffers are programmed into slots in queueing order.
> 
> When returning pending buffers we can't simply start from the first slot
> but we should actually iterate slots starting from the one is use. The
> rzg3e_cru_irq() handler already uses 'active_slot', make rzg2l_cru_irq()
> use it as well to know where to start iterating from.
> 
> As the pattern of iterating over slots in order will be used for slots
> programming in the next patches, provide an helper macro to do that.
> 
> While at it, rename return_unused_buffers() to rzg2l_cru_return_buffers().
>

Tested on RZ/G3E + ov5645 image sensor.

Tested-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Kind Regards,
Tommaso

> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>  .../media/platform/renesas/rzg2l-cru/rzg2l-video.c | 68 +++++++++++++++-------
>  1 file changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index 17e0153052e1..a6b606c63f90 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -43,6 +43,24 @@ struct rzg2l_cru_buffer {
>  #define to_buf_list(vb2_buffer) \
>  	(&container_of(vb2_buffer, struct rzg2l_cru_buffer, vb)->list)
>  
> +/*
> + * The CRU hardware cycles over its slots when transferring frames. All drivers
> + * structure that contains programming data for the slots, such as the memory
> + * destination addresses have to be iterated as they were circular buffers.
> + *
> + * Provide here utilities to iterate over slots and the associated data.
> + */
> +static inline unsigned int rzg2l_cru_slot_next(struct rzg2l_cru_dev *cru,
> +					       unsigned int slot)
> +{
> +	return (slot + 1) % cru->num_buf;
> +}
> +
> +/* Start cycling on cru slots from the one after 'start'. */
> +#define for_each_cru_slot_from(cru, slot, start)			\
> +	for (slot = rzg2l_cru_slot_next(cru, start);			\
> +	     slot != start; slot = rzg2l_cru_slot_next(cru, slot))
> +
>  /* -----------------------------------------------------------------------------
>   * DMA operations
>   */
> @@ -106,28 +124,36 @@ __rzg2l_cru_read_constant(struct rzg2l_cru_dev *cru, u32 offset)
>  	 __rzg2l_cru_read_constant(cru, offset) : \
>  	 __rzg2l_cru_read(cru, offset))
>  
> -static void return_unused_buffers(struct rzg2l_cru_dev *cru,
> -				  enum vb2_buffer_state state)
> +static void rzg2l_cru_return_buffers(struct rzg2l_cru_dev *cru,
> +				     enum vb2_buffer_state state)
>  {
>  	struct rzg2l_cru_buffer *buf, *node;
> -	unsigned int i;
>  
>  	scoped_guard(spinlock_irq, &cru->hw_lock) {
> -		for (i = 0; i < cru->num_buf; i++) {
> -			if (cru->queue_buf[i]) {
> -				vb2_buffer_done(&cru->queue_buf[i]->vb2_buf,
> -						state);
> -				cru->queue_buf[i] = NULL;
> -			}
> +		/* Return the buffer in progress first, if not completed yet. */
> +		unsigned int slot = cru->active_slot;
> +
> +		if (cru->queue_buf[slot]) {
> +			vb2_buffer_done(&cru->queue_buf[slot]->vb2_buf, state);
> +			cru->queue_buf[slot] = NULL;
>  		}
> -	}
>  
> -	scoped_guard(spinlock_irq, &cru->qlock) {
> -		list_for_each_entry_safe(buf, node, &cru->buf_list, list) {
> -			vb2_buffer_done(&buf->vb.vb2_buf, state);
> -			list_del(&buf->list);
> +		/* Return all the pending buffers after the active one. */
> +		for_each_cru_slot_from(cru, slot, cru->active_slot) {
> +			if (!cru->queue_buf[slot])
> +				continue;
> +
> +			vb2_buffer_done(&cru->queue_buf[slot]->vb2_buf, state);
> +			cru->queue_buf[slot] = NULL;
>  		}
>  	}
> +
> +	guard(spinlock_irq)(&cru->qlock);
> +
> +	list_for_each_entry_safe(buf, node, &cru->buf_list, list) {
> +		vb2_buffer_done(&buf->vb.vb2_buf, state);
> +		list_del(&buf->list);
> +	}
>  }
>  
>  static int rzg2l_cru_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
> @@ -591,16 +617,16 @@ irqreturn_t rzg2l_cru_irq(int irq, void *data)
>  
>  	/* Prepare for capture and update state */
>  	amnmbs = rzg2l_cru_read(cru, AMnMBS);
> -	slot = amnmbs & AMnMBS_MBSTS;
> +	cru->active_slot = amnmbs & AMnMBS_MBSTS;
>  
>  	/*
>  	 * AMnMBS.MBSTS indicates the destination of Memory Bank (MB).
>  	 * Recalculate to get the current transfer complete MB.
>  	 */
> -	if (slot == 0)
> +	if (cru->active_slot == 0)
>  		slot = cru->num_buf - 1;
>  	else
> -		slot--;
> +		slot = cru->active_slot - 1;
>  
>  	/*
>  	 * To hand buffers back in a known order to userspace start
> @@ -669,7 +695,7 @@ irqreturn_t rzg3e_cru_irq(int irq, void *data)
>  	}
>  
>  	slot = cru->active_slot;
> -	cru->active_slot = (cru->active_slot + 1) % cru->num_buf;
> +	cru->active_slot = rzg2l_cru_slot_next(cru, cru->active_slot);
>  
>  	dev_dbg(cru->dev, "Current written slot: %d\n", slot);
>  	cru->buf_addr[slot] = 0;
> @@ -743,7 +769,7 @@ static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count
>  	cru->scratch = dma_alloc_coherent(cru->dev, cru->format.sizeimage,
>  					  &cru->scratch_phys, GFP_KERNEL);
>  	if (!cru->scratch) {
> -		return_unused_buffers(cru, VB2_BUF_STATE_QUEUED);
> +		rzg2l_cru_return_buffers(cru, VB2_BUF_STATE_QUEUED);
>  		dev_err(cru->dev, "Failed to allocate scratch buffer\n");
>  		ret = -ENOMEM;
>  		goto assert_presetn;
> @@ -751,7 +777,7 @@ static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count
>  
>  	ret = rzg2l_cru_set_stream(cru, 1);
>  	if (ret) {
> -		return_unused_buffers(cru, VB2_BUF_STATE_QUEUED);
> +		rzg2l_cru_return_buffers(cru, VB2_BUF_STATE_QUEUED);
>  		goto out;
>  	}
>  
> @@ -788,7 +814,7 @@ static void rzg2l_cru_stop_streaming_vq(struct vb2_queue *vq)
>  	dma_free_coherent(cru->dev, cru->format.sizeimage,
>  			  cru->scratch, cru->scratch_phys);
>  
> -	return_unused_buffers(cru, VB2_BUF_STATE_ERROR);
> +	rzg2l_cru_return_buffers(cru, VB2_BUF_STATE_ERROR);
>  
>  	reset_control_assert(cru->presetn);
>  	clk_disable_unprepare(cru->vclk);
> 
> -- 
> 2.53.0
>