Re: [PATCH v14 53/56] media: core: Free range of buffers
From: Tomasz Figa <tfiga@chromium.org>
Date: 2023-11-09 09:10:28
Also in:
linux-arm-msm, linux-media, linux-mediatek, linux-rockchip, linux-staging, lkml
On Tue, Oct 31, 2023 at 05:31:01PM +0100, Benjamin Gaignard wrote:
quoted hunk ↗ jump to hunk
Improve __vb2_queue_free() and __vb2_free_mem() to free range of buffers and not only the last few buffers. Intoduce starting index to be flexible on range and change the loops according to this parameters. Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- .../media/common/videobuf2/videobuf2-core.c | 59 +++++++++---------- 1 file changed, 28 insertions(+), 31 deletions(-)diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 6e88406fcae9..010a8bca0240 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c@@ -519,15 +519,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, /* * __vb2_free_mem() - release all video buffer memory for a given queue
This comment is kind of outdated. Maybe we should replace it with release video buffer memory for a given range of buffers in a given queue ?
*/
-static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers)
+static void __vb2_free_mem(struct vb2_queue *q, unsigned int start, unsigned int count)
{
- unsigned int buffer;
+ unsigned int i;
struct vb2_buffer *vb;
- unsigned int q_num_buffers = vb2_get_num_buffers(q);
- for (buffer = q_num_buffers - buffers; buffer < q_num_buffers;
- ++buffer) {
- vb = vb2_get_buffer(q, buffer);
+ for (i = start; i < q->max_num_buffers && i < start + count; i++) {We could make this (and all those numerous simialr iterations) more efficient by using bitmap helpers (probably wrapped in some vb2 helpers), e.g. for_each_set_bit_from() (vb2_for_each_buffer_from()?). It can be done in a follow up patch separately from this series though.
quoted hunk ↗ jump to hunk
+ vb = vb2_get_buffer(q, i); if (!vb) continue;@@ -542,35 +540,35 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) } /* - * __vb2_queue_free() - free buffers at the end of the queue - video memory and + * __vb2_queue_free() - free count buffers from start index of the queue - video memory and
nit: How about using the @count and @start notation to refer the argument names? (Can be done with a follow up patch outside of this series later.)
* related information, if no buffers are left return the queue to an
* uninitialized state. Might be called even if the queue has already been freed.
*/
-static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
+static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned int count)
{
- unsigned int buffer;
- unsigned int q_num_buffers = vb2_get_num_buffers(q);
+ unsigned int i;
lockdep_assert_held(&q->mmap_lock);
/* Call driver-provided cleanup function for each buffer, if provided */
- for (buffer = q_num_buffers - buffers; buffer < q_num_buffers;
- ++buffer) {
- struct vb2_buffer *vb = vb2_get_buffer(q, buffer);
+ for (i = start; i < q->max_num_buffers && i < start + count; i++) {
+ struct vb2_buffer *vb = vb2_get_buffer(q, i);
- if (vb && vb->planes[0].mem_priv)
+ if (!vb)
+ continue;
+ if (vb->planes[0].mem_priv)nit: Not sure if we really had to change this, but I'm fine with either. Best regards, Tomasz _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel