[PATCH 00/13] media: rockchip: rga: add support for multi-planar formats

[PATCH 00/13] media: rockchip: rga: add support for multi-planar formats

[Michael Tretter]

This series adds support for the V4L2 multi-planar API to the Rockchip
RGA driver. Once the RGA driver supports the multi-planar API, it is
easier to share buffers with other V4L2 drivers that also support the
multi-planar API and may not expose planar formats with contiguous
planes.

With non-contiguous planes, the U and V planes may not start at the same
offset as with the continuous planes. Therefore, the RGA driver cannot
rely on its calculation of the plane offsets based on the format and
frame size anymore, but must remember the offsets when it created the
mapping. Therefore, I also reworked how the DMA mapping is handled.

As a bonus, the RGA driver should now work correctly on devices with
more than 4 GB of memory. Video buffers should now be allocated within
the 4 GB boundary and an import of buffers that have higher addresses
into the driver should fail.

Patch 1 fixes the swizzling of RGA formats. While testing all formats of
the driver on rk3568, I discovered that the color channels of the RGB
formats are wrong when converting to NV12. I didn't test this on other
SoCs with an RGA and I am not sure, if they behave differently regarding
the color channels. Please report, if this breaks the color conversion
on other SoCs, and I will make this SoC-specific.

Patches 2 to 6 are the rework the DMA descriptor handling for the RGA
MMU. The patches clean up, how the driver uses the DMA API, and make the
creation of the descriptor list more explicit. Furthermore, the driver
is changed to keep the mapping per video buffer instead of using a
single mapping that is updated with every buffer.

Patches 7 to 11 prepare the driver for the multi-planar API including a
cleanup of the format handling in the buffer, and finally switch to the
multi-planar API.

Patch 12 updates the code that creates the DMA-descriptor mapping to
correctly handle buffers with multiple planes. The driver has to iterate
all planes and make them the continuous for the RGA.

Patch 13 enables the NV12M format, which is the multi-planar variant of
the NV12 format.

Michael

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
Michael Tretter (13):
      media: rockchip: rga: fix swizzling for RGB formats
      media: rockchip: rga: extract helper to fill descriptors
      media: rockchip: rga: allocate DMA descriptors per buffer
      media: rockchip: rga: split src and dst buffer setup
      media: rockchip: rga: pre-calculate plane offsets
      media: rockchip: rga: set dma mask to 32 bits
      media: rockchip: rga: use clamp() to clamp size to limits
      media: rockchip: rga: use pixelformat to find format
      media: rockchip: rga: add local variable for pix_format
      media: rockchip: rga: use macros for testing buffer type
      media: rockchip: rga: switch to multi-planar API
      media: rockchip: rga: rework buffer handling for multi-planar formats
      media: rockchip: rga: add NV12M support

 drivers/media/platform/rockchip/rga/rga-buf.c | 166 ++++++++++++++++-------
 drivers/media/platform/rockchip/rga/rga-hw.c  | 143 +++++++++++++-------
 drivers/media/platform/rockchip/rga/rga.c     | 187 ++++++++++++--------------
 drivers/media/platform/rockchip/rga/rga.h     |  35 ++++-
 4 files changed, 329 insertions(+), 202 deletions(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230914-rockchip-rga-multiplanar-0d7c79b1ba93

Best regards,
-- 
Michael Tretter <m.tretter@pengutronix.de>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 01/13] media: rockchip: rga: fix swizzling for RGB formats

[Michael Tretter]

When using 32 bit RGB formats, the RGA on the rk3568 produces wrong
colors as the wrong color channels are read or written.  The reason is
that the format description for the channel swizzeling is wrong and the
wrong bits are configured. For example, when converting ARGB32 to NV12,
the alpha channel is used as blue channel.. This doesn't happen if the
color format is the same on both sides.

Fix the color_swap settings of the formats to correctly handle 32 bit
RGB formats.

For RGA_COLOR_FMT_XBGR8888, the RGA_COLOR_ALPHA_SWAP bit doesn't have an
effect. Thus, it isn't possible to handle the V4L2_PIX_FMT_XRGB32. Thus,
it is removed from the list of supported formats.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index f1c532a5802a..25f5b5eebf13 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -184,25 +184,16 @@ static int rga_setup_ctrls(struct rga_ctx *ctx)
 static struct rga_fmt formats[] = {
 	{
 		.fourcc = V4L2_PIX_FMT_ARGB32,
-		.color_swap = RGA_COLOR_RB_SWAP,
+		.color_swap = RGA_COLOR_ALPHA_SWAP,
 		.hw_format = RGA_COLOR_FMT_ABGR8888,
 		.depth = 32,
 		.uv_factor = 1,
 		.y_div = 1,
 		.x_div = 1,
 	},
-	{
-		.fourcc = V4L2_PIX_FMT_XRGB32,
-		.color_swap = RGA_COLOR_RB_SWAP,
-		.hw_format = RGA_COLOR_FMT_XBGR8888,
-		.depth = 32,
-		.uv_factor = 1,
-		.y_div = 1,
-		.x_div = 1,
-	},
 	{
 		.fourcc = V4L2_PIX_FMT_ABGR32,
-		.color_swap = RGA_COLOR_ALPHA_SWAP,
+		.color_swap = RGA_COLOR_RB_SWAP,
 		.hw_format = RGA_COLOR_FMT_ABGR8888,
 		.depth = 32,
 		.uv_factor = 1,
@@ -211,7 +202,7 @@ static struct rga_fmt formats[] = {
 	},
 	{
 		.fourcc = V4L2_PIX_FMT_XBGR32,
-		.color_swap = RGA_COLOR_ALPHA_SWAP,
+		.color_swap = RGA_COLOR_RB_SWAP,
 		.hw_format = RGA_COLOR_FMT_XBGR8888,
 		.depth = 32,
 		.uv_factor = 1,

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michael Tretter]

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@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;
 };
 
 struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type);

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 03/13] media: rockchip: rga: allocate DMA descriptors per buffer

[Michael Tretter]

The RGA driver allocates two buffers for the DMA descriptors of the
input and output buffers. Whenever a new job is processed, the
descriptor list is updated for the current buffers.

By updating the descriptor list during buf_prepare, it is possible to
correctly fail DMABUF imports if the buffers that shall be imported are
not within the 32 bit address range that can be addressed by the RGA.

Managing the DMA descriptor list with the buffer also makes it easier to
track the buffer mapping and the plane offsets into this mapping.

The cost is that the driver now requires DMA coherent memory per buffer
for the descriptor list. However, the size scales with the size of the
video buffers and is not allocated if the RGA is not used.

While at it, use dma_alloc_coherent to allocate the descriptors and get
rid of the virt_to_phys calls to get the physical addresses.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga-buf.c | 78 +++++++++++++++++----------
 drivers/media/platform/rockchip/rga/rga-hw.c  | 26 ++++-----
 drivers/media/platform/rockchip/rga/rga.c     | 31 ++---------
 drivers/media/platform/rockchip/rga/rga.h     | 21 ++++++--
 4 files changed, 86 insertions(+), 70 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index df5ebc90e32d..e8dcc0d5cb90 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -55,16 +55,54 @@ rga_queue_setup(struct vb2_queue *vq,
 	return 0;
 }
 
+static int rga_buf_init(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct rga_vb_buffer *rbuf = vb_to_rga(vbuf);
+	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+	struct rockchip_rga *rga = ctx->rga;
+	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
+	int n_desc = 0;
+
+	n_desc = DIV_ROUND_UP(f->size, PAGE_SIZE);
+
+	rbuf->n_desc = n_desc;
+	rbuf->dma_desc = dma_alloc_coherent(rga->dev,
+					    rbuf->n_desc * sizeof(*rbuf->dma_desc),
+					    &rbuf->dma_desc_pa, GFP_KERNEL);
+	if (!rbuf->dma_desc)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int rga_buf_prepare(struct vb2_buffer *vb)
 {
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct rga_vb_buffer *rbuf = vb_to_rga(vbuf);
 	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
 	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
+	struct rockchip_rga *rga = ctx->rga;
+	int n_desc;
 
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
 	vb2_set_plane_payload(vb, 0, f->size);
 
+	/* Create local MMU table for RGA */
+	n_desc = fill_descriptors(rbuf->dma_desc,
+				  vb2_dma_sg_plane_desc(vb, 0));
+	if (n_desc < 0) {
+		dev_err(rga->dev, "Failed to map buffer");
+		return n_desc;
+	}
+
+	/* sync local MMU table for RGA */
+	dma_sync_single_for_device(rga->dev, rbuf->dma_desc_pa,
+				   n_desc * sizeof(*rbuf->dma_desc),
+				   DMA_BIDIRECTIONAL);
+
 	return 0;
 }
 
@@ -76,6 +114,17 @@ static void rga_buf_queue(struct vb2_buffer *vb)
 	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 }
 
+static void rga_buf_cleanup(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct rga_vb_buffer *rbuf = vb_to_rga(vbuf);
+	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+	struct rockchip_rga *rga = ctx->rga;
+
+	dma_free_coherent(rga->dev, rbuf->n_desc * sizeof(*rbuf->dma_desc),
+			  rbuf->dma_desc, rbuf->dma_desc_pa);
+}
+
 static void rga_buf_return_buffers(struct vb2_queue *q,
 				   enum vb2_buffer_state state)
 {
@@ -119,37 +168,12 @@ static void rga_buf_stop_streaming(struct vb2_queue *q)
 
 const struct vb2_ops rga_qops = {
 	.queue_setup = rga_queue_setup,
+	.buf_init = rga_buf_init,
 	.buf_prepare = rga_buf_prepare,
 	.buf_queue = rga_buf_queue,
+	.buf_cleanup = rga_buf_cleanup,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
 	.start_streaming = rga_buf_start_streaming,
 	.stop_streaming = rga_buf_stop_streaming,
 };
-
-/* RGA MMU is a 1-Level MMU, so it can't be used through the IOMMU API.
- * We use it more like a scatter-gather list.
- */
-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 rga_dma_desc *pages;
-	unsigned int num_desc = 0;
-
-	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
-		pages = rga->src_mmu_pages;
-	else
-		pages = rga->dst_mmu_pages;
-
-	/* Create local MMU table for RGA */
-	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),
-				   num_desc * sizeof(*pages), DMA_BIDIRECTIONAL);
-}
diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index aaa96f256356..b391d97d4632 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -119,40 +119,40 @@ static struct rga_addr_offset *rga_lookup_draw_pos(struct
 	return NULL;
 }
 
-static void rga_cmd_set_src_addr(struct rga_ctx *ctx, void *mmu_pages)
+static void rga_cmd_set_src_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
 {
 	struct rockchip_rga *rga = ctx->rga;
 	u32 *dest = rga->cmdbuf_virt;
 	unsigned int reg;
 
 	reg = RGA_MMU_SRC_BASE - RGA_MODE_BASE_REG;
-	dest[reg >> 2] = virt_to_phys(mmu_pages) >> 4;
+	dest[reg >> 2] = dma_addr >> 4;
 
 	reg = RGA_MMU_CTRL1 - RGA_MODE_BASE_REG;
 	dest[reg >> 2] |= 0x7;
 }
 
-static void rga_cmd_set_src1_addr(struct rga_ctx *ctx, void *mmu_pages)
+static void rga_cmd_set_src1_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
 {
 	struct rockchip_rga *rga = ctx->rga;
 	u32 *dest = rga->cmdbuf_virt;
 	unsigned int reg;
 
 	reg = RGA_MMU_SRC1_BASE - RGA_MODE_BASE_REG;
-	dest[reg >> 2] = virt_to_phys(mmu_pages) >> 4;
+	dest[reg >> 2] = dma_addr >> 4;
 
 	reg = RGA_MMU_CTRL1 - RGA_MODE_BASE_REG;
 	dest[reg >> 2] |= 0x7 << 4;
 }
 
-static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, void *mmu_pages)
+static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
 {
 	struct rockchip_rga *rga = ctx->rga;
 	u32 *dest = rga->cmdbuf_virt;
 	unsigned int reg;
 
 	reg = RGA_MMU_DST_BASE - RGA_MODE_BASE_REG;
-	dest[reg >> 2] = virt_to_phys(mmu_pages) >> 4;
+	dest[reg >> 2] = dma_addr >> 4;
 
 	reg = RGA_MMU_CTRL1 - RGA_MODE_BASE_REG;
 	dest[reg >> 2] |= 0x7 << 8;
@@ -375,20 +375,21 @@ static void rga_cmd_set_mode(struct rga_ctx *ctx)
 	dest[(RGA_MODE_CTRL - RGA_MODE_BASE_REG) >> 2] = mode.val;
 }
 
-static void rga_cmd_set(struct rga_ctx *ctx)
+static void rga_cmd_set(struct rga_ctx *ctx,
+			struct rga_vb_buffer *src, struct rga_vb_buffer *dst)
 {
 	struct rockchip_rga *rga = ctx->rga;
 
 	memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE * 4);
 
-	rga_cmd_set_src_addr(ctx, rga->src_mmu_pages);
+	rga_cmd_set_src_addr(ctx, src->dma_desc_pa);
 	/*
 	 * Due to hardware bug,
 	 * src1 mmu also should be configured when using alpha blending.
 	 */
-	rga_cmd_set_src1_addr(ctx, rga->dst_mmu_pages);
+	rga_cmd_set_src1_addr(ctx, dst->dma_desc_pa);
 
-	rga_cmd_set_dst_addr(ctx, rga->dst_mmu_pages);
+	rga_cmd_set_dst_addr(ctx, dst->dma_desc_pa);
 	rga_cmd_set_mode(ctx);
 
 	rga_cmd_set_trans_info(ctx);
@@ -400,11 +401,12 @@ static void rga_cmd_set(struct rga_ctx *ctx)
 		PAGE_SIZE, DMA_BIDIRECTIONAL);
 }
 
-void rga_hw_start(struct rockchip_rga *rga)
+void rga_hw_start(struct rockchip_rga *rga,
+		  struct rga_vb_buffer *src, struct rga_vb_buffer *dst)
 {
 	struct rga_ctx *ctx = rga->curr;
 
-	rga_cmd_set(ctx);
+	rga_cmd_set(ctx, src, dst);
 
 	rga_write(rga, RGA_SYS_CTRL, 0x00);
 
diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 25f5b5eebf13..f18fccc7b204 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -45,10 +45,7 @@ static void device_run(void *prv)
 	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
-	rga_buf_map(&src->vb2_buf);
-	rga_buf_map(&dst->vb2_buf);
-
-	rga_hw_start(rga);
+	rga_hw_start(rga, vb_to_rga(src), vb_to_rga(dst));
 
 	spin_unlock_irqrestore(&rga->ctrl_lock, flags);
 }
@@ -101,7 +98,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	src_vq->drv_priv = ctx;
 	src_vq->ops = &rga_qops;
 	src_vq->mem_ops = &vb2_dma_sg_memops;
-	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	src_vq->buf_struct_size = sizeof(struct rga_vb_buffer);
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	src_vq->lock = &ctx->rga->mutex;
 	src_vq->dev = ctx->rga->v4l2_dev.dev;
@@ -115,7 +112,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	dst_vq->drv_priv = ctx;
 	dst_vq->ops = &rga_qops;
 	dst_vq->mem_ops = &vb2_dma_sg_memops;
-	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	dst_vq->buf_struct_size = sizeof(struct rga_vb_buffer);
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->lock = &ctx->rga->mutex;
 	dst_vq->dev = ctx->rga->v4l2_dev.dev;
@@ -872,26 +869,13 @@ static int rga_probe(struct platform_device *pdev)
 		goto rel_m2m;
 	}
 
-	rga->src_mmu_pages =
-		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
-	if (!rga->src_mmu_pages) {
-		ret = -ENOMEM;
-		goto free_dma;
-	}
-	rga->dst_mmu_pages =
-		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
-	if (!rga->dst_mmu_pages) {
-		ret = -ENOMEM;
-		goto free_src_pages;
-	}
-
 	def_frame.stride = (def_frame.width * def_frame.fmt->depth) >> 3;
 	def_frame.size = def_frame.stride * def_frame.height;
 
 	ret = video_register_device(vfd, VFL_TYPE_VIDEO, -1);
 	if (ret) {
 		v4l2_err(&rga->v4l2_dev, "Failed to register video device\n");
-		goto free_dst_pages;
+		goto free_dma;
 	}
 
 	v4l2_info(&rga->v4l2_dev, "Registered %s as /dev/%s\n",
@@ -899,10 +883,6 @@ static int rga_probe(struct platform_device *pdev)
 
 	return 0;
 
-free_dst_pages:
-	free_pages((unsigned long)rga->dst_mmu_pages, 3);
-free_src_pages:
-	free_pages((unsigned long)rga->src_mmu_pages, 3);
 free_dma:
 	dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
 		       rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
@@ -925,9 +905,6 @@ static void rga_remove(struct platform_device *pdev)
 	dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
 		       rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
 
-	free_pages((unsigned long)rga->src_mmu_pages, 3);
-	free_pages((unsigned long)rga->dst_mmu_pages, 3);
-
 	v4l2_info(&rga->v4l2_dev, "Removing\n");
 
 	v4l2_m2m_release(rga->m2m_dev);
diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
index 22f7da28ac51..ae984d5a236d 100644
--- a/drivers/media/platform/rockchip/rga/rga.h
+++ b/drivers/media/platform/rockchip/rga/rga.h
@@ -85,15 +85,27 @@ struct rockchip_rga {
 	struct rga_ctx *curr;
 	dma_addr_t cmdbuf_phy;
 	void *cmdbuf_virt;
-	struct rga_dma_desc *src_mmu_pages;
-	struct rga_dma_desc *dst_mmu_pages;
 };
 
+struct rga_vb_buffer {
+	struct vb2_v4l2_buffer vb_buf;
+	struct list_head queue;
+
+	/* RGA MMU mapping for this buffer */
+	struct rga_dma_desc *dma_desc;
+	dma_addr_t dma_desc_pa;
+	int n_desc;
+};
+
+static inline struct rga_vb_buffer *vb_to_rga(struct vb2_v4l2_buffer *vb)
+{
+	return container_of(vb, struct rga_vb_buffer, vb_buf);
+}
+
 struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type);
 
 /* RGA Buffers Manage */
 extern const struct vb2_ops rga_qops;
-void rga_buf_map(struct vb2_buffer *vb);
 
 /* RGA Hardware */
 static inline void rga_write(struct rockchip_rga *rga, u32 reg, u32 value)
@@ -114,6 +126,7 @@ static inline void rga_mod(struct rockchip_rga *rga, u32 reg, u32 val, u32 mask)
 	rga_write(rga, reg, temp);
 };
 
-void rga_hw_start(struct rockchip_rga *rga);
+void rga_hw_start(struct rockchip_rga *rga,
+		  struct rga_vb_buffer *src, struct rga_vb_buffer *dst);
 
 #endif

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 04/13] media: rockchip: rga: split src and dst buffer setup

[Michael Tretter]

Split the register setup for the source and destination video buffers
into separate functions.

This is a cleanup to make the code more readable by separating the
offset calculation for the different buffers and prepares the driver for
using pre-calculated offsets of planes.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga-hw.c | 94 ++++++++++++++++++++--------
 1 file changed, 68 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index b391d97d4632..836ec7721b21 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -163,7 +163,7 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	struct rockchip_rga *rga = ctx->rga;
 	u32 *dest = rga->cmdbuf_virt;
 	unsigned int scale_dst_w, scale_dst_h;
-	unsigned int src_h, src_w, src_x, src_y, dst_h, dst_w, dst_x, dst_y;
+	unsigned int src_h, src_w, dst_h, dst_w;
 	union rga_src_info src_info;
 	union rga_dst_info dst_info;
 	union rga_src_x_factor x_factor;
@@ -173,18 +173,10 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	union rga_dst_vir_info dst_vir_info;
 	union rga_dst_act_info dst_act_info;
 
-	struct rga_addr_offset *dst_offset;
-	struct rga_corners_addr_offset offsets;
-	struct rga_corners_addr_offset src_offsets;
-
 	src_h = ctx->in.crop.height;
 	src_w = ctx->in.crop.width;
-	src_x = ctx->in.crop.left;
-	src_y = ctx->in.crop.top;
 	dst_h = ctx->out.crop.height;
 	dst_w = ctx->out.crop.width;
-	dst_x = ctx->out.crop.left;
-	dst_y = ctx->out.crop.top;
 
 	src_info.val = dest[(RGA_SRC_INFO - RGA_MODE_BASE_REG) >> 2];
 	dst_info.val = dest[(RGA_DST_INFO - RGA_MODE_BASE_REG) >> 2];
@@ -312,32 +304,85 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	dst_act_info.data.act_height = dst_h - 1;
 	dst_act_info.data.act_width = dst_w - 1;
 
+	dest[(RGA_SRC_X_FACTOR - RGA_MODE_BASE_REG) >> 2] = x_factor.val;
+	dest[(RGA_SRC_Y_FACTOR - RGA_MODE_BASE_REG) >> 2] = y_factor.val;
+	dest[(RGA_SRC_VIR_INFO - RGA_MODE_BASE_REG) >> 2] = src_vir_info.val;
+	dest[(RGA_SRC_ACT_INFO - RGA_MODE_BASE_REG) >> 2] = src_act_info.val;
+
+	dest[(RGA_SRC_INFO - RGA_MODE_BASE_REG) >> 2] = src_info.val;
+
+	dest[(RGA_DST_VIR_INFO - RGA_MODE_BASE_REG) >> 2] = dst_vir_info.val;
+	dest[(RGA_DST_ACT_INFO - RGA_MODE_BASE_REG) >> 2] = dst_act_info.val;
+
+	dest[(RGA_DST_INFO - RGA_MODE_BASE_REG) >> 2] = dst_info.val;
+}
+
+static void rga_cmd_set_src_info(struct rga_ctx *ctx)
+{
+	struct rga_corners_addr_offset src_offsets;
+	struct rockchip_rga *rga = ctx->rga;
+	u32 *dest = rga->cmdbuf_virt;
+	unsigned int src_h, src_w, src_x, src_y;
+
+	src_h = ctx->in.crop.height;
+	src_w = ctx->in.crop.width;
+	src_x = ctx->in.crop.left;
+	src_y = ctx->in.crop.top;
+
 	/*
 	 * Calculate the source framebuffer base address with offset pixel.
 	 */
 	src_offsets = rga_get_addr_offset(&ctx->in, src_x, src_y,
 					  src_w, src_h);
 
-	/*
-	 * Configure the dest framebuffer base address with pixel offset.
-	 */
-	offsets = rga_get_addr_offset(&ctx->out, dst_x, dst_y, dst_w, dst_h);
-	dst_offset = rga_lookup_draw_pos(&offsets, src_info.data.rot_mode,
-					 src_info.data.mir_mode);
-
 	dest[(RGA_SRC_Y_RGB_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
 		src_offsets.left_top.y_off;
 	dest[(RGA_SRC_CB_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
 		src_offsets.left_top.u_off;
 	dest[(RGA_SRC_CR_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
 		src_offsets.left_top.v_off;
+}
 
-	dest[(RGA_SRC_X_FACTOR - RGA_MODE_BASE_REG) >> 2] = x_factor.val;
-	dest[(RGA_SRC_Y_FACTOR - RGA_MODE_BASE_REG) >> 2] = y_factor.val;
-	dest[(RGA_SRC_VIR_INFO - RGA_MODE_BASE_REG) >> 2] = src_vir_info.val;
-	dest[(RGA_SRC_ACT_INFO - RGA_MODE_BASE_REG) >> 2] = src_act_info.val;
+static void rga_cmd_set_dst_info(struct rga_ctx *ctx)
+{
+	struct rga_addr_offset *dst_offset;
+	struct rga_corners_addr_offset offsets;
+	struct rockchip_rga *rga = ctx->rga;
+	u32 *dest = rga->cmdbuf_virt;
+	unsigned int dst_h, dst_w, dst_x, dst_y;
+	unsigned int mir_mode = 0;
+	unsigned int rot_mode = 0;
 
-	dest[(RGA_SRC_INFO - RGA_MODE_BASE_REG) >> 2] = src_info.val;
+	dst_h = ctx->out.crop.height;
+	dst_w = ctx->out.crop.width;
+	dst_x = ctx->out.crop.left;
+	dst_y = ctx->out.crop.top;
+
+	if (ctx->vflip)
+		mir_mode |= RGA_SRC_MIRR_MODE_X;
+	if (ctx->hflip)
+		mir_mode |= RGA_SRC_MIRR_MODE_Y;
+
+	switch (ctx->rotate) {
+	case 90:
+		rot_mode = RGA_SRC_ROT_MODE_90_DEGREE;
+		break;
+	case 180:
+		rot_mode = RGA_SRC_ROT_MODE_180_DEGREE;
+		break;
+	case 270:
+		rot_mode = RGA_SRC_ROT_MODE_270_DEGREE;
+		break;
+	default:
+		rot_mode = RGA_SRC_ROT_MODE_0_DEGREE;
+		break;
+	}
+
+	/*
+	 * Configure the dest framebuffer base address with pixel offset.
+	 */
+	offsets = rga_get_addr_offset(&ctx->out, dst_x, dst_y, dst_w, dst_h);
+	dst_offset = rga_lookup_draw_pos(&offsets, mir_mode, rot_mode);
 
 	dest[(RGA_DST_Y_RGB_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
 		dst_offset->y_off;
@@ -345,11 +390,6 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 		dst_offset->u_off;
 	dest[(RGA_DST_CR_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
 		dst_offset->v_off;
-
-	dest[(RGA_DST_VIR_INFO - RGA_MODE_BASE_REG) >> 2] = dst_vir_info.val;
-	dest[(RGA_DST_ACT_INFO - RGA_MODE_BASE_REG) >> 2] = dst_act_info.val;
-
-	dest[(RGA_DST_INFO - RGA_MODE_BASE_REG) >> 2] = dst_info.val;
 }
 
 static void rga_cmd_set_mode(struct rga_ctx *ctx)
@@ -392,6 +432,8 @@ static void rga_cmd_set(struct rga_ctx *ctx,
 	rga_cmd_set_dst_addr(ctx, dst->dma_desc_pa);
 	rga_cmd_set_mode(ctx);
 
+	rga_cmd_set_src_info(ctx);
+	rga_cmd_set_dst_info(ctx);
 	rga_cmd_set_trans_info(ctx);
 
 	rga_write(rga, RGA_CMD_BASE, rga->cmdbuf_phy);

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 05/13] media: rockchip: rga: pre-calculate plane offsets

[Michael Tretter]

Calculate the plane offsets and store them with the video buffer while
creating the buffer mapping.

This allows the driver to more freely handle the memory of the DMA
mapping as the offsets and the mapping can be kept in sync.

The driver still has to update the offsets to respect the configured
cropping and rotation, but this calculation is now separated from the
calculation of the plane offsets.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga-buf.c | 15 ++++++++++++
 drivers/media/platform/rockchip/rga/rga-hw.c  | 33 ++++++++++++---------------
 drivers/media/platform/rockchip/rga/rga.h     |  9 ++++++++
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index e8dcc0d5cb90..137f4f4be14c 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -76,6 +76,18 @@ static int rga_buf_init(struct vb2_buffer *vb)
 	return 0;
 }
 
+static int get_plane_offset(struct rga_frame *f, int plane)
+{
+	if (plane == 0)
+		return 0;
+	if (plane == 1)
+		return f->width * f->height;
+	if (plane == 2)
+		return f->width * f->height + (f->width * f->height / f->fmt->uv_factor);
+
+	return -EINVAL;
+}
+
 static int rga_buf_prepare(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -97,6 +109,9 @@ static int rga_buf_prepare(struct vb2_buffer *vb)
 		dev_err(rga->dev, "Failed to map buffer");
 		return n_desc;
 	}
+	rbuf->offset.y_off = get_plane_offset(f, 0);
+	rbuf->offset.u_off = get_plane_offset(f, 1);
+	rbuf->offset.v_off = get_plane_offset(f, 2);
 
 	/* sync local MMU table for RGA */
 	dma_sync_single_for_device(rga->dev, rbuf->dma_desc_pa,
diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
index 836ec7721b21..8cbee059100b 100644
--- a/drivers/media/platform/rockchip/rga/rga-hw.c
+++ b/drivers/media/platform/rockchip/rga/rga-hw.c
@@ -16,12 +16,6 @@ enum e_rga_start_pos {
 	RB = 3,
 };
 
-struct rga_addr_offset {
-	unsigned int y_off;
-	unsigned int u_off;
-	unsigned int v_off;
-};
-
 struct rga_corners_addr_offset {
 	struct rga_addr_offset left_top;
 	struct rga_addr_offset right_top;
@@ -43,8 +37,8 @@ static unsigned int rga_get_scaling(unsigned int src, unsigned int dst)
 }
 
 static struct rga_corners_addr_offset
-rga_get_addr_offset(struct rga_frame *frm, unsigned int x, unsigned int y,
-		    unsigned int w, unsigned int h)
+rga_get_addr_offset(struct rga_frame *frm, struct rga_addr_offset *offset,
+		    unsigned int x, unsigned int y, unsigned int w, unsigned int h)
 {
 	struct rga_corners_addr_offset offsets;
 	struct rga_addr_offset *lt, *lb, *rt, *rb;
@@ -62,10 +56,9 @@ rga_get_addr_offset(struct rga_frame *frm, unsigned int x, unsigned int y,
 	uv_stride = frm->stride / x_div;
 	pixel_width = frm->stride / frm->width;
 
-	lt->y_off = y * frm->stride + x * pixel_width;
-	lt->u_off =
-		frm->width * frm->height + (y / y_div) * uv_stride + x / x_div;
-	lt->v_off = lt->u_off + frm->width * frm->height / uv_factor;
+	lt->y_off = offset->y_off + y * frm->stride + x * pixel_width;
+	lt->u_off = offset->u_off + (y / y_div) * uv_stride + x / x_div;
+	lt->v_off = offset->v_off + (y / y_div) * uv_stride + x / x_div;
 
 	lb->y_off = lt->y_off + (h - 1) * frm->stride;
 	lb->u_off = lt->u_off + (h / y_div - 1) * uv_stride;
@@ -317,7 +310,8 @@ static void rga_cmd_set_trans_info(struct rga_ctx *ctx)
 	dest[(RGA_DST_INFO - RGA_MODE_BASE_REG) >> 2] = dst_info.val;
 }
 
-static void rga_cmd_set_src_info(struct rga_ctx *ctx)
+static void rga_cmd_set_src_info(struct rga_ctx *ctx,
+				 struct rga_addr_offset *offset)
 {
 	struct rga_corners_addr_offset src_offsets;
 	struct rockchip_rga *rga = ctx->rga;
@@ -332,8 +326,8 @@ static void rga_cmd_set_src_info(struct rga_ctx *ctx)
 	/*
 	 * Calculate the source framebuffer base address with offset pixel.
 	 */
-	src_offsets = rga_get_addr_offset(&ctx->in, src_x, src_y,
-					  src_w, src_h);
+	src_offsets = rga_get_addr_offset(&ctx->in, offset,
+					  src_x, src_y, src_w, src_h);
 
 	dest[(RGA_SRC_Y_RGB_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
 		src_offsets.left_top.y_off;
@@ -343,7 +337,8 @@ static void rga_cmd_set_src_info(struct rga_ctx *ctx)
 		src_offsets.left_top.v_off;
 }
 
-static void rga_cmd_set_dst_info(struct rga_ctx *ctx)
+static void rga_cmd_set_dst_info(struct rga_ctx *ctx,
+				 struct rga_addr_offset *offset)
 {
 	struct rga_addr_offset *dst_offset;
 	struct rga_corners_addr_offset offsets;
@@ -381,7 +376,7 @@ static void rga_cmd_set_dst_info(struct rga_ctx *ctx)
 	/*
 	 * Configure the dest framebuffer base address with pixel offset.
 	 */
-	offsets = rga_get_addr_offset(&ctx->out, dst_x, dst_y, dst_w, dst_h);
+	offsets = rga_get_addr_offset(&ctx->out, offset, dst_x, dst_y, dst_w, dst_h);
 	dst_offset = rga_lookup_draw_pos(&offsets, mir_mode, rot_mode);
 
 	dest[(RGA_DST_Y_RGB_BASE_ADDR - RGA_MODE_BASE_REG) >> 2] =
@@ -432,8 +427,8 @@ static void rga_cmd_set(struct rga_ctx *ctx,
 	rga_cmd_set_dst_addr(ctx, dst->dma_desc_pa);
 	rga_cmd_set_mode(ctx);
 
-	rga_cmd_set_src_info(ctx);
-	rga_cmd_set_dst_info(ctx);
+	rga_cmd_set_src_info(ctx, &src->offset);
+	rga_cmd_set_dst_info(ctx, &dst->offset);
 	rga_cmd_set_trans_info(ctx);
 
 	rga_write(rga, RGA_CMD_BASE, rga->cmdbuf_phy);
diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
index ae984d5a236d..d8e76ab9c7e4 100644
--- a/drivers/media/platform/rockchip/rga/rga.h
+++ b/drivers/media/platform/rockchip/rga/rga.h
@@ -87,6 +87,12 @@ struct rockchip_rga {
 	void *cmdbuf_virt;
 };
 
+struct rga_addr_offset {
+	unsigned int y_off;
+	unsigned int u_off;
+	unsigned int v_off;
+};
+
 struct rga_vb_buffer {
 	struct vb2_v4l2_buffer vb_buf;
 	struct list_head queue;
@@ -95,6 +101,9 @@ struct rga_vb_buffer {
 	struct rga_dma_desc *dma_desc;
 	dma_addr_t dma_desc_pa;
 	int n_desc;
+
+	/* Plane offsets of this buffer into the mapping */
+	struct rga_addr_offset offset;
 };
 
 static inline struct rga_vb_buffer *vb_to_rga(struct vb2_v4l2_buffer *vb)

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 06/13] media: rockchip: rga: set dma mask to 32 bits

[Michael Tretter]

The RGA DMA descriptor list contains only 32-bit addresses. Set the
dma_mask to only allocate memory that is addressable by the descriptors.

This prevents errors when preparing vb2 buffers that were allocated by
the RGA. Imported buffers may still fail the preparation, as they may be
allocated above the 4 GB boundary.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index f18fccc7b204..149deb1f1e03 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -824,6 +824,12 @@ static int rga_probe(struct platform_device *pdev)
 		goto err_put_clk;
 	}
 
+	ret = dma_set_mask(rga->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(rga->dev, "32-bit DMA not supported");
+		goto err_put_clk;
+	}
+
 	ret = v4l2_device_register(&pdev->dev, &rga->v4l2_dev);
 	if (ret)
 		goto err_put_clk;

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 07/13] media: rockchip: rga: use clamp() to clamp size to limits

[Michael Tretter]

The try_fmt should limit the width and height to the know limits of the
RGA. Use the clamp() helper instead of open coding the clamping.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 149deb1f1e03..49403498bc17 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -480,15 +480,10 @@ static int vidioc_try_fmt(struct file *file, void *prv, struct v4l2_format *f)
 
 	f->fmt.pix.field = V4L2_FIELD_NONE;
 
-	if (f->fmt.pix.width > MAX_WIDTH)
-		f->fmt.pix.width = MAX_WIDTH;
-	if (f->fmt.pix.height > MAX_HEIGHT)
-		f->fmt.pix.height = MAX_HEIGHT;
-
-	if (f->fmt.pix.width < MIN_WIDTH)
-		f->fmt.pix.width = MIN_WIDTH;
-	if (f->fmt.pix.height < MIN_HEIGHT)
-		f->fmt.pix.height = MIN_HEIGHT;
+	f->fmt.pix.width = clamp(f->fmt.pix.width,
+				 (u32)MIN_WIDTH, (u32)MAX_WIDTH);
+	f->fmt.pix.height = clamp(f->fmt.pix.height,
+				  (u32)MIN_HEIGHT, (u32)MAX_HEIGHT);
 
 	if (fmt->hw_format >= RGA_COLOR_FMT_YUV422SP)
 		f->fmt.pix.bytesperline = f->fmt.pix.width;

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 08/13] media: rockchip: rga: use pixelformat to find format

[Michael Tretter]

Use the pixelformat instead of the v4l2_format to find the rga_fmt. This
avoids knowing the structure and type of v4l2_format in rga_fmt_find and
simplifies the function.

Also cleanup the users of the function. In try_fmt always return the
found pixel format to make sure that the pixel format is always set.
Thus, we can be sure that we will find the rga_fmt in s_fmt and can drop
the check if a given format has been found.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 49403498bc17..4579023af2ff 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -318,12 +318,12 @@ static struct rga_fmt formats[] = {
 
 #define NUM_FORMATS ARRAY_SIZE(formats)
 
-static struct rga_fmt *rga_fmt_find(struct v4l2_format *f)
+static struct rga_fmt *rga_fmt_find(u32 pixelformat)
 {
 	unsigned int i;
 
 	for (i = 0; i < NUM_FORMATS; i++) {
-		if (formats[i].fourcc == f->fmt.pix.pixelformat)
+		if (formats[i].fourcc == pixelformat)
 			return &formats[i];
 	}
 	return NULL;
@@ -472,11 +472,11 @@ static int vidioc_try_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
 	struct rga_fmt *fmt;
 
-	fmt = rga_fmt_find(f);
-	if (!fmt) {
+	fmt = rga_fmt_find(f->fmt.pix.pixelformat);
+	if (!fmt)
 		fmt = &formats[0];
-		f->fmt.pix.pixelformat = fmt->fourcc;
-	}
+
+	f->fmt.pix.pixelformat = fmt->fourcc;
 
 	f->fmt.pix.field = V4L2_FIELD_NONE;
 
@@ -502,7 +502,6 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	struct rockchip_rga *rga = ctx->rga;
 	struct vb2_queue *vq;
 	struct rga_frame *frm;
-	struct rga_fmt *fmt;
 	int ret = 0;
 
 	/* Adjust all values accordingly to the hardware capabilities
@@ -519,13 +518,10 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	frm = rga_get_frame(ctx, f->type);
 	if (IS_ERR(frm))
 		return PTR_ERR(frm);
-	fmt = rga_fmt_find(f);
-	if (!fmt)
-		return -EINVAL;
 	frm->width = f->fmt.pix.width;
 	frm->height = f->fmt.pix.height;
 	frm->size = f->fmt.pix.sizeimage;
-	frm->fmt = fmt;
+	frm->fmt = rga_fmt_find(f->fmt.pix.pixelformat);
 	frm->stride = f->fmt.pix.bytesperline;
 	frm->colorspace = f->fmt.pix.colorspace;
 

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 09/13] media: rockchip: rga: add local variable for pix_format

[Michael Tretter]

The local variable allows to simplify the accessed to the format and
makes it easier to change the type of the format.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 55 ++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 4579023af2ff..303ee762bec4 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -446,6 +446,7 @@ static int vidioc_enum_fmt(struct file *file, void *prv, struct v4l2_fmtdesc *f)
 
 static int vidioc_g_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
+	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
 	struct rga_ctx *ctx = prv;
 	struct vb2_queue *vq;
 	struct rga_frame *frm;
@@ -457,47 +458,49 @@ static int vidioc_g_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	if (IS_ERR(frm))
 		return PTR_ERR(frm);
 
-	f->fmt.pix.width = frm->width;
-	f->fmt.pix.height = frm->height;
-	f->fmt.pix.field = V4L2_FIELD_NONE;
-	f->fmt.pix.pixelformat = frm->fmt->fourcc;
-	f->fmt.pix.bytesperline = frm->stride;
-	f->fmt.pix.sizeimage = frm->size;
-	f->fmt.pix.colorspace = frm->colorspace;
+	pix_fmt->width = frm->width;
+	pix_fmt->height = frm->height;
+	pix_fmt->field = V4L2_FIELD_NONE;
+	pix_fmt->pixelformat = frm->fmt->fourcc;
+	pix_fmt->bytesperline = frm->stride;
+	pix_fmt->sizeimage = frm->size;
+	pix_fmt->colorspace = frm->colorspace;
 
 	return 0;
 }
 
 static int vidioc_try_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
+	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
 	struct rga_fmt *fmt;
 
 	fmt = rga_fmt_find(f->fmt.pix.pixelformat);
 	if (!fmt)
 		fmt = &formats[0];
 
-	f->fmt.pix.pixelformat = fmt->fourcc;
+	pix_fmt->pixelformat = fmt->fourcc;
 
-	f->fmt.pix.field = V4L2_FIELD_NONE;
+	pix_fmt->field = V4L2_FIELD_NONE;
 
-	f->fmt.pix.width = clamp(f->fmt.pix.width,
-				 (u32)MIN_WIDTH, (u32)MAX_WIDTH);
-	f->fmt.pix.height = clamp(f->fmt.pix.height,
-				  (u32)MIN_HEIGHT, (u32)MAX_HEIGHT);
+	pix_fmt->width = clamp(pix_fmt->width,
+			       (u32)MIN_WIDTH, (u32)MAX_WIDTH);
+	pix_fmt->height = clamp(pix_fmt->height,
+				(u32)MIN_HEIGHT, (u32)MAX_HEIGHT);
 
 	if (fmt->hw_format >= RGA_COLOR_FMT_YUV422SP)
-		f->fmt.pix.bytesperline = f->fmt.pix.width;
+		pix_fmt->bytesperline = pix_fmt->width;
 	else
-		f->fmt.pix.bytesperline = (f->fmt.pix.width * fmt->depth) >> 3;
+		pix_fmt->bytesperline = (pix_fmt->width * fmt->depth) >> 3;
 
-	f->fmt.pix.sizeimage =
-		f->fmt.pix.height * (f->fmt.pix.width * fmt->depth) >> 3;
+	pix_fmt->sizeimage =
+		pix_fmt->height * (pix_fmt->width * fmt->depth) >> 3;
 
 	return 0;
 }
 
 static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
+	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
 	struct rga_ctx *ctx = prv;
 	struct rockchip_rga *rga = ctx->rga;
 	struct vb2_queue *vq;
@@ -518,12 +521,12 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	frm = rga_get_frame(ctx, f->type);
 	if (IS_ERR(frm))
 		return PTR_ERR(frm);
-	frm->width = f->fmt.pix.width;
-	frm->height = f->fmt.pix.height;
-	frm->size = f->fmt.pix.sizeimage;
-	frm->fmt = rga_fmt_find(f->fmt.pix.pixelformat);
-	frm->stride = f->fmt.pix.bytesperline;
-	frm->colorspace = f->fmt.pix.colorspace;
+	frm->width = pix_fmt->width;
+	frm->height = pix_fmt->height;
+	frm->size = pix_fmt->sizeimage;
+	frm->fmt = rga_fmt_find(pix_fmt->pixelformat);
+	frm->stride = pix_fmt->bytesperline;
+	frm->colorspace = pix_fmt->colorspace;
 
 	/* Reset crop settings */
 	frm->crop.left = 0;
@@ -531,6 +534,12 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	frm->crop.width = frm->width;
 	frm->crop.height = frm->height;
 
+	v4l2_dbg(debug, 1, &rga->v4l2_dev,
+		 "[%s] fmt - %p4cc %dx%d (stride %d, sizeimage %d)\n",
+		  V4L2_TYPE_IS_OUTPUT(f->type) ? "OUTPUT" : "CAPTURE",
+		  &frm->fmt->fourcc, frm->width, frm->height,
+		  frm->stride, frm->size);
+
 	return 0;
 }
 

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 10/13] media: rockchip: rga: use macros for testing buffer type

[Michael Tretter]

Use the provided V4L2_TYPE_IS_{OUTPUT,CAPTURE} macros to check if the
buffer or queue is OUTPUT or CAPTURE. The macros work also work for the
_MPLANE buffer and queue types and make it easier to switch to the
multi-planar API.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 303ee762bec4..516069d720fa 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -342,14 +342,11 @@ static struct rga_frame def_frame = {
 
 struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type)
 {
-	switch (type) {
-	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+	if (V4L2_TYPE_IS_OUTPUT(type))
 		return &ctx->in;
-	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+	if (V4L2_TYPE_IS_CAPTURE(type))
 		return &ctx->out;
-	default:
-		return ERR_PTR(-EINVAL);
-	}
+	return ERR_PTR(-EINVAL);
 }
 
 static int rga_open(struct file *file)
@@ -557,21 +554,21 @@ static int vidioc_g_selection(struct file *file, void *prv,
 	switch (s->target) {
 	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
 	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
-		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		if (!V4L2_TYPE_IS_CAPTURE(s->type))
 			return -EINVAL;
 		break;
 	case V4L2_SEL_TGT_CROP_DEFAULT:
 	case V4L2_SEL_TGT_CROP_BOUNDS:
-		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+		if (!V4L2_TYPE_IS_OUTPUT(s->type))
 			return -EINVAL;
 		break;
 	case V4L2_SEL_TGT_COMPOSE:
-		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		if (!V4L2_TYPE_IS_CAPTURE(s->type))
 			return -EINVAL;
 		use_frame = true;
 		break;
 	case V4L2_SEL_TGT_CROP:
-		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+		if (!V4L2_TYPE_IS_OUTPUT(s->type))
 			return -EINVAL;
 		use_frame = true;
 		break;
@@ -609,7 +606,7 @@ static int vidioc_s_selection(struct file *file, void *prv,
 		 * COMPOSE target is only valid for capture buffer type, return
 		 * error for output buffer type
 		 */
-		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		if (!V4L2_TYPE_IS_CAPTURE(s->type))
 			return -EINVAL;
 		break;
 	case V4L2_SEL_TGT_CROP:
@@ -617,7 +614,7 @@ static int vidioc_s_selection(struct file *file, void *prv,
 		 * CROP target is only valid for output buffer type, return
 		 * error for capture buffer type
 		 */
-		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+		if (!V4L2_TYPE_IS_OUTPUT(s->type))
 			return -EINVAL;
 		break;
 	/*

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 11/13] media: rockchip: rga: switch to multi-planar API

[Michael Tretter]

Switch to the multi-planar API, which allows to handle buffers with
separate planes.

The RGA driver doesn't expose multi-planar formats, yet. The existing
contiguous planar formats can be used with the multi-planar API as well,
but the multi-planar API is required for multi-planar formats.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 60 +++++++++++++++----------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 516069d720fa..db2160407b83 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -93,7 +93,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	struct rga_ctx *ctx = priv;
 	int ret;
 
-	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
 	src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
 	src_vq->drv_priv = ctx;
 	src_vq->ops = &rga_qops;
@@ -107,7 +107,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	if (ret)
 		return ret;
 
-	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
 	dst_vq->drv_priv = ctx;
 	dst_vq->ops = &rga_qops;
@@ -443,7 +443,7 @@ static int vidioc_enum_fmt(struct file *file, void *prv, struct v4l2_fmtdesc *f)
 
 static int vidioc_g_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
-	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
+	struct v4l2_pix_format_mplane *pix_fmt = &f->fmt.pix_mp;
 	struct rga_ctx *ctx = prv;
 	struct vb2_queue *vq;
 	struct rga_frame *frm;
@@ -455,12 +455,9 @@ static int vidioc_g_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	if (IS_ERR(frm))
 		return PTR_ERR(frm);
 
-	pix_fmt->width = frm->width;
-	pix_fmt->height = frm->height;
+	v4l2_fill_pixfmt_mp(pix_fmt, frm->fmt->fourcc, frm->width, frm->height);
+
 	pix_fmt->field = V4L2_FIELD_NONE;
-	pix_fmt->pixelformat = frm->fmt->fourcc;
-	pix_fmt->bytesperline = frm->stride;
-	pix_fmt->sizeimage = frm->size;
 	pix_fmt->colorspace = frm->colorspace;
 
 	return 0;
@@ -468,41 +465,33 @@ static int vidioc_g_fmt(struct file *file, void *prv, struct v4l2_format *f)
 
 static int vidioc_try_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
-	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
+	struct v4l2_pix_format_mplane *pix_fmt = &f->fmt.pix_mp;
 	struct rga_fmt *fmt;
 
-	fmt = rga_fmt_find(f->fmt.pix.pixelformat);
+	fmt = rga_fmt_find(pix_fmt->pixelformat);
 	if (!fmt)
 		fmt = &formats[0];
 
-	pix_fmt->pixelformat = fmt->fourcc;
-
-	pix_fmt->field = V4L2_FIELD_NONE;
-
 	pix_fmt->width = clamp(pix_fmt->width,
 			       (u32)MIN_WIDTH, (u32)MAX_WIDTH);
 	pix_fmt->height = clamp(pix_fmt->height,
 				(u32)MIN_HEIGHT, (u32)MAX_HEIGHT);
 
-	if (fmt->hw_format >= RGA_COLOR_FMT_YUV422SP)
-		pix_fmt->bytesperline = pix_fmt->width;
-	else
-		pix_fmt->bytesperline = (pix_fmt->width * fmt->depth) >> 3;
-
-	pix_fmt->sizeimage =
-		pix_fmt->height * (pix_fmt->width * fmt->depth) >> 3;
+	v4l2_fill_pixfmt_mp(pix_fmt, fmt->fourcc, pix_fmt->width, pix_fmt->height);
+	pix_fmt->field = V4L2_FIELD_NONE;
 
 	return 0;
 }
 
 static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 {
-	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
+	struct v4l2_pix_format_mplane *pix_fmt = &f->fmt.pix_mp;
 	struct rga_ctx *ctx = prv;
 	struct rockchip_rga *rga = ctx->rga;
 	struct vb2_queue *vq;
 	struct rga_frame *frm;
 	int ret = 0;
+	int i;
 
 	/* Adjust all values accordingly to the hardware capabilities
 	 * and chosen format.
@@ -520,9 +509,11 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 		return PTR_ERR(frm);
 	frm->width = pix_fmt->width;
 	frm->height = pix_fmt->height;
-	frm->size = pix_fmt->sizeimage;
+	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(pix_fmt->pixelformat);
-	frm->stride = pix_fmt->bytesperline;
+	frm->stride = pix_fmt->plane_fmt[0].bytesperline;
 	frm->colorspace = pix_fmt->colorspace;
 
 	/* Reset crop settings */
@@ -537,6 +528,13 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 		  &frm->fmt->fourcc, frm->width, frm->height,
 		  frm->stride, frm->size);
 
+	for (i = 0; i < pix_fmt->num_planes; i++) {
+		v4l2_dbg(debug, 1, &rga->v4l2_dev,
+			 "plane[%d]: size %d, bytesperline %d\n",
+			 i, pix_fmt->plane_fmt[i].sizeimage,
+			 pix_fmt->plane_fmt[i].bytesperline);
+	}
+
 	return 0;
 }
 
@@ -647,14 +645,14 @@ static const struct v4l2_ioctl_ops rga_ioctl_ops = {
 	.vidioc_querycap = vidioc_querycap,
 
 	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt,
-	.vidioc_g_fmt_vid_cap = vidioc_g_fmt,
-	.vidioc_try_fmt_vid_cap = vidioc_try_fmt,
-	.vidioc_s_fmt_vid_cap = vidioc_s_fmt,
+	.vidioc_g_fmt_vid_cap_mplane = vidioc_g_fmt,
+	.vidioc_try_fmt_vid_cap_mplane = vidioc_try_fmt,
+	.vidioc_s_fmt_vid_cap_mplane = vidioc_s_fmt,
 
 	.vidioc_enum_fmt_vid_out = vidioc_enum_fmt,
-	.vidioc_g_fmt_vid_out = vidioc_g_fmt,
-	.vidioc_try_fmt_vid_out = vidioc_try_fmt,
-	.vidioc_s_fmt_vid_out = vidioc_s_fmt,
+	.vidioc_g_fmt_vid_out_mplane = vidioc_g_fmt,
+	.vidioc_try_fmt_vid_out_mplane = vidioc_try_fmt,
+	.vidioc_s_fmt_vid_out_mplane = vidioc_s_fmt,
 
 	.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
 	.vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
@@ -681,7 +679,7 @@ static const struct video_device rga_videodev = {
 	.minor = -1,
 	.release = video_device_release,
 	.vfl_dir = VFL_DIR_M2M,
-	.device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING,
+	.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING,
 };
 
 static int rga_enable_clocks(struct rockchip_rga *rga)

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 12/13] media: rockchip: rga: rework buffer handling for multi-planar formats

[Michael Tretter]

Multi-planar formats may have multiple planes that must be handled and
correctly mapped into a continuous buffer for the RGA by using the DMA
descriptors.

The plane offsets in the continuous mapping may now start at page
boundaries and the previous calculation based on the frame sizes is only
valid for planar buffers in a single memory. Therefore, the offsets must
be detected and set while creating the mapping.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga-buf.c | 60 ++++++++++++++++++++-------
 drivers/media/platform/rockchip/rga/rga.c     |  7 ++++
 drivers/media/platform/rockchip/rga/rga.h     |  1 +
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index 137f4f4be14c..8eb89b28ba48 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -7,6 +7,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/scatterlist.h>
 
+#include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-mem2mem.h>
@@ -42,15 +43,29 @@ rga_queue_setup(struct vb2_queue *vq,
 {
 	struct rga_ctx *ctx = vb2_get_drv_priv(vq);
 	struct rga_frame *f = rga_get_frame(ctx, vq->type);
+	const struct v4l2_pix_format_mplane *pix_fmt;
+	int i;
 
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
-	if (*nplanes)
-		return sizes[0] < f->size ? -EINVAL : 0;
+	pix_fmt = &f->pix;
 
-	sizes[0] = f->size;
-	*nplanes = 1;
+	if (*nplanes) {
+		if (*nplanes != pix_fmt->num_planes)
+			return -EINVAL;
+
+		for (i = 0; i < pix_fmt->num_planes; i++)
+			if (sizes[i] < pix_fmt->plane_fmt[i].sizeimage)
+				return -EINVAL;
+
+		return 0;
+	}
+
+	*nplanes = pix_fmt->num_planes;
+
+	for (i = 0; i < pix_fmt->num_planes; i++)
+		sizes[i] = pix_fmt->plane_fmt[i].sizeimage;
 
 	return 0;
 }
@@ -95,23 +110,38 @@ static int rga_buf_prepare(struct vb2_buffer *vb)
 	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
 	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
 	struct rockchip_rga *rga = ctx->rga;
-	int n_desc;
+	int n_desc = 0;
+	int i;
+	const struct v4l2_format_info *info;
+	unsigned int offsets[VIDEO_MAX_PLANES];
 
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
-	vb2_set_plane_payload(vb, 0, f->size);
+	for (i = 0; i < vb->num_planes; i++) {
+		unsigned int ret;
+
+		vb2_set_plane_payload(vb, i, f->pix.plane_fmt[i].sizeimage);
 
-	/* Create local MMU table for RGA */
-	n_desc = fill_descriptors(rbuf->dma_desc,
-				  vb2_dma_sg_plane_desc(vb, 0));
-	if (n_desc < 0) {
-		dev_err(rga->dev, "Failed to map buffer");
-		return n_desc;
+		/* Create local MMU table for RGA */
+		ret = fill_descriptors(&rbuf->dma_desc[n_desc],
+				       vb2_dma_sg_plane_desc(vb, i));
+		if (ret < 0) {
+			dev_err(rga->dev, "Failed to map buffer");
+			return ret;
+		}
+		offsets[i] = n_desc << PAGE_SHIFT;
+		n_desc += ret;
 	}
-	rbuf->offset.y_off = get_plane_offset(f, 0);
-	rbuf->offset.u_off = get_plane_offset(f, 1);
-	rbuf->offset.v_off = get_plane_offset(f, 2);
+
+	/* Fill the remaining planes */
+	info = v4l2_format_info(f->fmt->fourcc);
+	for (i = info->mem_planes; i < info->comp_planes; i++)
+		offsets[i] = get_plane_offset(f, i);
+
+	rbuf->offset.y_off = offsets[0];
+	rbuf->offset.u_off = offsets[1];
+	rbuf->offset.v_off = offsets[2];
 
 	/* sync local MMU table for RGA */
 	dma_sync_single_for_device(rga->dev, rbuf->dma_desc_pa,
diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index db2160407b83..e0324341e702 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -363,6 +363,11 @@ static int rga_open(struct file *file)
 	ctx->in = def_frame;
 	ctx->out = def_frame;
 
+	v4l2_fill_pixfmt_mp(&ctx->in.pix,
+			    ctx->in.fmt->fourcc, ctx->out.width, ctx->out.height);
+	v4l2_fill_pixfmt_mp(&ctx->out.pix,
+			    ctx->out.fmt->fourcc, ctx->out.width, ctx->out.height);
+
 	if (mutex_lock_interruptible(&rga->mutex)) {
 		kfree(ctx);
 		return -ERESTARTSYS;
@@ -522,6 +527,8 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	frm->crop.width = frm->width;
 	frm->crop.height = frm->height;
 
+	frm->pix = *pix_fmt;
+
 	v4l2_dbg(debug, 1, &rga->v4l2_dev,
 		 "[%s] fmt - %p4cc %dx%d (stride %d, sizeimage %d)\n",
 		  V4L2_TYPE_IS_OUTPUT(f->type) ? "OUTPUT" : "CAPTURE",
diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
index d8e76ab9c7e4..b279483aecf8 100644
--- a/drivers/media/platform/rockchip/rga/rga.h
+++ b/drivers/media/platform/rockchip/rga/rga.h
@@ -34,6 +34,7 @@ struct rga_frame {
 
 	/* Image format */
 	struct rga_fmt *fmt;
+	struct v4l2_pix_format_mplane pix;
 
 	/* Variables that can calculated once and reused */
 	u32 stride;

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 13/13] media: rockchip: rga: add NV12M support

[Michael Tretter]

Add support for the multi-planar variants of NV12. The RGA is now able
to exchange DMABUFs with other devices that only support multi-planar
NV12, for example the Hantro JPEG encoder.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index e0324341e702..a9b4d4bca733 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -278,6 +278,15 @@ static struct rga_fmt formats[] = {
 		.y_div = 2,
 		.x_div = 1,
 	},
+	{
+		.fourcc = V4L2_PIX_FMT_NV12M,
+		.color_swap = RGA_COLOR_NONE_SWAP,
+		.hw_format = RGA_COLOR_FMT_YUV420SP,
+		.depth = 12,
+		.uv_factor = 4,
+		.y_div = 2,
+		.x_div = 1,
+	},
 	{
 		.fourcc = V4L2_PIX_FMT_NV16,
 		.color_swap = RGA_COLOR_NONE_SWAP,

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 05/13] media: rockchip: rga: pre-calculate plane offsets

[kernel test robot]

Hi Michael,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 0bb80ecc33a8fb5a682236443c1e740d5c917d1d]

url:    https://github.com/intel-lab-lkp/linux/commits/Michael-Tretter/media-rockchip-rga-fix-swizzling-for-RGB-formats/20230914-204330
base:   0bb80ecc33a8fb5a682236443c1e740d5c917d1d
patch link:    https://lore.kernel.org/r/20230914-rockchip-rga-multiplanar-v1-5-abfd77260ae3%40pengutronix.de
patch subject: [PATCH 05/13] media: rockchip: rga: pre-calculate plane offsets
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230914/202309142156.JJwE4Eke-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309142156.JJwE4Eke-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309142156.JJwE4Eke-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/media/platform/rockchip/rga/rga-hw.c: In function 'rga_get_addr_offset':
>> drivers/media/platform/rockchip/rga/rga-hw.c:46:65: warning: variable 'uv_factor' set but not used [-Wunused-but-set-variable]
      46 |                      y_div = 0, uv_stride = 0, pixel_width = 0, uv_factor = 0;
         |                                                                 ^~~~~~~~~


vim +/uv_factor +46 drivers/media/platform/rockchip/rga/rga-hw.c

f7e7b48e6d796d Jacob Chen      2017-10-11  38  
f7e7b48e6d796d Jacob Chen      2017-10-11  39  static struct rga_corners_addr_offset
7c2b289b29ea42 Michael Tretter 2023-09-14  40  rga_get_addr_offset(struct rga_frame *frm, struct rga_addr_offset *offset,
7c2b289b29ea42 Michael Tretter 2023-09-14  41  		    unsigned int x, unsigned int y, unsigned int w, unsigned int h)
f7e7b48e6d796d Jacob Chen      2017-10-11  42  {
f7e7b48e6d796d Jacob Chen      2017-10-11  43  	struct rga_corners_addr_offset offsets;
f7e7b48e6d796d Jacob Chen      2017-10-11  44  	struct rga_addr_offset *lt, *lb, *rt, *rb;
f7e7b48e6d796d Jacob Chen      2017-10-11  45  	unsigned int x_div = 0,
f7e7b48e6d796d Jacob Chen      2017-10-11 @46  		     y_div = 0, uv_stride = 0, pixel_width = 0, uv_factor = 0;
f7e7b48e6d796d Jacob Chen      2017-10-11  47  
f7e7b48e6d796d Jacob Chen      2017-10-11  48  	lt = &offsets.left_top;
f7e7b48e6d796d Jacob Chen      2017-10-11  49  	lb = &offsets.left_bottom;
f7e7b48e6d796d Jacob Chen      2017-10-11  50  	rt = &offsets.right_top;
f7e7b48e6d796d Jacob Chen      2017-10-11  51  	rb = &offsets.right_bottom;
f7e7b48e6d796d Jacob Chen      2017-10-11  52  
f7e7b48e6d796d Jacob Chen      2017-10-11  53  	x_div = frm->fmt->x_div;
f7e7b48e6d796d Jacob Chen      2017-10-11  54  	y_div = frm->fmt->y_div;
f7e7b48e6d796d Jacob Chen      2017-10-11  55  	uv_factor = frm->fmt->uv_factor;
f7e7b48e6d796d Jacob Chen      2017-10-11  56  	uv_stride = frm->stride / x_div;
f7e7b48e6d796d Jacob Chen      2017-10-11  57  	pixel_width = frm->stride / frm->width;
f7e7b48e6d796d Jacob Chen      2017-10-11  58  
7c2b289b29ea42 Michael Tretter 2023-09-14  59  	lt->y_off = offset->y_off + y * frm->stride + x * pixel_width;
7c2b289b29ea42 Michael Tretter 2023-09-14  60  	lt->u_off = offset->u_off + (y / y_div) * uv_stride + x / x_div;
7c2b289b29ea42 Michael Tretter 2023-09-14  61  	lt->v_off = offset->v_off + (y / y_div) * uv_stride + x / x_div;
f7e7b48e6d796d Jacob Chen      2017-10-11  62  
f7e7b48e6d796d Jacob Chen      2017-10-11  63  	lb->y_off = lt->y_off + (h - 1) * frm->stride;
f7e7b48e6d796d Jacob Chen      2017-10-11  64  	lb->u_off = lt->u_off + (h / y_div - 1) * uv_stride;
f7e7b48e6d796d Jacob Chen      2017-10-11  65  	lb->v_off = lt->v_off + (h / y_div - 1) * uv_stride;
f7e7b48e6d796d Jacob Chen      2017-10-11  66  
f7e7b48e6d796d Jacob Chen      2017-10-11  67  	rt->y_off = lt->y_off + (w - 1) * pixel_width;
f7e7b48e6d796d Jacob Chen      2017-10-11  68  	rt->u_off = lt->u_off + w / x_div - 1;
f7e7b48e6d796d Jacob Chen      2017-10-11  69  	rt->v_off = lt->v_off + w / x_div - 1;
f7e7b48e6d796d Jacob Chen      2017-10-11  70  
f7e7b48e6d796d Jacob Chen      2017-10-11  71  	rb->y_off = lb->y_off + (w - 1) * pixel_width;
f7e7b48e6d796d Jacob Chen      2017-10-11  72  	rb->u_off = lb->u_off + w / x_div - 1;
f7e7b48e6d796d Jacob Chen      2017-10-11  73  	rb->v_off = lb->v_off + w / x_div - 1;
f7e7b48e6d796d Jacob Chen      2017-10-11  74  
f7e7b48e6d796d Jacob Chen      2017-10-11  75  	return offsets;
f7e7b48e6d796d Jacob Chen      2017-10-11  76  }
f7e7b48e6d796d Jacob Chen      2017-10-11  77  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/13] media: rockchip: rga: set dma mask to 32 bits

[Robin Murphy]

On 2023-09-14 13:40, Michael Tretter wrote:
> The RGA DMA descriptor list contains only 32-bit addresses. Set the
> dma_mask to only allocate memory that is addressable by the descriptors.
> 
> This prevents errors when preparing vb2 buffers that were allocated by
> the RGA. Imported buffers may still fail the preparation, as they may be
> allocated above the 4 GB boundary.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>   drivers/media/platform/rockchip/rga/rga.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index f18fccc7b204..149deb1f1e03 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -824,6 +824,12 @@ static int rga_probe(struct platform_device *pdev)
>   		goto err_put_clk;
>   	}
>   
> +	ret = dma_set_mask(rga->dev, DMA_BIT_MASK(32));

You want dma_set_mask_and_coherent(), given that you are using coherent 
allocations as well. The driver's getting away with it since the default 
masks happen to be 32-bit anyway, but it's better to be explicit.

Thanks,
Robin.

> +	if (ret) {
> +		dev_err(rga->dev, "32-bit DMA not supported");
> +		goto err_put_clk;
> +	}
> +
>   	ret = v4l2_device_register(&pdev->dev, &rga->v4l2_dev);
>   	if (ret)
>   		goto err_put_clk;
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Robin Murphy]

On 2023-09-14 13: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.

That sounds unnecessary, since the only DMA addresses the RGA should be 
seeing are those from a dma_map_sg() call using the RGA device itself, 
and that would have failed if it was unable to provide a valid DMA 
address for the device.

The existing rga_buf_map() code is so clearly wrong I can't tell whether 
that mapping is done somewhere out in the core framework or whether the 
driver's supposed to be doing it for itself.

Thanks,
Robin.

> 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@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;
>   };
>   
>   struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type);
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 03/13] media: rockchip: rga: allocate DMA descriptors per buffer

[Robin Murphy]

On 2023-09-14 13:40, Michael Tretter wrote:
> The RGA driver allocates two buffers for the DMA descriptors of the
> input and output buffers. Whenever a new job is processed, the
> descriptor list is updated for the current buffers.
> 
> By updating the descriptor list during buf_prepare, it is possible to
> correctly fail DMABUF imports if the buffers that shall be imported are
> not within the 32 bit address range that can be addressed by the RGA.
> 
> Managing the DMA descriptor list with the buffer also makes it easier to
> track the buffer mapping and the plane offsets into this mapping.
> 
> The cost is that the driver now requires DMA coherent memory per buffer
> for the descriptor list. However, the size scales with the size of the
> video buffers and is not allocated if the RGA is not used.
> 
> While at it, use dma_alloc_coherent to allocate the descriptors and get
> rid of the virt_to_phys calls to get the physical addresses.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>   drivers/media/platform/rockchip/rga/rga-buf.c | 78 +++++++++++++++++----------
>   drivers/media/platform/rockchip/rga/rga-hw.c  | 26 ++++-----
>   drivers/media/platform/rockchip/rga/rga.c     | 31 ++---------
>   drivers/media/platform/rockchip/rga/rga.h     | 21 ++++++--
>   4 files changed, 86 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index df5ebc90e32d..e8dcc0d5cb90 100644
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> @@ -55,16 +55,54 @@ rga_queue_setup(struct vb2_queue *vq,
>   	return 0;
>   }
>   
> +static int rga_buf_init(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct rga_vb_buffer *rbuf = vb_to_rga(vbuf);
> +	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +	struct rockchip_rga *rga = ctx->rga;
> +	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
> +	int n_desc = 0;
> +
> +	n_desc = DIV_ROUND_UP(f->size, PAGE_SIZE);
> +
> +	rbuf->n_desc = n_desc;
> +	rbuf->dma_desc = dma_alloc_coherent(rga->dev,
> +					    rbuf->n_desc * sizeof(*rbuf->dma_desc),
> +					    &rbuf->dma_desc_pa, GFP_KERNEL);
> +	if (!rbuf->dma_desc)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
>   static int rga_buf_prepare(struct vb2_buffer *vb)
>   {
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct rga_vb_buffer *rbuf = vb_to_rga(vbuf);
>   	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
>   	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
> +	struct rockchip_rga *rga = ctx->rga;
> +	int n_desc;
>   
>   	if (IS_ERR(f))
>   		return PTR_ERR(f);
>   
>   	vb2_set_plane_payload(vb, 0, f->size);
>   
> +	/* Create local MMU table for RGA */
> +	n_desc = fill_descriptors(rbuf->dma_desc,
> +				  vb2_dma_sg_plane_desc(vb, 0));
> +	if (n_desc < 0) {
> +		dev_err(rga->dev, "Failed to map buffer");
> +		return n_desc;
> +	}
> +
> +	/* sync local MMU table for RGA */
> +	dma_sync_single_for_device(rga->dev, rbuf->dma_desc_pa,
> +				   n_desc * sizeof(*rbuf->dma_desc),
> +				   DMA_BIDIRECTIONAL);

This sync is still wrong (and DMA_API_DEBUG should complain), but also 
now pointless if you're using a coherent buffer (as is the one in 
rga_cmd_set(), seemingly)

Thanks,
Robin.

> +
>   	return 0;
>   }
>   
> @@ -76,6 +114,17 @@ static void rga_buf_queue(struct vb2_buffer *vb)
>   	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
>   }
>   
> +static void rga_buf_cleanup(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct rga_vb_buffer *rbuf = vb_to_rga(vbuf);
> +	struct rga_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +	struct rockchip_rga *rga = ctx->rga;
> +
> +	dma_free_coherent(rga->dev, rbuf->n_desc * sizeof(*rbuf->dma_desc),
> +			  rbuf->dma_desc, rbuf->dma_desc_pa);
> +}
> +
>   static void rga_buf_return_buffers(struct vb2_queue *q,
>   				   enum vb2_buffer_state state)
>   {
> @@ -119,37 +168,12 @@ static void rga_buf_stop_streaming(struct vb2_queue *q)
>   
>   const struct vb2_ops rga_qops = {
>   	.queue_setup = rga_queue_setup,
> +	.buf_init = rga_buf_init,
>   	.buf_prepare = rga_buf_prepare,
>   	.buf_queue = rga_buf_queue,
> +	.buf_cleanup = rga_buf_cleanup,
>   	.wait_prepare = vb2_ops_wait_prepare,
>   	.wait_finish = vb2_ops_wait_finish,
>   	.start_streaming = rga_buf_start_streaming,
>   	.stop_streaming = rga_buf_stop_streaming,
>   };
> -
> -/* RGA MMU is a 1-Level MMU, so it can't be used through the IOMMU API.
> - * We use it more like a scatter-gather list.
> - */
> -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 rga_dma_desc *pages;
> -	unsigned int num_desc = 0;
> -
> -	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> -		pages = rga->src_mmu_pages;
> -	else
> -		pages = rga->dst_mmu_pages;
> -
> -	/* Create local MMU table for RGA */
> -	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),
> -				   num_desc * sizeof(*pages), DMA_BIDIRECTIONAL);
> -}
> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
> index aaa96f256356..b391d97d4632 100644
> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
> @@ -119,40 +119,40 @@ static struct rga_addr_offset *rga_lookup_draw_pos(struct
>   	return NULL;
>   }
>   
> -static void rga_cmd_set_src_addr(struct rga_ctx *ctx, void *mmu_pages)
> +static void rga_cmd_set_src_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
>   {
>   	struct rockchip_rga *rga = ctx->rga;
>   	u32 *dest = rga->cmdbuf_virt;
>   	unsigned int reg;
>   
>   	reg = RGA_MMU_SRC_BASE - RGA_MODE_BASE_REG;
> -	dest[reg >> 2] = virt_to_phys(mmu_pages) >> 4;
> +	dest[reg >> 2] = dma_addr >> 4;
>   
>   	reg = RGA_MMU_CTRL1 - RGA_MODE_BASE_REG;
>   	dest[reg >> 2] |= 0x7;
>   }
>   
> -static void rga_cmd_set_src1_addr(struct rga_ctx *ctx, void *mmu_pages)
> +static void rga_cmd_set_src1_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
>   {
>   	struct rockchip_rga *rga = ctx->rga;
>   	u32 *dest = rga->cmdbuf_virt;
>   	unsigned int reg;
>   
>   	reg = RGA_MMU_SRC1_BASE - RGA_MODE_BASE_REG;
> -	dest[reg >> 2] = virt_to_phys(mmu_pages) >> 4;
> +	dest[reg >> 2] = dma_addr >> 4;
>   
>   	reg = RGA_MMU_CTRL1 - RGA_MODE_BASE_REG;
>   	dest[reg >> 2] |= 0x7 << 4;
>   }
>   
> -static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, void *mmu_pages)
> +static void rga_cmd_set_dst_addr(struct rga_ctx *ctx, dma_addr_t dma_addr)
>   {
>   	struct rockchip_rga *rga = ctx->rga;
>   	u32 *dest = rga->cmdbuf_virt;
>   	unsigned int reg;
>   
>   	reg = RGA_MMU_DST_BASE - RGA_MODE_BASE_REG;
> -	dest[reg >> 2] = virt_to_phys(mmu_pages) >> 4;
> +	dest[reg >> 2] = dma_addr >> 4;
>   
>   	reg = RGA_MMU_CTRL1 - RGA_MODE_BASE_REG;
>   	dest[reg >> 2] |= 0x7 << 8;
> @@ -375,20 +375,21 @@ static void rga_cmd_set_mode(struct rga_ctx *ctx)
>   	dest[(RGA_MODE_CTRL - RGA_MODE_BASE_REG) >> 2] = mode.val;
>   }
>   
> -static void rga_cmd_set(struct rga_ctx *ctx)
> +static void rga_cmd_set(struct rga_ctx *ctx,
> +			struct rga_vb_buffer *src, struct rga_vb_buffer *dst)
>   {
>   	struct rockchip_rga *rga = ctx->rga;
>   
>   	memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE * 4);
>   
> -	rga_cmd_set_src_addr(ctx, rga->src_mmu_pages);
> +	rga_cmd_set_src_addr(ctx, src->dma_desc_pa);
>   	/*
>   	 * Due to hardware bug,
>   	 * src1 mmu also should be configured when using alpha blending.
>   	 */
> -	rga_cmd_set_src1_addr(ctx, rga->dst_mmu_pages);
> +	rga_cmd_set_src1_addr(ctx, dst->dma_desc_pa);
>   
> -	rga_cmd_set_dst_addr(ctx, rga->dst_mmu_pages);
> +	rga_cmd_set_dst_addr(ctx, dst->dma_desc_pa);
>   	rga_cmd_set_mode(ctx);
>   
>   	rga_cmd_set_trans_info(ctx);
> @@ -400,11 +401,12 @@ static void rga_cmd_set(struct rga_ctx *ctx)
>   		PAGE_SIZE, DMA_BIDIRECTIONAL);
>   }
>   
> -void rga_hw_start(struct rockchip_rga *rga)
> +void rga_hw_start(struct rockchip_rga *rga,
> +		  struct rga_vb_buffer *src, struct rga_vb_buffer *dst)
>   {
>   	struct rga_ctx *ctx = rga->curr;
>   
> -	rga_cmd_set(ctx);
> +	rga_cmd_set(ctx, src, dst);
>   
>   	rga_write(rga, RGA_SYS_CTRL, 0x00);
>   
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index 25f5b5eebf13..f18fccc7b204 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -45,10 +45,7 @@ static void device_run(void *prv)
>   	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>   	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>   
> -	rga_buf_map(&src->vb2_buf);
> -	rga_buf_map(&dst->vb2_buf);
> -
> -	rga_hw_start(rga);
> +	rga_hw_start(rga, vb_to_rga(src), vb_to_rga(dst));
>   
>   	spin_unlock_irqrestore(&rga->ctrl_lock, flags);
>   }
> @@ -101,7 +98,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>   	src_vq->drv_priv = ctx;
>   	src_vq->ops = &rga_qops;
>   	src_vq->mem_ops = &vb2_dma_sg_memops;
> -	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	src_vq->buf_struct_size = sizeof(struct rga_vb_buffer);
>   	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>   	src_vq->lock = &ctx->rga->mutex;
>   	src_vq->dev = ctx->rga->v4l2_dev.dev;
> @@ -115,7 +112,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>   	dst_vq->drv_priv = ctx;
>   	dst_vq->ops = &rga_qops;
>   	dst_vq->mem_ops = &vb2_dma_sg_memops;
> -	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	dst_vq->buf_struct_size = sizeof(struct rga_vb_buffer);
>   	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>   	dst_vq->lock = &ctx->rga->mutex;
>   	dst_vq->dev = ctx->rga->v4l2_dev.dev;
> @@ -872,26 +869,13 @@ static int rga_probe(struct platform_device *pdev)
>   		goto rel_m2m;
>   	}
>   
> -	rga->src_mmu_pages =
> -		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
> -	if (!rga->src_mmu_pages) {
> -		ret = -ENOMEM;
> -		goto free_dma;
> -	}
> -	rga->dst_mmu_pages =
> -		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
> -	if (!rga->dst_mmu_pages) {
> -		ret = -ENOMEM;
> -		goto free_src_pages;
> -	}
> -
>   	def_frame.stride = (def_frame.width * def_frame.fmt->depth) >> 3;
>   	def_frame.size = def_frame.stride * def_frame.height;
>   
>   	ret = video_register_device(vfd, VFL_TYPE_VIDEO, -1);
>   	if (ret) {
>   		v4l2_err(&rga->v4l2_dev, "Failed to register video device\n");
> -		goto free_dst_pages;
> +		goto free_dma;
>   	}
>   
>   	v4l2_info(&rga->v4l2_dev, "Registered %s as /dev/%s\n",
> @@ -899,10 +883,6 @@ static int rga_probe(struct platform_device *pdev)
>   
>   	return 0;
>   
> -free_dst_pages:
> -	free_pages((unsigned long)rga->dst_mmu_pages, 3);
> -free_src_pages:
> -	free_pages((unsigned long)rga->src_mmu_pages, 3);
>   free_dma:
>   	dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
>   		       rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
> @@ -925,9 +905,6 @@ static void rga_remove(struct platform_device *pdev)
>   	dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
>   		       rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
>   
> -	free_pages((unsigned long)rga->src_mmu_pages, 3);
> -	free_pages((unsigned long)rga->dst_mmu_pages, 3);
> -
>   	v4l2_info(&rga->v4l2_dev, "Removing\n");
>   
>   	v4l2_m2m_release(rga->m2m_dev);
> diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
> index 22f7da28ac51..ae984d5a236d 100644
> --- a/drivers/media/platform/rockchip/rga/rga.h
> +++ b/drivers/media/platform/rockchip/rga/rga.h
> @@ -85,15 +85,27 @@ struct rockchip_rga {
>   	struct rga_ctx *curr;
>   	dma_addr_t cmdbuf_phy;
>   	void *cmdbuf_virt;
> -	struct rga_dma_desc *src_mmu_pages;
> -	struct rga_dma_desc *dst_mmu_pages;
>   };
>   
> +struct rga_vb_buffer {
> +	struct vb2_v4l2_buffer vb_buf;
> +	struct list_head queue;
> +
> +	/* RGA MMU mapping for this buffer */
> +	struct rga_dma_desc *dma_desc;
> +	dma_addr_t dma_desc_pa;
> +	int n_desc;
> +};
> +
> +static inline struct rga_vb_buffer *vb_to_rga(struct vb2_v4l2_buffer *vb)
> +{
> +	return container_of(vb, struct rga_vb_buffer, vb_buf);
> +}
> +
>   struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type);
>   
>   /* RGA Buffers Manage */
>   extern const struct vb2_ops rga_qops;
> -void rga_buf_map(struct vb2_buffer *vb);
>   
>   /* RGA Hardware */
>   static inline void rga_write(struct rockchip_rga *rga, u32 reg, u32 value)
> @@ -114,6 +126,7 @@ static inline void rga_mod(struct rockchip_rga *rga, u32 reg, u32 val, u32 mask)
>   	rga_write(rga, reg, temp);
>   };
>   
> -void rga_hw_start(struct rockchip_rga *rga);
> +void rga_hw_start(struct rockchip_rga *rga,
> +		  struct rga_vb_buffer *src, struct rga_vb_buffer *dst);
>   
>   #endif
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michael Tretter]

Hi Robin,

On Thu, 14 Sep 2023 16:06:27 +0100, Robin Murphy wrote:
> On 2023-09-14 13: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.
> 
> That sounds unnecessary, since the only DMA addresses the RGA should be
> seeing are those from a dma_map_sg() call using the RGA device itself, and
> that would have failed if it was unable to provide a valid DMA address for
> the device.
> 
> The existing rga_buf_map() code is so clearly wrong I can't tell whether
> that mapping is done somewhere out in the core framework or whether the
> driver's supposed to be doing it for itself.

The sg_table is filled by dma_map_sgtable in
drivers/media/common/videobuf2/videobuf2-dma-sg.c.

Do you suggest to just drop the check for the addresses or is there something
fundamentally wrong with filling the descriptor list in the driver? Can you
explain what exactly is wrong with this code and do you have a pointer how to
implement this correctly?

Thanks,

Michael

> 
> Thanks,
> Robin.
> 
> > 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@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;
> >   };
> >   struct rga_frame *rga_get_frame(struct rga_ctx *ctx, enum v4l2_buf_type type);
> > 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Robin Murphy]

On 2023-09-14 18:57, Michael Tretter wrote:
> Hi Robin,
> 
> On Thu, 14 Sep 2023 16:06:27 +0100, Robin Murphy wrote:
>> On 2023-09-14 13: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.
>>
>> That sounds unnecessary, since the only DMA addresses the RGA should be
>> seeing are those from a dma_map_sg() call using the RGA device itself, and
>> that would have failed if it was unable to provide a valid DMA address for
>> the device.
>>
>> The existing rga_buf_map() code is so clearly wrong I can't tell whether
>> that mapping is done somewhere out in the core framework or whether the
>> driver's supposed to be doing it for itself.
> 
> The sg_table is filled by dma_map_sgtable in
> drivers/media/common/videobuf2/videobuf2-dma-sg.c.

Ah, great - in that case all you should need to do here is fill out the 
DMA descriptors with the correct DMA address as you are doing. If 
buf->dev is the right thing and you've set your DMA masks correctly then 
you can rely on the DMA addresses being appropriate already, since it's 
vb2-dma-sg's job to get that right (which per another recent thread it 
will do, but could do better...)

> Do you suggest to just drop the check for the addresses or is there something
> fundamentally wrong with filling the descriptor list in the driver? Can you
> explain what exactly is wrong with this code and do you have a pointer how to
> implement this correctly?

The rest of your patch is frankly more correct than what it's replacing, 
you can just drop the redundant upper_32_bits() check :)

Cheers,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Hans Verkuil]

On 14/09/2023 22:33, Robin Murphy wrote:
> On 2023-09-14 18:57, Michael Tretter wrote:
>> Hi Robin,
>>
>> On Thu, 14 Sep 2023 16:06:27 +0100, Robin Murphy wrote:
>>> On 2023-09-14 13: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.
>>>
>>> That sounds unnecessary, since the only DMA addresses the RGA should be
>>> seeing are those from a dma_map_sg() call using the RGA device itself, and
>>> that would have failed if it was unable to provide a valid DMA address for
>>> the device.
>>>
>>> The existing rga_buf_map() code is so clearly wrong I can't tell whether
>>> that mapping is done somewhere out in the core framework or whether the
>>> driver's supposed to be doing it for itself.
>>
>> The sg_table is filled by dma_map_sgtable in
>> drivers/media/common/videobuf2/videobuf2-dma-sg.c.
> 
> Ah, great - in that case all you should need to do here is fill out the DMA descriptors with the correct DMA address as you are doing. If buf->dev is the right thing and you've set your DMA masks
> correctly then you can rely on the DMA addresses being appropriate already, since it's vb2-dma-sg's job to get that right (which per another recent thread it will do, but could do better...)

A lot of media drivers that require 32 bit DMA set the vb2_queue gfp_flags:

q->gfp_flags = __GFP_DMA32;

This is ORed in the vb2 framework when calling alloc_pages().

That said, that's only used for the V4L2_MEMORY_MMAP streaming model, i.e. when
the driver allocates the memory. For MEMORY_DMABUF this isn't used and you still
need the correct DMA mask for the device.

Since I do not see __GFP_DMA32, I suspect that allocating buffers in vb2 might
still choose buffers above 4 GB, which would be wrong for this driver.

Regards,

	Hans

> 
>> Do you suggest to just drop the check for the addresses or is there something
>> fundamentally wrong with filling the descriptor list in the driver? Can you
>> explain what exactly is wrong with this code and do you have a pointer how to
>> implement this correctly?
> 
> The rest of your patch is frankly more correct than what it's replacing, you can just drop the redundant upper_32_bits() check :)
> 
> Cheers,
> Robin.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [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@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);
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/13] media: rockchip: rga: add support for multi-planar formats

[Hans Verkuil]

On 14/09/2023 14:40, Michael Tretter wrote:
> This series adds support for the V4L2 multi-planar API to the Rockchip
> RGA driver. Once the RGA driver supports the multi-planar API, it is
> easier to share buffers with other V4L2 drivers that also support the
> multi-planar API and may not expose planar formats with contiguous
> planes.
> 
> With non-contiguous planes, the U and V planes may not start at the same
> offset as with the continuous planes. Therefore, the RGA driver cannot
> rely on its calculation of the plane offsets based on the format and
> frame size anymore, but must remember the offsets when it created the
> mapping. Therefore, I also reworked how the DMA mapping is handled.
> 
> As a bonus, the RGA driver should now work correctly on devices with
> more than 4 GB of memory. Video buffers should now be allocated within
> the 4 GB boundary and an import of buffers that have higher addresses
> into the driver should fail.
> 
> Patch 1 fixes the swizzling of RGA formats. While testing all formats of
> the driver on rk3568, I discovered that the color channels of the RGB
> formats are wrong when converting to NV12. I didn't test this on other
> SoCs with an RGA and I am not sure, if they behave differently regarding
> the color channels. Please report, if this breaks the color conversion
> on other SoCs, and I will make this SoC-specific.
> 
> Patches 2 to 6 are the rework the DMA descriptor handling for the RGA
> MMU. The patches clean up, how the driver uses the DMA API, and make the
> creation of the descriptor list more explicit. Furthermore, the driver
> is changed to keep the mapping per video buffer instead of using a
> single mapping that is updated with every buffer.
> 
> Patches 7 to 11 prepare the driver for the multi-planar API including a
> cleanup of the format handling in the buffer, and finally switch to the
> multi-planar API.
> 
> Patch 12 updates the code that creates the DMA-descriptor mapping to
> correctly handle buffers with multiple planes. The driver has to iterate
> all planes and make them the continuous for the RGA.
> 
> Patch 13 enables the NV12M format, which is the multi-planar variant of
> the NV12 format.

I get these smatch warnings for this series:

drivers/media/platform/rockchip/rga/rga-hw.c: In function 'rga_get_addr_offset':
drivers/media/platform/rockchip/rga/rga-hw.c:46:65: warning: variable 'uv_factor' set but not used [-Wunused-but-set-variable]
   46 |                      y_div = 0, uv_stride = 0, pixel_width = 0, uv_factor = 0;
      |                                                                 ^~~~~~~~~
drivers/media/platform/rockchip/rga/rga-buf.c:129 rga_buf_prepare() warn: unsigned 'ret' is never less than zero.

Regards,

	Hans

> 
> Michael
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> Michael Tretter (13):
>       media: rockchip: rga: fix swizzling for RGB formats
>       media: rockchip: rga: extract helper to fill descriptors
>       media: rockchip: rga: allocate DMA descriptors per buffer
>       media: rockchip: rga: split src and dst buffer setup
>       media: rockchip: rga: pre-calculate plane offsets
>       media: rockchip: rga: set dma mask to 32 bits
>       media: rockchip: rga: use clamp() to clamp size to limits
>       media: rockchip: rga: use pixelformat to find format
>       media: rockchip: rga: add local variable for pix_format
>       media: rockchip: rga: use macros for testing buffer type
>       media: rockchip: rga: switch to multi-planar API
>       media: rockchip: rga: rework buffer handling for multi-planar formats
>       media: rockchip: rga: add NV12M support
> 
>  drivers/media/platform/rockchip/rga/rga-buf.c | 166 ++++++++++++++++-------
>  drivers/media/platform/rockchip/rga/rga-hw.c  | 143 +++++++++++++-------
>  drivers/media/platform/rockchip/rga/rga.c     | 187 ++++++++++++--------------
>  drivers/media/platform/rockchip/rga/rga.h     |  35 ++++-
>  4 files changed, 329 insertions(+), 202 deletions(-)
> ---
> base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
> change-id: 20230914-rockchip-rga-multiplanar-0d7c79b1ba93
> 
> Best regards,


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/13] media: rockchip: rga: add support for multi-planar formats

[Michael Tretter]

On Mon, 25 Sep 2023 09:45:10 +0200, Hans Verkuil wrote:
> On 14/09/2023 14:40, Michael Tretter wrote:
> > This series adds support for the V4L2 multi-planar API to the Rockchip
> > RGA driver. Once the RGA driver supports the multi-planar API, it is
> > easier to share buffers with other V4L2 drivers that also support the
> > multi-planar API and may not expose planar formats with contiguous
> > planes.
> > 
> > With non-contiguous planes, the U and V planes may not start at the same
> > offset as with the continuous planes. Therefore, the RGA driver cannot
> > rely on its calculation of the plane offsets based on the format and
> > frame size anymore, but must remember the offsets when it created the
> > mapping. Therefore, I also reworked how the DMA mapping is handled.
> > 
> > As a bonus, the RGA driver should now work correctly on devices with
> > more than 4 GB of memory. Video buffers should now be allocated within
> > the 4 GB boundary and an import of buffers that have higher addresses
> > into the driver should fail.
> > 
> > Patch 1 fixes the swizzling of RGA formats. While testing all formats of
> > the driver on rk3568, I discovered that the color channels of the RGB
> > formats are wrong when converting to NV12. I didn't test this on other
> > SoCs with an RGA and I am not sure, if they behave differently regarding
> > the color channels. Please report, if this breaks the color conversion
> > on other SoCs, and I will make this SoC-specific.
> > 
> > Patches 2 to 6 are the rework the DMA descriptor handling for the RGA
> > MMU. The patches clean up, how the driver uses the DMA API, and make the
> > creation of the descriptor list more explicit. Furthermore, the driver
> > is changed to keep the mapping per video buffer instead of using a
> > single mapping that is updated with every buffer.
> > 
> > Patches 7 to 11 prepare the driver for the multi-planar API including a
> > cleanup of the format handling in the buffer, and finally switch to the
> > multi-planar API.
> > 
> > Patch 12 updates the code that creates the DMA-descriptor mapping to
> > correctly handle buffers with multiple planes. The driver has to iterate
> > all planes and make them the continuous for the RGA.
> > 
> > Patch 13 enables the NV12M format, which is the multi-planar variant of
> > the NV12 format.
> 
> I get these smatch warnings for this series:
> 
> drivers/media/platform/rockchip/rga/rga-hw.c: In function 'rga_get_addr_offset':
> drivers/media/platform/rockchip/rga/rga-hw.c:46:65: warning: variable 'uv_factor' set but not used [-Wunused-but-set-variable]
>    46 |                      y_div = 0, uv_stride = 0, pixel_width = 0, uv_factor = 0;
>       |                                                                 ^~~~~~~~~
> drivers/media/platform/rockchip/rga/rga-buf.c:129 rga_buf_prepare() warn: unsigned 'ret' is never less than zero.

Thanks, I will run smatch on the series and send a v2.

Michael

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Michael
> > 
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > Michael Tretter (13):
> >       media: rockchip: rga: fix swizzling for RGB formats
> >       media: rockchip: rga: extract helper to fill descriptors
> >       media: rockchip: rga: allocate DMA descriptors per buffer
> >       media: rockchip: rga: split src and dst buffer setup
> >       media: rockchip: rga: pre-calculate plane offsets
> >       media: rockchip: rga: set dma mask to 32 bits
> >       media: rockchip: rga: use clamp() to clamp size to limits
> >       media: rockchip: rga: use pixelformat to find format
> >       media: rockchip: rga: add local variable for pix_format
> >       media: rockchip: rga: use macros for testing buffer type
> >       media: rockchip: rga: switch to multi-planar API
> >       media: rockchip: rga: rework buffer handling for multi-planar formats
> >       media: rockchip: rga: add NV12M support
> > 
> >  drivers/media/platform/rockchip/rga/rga-buf.c | 166 ++++++++++++++++-------
> >  drivers/media/platform/rockchip/rga/rga-hw.c  | 143 +++++++++++++-------
> >  drivers/media/platform/rockchip/rga/rga.c     | 187 ++++++++++++--------------
> >  drivers/media/platform/rockchip/rga/rga.h     |  35 ++++-
> >  4 files changed, 329 insertions(+), 202 deletions(-)
> > ---
> > base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
> > change-id: 20230914-rockchip-rga-multiplanar-0d7c79b1ba93
> > 
> > Best regards,
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Michael Tretter]

On Mon, 25 Sep 2023 09:32:49 +0200, Hans Verkuil wrote:
> 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@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.

Thanks. I'll fix this in v2.

Michael

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel