[PATCH 0/2] media: rockchip: rga: fix v4l2-compliance errors

[PATCH 0/2] media: rockchip: rga: fix v4l2-compliance errors

[Michael Tretter]

Fix the Streaming ioctl tests reported by v4l2-compliance for the
Rockchip RGA driver.

Patch 1 fixes the field of the returned buffers and Patch 2 the sequence
numbers.

---
Michael Tretter (2):
      media: rockchip: rga: fix field in OUTPUT buffers
      media: rockchip: rga: fix sequence number handling

 drivers/media/platform/rockchip/rga/rga-buf.c | 15 +++++++++++++++
 drivers/media/platform/rockchip/rga/rga.c     |  4 ++++
 drivers/media/platform/rockchip/rga/rga.h     |  3 +++
 3 files changed, 22 insertions(+)
---
base-commit: 8771b7f31b7fff91a998e6afdb60650d4bac59a5
change-id: 20240528-rk3568-rga-fff1dd0c2c75

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 1/2] media: rockchip: rga: fix field in OUTPUT buffers

[Michael Tretter]

Returned buffers shouldn't contain V4L2_FIELD_ANY as field. Set the
field to V4L2_FIELD_NONE, if it isn't set.

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

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index 662c81b6d0b5..77c7535893e3 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -119,6 +119,16 @@ static int rga_buf_prepare(struct vb2_buffer *vb)
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
+	if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
+		if (vbuf->field == V4L2_FIELD_ANY)
+			vbuf->field = V4L2_FIELD_NONE;
+		if (vbuf->field != V4L2_FIELD_NONE) {
+			v4l2_err(&ctx->rga->v4l2_dev, "Unsupported field %s\n",
+				 v4l2_field_names[vbuf->field]);
+			return -EINVAL;
+		}
+	}
+
 	for (i = 0; i < vb->num_planes; i++) {
 		vb2_set_plane_payload(vb, i, f->pix.plane_fmt[i].sizeimage);
 

-- 
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 1/2] media: rockchip: rga: fix field in OUTPUT buffers

[Hans Verkuil]

On 28/05/2024 16:05, Michael Tretter wrote:
> Returned buffers shouldn't contain V4L2_FIELD_ANY as field. Set the
> field to V4L2_FIELD_NONE, if it isn't set.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  drivers/media/platform/rockchip/rga/rga-buf.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index 662c81b6d0b5..77c7535893e3 100644
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> @@ -119,6 +119,16 @@ static int rga_buf_prepare(struct vb2_buffer *vb)
>  	if (IS_ERR(f))
>  		return PTR_ERR(f);
>  
> +	if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
> +		if (vbuf->field == V4L2_FIELD_ANY)
> +			vbuf->field = V4L2_FIELD_NONE;
> +		if (vbuf->field != V4L2_FIELD_NONE) {
> +			v4l2_err(&ctx->rga->v4l2_dev, "Unsupported field %s\n",
> +				 v4l2_field_names[vbuf->field]);

This should be v4l2_dbg: incorrect userspace data should not result in
spamming of the kernel log. It's fine to report it if debugging is enabled.

Regards,

	Hans

> +			return -EINVAL;
> +		}
> +	}
> +
>  	for (i = 0; i < vb->num_planes; i++) {
>  		vb2_set_plane_payload(vb, i, f->pix.plane_fmt[i].sizeimage);
>  
> 


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

Re: [PATCH 1/2] media: rockchip: rga: fix field in OUTPUT buffers

[Michael Tretter]

On Thu, 30 May 2024 14:47:48 +0200, Hans Verkuil wrote:
> On 28/05/2024 16:05, Michael Tretter wrote:
> > Returned buffers shouldn't contain V4L2_FIELD_ANY as field. Set the
> > field to V4L2_FIELD_NONE, if it isn't set.
> > 
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> >  drivers/media/platform/rockchip/rga/rga-buf.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> > index 662c81b6d0b5..77c7535893e3 100644
> > --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> > +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> > @@ -119,6 +119,16 @@ static int rga_buf_prepare(struct vb2_buffer *vb)
> >  	if (IS_ERR(f))
> >  		return PTR_ERR(f);
> >  
> > +	if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
> > +		if (vbuf->field == V4L2_FIELD_ANY)
> > +			vbuf->field = V4L2_FIELD_NONE;
> > +		if (vbuf->field != V4L2_FIELD_NONE) {
> > +			v4l2_err(&ctx->rga->v4l2_dev, "Unsupported field %s\n",
> > +				 v4l2_field_names[vbuf->field]);
> 
> This should be v4l2_dbg: incorrect userspace data should not result in
> spamming of the kernel log. It's fine to report it if debugging is enabled.

I cannot use v4l2_dbg here, as the debug module parameter for the RGA
driver is defined in a different file. I'll just drop the error message
in v2.

Michael

> 
> Regards,
> 
> 	Hans
> 
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> >  	for (i = 0; i < vb->num_planes; i++) {
> >  		vb2_set_plane_payload(vb, i, f->pix.plane_fmt[i].sizeimage);
> >  
> > 
> 
> 

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

[PATCH 2/2] media: rockchip: rga: fix sequence number handling

[Michael Tretter]

The RGA driver didn't set the sequence numbers of the returned buffers.

Keep track of the CAPTURE and OUTPUT sequence numbers, and set the
sequence numbers in the source and destination buffers.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/media/platform/rockchip/rga/rga-buf.c | 5 +++++
 drivers/media/platform/rockchip/rga/rga.c     | 4 ++++
 drivers/media/platform/rockchip/rga/rga.h     | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
index 77c7535893e3..48bb45ec0230 100644
--- a/drivers/media/platform/rockchip/rga/rga-buf.c
+++ b/drivers/media/platform/rockchip/rga/rga-buf.c
@@ -205,6 +205,11 @@ static int rga_buf_start_streaming(struct vb2_queue *q, unsigned int count)
 		return ret;
 	}
 
+	if (V4L2_TYPE_IS_OUTPUT(q->type))
+		ctx->osequence = 0;
+	else
+		ctx->csequence = 0;
+
 	return 0;
 }
 
diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index 00fdfa9e10bc..0e768f3e9eda 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -43,6 +43,8 @@ static void device_run(void *prv)
 	rga->curr = ctx;
 
 	src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+	src->sequence = ctx->osequence++;
+
 	dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
 	rga_hw_start(rga, vb_to_rga(src), vb_to_rga(dst));
@@ -75,6 +77,8 @@ static irqreturn_t rga_isr(int irq, void *prv)
 
 		v4l2_m2m_buf_copy_metadata(src, dst, true);
 
+		dst->sequence = ctx->csequence++;
+
 		v4l2_m2m_buf_done(src, VB2_BUF_STATE_DONE);
 		v4l2_m2m_buf_done(dst, VB2_BUF_STATE_DONE);
 		v4l2_m2m_job_finish(rga->m2m_dev, ctx->fh.m2m_ctx);
diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
index 3502dff6055c..8105bb2efe57 100644
--- a/drivers/media/platform/rockchip/rga/rga.h
+++ b/drivers/media/platform/rockchip/rga/rga.h
@@ -57,6 +57,9 @@ struct rga_ctx {
 	struct rga_frame out;
 	struct v4l2_ctrl_handler ctrl_handler;
 
+	int osequence;
+	int csequence;
+
 	/* Control values */
 	u32 op;
 	u32 hflip;

-- 
2.39.2


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