On 27/04/2022 09:01, Ming Qian wrote: >> From: Hans Verkuil [mailto:hverkuil-cisco@xxxxxxxxx] >> Sent: Wednesday, April 27, 2022 2:38 PM >> To: Ming Qian <ming.qian@xxxxxxx>; mchehab@xxxxxxxxxx; >> shawnguo@xxxxxxxxxx >> Cc: robh+dt@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; >> festevam@xxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Aisheng Dong >> <aisheng.dong@xxxxxxx>; linux-media@xxxxxxxxxxxxxxx; >> linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx >> Subject: [EXT] Re: [PATCH] media: amphion: ensure the buffer count is not less >> than min_buffer >> >> Caution: EXT Email >> >> Hi Ming Qian, >> >> On 22/03/2022 09:28, Ming Qian wrote: >>> the output buffer count should >= min_buffer_out the capture buffer >>> count should >= min_buffer_cap >>> >>> Signed-off-by: Ming Qian <ming.qian@xxxxxxx> >>> --- >>> drivers/media/platform/amphion/vpu_v4l2.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/media/platform/amphion/vpu_v4l2.c >>> b/drivers/media/platform/amphion/vpu_v4l2.c >>> index cbf3315605a9..72a0544f4da3 100644 >>> --- a/drivers/media/platform/amphion/vpu_v4l2.c >>> +++ b/drivers/media/platform/amphion/vpu_v4l2.c >>> @@ -355,6 +355,10 @@ static int vpu_vb2_queue_setup(struct vb2_queue >> *vq, >>> return 0; >>> } >>> >>> + if (V4L2_TYPE_IS_OUTPUT(vq->type)) >>> + *buf_count = max_t(unsigned int, *buf_count, >> inst->min_buffer_out); >>> + else >>> + *buf_count = max_t(unsigned int, *buf_count, >>> + inst->min_buffer_cap); >> >> I noticed that min_buffer_out/cap is set to 2, but min_buffers_needed is set to >> 1. Wouldn't it make more sense to set min_buffers_needed to >> 2 as well? >> >> If you do that, then the vb2 core will already take care of ensuring that the >> buf_count is adjusted. >> >> If you *do* have to do this manually, then you need to place the whole if-else >> under 'if (!*num_planes) {', otherwise it will mess up the VIDIOC_CREATE_BUFS >> ioctl. See the queue_setup in include/media/videobuf2-core.h documentation >> for the sordid details. >> >> Regards, >> >> Hans >> > > Hi Hans, > I want to make the vpu start when 1 frames is queued, so I set the min_buffers_needed to 1. > And the min_buffer_cap may be changed when a source change event is triggered. So in most case, it will be larger than 2. Ah, I only grepped for min_buffer_out, not _cap, so I missed that that one isn't constant. > I'll make a v2 patch that place the whole if-else under 'if (!*num_planes) {' Great, thank you! Hans > Thanks for your reminder > > Ming > >>> *plane_count = cur_fmt->num_planes; >>> for (i = 0; i < cur_fmt->num_planes; i++) >>> psize[i] = cur_fmt->sizeimage[i]; >
