[v6,00/14] Implement V4L2_BUF_FLAG_NO_CACHE_* flags

Message ID 20200514160153.3646-1-sergey.senozhatsky@gmail.com (mailing list archive)
Headers
Series Implement V4L2_BUF_FLAG_NO_CACHE_* flags |

Message

Sergey Senozhatsky May 14, 2020, 4:01 p.m. UTC
  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.

Previous versions:
v5 link: https://lore.kernel.org/lkml/20200424092920.4801-1-sergey.senozhatsky@gmail.com
v4 link: https://lore.kernel.org/lkml/20200302041213.27662-1-senozhatsky@chromium.org/
v3 link: https://lore.kernel.org/lkml/20200226111529.180197-1-senozhatsky@chromium.org
v2 link: https://lore.kernel.org/lkml/20200204025641.218376-1-senozhatsky@chromium.org/
v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/

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@vger.kernel.org/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(-)
  

Comments

Hans Verkuil May 18, 2020, 3:18 p.m. UTC | #1
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@gmail.com
> v4 link: https://lore.kernel.org/lkml/20200302041213.27662-1-senozhatsky@chromium.org/
> v3 link: https://lore.kernel.org/lkml/20200226111529.180197-1-senozhatsky@chromium.org
> v2 link: https://lore.kernel.org/lkml/20200204025641.218376-1-senozhatsky@chromium.org/
> v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/
> 
> 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@vger.kernel.org/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(-)
>
  
Sergey Senozhatsky May 19, 2020, 8:06 a.m. UTC | #2
Hi Hans,

On (20/05/18 17:18), Hans Verkuil wrote:
> 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!

Hans, Tomasz, Ezequiel, thanks for all the help and guidance.

	-ss