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

From: Hans Verkuil <hverkuil@xs4all.nl>
To: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Tomasz Figa <tfiga@chromium.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Pawel Osciak <posciak@chromium.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCHv2 03/12] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
Date: Wed, 19 Feb 2020 09:56:09 +0100	[thread overview]
Message-ID: <17e99adc-d86d-ebd3-ab52-d6c8e3ac14f3@xs4all.nl> (raw)
In-Reply-To: <20200204025641.218376-4-senozhatsky@chromium.org>

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 {
>  	/*
> 