Re: [PATCH 2/2] media: rockchip: rkcif: comply with minimum number of buffers requirement
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2026-02-19 17:00:51
Also in:
linux-media, linux-rockchip, lkml
On Thu, Feb 19, 2026 at 05:19:46PM +0100, Michael Riesch wrote:
On 2/19/26 10:13, Laurent Pinchart wrote:quoted
On Mon, Feb 16, 2026 at 02:49:57PM +0100, Michael Riesch via B4 Relay wrote:quoted
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.
quoted
quoted
-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.quoted
quoted
}
-- Regards, Laurent Pinchart