| Message ID | 20200226111529.180197-3-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement V4L2_BUF_FLAG_NO_CACHE_* flags | expand |
On 2/26/20 12:15 PM, Sergey Senozhatsky wrote: > Set video buffer cache management flags corresponding to V4L2 cache > flags. > > Both ->prepare() and ->finish() cache management hints should be > passed during this stage (buffer preparation), because there is no > other way for user-space to skip ->finish() cache flush. > > There are two possible alternative approaches: > - The first one is to move cache sync from ->finish() to dqbuf(). > But this breaks some drivers, that need to fix-up buffers before > dequeueing them. > > - The second one is to move ->finish() call from ->done() to dqbuf. It's not clear what the purpose is of describing these alternative approaches. I'd just drop that. It's a bit confusing. > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > --- > .../media/common/videobuf2/videobuf2-v4l2.c | 49 +++++++++++++++++++ > include/media/videobuf2-core.h | 11 +++++ > 2 files changed, 60 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index eb5d5db96552..2a604bd7793a 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -199,6 +199,15 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b > vbuf->request_fd = -1; > vbuf->is_held = false; > > + /* > + * Clear buffer cache flags if queue does not support user space hints. > + * That's to indicate to userspace that these flags won't work. > + */ > + if (!vb2_queue_allows_cache_hints(q)) { > + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_INVALIDATE; > + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_CLEAN; > + } > + > if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { > switch (b->memory) { > case VB2_MEMORY_USERPTR: > @@ -337,6 +346,45 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b > return 0; > } > > +static void set_buffer_cache_hints(struct vb2_queue *q, > + struct vb2_buffer *vb, > + struct v4l2_buffer *b) > +{ > + /* > + * DMA exporter should take care of cache syncs, so we can avoid > + * explicit ->prepare()/->finish() syncs. For other ->memory types > + * we always need ->prepare() or/and ->finish() cache sync. > + */ > + if (q->memory == VB2_MEMORY_DMABUF) { > + vb->need_cache_sync_on_finish = 0; > + vb->need_cache_sync_on_prepare = 0; > + return; > + } > + > + /* > + * Cache sync/invalidation flags are set by default in order to > + * preserve existing behaviour for old apps/drivers. > + */ > + vb->need_cache_sync_on_prepare = 1; > + vb->need_cache_sync_on_finish = 1; > + > + if (!vb2_queue_allows_cache_hints(q)) > + return; > + > + /* > + * ->finish() cache sync can be avoided when queue direction is > + * TO_DEVICE. > + */ > + if (q->dma_dir == DMA_TO_DEVICE) > + vb->need_cache_sync_on_finish = 0; > + > + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE) > + vb->need_cache_sync_on_finish = 0; > + > + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN) > + vb->need_cache_sync_on_prepare = 0; > +} > + > static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, > struct v4l2_buffer *b, bool is_prepare, > struct media_request **p_req) > @@ -381,6 +429,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md > } > > if (!vb->prepared) { > + set_buffer_cache_hints(q, vb, b); > /* Copy relevant information provided by the userspace */ > memset(vbuf->planes, 0, > sizeof(vbuf->planes[0]) * vb->num_planes); > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 4a19170672ac..d307869bfb15 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -632,6 +632,17 @@ struct vb2_queue { > #endif > }; > > +/** > + * vb2_queue_allows_cache_hints() - Return true if the queue allows cache > + * and memory consistency hints. > + * > + * @q: pointer to &struct vb2_queue with videobuf2 queue > + */ > +static inline bool vb2_queue_allows_cache_hints(struct vb2_queue *q) > +{ > + return (q->allow_cache_hints != 0) && (q->memory == VB2_MEMORY_MMAP); Simply to: return q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP; Regards, Hans > +} > + > /** > * vb2_plane_vaddr() - Return a kernel virtual address of a given plane. > * @vb: pointer to &struct vb2_buffer to which the plane in >
On (20/02/27 12:58), Hans Verkuil wrote: [..] > > There are two possible alternative approaches: > > - The first one is to move cache sync from ->finish() to dqbuf(). > > But this breaks some drivers, that need to fix-up buffers before > > dequeueing them. > > > > - The second one is to move ->finish() call from ->done() to dqbuf. > > It's not clear what the purpose is of describing these alternative approaches. > I'd just drop that. It's a bit confusing. OK. > > +/** > > + * vb2_queue_allows_cache_hints() - Return true if the queue allows cache > > + * and memory consistency hints. > > + * > > + * @q: pointer to &struct vb2_queue with videobuf2 queue > > + */ > > +static inline bool vb2_queue_allows_cache_hints(struct vb2_queue *q) > > +{ > > + return (q->allow_cache_hints != 0) && (q->memory == VB2_MEMORY_MMAP); > > Simply to: > > return q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP; > OK. I saw vb2_is_busy() static inline bool vb2_is_busy(struct vb2_queue *q) { return (q->num_buffers > 0); } in the very same header file and concluded that maybe this is the preferred style. -ss
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index eb5d5db96552..2a604bd7793a 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -199,6 +199,15 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b vbuf->request_fd = -1; vbuf->is_held = false; + /* + * Clear buffer cache flags if queue does not support user space hints. + * That's to indicate to userspace that these flags won't work. + */ + if (!vb2_queue_allows_cache_hints(q)) { + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_INVALIDATE; + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_CLEAN; + } + if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { switch (b->memory) { case VB2_MEMORY_USERPTR: @@ -337,6 +346,45 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b return 0; } +static void set_buffer_cache_hints(struct vb2_queue *q, + struct vb2_buffer *vb, + struct v4l2_buffer *b) +{ + /* + * DMA exporter should take care of cache syncs, so we can avoid + * explicit ->prepare()/->finish() syncs. For other ->memory types + * we always need ->prepare() or/and ->finish() cache sync. + */ + if (q->memory == VB2_MEMORY_DMABUF) { + vb->need_cache_sync_on_finish = 0; + vb->need_cache_sync_on_prepare = 0; + return; + } + + /* + * Cache sync/invalidation flags are set by default in order to + * preserve existing behaviour for old apps/drivers. + */ + vb->need_cache_sync_on_prepare = 1; + vb->need_cache_sync_on_finish = 1; + + if (!vb2_queue_allows_cache_hints(q)) + return; + + /* + * ->finish() cache sync can be avoided when queue direction is + * TO_DEVICE. + */ + if (q->dma_dir == DMA_TO_DEVICE) + vb->need_cache_sync_on_finish = 0; + + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE) + vb->need_cache_sync_on_finish = 0; + + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN) + vb->need_cache_sync_on_prepare = 0; +} + static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, struct v4l2_buffer *b, bool is_prepare, struct media_request **p_req) @@ -381,6 +429,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md } if (!vb->prepared) { + set_buffer_cache_hints(q, vb, b); /* Copy relevant information provided by the userspace */ memset(vbuf->planes, 0, sizeof(vbuf->planes[0]) * vb->num_planes); diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 4a19170672ac..d307869bfb15 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -632,6 +632,17 @@ struct vb2_queue { #endif }; +/** + * vb2_queue_allows_cache_hints() - Return true if the queue allows cache + * and memory consistency hints. + * + * @q: pointer to &struct vb2_queue with videobuf2 queue + */ +static inline bool vb2_queue_allows_cache_hints(struct vb2_queue *q) +{ + return (q->allow_cache_hints != 0) && (q->memory == VB2_MEMORY_MMAP); +} + /** * vb2_plane_vaddr() - Return a kernel virtual address of a given plane. * @vb: pointer to &struct vb2_buffer to which the plane in
Set video buffer cache management flags corresponding to V4L2 cache flags. Both ->prepare() and ->finish() cache management hints should be passed during this stage (buffer preparation), because there is no other way for user-space to skip ->finish() cache flush. There are two possible alternative approaches: - The first one is to move cache sync from ->finish() to dqbuf(). But this breaks some drivers, that need to fix-up buffers before dequeueing them. - The second one is to move ->finish() call from ->done() to dqbuf. Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> --- .../media/common/videobuf2/videobuf2-v4l2.c | 49 +++++++++++++++++++ include/media/videobuf2-core.h | 11 +++++ 2 files changed, 60 insertions(+)