On Tue, Nov 28, 2023 at 7:26 PM Hans Verkuil <hverkuil@xxxxxxxxx> 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@xxxxxxxxxxxxx> > > CC: Stanimir Varbanov <stanimir.k.varbanov@xxxxxxxxx> (maintainer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER) > > CC: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx> (maintainer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER) > > CC: "Bryan O'Donoghue" <bryan.odonoghue@xxxxxxxxxx> (reviewer:QUALCOMM VENUS VIDEO ACCELERATOR DRIVER) > > CC: Andy Gross <agross@xxxxxxxxxx> (maintainer:ARM/QUALCOMM SUPPORT) > > CC: Bjorn Andersson <andersson@xxxxxxxxxx> (maintainer:ARM/QUALCOMM SUPPORT) > > CC: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> (maintainer:ARM/QUALCOMM SUPPORT) > > CC: linux-arm-msm@xxxxxxxxxxxxxxx > > --- > > 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
