Re: [PATCH v2 16/22] media: rockchip: rga: remove size from rga_frame

[Nicolas Dufresne]

Le mercredi 03 décembre 2025 à 16:52 +0100, Sven Püschel a écrit :
> The size member is only used for the mmu page table mapping.
> Therefore avoid storing the value and instead only calculate it
> in place. This also avoids the calculation entirely when an external
> iommu is used.
> 
> Signed-off-by: Sven Püschel <s.pueschel@xxxxxxxxxxxxxx>
> ---
>  drivers/media/platform/rockchip/rga/rga-buf.c | 6 +++++-
>  drivers/media/platform/rockchip/rga/rga.c     | 8 ++------
>  drivers/media/platform/rockchip/rga/rga.h     | 1 -
>  3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index e8d1e2e579140..8de6b9c3fd615 100644
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> @@ -79,9 +79,13 @@ static int rga_buf_init(struct vb2_buffer *vb)
>  	struct rockchip_rga *rga = ctx->rga;
>  	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
>  	size_t n_desc = 0;
> +	u32 size = 0;
> +	u8 i;

Move in it the scope it is being used.

>  
>  	if (rga_has_internal_iommu(rga)) {

though, after looking more attentively, this function is a no-op when the is an
external iommu. Using an early return would make this so much clearer, and you
can then drop the indent.

	if (rga_has_internal_iommu(rga))
		return 0;

Perhaps a comment to be applied on patch 14 though.

> -		n_desc = DIV_ROUND_UP(f->size, PAGE_SIZE);
> +		for (i = 0; i < f->pix.num_planes; i++)
> +			size += f->pix.plane_fmt[i].sizeimage;
> +		n_desc = DIV_ROUND_UP(size, PAGE_SIZE);
>  
>  		rbuf->n_desc = n_desc;
>  		rbuf->dma_desc = dma_alloc_coherent(rga->dev,
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index 1bfc4021f4a7b..0d7b0bcac950e 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -221,7 +221,6 @@ static int rga_open(struct file *file)
>  	};
>  
>  	def_frame.stride = (def_width * def_frame.fmt->depth) >> 3;
> -	def_frame.size = def_frame.stride * def_height;
>  
>  	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
> @@ -428,9 +427,6 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
>  	frm = rga_get_frame(ctx, f->type);
>  	if (IS_ERR(frm))
>  		return PTR_ERR(frm);
> -	frm->size = 0;
> -	for (i = 0; i < pix_fmt->num_planes; i++)
> -		frm->size += pix_fmt->plane_fmt[i].sizeimage;
>  	frm->fmt = rga_fmt_find(rga, pix_fmt->pixelformat);
>  	frm->stride = pix_fmt->plane_fmt[0].bytesperline;
>  
> @@ -443,10 +439,10 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
>  	frm->pix = *pix_fmt;
>  
>  	v4l2_dbg(debug, 1, &rga->v4l2_dev,
> -		 "[%s] fmt - %p4cc %dx%d (stride %d, sizeimage %d)\n",
> +		 "[%s] fmt - %p4cc %dx%d (stride %d)\n",
>  		  V4L2_TYPE_IS_OUTPUT(f->type) ? "OUTPUT" : "CAPTURE",
>  		  &frm->fmt->fourcc, pix_fmt->width, pix_fmt->height,
> -		  frm->stride, frm->size);
> +		  frm->stride);
>  
>  	for (i = 0; i < pix_fmt->num_planes; i++) {
>  		v4l2_dbg(debug, 1, &rga->v4l2_dev,
> diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
> index fc81bc5562792..466d568b9e614 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -37,7 +37,6 @@ struct rga_frame {
>  
>  	/* Variables that can calculated once and reused */
>  	u32 stride;
> -	u32 size;
>  };
>  
>  struct rga_dma_desc {

Attachment: signature.asc
Description: This is a digitally signed message part