[PATCH 02/13] media: rockchip: rga: extract helper to fill descriptors

[Hans Verkuil]

On 14/09/2023 14:40, Michael Tretter wrote:
> The IOMMU of the RGA is programmed with a list of DMA descriptors that
> contain an 32 bit address per 4k page in the video buffers. The address
> in the descriptor points to the start address of the page.
> 
> Introduce 'struct rga_dma_desc' to make the handling of the DMA
> descriptors explicit instead of hiding them behind standard types.
> 
> As the descriptors only handle 32 bit addresses, addresses above 4 GB
> cannot be addressed. If this is detected, stop filling the descriptor
> list and report an error.
> 
> While at it, use provided helpers for iterating the sg_table instead of
> manually calculating the DMA addresses.
> 
> Signed-off-by: Michael Tretter <m.tretter at pengutronix.de>
> ---
>  drivers/media/platform/rockchip/rga/rga-buf.c | 47 +++++++++++++++------------
>  drivers/media/platform/rockchip/rga/rga.h     |  8 +++--
>  2 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index 81508ed5abf3..df5ebc90e32d 100644
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/pm_runtime.h>
> +#include <linux/scatterlist.h>
>  
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ioctl.h>
> @@ -15,6 +16,25 @@
>  #include "rga-hw.h"
>  #include "rga.h"
>  
> +static int fill_descriptors(struct rga_dma_desc *desc, struct sg_table *sgt)
> +{
> +	struct sg_dma_page_iter iter;
> +	struct rga_dma_desc *tmp = desc;
> +	unsigned int num_desc = 0;
> +	dma_addr_t addr;
> +
> +	for_each_sgtable_dma_page(sgt, &iter, 0) {
> +		addr = sg_page_iter_dma_address(&iter);
> +		if (upper_32_bits(addr) != 0L)
> +			return -1;
> +		tmp->addr = lower_32_bits(addr);
> +		tmp++;
> +		num_desc++;
> +	}
> +
> +	return num_desc;
> +}
> +
>  static int
>  rga_queue_setup(struct vb2_queue *vq,
>  		unsigned int *nbuffers, unsigned int *nplanes,
> @@ -114,11 +134,8 @@ void rga_buf_map(struct vb2_buffer *vb)
>  {
>  	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
>  	struct rockchip_rga *rga = ctx->rga;
> -	struct sg_table *sgt;
> -	struct scatterlist *sgl;
> -	unsigned int *pages;
> -	unsigned int address, len, i, p;
> -	unsigned int mapped_size = 0;
> +	struct rga_dma_desc *pages;
> +	unsigned int num_desc = 0;
>  
>  	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
>  		pages = rga->src_mmu_pages;
> @@ -126,23 +143,13 @@ void rga_buf_map(struct vb2_buffer *vb)
>  		pages = rga->dst_mmu_pages;
>  
>  	/* Create local MMU table for RGA */
> -	sgt = vb2_plane_cookie(vb, 0);
> -
> -	for_each_sg(sgt->sgl, sgl, sgt->nents, i) {
> -		len = sg_dma_len(sgl) >> PAGE_SHIFT;
> -		address = sg_phys(sgl);
> -
> -		for (p = 0; p < len; p++) {
> -			dma_addr_t phys = address +
> -					  ((dma_addr_t)p << PAGE_SHIFT);
> -
> -			pages[mapped_size + p] = phys;
> -		}
> -
> -		mapped_size += len;
> +	num_desc = fill_descriptors(pages, vb2_dma_sg_plane_desc(vb, 0));
> +	if (num_desc < 0) {
> +		dev_err(rga->dev, "Failed to map buffer");
> +		return;
>  	}
>  
>  	/* sync local MMU table for RGA */
>  	dma_sync_single_for_device(rga->dev, virt_to_phys(pages),
> -				   8 * PAGE_SIZE, DMA_BIDIRECTIONAL);
> +				   num_desc * sizeof(*pages), DMA_BIDIRECTIONAL);
>  }
> diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
> index 5fa9d2f366dc..22f7da28ac51 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -40,6 +40,10 @@ struct rga_frame {
>  	u32 size;
>  };
>  
> +struct rga_dma_desc {
> +	u32 addr;
> +};
> +
>  struct rockchip_rga_version {
>  	u32 major;
>  	u32 minor;
> @@ -81,8 +85,8 @@ struct rockchip_rga {
>  	struct rga_ctx *curr;
>  	dma_addr_t cmdbuf_phy;
>  	void *cmdbuf_virt;
> -	unsigned int *src_mmu_pages;
> -	unsigned int *dst_mmu_pages;
> +	struct rga_dma_desc *src_mmu_pages;
> +	struct rga_dma_desc *dst_mmu_pages;

This breaks compilation:

drivers/media/platform/rockchip/rga/rga.c: In function ‘rga_probe’:
drivers/media/platform/rockchip/rga/rga.c:875:28: error: assignment to ‘struct rga_dma_desc *’ from incompatible pointer type ‘unsigned int *’ [-Werror=incompatible-pointer-types]
  875 |         rga->src_mmu_pages =
      |                            ^
drivers/media/platform/rockchip/rga/rga.c:881:28: error: assignment to ‘struct rga_dma_desc *’ from incompatible pointer type ‘unsigned int *’ [-Werror=incompatible-pointer-types]
  881 |         rga->dst_mmu_pages =
      |                            ^

It's probably something that is fixed in a later patch, but as it is right
now this series breaks bisect.

Regards,

	Hans

>  };
>  
>  struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type);
>