[RFC,PATCHv2,03/12] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag

  By setting or clearing V4L2_FLAG_MEMORY_NON_CONSISTENT flag
user-space should be able to set or clear queue's NON_CONSISTENT
->dma_attrs. Queue's ->dma_attrs are passed to the underlying
allocator in __vb2_buf_mem_alloc(), so thus user-space is able
to request vb2 buffer's memory to be either consistent (coherent)
or non-consistent.

Change-Id: Ib333081c482e23c9a89386078293e19c3fd59076
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 Documentation/media/uapi/v4l/buffer.rst | 27 +++++++++++++++++++++++++
 include/uapi/linux/videodev2.h          |  2 ++
 2 files changed, 29 insertions(+)
  

On Tue, Feb 4, 2020 at 11:57 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> By setting or clearing V4L2_FLAG_MEMORY_NON_CONSISTENT flag
> user-space should be able to set or clear queue's NON_CONSISTENT
> ->dma_attrs. Queue's ->dma_attrs are passed to the underlying
> allocator in __vb2_buf_mem_alloc(), so thus user-space is able
> to request vb2 buffer's memory to be either consistent (coherent)
> or non-consistent.
>
> Change-Id: Ib333081c482e23c9a89386078293e19c3fd59076
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  Documentation/media/uapi/v4l/buffer.rst | 27 +++++++++++++++++++++++++
>  include/uapi/linux/videodev2.h          |  2 ++
>  2 files changed, 29 insertions(+)
>
> diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
> index 9149b57728e5..af007daf0591 100644
> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -705,6 +705,33 @@ Buffer Flags
>
>  .. c:type:: v4l2_memory
>
> +Memory Consistency Flags
> +========================
> +
> +.. tabularcolumns:: |p{7.0cm}|p{2.2cm}|p{8.3cm}|
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * .. _`V4L2_FLAG_MEMORY_NON_CONSISTENT`:
> +
> +      - ``V4L2_FLAG_MEMORY_NON_CONSISTENT``
> +      - 0x00000001
> +      - vb2 buffer is allocated either in consistent (it will be automatically
> +       coherent between CPU and bus) or non-consistent memory. The latter
> +       can provide performance gains, for instance CPU cache sync/flush
> +       operations can be avoided if the buffer is accesed by the corresponding
> +       device only and CPU does not read/write to/from that buffer. However,
> +       this requires extra care from the driver -- it must guarantee memory
> +       consistency by issuing cache flush/sync when consistency is needed.
> +       If this flag is set V4L2 will attempt to allocate vb2 buffer in
> +       non-consistent memory. This flag is ignored if queue does not report
> +        :ret:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.

nit: Should the patch adding the capability flag precede this one?

> +
>  enum v4l2_memory
>  ================
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 5f9357dcb060..72efc1c544cd 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -189,6 +189,8 @@ enum v4l2_memory {
>         V4L2_MEMORY_DMABUF           = 4,
>  };
>
> +#define V4L2_FLAG_MEMORY_NON_CONSISTENT                (1 << 0)
> +
>  /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
>  enum v4l2_colorspace {
>         /*
> --
> 2.25.0.341.g760bfbb309-goog
>
  

On 2/4/20 3:56 AM, Sergey Senozhatsky wrote:
> By setting or clearing V4L2_FLAG_MEMORY_NON_CONSISTENT flag
> user-space should be able to set or clear queue's NON_CONSISTENT
> ->dma_attrs. Queue's ->dma_attrs are passed to the underlying
> allocator in __vb2_buf_mem_alloc(), so thus user-space is able
> to request vb2 buffer's memory to be either consistent (coherent)
> or non-consistent.
> 
> Change-Id: Ib333081c482e23c9a89386078293e19c3fd59076
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  Documentation/media/uapi/v4l/buffer.rst | 27 +++++++++++++++++++++++++
>  include/uapi/linux/videodev2.h          |  2 ++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
> index 9149b57728e5..af007daf0591 100644
> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -705,6 +705,33 @@ Buffer Flags
>  
>  .. c:type:: v4l2_memory
>  
> +Memory Consistency Flags

This new part should be added *above* the '.. c:type:: v4l2_memory' line.

You also need to add '.. _memory-flags:' just before this section so that
you can link to it from the create_bufs and reqbufs ioctl descriptions.

> +========================
> +
> +.. tabularcolumns:: |p{7.0cm}|p{2.2cm}|p{8.3cm}|
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * .. _`V4L2_FLAG_MEMORY_NON_CONSISTENT`:
> +
> +      - ``V4L2_FLAG_MEMORY_NON_CONSISTENT``
> +      - 0x00000001
> +      - vb2 buffer is allocated either in consistent (it will be automatically
> +	coherent between CPU and bus) or non-consistent memory. The latter
> +	can provide performance gains, for instance CPU cache sync/flush
> +	operations can be avoided if the buffer is accesed by the corresponding

accesed -> accessed

> +	device only and CPU does not read/write to/from that buffer. However,
> +	this requires extra care from the driver -- it must guarantee memory
> +	consistency by issuing cache flush/sync when consistency is needed.
> +	If this flag is set V4L2 will attempt to allocate vb2 buffer in
> +	non-consistent memory. This flag is ignored if queue does not report
> +        :ret:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.

V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS is not defined. Add that first in a separate
patch.

Also update the current description of the V4L2_BUF_FLAG_NO_CACHE_INVALIDATE/CLEAN
flags to indicate that they are only valid if V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS
is set (that should be done in the patch adding the V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS
capability).

Regards,

	Hans

> +
>  enum v4l2_memory
>  ================
>  
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 5f9357dcb060..72efc1c544cd 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -189,6 +189,8 @@ enum v4l2_memory {
>  	V4L2_MEMORY_DMABUF           = 4,
>  };
>  
> +#define V4L2_FLAG_MEMORY_NON_CONSISTENT		(1 << 0)
> +
>  /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
>  enum v4l2_colorspace {
>  	/*
>
  

On 2/4/20 3:56 AM, Sergey Senozhatsky wrote:
> By setting or clearing V4L2_FLAG_MEMORY_NON_CONSISTENT flag
> user-space should be able to set or clear queue's NON_CONSISTENT
> ->dma_attrs. Queue's ->dma_attrs are passed to the underlying
> allocator in __vb2_buf_mem_alloc(), so thus user-space is able
> to request vb2 buffer's memory to be either consistent (coherent)
> or non-consistent.
> 
> Change-Id: Ib333081c482e23c9a89386078293e19c3fd59076
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  Documentation/media/uapi/v4l/buffer.rst | 27 +++++++++++++++++++++++++
>  include/uapi/linux/videodev2.h          |  2 ++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
> index 9149b57728e5..af007daf0591 100644
> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -705,6 +705,33 @@ Buffer Flags
>  
>  .. c:type:: v4l2_memory
>  
> +Memory Consistency Flags
> +========================
> +
> +.. tabularcolumns:: |p{7.0cm}|p{2.2cm}|p{8.3cm}|
> +
> +.. cssclass:: longtable
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * .. _`V4L2_FLAG_MEMORY_NON_CONSISTENT`:
> +
> +      - ``V4L2_FLAG_MEMORY_NON_CONSISTENT``
> +      - 0x00000001
> +      - vb2 buffer is allocated either in consistent (it will be automatically
> +	coherent between CPU and bus) or non-consistent memory. The latter
> +	can provide performance gains, for instance CPU cache sync/flush
> +	operations can be avoided if the buffer is accesed by the corresponding
> +	device only and CPU does not read/write to/from that buffer. However,
> +	this requires extra care from the driver -- it must guarantee memory
> +	consistency by issuing cache flush/sync when consistency is needed.
> +	If this flag is set V4L2 will attempt to allocate vb2 buffer in
> +	non-consistent memory. This flag is ignored if queue does not report
> +        :ret:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.

This flag only makes sense for the MMAP memory model, right? That should be
documented and checked in the code.

An attempt to use this flag with the wrong memory model should just clear it,
I think (something to test as well in v4l2-compliance).

Regards,

	Hans

> +
>  enum v4l2_memory
>  ================
>  
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 5f9357dcb060..72efc1c544cd 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -189,6 +189,8 @@ enum v4l2_memory {
>  	V4L2_MEMORY_DMABUF           = 4,
>  };
>  
> +#define V4L2_FLAG_MEMORY_NON_CONSISTENT		(1 << 0)
> +
>  /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
>  enum v4l2_colorspace {
>  	/*
>
  

On (20/02/19 09:56), Hans Verkuil wrote:
> > +Memory Consistency Flags
> > +========================
> > +
> > +.. tabularcolumns:: |p{7.0cm}|p{2.2cm}|p{8.3cm}|
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +    :widths:       3 1 4
> > +
> > +    * .. _`V4L2_FLAG_MEMORY_NON_CONSISTENT`:
> > +
> > +      - ``V4L2_FLAG_MEMORY_NON_CONSISTENT``
> > +      - 0x00000001
> > +      - vb2 buffer is allocated either in consistent (it will be automatically
> > +	coherent between CPU and bus) or non-consistent memory. The latter
> > +	can provide performance gains, for instance CPU cache sync/flush
> > +	operations can be avoided if the buffer is accesed by the corresponding
> > +	device only and CPU does not read/write to/from that buffer. However,
> > +	this requires extra care from the driver -- it must guarantee memory
> > +	consistency by issuing cache flush/sync when consistency is needed.
> > +	If this flag is set V4L2 will attempt to allocate vb2 buffer in
> > +	non-consistent memory. This flag is ignored if queue does not report
> > +        :ret:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.
> 
> This flag only makes sense for the MMAP memory model, right? That should be
> documented and checked in the code.

Not all buffer allocators respect DMA attrs (V4L2_FLAG_MEMORY_NON_CONSISTENT
is a DMA attribute) even if we use MMAP memory model. Right? E.g. dma-cont
does, dma-sg - does not.

So the list is:

  a) buffer allocated for MMAP I/O
  b) buffer allocator which does not allocate pages from kernel page
     allocator
  c) queue that supports user space cache hints

If the driver does not set vb2_dma_contig_memops as q->mem_ops then
that queue should not have ->allow_cache_hints set. But even if it does,
the flag is ignored by the allocator.

So maybe the text can be:

+       ................... The flag takes effect only if the buffer is
+       used for :ref:`memory mapping <mmap>` I/O and the queue reports
+       :ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.

	-ss