lkml.org 
[lkml]   [2020]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC][PATCH 06/15] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT in CREATE_BUFS
On (20/01/23 12:41), Hans Verkuil wrote:
[..]
> >>>
> >>> fill_buf_caps(q, &create->capabilities);
> >>> @@ -775,7 +776,11 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> >>> for (i = 0; i < requested_planes; i++)
> >>> if (requested_sizes[i] == 0)
> >>> return -EINVAL;
> >>> - return ret ? ret : vb2_core_create_bufs(q, create->memory,
> >>> +
> >>> + if (create->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
> >>> + consistent = false;
> >>> +
> >>> + return ret ? ret : vb2_core_create_bufs(q, create->memory, consistent,
> >>> &create->count, requested_planes, requested_sizes);
> >>
> >> As mentioned before: we need a V4L2_BUF_CAP capability.
> >
> > I can add V4L2_BUF_CAP for memory consistency. Isn't it just q->memory
> > property though? User space may request MMAP consistent memory or MMAP
> > inconsistent memory.
>
> So instead of adding a flag we add a V4L2_MEMORY_MMAP_NON_CONSISTENT memory
> type and add a V4L2_BUF_CAP_SUPPORTS_MMAP_NON_CONSISTENT to signal support
> for this?
>
> I like that better than a flag. It also automatically enforces that all
> buffers must be of that type.

Yes, we had this idea as well. The conclusion was it makes the patch
set bigger and harder to verify and review. Passing memory consistency
attribute via ->flags was the shortest path at the end. Namely due to all
those numerous places that test q->memory:

455 if (q->memory == VB2_MEMORY_MMAP)
456 __vb2_buf_mem_free(vb);
457 else if (q->memory == VB2_MEMORY_DMABUF)
458 __vb2_buf_dmabuf_put(vb);
459 else
460 __vb2_buf_userptr_put(vb);

[..]

737 mutex_lock(&q->mmap_lock);
738 if (debug && q->memory == VB2_MEMORY_MMAP &&
739 __buffers_in_use(q))
740 dprintk(1, "memory in use, orphaning buffers\n");

[..]

etc.

As a workaround we looked at the idea that V4L2_MEMORY_MMAP_NON_CONSISTENT
flag might make sense only on the very high level - when user space requests
V4L2_MEMORY_MMAP_NON_CONSISTENT then we simply set DMA attribute and then
"downgrade" requested V4L2_MEMORY_MMAP_NON_CONSISTENT memory type to
V4L2_MEMORY_MMAP.


Something like this.

---

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 4489744fbbd9..60afbfcca995 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -803,6 +803,14 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
}
EXPORT_SYMBOL_GPL(vb2_core_reqbufs);

+static void __set_queue_consistency(struct vb2_queue *q, bool consistent_mem)
+{
+ if (consistent_mem)
+ q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT;
+ else
+ q->dma_attrs |= DMA_ATTR_NON_CONSISTENT;
+}
+
int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int *count, unsigned requested_planes,
const unsigned requested_sizes[])
@@ -810,6 +818,10 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int num_planes = 0, num_buffers, allocated_buffers;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
int ret;
+ bool consistent_mem = (memory == V4L2_MEMORY_MMAP_NON_CONSISTENT);
+
+ if (consistent_mem)
+ memory = V4L2_MEMORY_MMAP;

if (q->num_buffers == VB2_MAX_FRAME) {
dprintk(1, "maximum number of buffers already allocated\n");
@@ -822,6 +834,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
return -EBUSY;
}
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
+ __set_queue_consistency(q, consistent_mem);
q->memory = memory;
q->waiting_for_buffers = !q->is_output;
} else if (q->memory != memory) {
\
 
 \ /
  Last update: 2020-01-24 02:28    [W:0.067 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site