[PATCH 0/2] media: rockchip: rkcif: various fixes

[PATCH 0/2] media: rockchip: rkcif: various fixes

[Michael Riesch via B4 Relay]

Habidere,

This series contains

 1) a re-spin of Dan's patch that fixes some more stupid off-by-one issues.
    This patch has been around on the list for some time, but apparently
    has not been applied yet.
 2) a fix that makes the DMA abstraction respect the minimum number of
    buffers requirement

Best regards,
Michael

Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
Dan Carpenter (1):
      media: rockchip: rkcif: fix off by one bugs

Michael Riesch (1):
      media: rockchip: rkcif: comply with minimum number of buffers requirement

 .../platform/rockchip/rkcif/rkcif-capture-mipi.c   | 10 +++---
 .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
 2 files changed, 26 insertions(+), 25 deletions(-)
---
base-commit: c824345288d11e269ce41b36c105715bc2286050
change-id: 20260216-rkcif-fixes-bdd9d3c7e4b0

Best regards,
-- 
Michael Riesch <michael.riesch@collabora.com>

[PATCH 1/2] media: rockchip: rkcif: fix off by one bugs

[Michael Riesch via B4 Relay]

From: Dan Carpenter <dan.carpenter@linaro.org>

Change these comparisons from > vs >= to avoid accessing one element
beyond the end of the arrays.

Fixes: 1f2353f5a1af ("media: rockchip: rkcif: add support for rk3568 vicap mipi capture")
Cc: stable@kernel.org
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
 drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
index 1b81bcc067ef..a933df682acc 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
@@ -489,8 +489,8 @@ static inline unsigned int rkcif_mipi_get_reg(struct rkcif_interface *interface,
 
 	block = interface->index - RKCIF_MIPI_BASE;
 
-	if (WARN_ON_ONCE(block > RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
-	    WARN_ON_ONCE(index > RKCIF_MIPI_REGISTER_MAX))
+	if (WARN_ON_ONCE(block >= RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
+	    WARN_ON_ONCE(index >= RKCIF_MIPI_REGISTER_MAX))
 		return RKCIF_REGISTER_NOTSUPPORTED;
 
 	offset = rkcif->match_data->mipi->blocks[block].offset;
@@ -510,9 +510,9 @@ static inline unsigned int rkcif_mipi_id_get_reg(struct rkcif_stream *stream,
 	block = stream->interface->index - RKCIF_MIPI_BASE;
 	id = stream->id;
 
-	if (WARN_ON_ONCE(block > RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
-	    WARN_ON_ONCE(id > RKCIF_ID_MAX) ||
-	    WARN_ON_ONCE(index > RKCIF_MIPI_ID_REGISTER_MAX))
+	if (WARN_ON_ONCE(block >= RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
+	    WARN_ON_ONCE(id >= RKCIF_ID_MAX) ||
+	    WARN_ON_ONCE(index >= RKCIF_MIPI_ID_REGISTER_MAX))
 		return RKCIF_REGISTER_NOTSUPPORTED;
 
 	offset = rkcif->match_data->mipi->blocks[block].offset;

-- 
2.39.5

[PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement

[Michael Riesch via B4 Relay]

From: Michael Riesch <michael.riesch@collabora.com>

Each stream requires CIF_REQ_BUFS_MIN=1 buffers to enable streaming.
However, it failed with only one buffer provided.

Comply with the minimum number of buffers requirement and accept
exactly one buffer.

Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
Cc: stable@kernel.org
Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
---
 .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
index e00010a91e8b..5a5ab9e7e86e 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
@@ -106,19 +106,6 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
 {
 	struct v4l2_pix_format_mplane *pix = &stream->pix;
 
-	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
-	if (!stream->buffers[0])
-		goto err_buff_0;
-
-	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
-	if (!stream->buffers[1])
-		goto err_buff_1;
-
-	if (stream->queue_buffer) {
-		stream->queue_buffer(stream, 0);
-		stream->queue_buffer(stream, 1);
-	}
-
 	stream->dummy.size = pix->num_planes * pix->plane_fmt[0].sizeimage;
 	stream->dummy.vaddr =
 		dma_alloc_attrs(stream->rkcif->dev, stream->dummy.size,
@@ -132,16 +119,30 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
 			stream->dummy.buffer.buff_addr[i - 1] +
 			pix->plane_fmt[i - 1].bytesperline * pix->height;
 
-	return 0;
+	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
+	if (!stream->buffers[0])
+		goto err_buff_0;
 
-err_dummy:
-	rkcif_stream_return_buffer(stream->buffers[1], VB2_BUF_STATE_QUEUED);
-	stream->buffers[1] = NULL;
+	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
+	if (!stream->buffers[1]) {
+		stream->buffers[stream->frame_phase] = &stream->dummy.buffer;
+		stream->buffers[stream->frame_phase]->is_dummy = true;
+	}
+
+	if (stream->queue_buffer) {
+		stream->queue_buffer(stream, 0);
+		stream->queue_buffer(stream, 1);
+	}
+
+	return 0;
 
-err_buff_1:
-	rkcif_stream_return_buffer(stream->buffers[0], VB2_BUF_STATE_QUEUED);
-	stream->buffers[0] = NULL;
 err_buff_0:
+	dma_free_attrs(stream->rkcif->dev, stream->dummy.size,
+		       stream->dummy.vaddr,
+		       stream->dummy.buffer.buff_addr[0],
+		       DMA_ATTR_NO_KERNEL_MAPPING);
+	stream->dummy.vaddr = NULL;
+err_dummy:
 	return -EINVAL;
 }
 

-- 
2.39.5

Re: [PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement

[Michael Riesch]

Hi all,

On 2/16/26 14:49, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
> 
> Each stream requires CIF_REQ_BUFS_MIN=1 buffers to enable streaming.
> However, it failed with only one buffer provided.
> 
> Comply with the minimum number of buffers requirement and accept
> exactly one buffer.
> 
> Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
> Cc: stable@kernel.org
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
>  .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
>  1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> index e00010a91e8b..5a5ab9e7e86e 100644
> --- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> @@ -106,19 +106,6 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
>  {
>  	struct v4l2_pix_format_mplane *pix = &stream->pix;
>  
> -	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> -	if (!stream->buffers[0])
> -		goto err_buff_0;
> -
> -	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> -	if (!stream->buffers[1])
> -		goto err_buff_1;
> -
> -	if (stream->queue_buffer) {
> -		stream->queue_buffer(stream, 0);
> -		stream->queue_buffer(stream, 1);
> -	}
> -
>  	stream->dummy.size = pix->num_planes * pix->plane_fmt[0].sizeimage;
>  	stream->dummy.vaddr =
>  		dma_alloc_attrs(stream->rkcif->dev, stream->dummy.size,
> @@ -132,16 +119,30 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
>  			stream->dummy.buffer.buff_addr[i - 1] +
>  			pix->plane_fmt[i - 1].bytesperline * pix->height;
>  
> -	return 0;
> +	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> +	if (!stream->buffers[0])
> +		goto err_buff_0;
>  
> -err_dummy:
> -	rkcif_stream_return_buffer(stream->buffers[1], VB2_BUF_STATE_QUEUED);
> -	stream->buffers[1] = NULL;
> +	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> +	if (!stream->buffers[1]) {
> +		stream->buffers[stream->frame_phase] = &stream->dummy.buffer;
> +		stream->buffers[stream->frame_phase]->is_dummy = true;

Apparently I was too quick on the trigger here. This should read
"stream->buffers[1]" in both lines *facepalm*. Will wait for other
responses and send a v2.

Best regards,
Michael

> +	}
> +
> +	if (stream->queue_buffer) {
> +		stream->queue_buffer(stream, 0);
> +		stream->queue_buffer(stream, 1);
> +	}
> +
> +	return 0;
>  
> -err_buff_1:
> -	rkcif_stream_return_buffer(stream->buffers[0], VB2_BUF_STATE_QUEUED);
> -	stream->buffers[0] = NULL;
>  err_buff_0:
> +	dma_free_attrs(stream->rkcif->dev, stream->dummy.size,
> +		       stream->dummy.vaddr,
> +		       stream->dummy.buffer.buff_addr[0],
> +		       DMA_ATTR_NO_KERNEL_MAPPING);
> +	stream->dummy.vaddr = NULL;
> +err_dummy:
>  	return -EINVAL;
>  }
>  
> 

Re: [PATCH 1/2] media: rockchip: rkcif: fix off by one bugs

[Paul Elder]

Quoting Michael Riesch (2026-02-16 22:49:56)
> From: Dan Carpenter <dan.carpenter@linaro.org>
> 
> Change these comparisons from > vs >= to avoid accessing one element
> beyond the end of the arrays.
> 
> Fixes: 1f2353f5a1af ("media: rockchip: rkcif: add support for rk3568 vicap mipi capture")
> Cc: stable@kernel.org
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
> index 1b81bcc067ef..a933df682acc 100644
> --- a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
> @@ -489,8 +489,8 @@ static inline unsigned int rkcif_mipi_get_reg(struct rkcif_interface *interface,
>  
>         block = interface->index - RKCIF_MIPI_BASE;
>  
> -       if (WARN_ON_ONCE(block > RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> -           WARN_ON_ONCE(index > RKCIF_MIPI_REGISTER_MAX))
> +       if (WARN_ON_ONCE(block >= RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> +           WARN_ON_ONCE(index >= RKCIF_MIPI_REGISTER_MAX))
>                 return RKCIF_REGISTER_NOTSUPPORTED;
>  
>         offset = rkcif->match_data->mipi->blocks[block].offset;
> @@ -510,9 +510,9 @@ static inline unsigned int rkcif_mipi_id_get_reg(struct rkcif_stream *stream,
>         block = stream->interface->index - RKCIF_MIPI_BASE;
>         id = stream->id;
>  
> -       if (WARN_ON_ONCE(block > RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> -           WARN_ON_ONCE(id > RKCIF_ID_MAX) ||
> -           WARN_ON_ONCE(index > RKCIF_MIPI_ID_REGISTER_MAX))
> +       if (WARN_ON_ONCE(block >= RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> +           WARN_ON_ONCE(id >= RKCIF_ID_MAX) ||
> +           WARN_ON_ONCE(index >= RKCIF_MIPI_ID_REGISTER_MAX))
>                 return RKCIF_REGISTER_NOTSUPPORTED;
>  
>         offset = rkcif->match_data->mipi->blocks[block].offset;
> 
> -- 
> 2.39.5
> 
>

Re: [PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement

[Paul Elder]

Hi Michael,

Thanks for the patch.

Quoting Michael Riesch (2026-02-17 21:25:10)
> Hi all,
> 
> On 2/16/26 14:49, Michael Riesch via B4 Relay wrote:
> > From: Michael Riesch <michael.riesch@collabora.com>
> > 
> > Each stream requires CIF_REQ_BUFS_MIN=1 buffers to enable streaming.
> > However, it failed with only one buffer provided.
> > 
> > Comply with the minimum number of buffers requirement and accept
> > exactly one buffer.
> > 
> > Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
> > Cc: stable@kernel.org
> > Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> > ---
> >  .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
> >  1 file changed, 21 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> > index e00010a91e8b..5a5ab9e7e86e 100644
> > --- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> > +++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> > @@ -106,19 +106,6 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
> >  {
> >       struct v4l2_pix_format_mplane *pix = &stream->pix;
> >  
> > -     stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> > -     if (!stream->buffers[0])
> > -             goto err_buff_0;
> > -
> > -     stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> > -     if (!stream->buffers[1])
> > -             goto err_buff_1;
> > -
> > -     if (stream->queue_buffer) {
> > -             stream->queue_buffer(stream, 0);
> > -             stream->queue_buffer(stream, 1);
> > -     }
> > -
> >       stream->dummy.size = pix->num_planes * pix->plane_fmt[0].sizeimage;
> >       stream->dummy.vaddr =
> >               dma_alloc_attrs(stream->rkcif->dev, stream->dummy.size,
> > @@ -132,16 +119,30 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
> >                       stream->dummy.buffer.buff_addr[i - 1] +
> >                       pix->plane_fmt[i - 1].bytesperline * pix->height;
> >  
> > -     return 0;
> > +     stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> > +     if (!stream->buffers[0])
> > +             goto err_buff_0;
> >  
> > -err_dummy:
> > -     rkcif_stream_return_buffer(stream->buffers[1], VB2_BUF_STATE_QUEUED);
> > -     stream->buffers[1] = NULL;
> > +     stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> > +     if (!stream->buffers[1]) {
> > +             stream->buffers[stream->frame_phase] = &stream->dummy.buffer;
> > +             stream->buffers[stream->frame_phase]->is_dummy = true;
> 
> Apparently I was too quick on the trigger here. This should read
> "stream->buffers[1]" in both lines *facepalm*. Will wait for other
> responses and send a v2.

With this fix I've tested that it works, and it looks good to me :)


Paul

> 
> Best regards,
> Michael
> 
> > +     }
> > +
> > +     if (stream->queue_buffer) {
> > +             stream->queue_buffer(stream, 0);
> > +             stream->queue_buffer(stream, 1);
> > +     }
> > +
> > +     return 0;
> >  
> > -err_buff_1:
> > -     rkcif_stream_return_buffer(stream->buffers[0], VB2_BUF_STATE_QUEUED);
> > -     stream->buffers[0] = NULL;
> >  err_buff_0:
> > +     dma_free_attrs(stream->rkcif->dev, stream->dummy.size,
> > +                    stream->dummy.vaddr,
> > +                    stream->dummy.buffer.buff_addr[0],
> > +                    DMA_ATTR_NO_KERNEL_MAPPING);
> > +     stream->dummy.vaddr = NULL;
> > +err_dummy:
> >       return -EINVAL;
> >  }
> >  
> > 
>

Re: [PATCH 0/2] media: rockchip: rkcif: various fixes

[Chen-Yu Tsai]

On Mon, Feb 16, 2026 at 9:50 PM Michael Riesch via B4 Relay
<devnull+michael.riesch.collabora.com@kernel.org> wrote:
>
> Habidere,
>
> This series contains
>
>  1) a re-spin of Dan's patch that fixes some more stupid off-by-one issues.
>     This patch has been around on the list for some time, but apparently
>     has not been applied yet.
>  2) a fix that makes the DMA abstraction respect the minimum number of
>     buffers requirement
>
> Best regards,
> Michael
>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>

This series, along with the minor fixup for patch 2, makes video capture
on my Rock 3A w/ IMX219 from a Raspberry Pi v2 camera module run.
Previously it would fail to queue buffers.


Tested-by: Chen-Yu Tsai <wens@kernel.org>

> ---
> Dan Carpenter (1):
>       media: rockchip: rkcif: fix off by one bugs
>
> Michael Riesch (1):
>       media: rockchip: rkcif: comply with minimum number of buffers requirement
>
>  .../platform/rockchip/rkcif/rkcif-capture-mipi.c   | 10 +++---
>  .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
>  2 files changed, 26 insertions(+), 25 deletions(-)
> ---
> base-commit: c824345288d11e269ce41b36c105715bc2286050
> change-id: 20260216-rkcif-fixes-bdd9d3c7e4b0
>
> Best regards,
> --
> Michael Riesch <michael.riesch@collabora.com>
>
>
>

Re: [PATCH 1/2] media: rockchip: rkcif: fix off by one bugs

[Laurent Pinchart]

On Mon, Feb 16, 2026 at 02:49:56PM +0100, Michael Riesch via B4 Relay wrote:
> From: Dan Carpenter <dan.carpenter@linaro.org>
> 
> Change these comparisons from > vs >= to avoid accessing one element
> beyond the end of the arrays.
> 
> Fixes: 1f2353f5a1af ("media: rockchip: rkcif: add support for rk3568 vicap mipi capture")
> Cc: stable@kernel.org
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
>  drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
> index 1b81bcc067ef..a933df682acc 100644
> --- a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
> @@ -489,8 +489,8 @@ static inline unsigned int rkcif_mipi_get_reg(struct rkcif_interface *interface,
>  
>  	block = interface->index - RKCIF_MIPI_BASE;
>  
> -	if (WARN_ON_ONCE(block > RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> -	    WARN_ON_ONCE(index > RKCIF_MIPI_REGISTER_MAX))
> +	if (WARN_ON_ONCE(block >= RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> +	    WARN_ON_ONCE(index >= RKCIF_MIPI_REGISTER_MAX))

While at it, I'd write

	if (WARN_ON_ONCE(block >= ARRAY_SIZE(rkcif->match_data->mipi->blocks)) ||
	    WARN_ON_ONCE(index >= ARRAY_SIZE(rkcif->match_data->mipi->regs)))

Same below.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		return RKCIF_REGISTER_NOTSUPPORTED;
>  
>  	offset = rkcif->match_data->mipi->blocks[block].offset;
> @@ -510,9 +510,9 @@ static inline unsigned int rkcif_mipi_id_get_reg(struct rkcif_stream *stream,
>  	block = stream->interface->index - RKCIF_MIPI_BASE;
>  	id = stream->id;
>  
> -	if (WARN_ON_ONCE(block > RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> -	    WARN_ON_ONCE(id > RKCIF_ID_MAX) ||
> -	    WARN_ON_ONCE(index > RKCIF_MIPI_ID_REGISTER_MAX))
> +	if (WARN_ON_ONCE(block >= RKCIF_MIPI_MAX - RKCIF_MIPI_BASE) ||
> +	    WARN_ON_ONCE(id >= RKCIF_ID_MAX) ||
> +	    WARN_ON_ONCE(index >= RKCIF_MIPI_ID_REGISTER_MAX))
>  		return RKCIF_REGISTER_NOTSUPPORTED;
>  
>  	offset = rkcif->match_data->mipi->blocks[block].offset;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement

[Laurent Pinchart]

On Mon, Feb 16, 2026 at 02:49:57PM +0100, Michael Riesch via B4 Relay wrote:
> From: Michael Riesch <michael.riesch@collabora.com>
> 
> Each stream requires CIF_REQ_BUFS_MIN=1 buffers to enable streaming.
> However, it failed with only one buffer provided.
> 
> Comply with the minimum number of buffers requirement and accept
> exactly one buffer.
> 
> Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
> Cc: stable@kernel.org
> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> ---
>  .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
>  1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> index e00010a91e8b..5a5ab9e7e86e 100644
> --- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> @@ -106,19 +106,6 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
>  {
>  	struct v4l2_pix_format_mplane *pix = &stream->pix;
>  
> -	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> -	if (!stream->buffers[0])
> -		goto err_buff_0;
> -
> -	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> -	if (!stream->buffers[1])
> -		goto err_buff_1;
> -
> -	if (stream->queue_buffer) {
> -		stream->queue_buffer(stream, 0);
> -		stream->queue_buffer(stream, 1);
> -	}
> -
>  	stream->dummy.size = pix->num_planes * pix->plane_fmt[0].sizeimage;
>  	stream->dummy.vaddr =
>  		dma_alloc_attrs(stream->rkcif->dev, stream->dummy.size,
> @@ -132,16 +119,30 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
>  			stream->dummy.buffer.buff_addr[i - 1] +
>  			pix->plane_fmt[i - 1].bytesperline * pix->height;
>  
> -	return 0;
> +	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> +	if (!stream->buffers[0])
> +		goto err_buff_0;

Why do you move this after allocation of the dummy buffer, to then add
dma_free_attrs() in the err_buff_0 error path ?

>  
> -err_dummy:
> -	rkcif_stream_return_buffer(stream->buffers[1], VB2_BUF_STATE_QUEUED);
> -	stream->buffers[1] = NULL;
> +	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> +	if (!stream->buffers[1]) {
> +		stream->buffers[stream->frame_phase] = &stream->dummy.buffer;
> +		stream->buffers[stream->frame_phase]->is_dummy = true;
> +	}
> +
> +	if (stream->queue_buffer) {
> +		stream->queue_buffer(stream, 0);
> +		stream->queue_buffer(stream, 1);
> +	}
> +
> +	return 0;
>  
> -err_buff_1:
> -	rkcif_stream_return_buffer(stream->buffers[0], VB2_BUF_STATE_QUEUED);
> -	stream->buffers[0] = NULL;
>  err_buff_0:
> +	dma_free_attrs(stream->rkcif->dev, stream->dummy.size,
> +		       stream->dummy.vaddr,
> +		       stream->dummy.buffer.buff_addr[0],
> +		       DMA_ATTR_NO_KERNEL_MAPPING);
> +	stream->dummy.vaddr = NULL;
> +err_dummy:
>  	return -EINVAL;

You can drop the err_dummy label and return -EINVAL directly. Except you
should probably return -ENOMEM as the failure comes from
dma_alloc_attrs().

>  }
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement

[Michael Riesch]

Hi Laurent,

On 2/19/26 10:13, Laurent Pinchart wrote:
> On Mon, Feb 16, 2026 at 02:49:57PM +0100, Michael Riesch via B4 Relay wrote:
>> From: Michael Riesch <michael.riesch@collabora.com>
>>
>> Each stream requires CIF_REQ_BUFS_MIN=1 buffers to enable streaming.
>> However, it failed with only one buffer provided.
>>
>> Comply with the minimum number of buffers requirement and accept
>> exactly one buffer.
>>
>> Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
>> Cc: stable@kernel.org
>> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
>> ---
>>  .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
>>  1 file changed, 21 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
>> index e00010a91e8b..5a5ab9e7e86e 100644
>> --- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
>> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
>> @@ -106,19 +106,6 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
>>  {
>>  	struct v4l2_pix_format_mplane *pix = &stream->pix;
>>  
>> -	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
>> -	if (!stream->buffers[0])
>> -		goto err_buff_0;
>> -
>> -	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
>> -	if (!stream->buffers[1])
>> -		goto err_buff_1;
>> -
>> -	if (stream->queue_buffer) {
>> -		stream->queue_buffer(stream, 0);
>> -		stream->queue_buffer(stream, 1);
>> -	}
>> -
>>  	stream->dummy.size = pix->num_planes * pix->plane_fmt[0].sizeimage;
>>  	stream->dummy.vaddr =
>>  		dma_alloc_attrs(stream->rkcif->dev, stream->dummy.size,
>> @@ -132,16 +119,30 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
>>  			stream->dummy.buffer.buff_addr[i - 1] +
>>  			pix->plane_fmt[i - 1].bytesperline * pix->height;
>>  
>> -	return 0;
>> +	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
>> +	if (!stream->buffers[0])
>> +		goto err_buff_0;
> 
> Why do you move this after allocation of the dummy buffer, to then add
> dma_free_attrs() in the err_buff_0 error path ?

To keep the two rkcif_stream_pop_buffer calls together. We need to
allocate the dummy in any case, but in case the second pop fails we use
it -- this was not the case before.

> 
>>  
>> -err_dummy:
>> -	rkcif_stream_return_buffer(stream->buffers[1], VB2_BUF_STATE_QUEUED);
>> -	stream->buffers[1] = NULL;
>> +	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
>> +	if (!stream->buffers[1]) {
>> +		stream->buffers[stream->frame_phase] = &stream->dummy.buffer;
>> +		stream->buffers[stream->frame_phase]->is_dummy = true;
>> +	}
>> +
>> +	if (stream->queue_buffer) {
>> +		stream->queue_buffer(stream, 0);
>> +		stream->queue_buffer(stream, 1);
>> +	}
>> +
>> +	return 0;
>>  
>> -err_buff_1:
>> -	rkcif_stream_return_buffer(stream->buffers[0], VB2_BUF_STATE_QUEUED);
>> -	stream->buffers[0] = NULL;
>>  err_buff_0:
>> +	dma_free_attrs(stream->rkcif->dev, stream->dummy.size,
>> +		       stream->dummy.vaddr,
>> +		       stream->dummy.buffer.buff_addr[0],
>> +		       DMA_ATTR_NO_KERNEL_MAPPING);
>> +	stream->dummy.vaddr = NULL;
>> +err_dummy:
>>  	return -EINVAL;
> 
> You can drop the err_dummy label and return -EINVAL directly. Except you
> should probably return -ENOMEM as the failure comes from
> dma_alloc_attrs().

Makes sense, will fix.

Best regards,
Michael

> 
>>  }
>>  
> 

Re: [PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement

[Laurent Pinchart]

On Thu, Feb 19, 2026 at 05:19:46PM +0100, Michael Riesch wrote:
> On 2/19/26 10:13, Laurent Pinchart wrote:
> > On Mon, Feb 16, 2026 at 02:49:57PM +0100, Michael Riesch via B4 Relay wrote:
> >> From: Michael Riesch <michael.riesch@collabora.com>
> >>
> >> Each stream requires CIF_REQ_BUFS_MIN=1 buffers to enable streaming.
> >> However, it failed with only one buffer provided.
> >>
> >> Comply with the minimum number of buffers requirement and accept
> >> exactly one buffer.
> >>
> >> Fixes: 501802e2ad51 ("media: rockchip: rkcif: add abstraction for dma blocks")
> >> Cc: stable@kernel.org
> >> Signed-off-by: Michael Riesch <michael.riesch@collabora.com>
> >> ---
> >>  .../media/platform/rockchip/rkcif/rkcif-stream.c   | 41 +++++++++++-----------
> >>  1 file changed, 21 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> >> index e00010a91e8b..5a5ab9e7e86e 100644
> >> --- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> >> +++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
> >> @@ -106,19 +106,6 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
> >>  {
> >>  	struct v4l2_pix_format_mplane *pix = &stream->pix;
> >>  
> >> -	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> >> -	if (!stream->buffers[0])
> >> -		goto err_buff_0;
> >> -
> >> -	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> >> -	if (!stream->buffers[1])
> >> -		goto err_buff_1;
> >> -
> >> -	if (stream->queue_buffer) {
> >> -		stream->queue_buffer(stream, 0);
> >> -		stream->queue_buffer(stream, 1);
> >> -	}
> >> -
> >>  	stream->dummy.size = pix->num_planes * pix->plane_fmt[0].sizeimage;
> >>  	stream->dummy.vaddr =
> >>  		dma_alloc_attrs(stream->rkcif->dev, stream->dummy.size,
> >> @@ -132,16 +119,30 @@ static int rkcif_stream_init_buffers(struct rkcif_stream *stream)
> >>  			stream->dummy.buffer.buff_addr[i - 1] +
> >>  			pix->plane_fmt[i - 1].bytesperline * pix->height;
> >>  
> >> -	return 0;
> >> +	stream->buffers[0] = rkcif_stream_pop_buffer(stream);
> >> +	if (!stream->buffers[0])
> >> +		goto err_buff_0;
> > 
> > Why do you move this after allocation of the dummy buffer, to then add
> > dma_free_attrs() in the err_buff_0 error path ?
> 
> To keep the two rkcif_stream_pop_buffer calls together. We need to
> allocate the dummy in any case, but in case the second pop fails we use
> it -- this was not the case before.

I suppose it's easier than returning buffers[0] in the dummy buffer
allocate error path. Works for me.

> >> -err_dummy:
> >> -	rkcif_stream_return_buffer(stream->buffers[1], VB2_BUF_STATE_QUEUED);
> >> -	stream->buffers[1] = NULL;
> >> +	stream->buffers[1] = rkcif_stream_pop_buffer(stream);
> >> +	if (!stream->buffers[1]) {
> >> +		stream->buffers[stream->frame_phase] = &stream->dummy.buffer;
> >> +		stream->buffers[stream->frame_phase]->is_dummy = true;
> >> +	}
> >> +
> >> +	if (stream->queue_buffer) {
> >> +		stream->queue_buffer(stream, 0);
> >> +		stream->queue_buffer(stream, 1);
> >> +	}
> >> +
> >> +	return 0;
> >>  
> >> -err_buff_1:
> >> -	rkcif_stream_return_buffer(stream->buffers[0], VB2_BUF_STATE_QUEUED);
> >> -	stream->buffers[0] = NULL;
> >>  err_buff_0:
> >> +	dma_free_attrs(stream->rkcif->dev, stream->dummy.size,
> >> +		       stream->dummy.vaddr,
> >> +		       stream->dummy.buffer.buff_addr[0],
> >> +		       DMA_ATTR_NO_KERNEL_MAPPING);
> >> +	stream->dummy.vaddr = NULL;
> >> +err_dummy:
> >>  	return -EINVAL;
> > 
> > You can drop the err_dummy label and return -EINVAL directly. Except you
> > should probably return -ENOMEM as the failure comes from
> > dma_alloc_attrs().
> 
> Makes sense, will fix.
> 
> >>  }
> >>  

-- 
Regards,

Laurent Pinchart