| Message ID | 20220322082859.9834-1-ming.qian@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: amphion: ensure the buffer count is not less than min_buffer | expand |
On 27/04/2022 11:31, Ming Qian wrote: >> From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl] >> Sent: Wednesday, April 27, 2022 3:25 PM >> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org; >> shawnguo@kernel.org >> Cc: robh+dt@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; >> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>; Aisheng Dong >> <aisheng.dong@nxp.com>; linux-media@vger.kernel.org; >> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org >> Subject: Re: [EXT] Re: [PATCH] media: amphion: ensure the buffer count is not >> less than min_buffer >> >> Caution: EXT Email >> >> On 27/04/2022 09:01, Ming Qian wrote: >>>> From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl] >>>> Sent: Wednesday, April 27, 2022 2:38 PM >>>> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org; >>>> shawnguo@kernel.org >>>> Cc: robh+dt@kernel.org; s.hauer@pengutronix.de; >>>> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx >>>> <linux-imx@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>; >>>> linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; >>>> linux-arm-kernel@lists.infradead.org >>>> 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@nxp.com> >>>>> --- >>>>> 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) >> {' >> > Hi Hans, > I send a v2 patch. > But I think the v1 is OK, as the full code has already guaranteed the condition ` if (!*num_planes)`, > > if (*plane_count) { > ... ... > 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); > You are correct, it wasn't visible in the patch that the *buf_count adjustments only happened if *plane_count == 0. I'm going back to v1. Sorry for the confusion! Hans > Ming > >> 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]; >>> >
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); *plane_count = cur_fmt->num_planes; for (i = 0; i < cur_fmt->num_planes; i++) psize[i] = cur_fmt->sizeimage[i];
the output buffer count should >= min_buffer_out the capture buffer count should >= min_buffer_cap Signed-off-by: Ming Qian <ming.qian@nxp.com> --- drivers/media/platform/amphion/vpu_v4l2.c | 4 ++++ 1 file changed, 4 insertions(+)