[16/55] media: qcom: venus: Stop abusing of min_buffers_needed field

'min_buffers_needed' is suppose to be used to indicate the number
of buffers needed by DMA engine to start streaming.
venus driver doesn't use DMA engine and just want to specify
the minimum number of buffers to allocate when calling VIDIOC_REQBUFS.
That 'min_reqbufs_allocation' field purpose so use it.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
CC: Stanimir Varbanov <stanimir.k.varbanov@gmail.com> (maintainer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER)
CC: Vikash Garodia <quic_vgarodia@quicinc.com> (maintainer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER)
CC: "Bryan O'Donoghue" <bryan.odonoghue@linaro.org> (reviewer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER)
CC: Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
CC: Bjorn Andersson <andersson@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
CC: Konrad Dybcio <konrad.dybcio@linaro.org> (maintainer:ARM/QUALCOMM SUPPORT)
CC: linux-arm-msm@vger.kernel.org
---
 drivers/media/platform/qcom/venus/vdec.c | 4 ++--
 drivers/media/platform/qcom/venus/venc.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

On Tue, Nov 28, 2023 at 7:26 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 27/11/2023 17:54, Benjamin Gaignard wrote:
> > 'min_buffers_needed' is suppose to be used to indicate the number
> > of buffers needed by DMA engine to start streaming.
> > venus driver doesn't use DMA engine and just want to specify
> > the minimum number of buffers to allocate when calling VIDIOC_REQBUFS.
> > That 'min_reqbufs_allocation' field purpose so use it.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > CC: Stanimir Varbanov <stanimir.k.varbanov@gmail.com> (maintainer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER)
> > CC: Vikash Garodia <quic_vgarodia@quicinc.com> (maintainer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER)
> > CC: "Bryan O'Donoghue" <bryan.odonoghue@linaro.org> (reviewer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER)
> > CC: Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > CC: Bjorn Andersson <andersson@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > CC: Konrad Dybcio <konrad.dybcio@linaro.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > CC: linux-arm-msm@vger.kernel.org
> > ---
> >  drivers/media/platform/qcom/venus/vdec.c | 4 ++--
> >  drivers/media/platform/qcom/venus/venc.c | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> > index dbf305cec120..16b8d0dde10d 100644
> > --- a/drivers/media/platform/qcom/venus/vdec.c
> > +++ b/drivers/media/platform/qcom/venus/vdec.c
> > @@ -1641,7 +1641,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
> >       src_vq->drv_priv = inst;
> >       src_vq->buf_struct_size = sizeof(struct venus_buffer);
> >       src_vq->allow_zero_bytesused = 1;
> > -     src_vq->min_buffers_needed = 0;
> > +     src_vq->min_reqbufs_allocation = 0;
>
> Just drop this.
>
> >       src_vq->dev = inst->core->dev;
> >       src_vq->lock = &inst->ctx_q_lock;
> >       ret = vb2_queue_init(src_vq);
> > @@ -1656,7 +1656,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
> >       dst_vq->drv_priv = inst;
> >       dst_vq->buf_struct_size = sizeof(struct venus_buffer);
> >       dst_vq->allow_zero_bytesused = 1;
> > -     dst_vq->min_buffers_needed = 0;
> > +     dst_vq->min_reqbufs_allocation = 0;
> >       dst_vq->dev = inst->core->dev;
> >       dst_vq->lock = &inst->ctx_q_lock;
> >       return vb2_queue_init(dst_vq);
> > diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> > index 44b13696cf82..e399d01c208c 100644
> > --- a/drivers/media/platform/qcom/venus/venc.c
> > +++ b/drivers/media/platform/qcom/venus/venc.c
> > @@ -1398,7 +1398,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
> >       src_vq->drv_priv = inst;
> >       src_vq->buf_struct_size = sizeof(struct venus_buffer);
> >       src_vq->allow_zero_bytesused = 1;
> > -     src_vq->min_buffers_needed = 1;
> > +     src_vq->min_reqbufs_allocation = 1;
>
> So for platform drivers like this it is going to be more difficult to
> determine which meaning min_buffers_needed had: is at least one queued
> buffer needed before you can start streaming, or is this for a minimum
> buffer allocation?

By the way, for stateful decoders, we also have the
V4L2_CID_MIN_BUFFERS_FOR_CAPTURE control, which tells the userspace
the minimum buffers that need to be allocated and the drivers also
should adjust the number requested in REQBUFS to be at least that.

>
> In the case of m2m devices using the v4l2-mem2mem framework it is almost
> certainly the minimum buffer allocation since the m2m framework already
> checks that there are input and output buffers queued (__v4l2_m2m_try_queue).
>
> So just delete the src_vq->min_buffers_needed = 1; line, and there is
> no need to set min_reqbufs_allocation.
>
> >       src_vq->dev = inst->core->dev;
> >       src_vq->lock = &inst->ctx_q_lock;
> >       if (inst->core->res->hfi_version == HFI_VERSION_1XX)
> > @@ -1415,7 +1415,7 @@ static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
> >       dst_vq->drv_priv = inst;
> >       dst_vq->buf_struct_size = sizeof(struct venus_buffer);
> >       dst_vq->allow_zero_bytesused = 1;
> > -     dst_vq->min_buffers_needed = 1;
> > +     dst_vq->min_reqbufs_allocation = 1;
> >       dst_vq->dev = inst->core->dev;
> >       dst_vq->lock = &inst->ctx_q_lock;
> >       return vb2_queue_init(dst_vq);
>
> Ditto.
>
> Regards,
>
>         Hans