Le 24/01/2024 à 16:44, Hans Verkuil a écrit :
On 24/01/2024 16:35, Benjamin Gaignard wrote:
Le 24/01/2024 à 13:52, Hans Verkuil a écrit :
On 19/01/2024 10:49, Benjamin Gaignard wrote:
Allow to delete buffers on capture queue because it the one which
own the decoded buffers. After a dynamic resolution change lot of
them could remain allocated but won't be used anymore so deleting
them save memory.
Do not add this feature on output queue because the buffers are
smaller, fewer and always recycled even after a dynamic resolution
change.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
---
drivers/media/platform/verisilicon/hantro_drv.c | 1 +
drivers/media/platform/verisilicon/hantro_v4l2.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index db3df6cc4513..f6b0a676a740 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -248,6 +248,7 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = &ctx->dev->vpu_mutex;
dst_vq->dev = ctx->dev->v4l2_dev.dev;
+ src_vq->supports_delete_bufs = true;
As I mentioned, I remain unconvinced by this. It is just making the API inconsistent
since if you support delete_bufs, then why support it for one queue only and not both?
Because the both queues don't handle the same type of data.
For example for a stateless decoder, for me, it makes sense to allow delete decoded frames
if they won't be used anymore but that won't makes sense for bitstream buffers.
But is there any reason why you wouldn't support this feature? We support VIDIOC_CREATE_BUFS
as well, even though most drivers do not need it, but it is cheap to add.
Deleting buffers is a generic feature, and I don't see why you wouldn't just offer it
for both queues.
You want me to remove supports_delete_bufs and V4L2_BUF_CAP_SUPPORTS_DELETE_BUFS ?
This way we can remove buffers from the both queues.
Sound good for you ?
Regards,
Benjamin
Regards,
Hans
return vb2_queue_init(dst_vq);
}
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 941fa23c211a..34eab90e8a42 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -756,6 +756,7 @@ const struct v4l2_ioctl_ops hantro_ioctl_ops = {
.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
+ .vidioc_delete_bufs = v4l2_m2m_ioctl_delete_bufs,
.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
In my view setting vidioc_delete_bufs should enable this feature, and if
for some strange reason only one queue support it, then make a wrapper
callback that returns an error when used with the wrong queue.
Also note that patch 6/8 never checks for q->supports_delete_bufs in
vb2_core_delete_bufs(), which is wrong!
I will fix that in next version.
Regards,
Benjamin
Regards,
Hans
