Re: [PATCH v6 00/14] Implement V4L2_BUF_FLAG_NO_CACHE_* flags

From: Hans Verkuil
Date: Mon May 18 2020 - 11:18:43 EST


Hi Sergey,

On 14/05/2020 18:01, Sergey Senozhatsky wrote:
> Hello
>
> v6 changes:
> The design has been slightly reworked. The cache-hints capability has
> been renamed to SUPPORTS_MMAP_CACHE_HINTS and is reported for all queues
> that support MMAP and allow cache hints. However, the actual hints and
> memory consistency are ignored unless the queue is used for the MMAP
> streaming I/O. Plus some cleanups, documentation updates, and so on.

This looks good. If there are no new comments then I plan to make a PR for 5.9 in
two weeks.

Thank you for all your work on this!

Regards,

Hans

>
> Previous versions:
> v5 link: https://lore.kernel.org/lkml/20200424092920.4801-1-sergey.senozhatsky@xxxxxxxxx
> v4 link: https://lore.kernel.org/lkml/20200302041213.27662-1-senozhatsky@xxxxxxxxxxxx/
> v3 link: https://lore.kernel.org/lkml/20200226111529.180197-1-senozhatsky@xxxxxxxxxxxx
> v2 link: https://lore.kernel.org/lkml/20200204025641.218376-1-senozhatsky@xxxxxxxxxxxx/
> v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@xxxxxxxxxxxx/
>
> Series Intro
> ========================================================================
>
> This is a reworked version of the vb2 cache hints
> (V4L2_BUF_FLAG_NO_CACHE_INVALIDATE / V4L2_BUF_FLAG_NO_CACHE_CLEAN)
> support patch series which previsouly was developed by Sakari and
> Laurent [0].
>
> The patch set attempts to preserve the existing behvaiour - cache
> sync is performed in ->prepare() and ->finish() (unless the buffer
> is DMA exported). User space can request âdefault behaviorâ override
> with cache management hints, which are handled on a per-buffer basis
> and should be supplied with v4l2_buffer ->flags during buffer
> preparation. There are two possible hints:
>
> - V4L2_BUF_FLAG_NO_CACHE_INVALIDATE
> No cache sync on ->finish()
>
> - V4L2_BUF_FLAG_NO_CACHE_CLEAN
> No cache sync on ->prepare()
>
> In order to keep things on the safe side, we also require driver
> to explicitly state which of its queues (if any) support user space
> cache management hints (such queues should have ->allow_cache_hints
> bit set).
>
> The patch set also (to some extent) simplifies allocators' ->prepare()
> and ->finish() callbacks. Namely, we move cache management decision
> making to the upper - core - layer. For example, if, previously, we
> would have something like this
>
> vb2_buffer_done()
> vb2_dc_finish()
> if (buf->db_attach)
> return;
>
> where each allocators' ->finish() callback would either bail
> out (DMA exported buffer, for instance) or sync, now that "bail
> out or sync" decision is made before we call into the allocator.
>
> Along with cache management hints, user space is also able to
> adjust queue's memory consistency attributes. Memory consistency
> attribute (dma_attrs) is per-queue, yet it plays its role on the
> allocator level, when we allocate buffersâ private memory (planes).
> For the time being, only one consistency attribute is supported:
> DMA_ATTR_NON_CONSISTENT.
>
> [0] https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg112459.html
>
> Sergey Senozhatsky (14):
> videobuf2: use explicit unsigned int in vb2_queue
> videobuf2: add cache management members
> videobuf2: handle V4L2 buffer cache flags
> videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
> videobuf2: add queue memory consistency parameter
> videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
> videobuf2: factor out planes prepare/finish functions
> videobuf2: do not sync caches when we are allowed not to
> videobuf2: check ->synced flag in prepare() and finish()
> videobuf2: add begin/end cpu_access callbacks to dma-contig
> videobuf2: add begin/end cpu_access callbacks to dma-sg
> videobuf2: don't test db_attach in dma-contig prepare and finish
> videobuf2: remove redundant if-statement
> media: vivid: add cache_hints module param
>
> Documentation/admin-guide/media/vivid.rst | 9 ++
> .../userspace-api/media/v4l/buffer.rst | 40 +++++-
> .../media/v4l/vidioc-create-bufs.rst | 7 +-
> .../media/v4l/vidioc-reqbufs.rst | 21 ++-
> .../media/common/videobuf2/videobuf2-core.c | 121 +++++++++++++-----
> .../common/videobuf2/videobuf2-dma-contig.c | 44 ++++++-
> .../media/common/videobuf2/videobuf2-dma-sg.c | 38 ++++--
> .../media/common/videobuf2/videobuf2-v4l2.c | 72 ++++++++++-
> drivers/media/dvb-core/dvb_vb2.c | 2 +-
> drivers/media/test-drivers/vivid/vivid-core.c | 9 ++
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 10 +-
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 +-
> include/media/videobuf2-core.h | 47 +++++--
> include/uapi/linux/videodev2.h | 14 +-
> 14 files changed, 366 insertions(+), 73 deletions(-)
>