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@xxxxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx
> > Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxx>
> > ---
> >  .../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;
> >  }
> >  
> > 
>