Commit 129134e5 authored by Sergey Senozhatsky's avatar Sergey Senozhatsky Committed by Mauro Carvalho Chehab
Browse files

media: media/v4l2: remove V4L2_FLAG_MEMORY_NON_CONSISTENT flag 



The patch partially reverts some of the UAPI bits of the buffer
cache management hints. Namely, the queue consistency (memory
coherency) user-space hint because, as it turned out, the kernel
implementation of this feature was misusing DMA_ATTR_NON_CONSISTENT.

The patch reverts both kernel and user space parts: removes the
DMA consistency attr functions, rolls back changes to v4l2_requestbuffers,
v4l2_create_buffers structures and corresponding UAPI functions
(plus compat32 layer) and cleans up the documentation.

[hverkuil: fixed a few typos in the commit log]
[hverkuil: fixed vb2_core_reqbufs call in drivers/media/dvb-core/dvb_vb2.c]
[mchehab: fixed a typo in the commit log: revers->reverts]

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+huawei@kernel.org>
parent 288eceb0

Documentation/userspace-api/media/v4l/buffer.rst

+0 −17
Original line number Diff line number Diff line
@@ -701,23 +701,6 @@ Memory Consistency Flags 
    :stub-columns: 0

    :widths:       3 1 4



    * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`:



      - ``V4L2_FLAG_MEMORY_NON_CONSISTENT``

      - 0x00000001

      - A buffer is allocated either in consistent (it will be automatically

	coherent between the CPU and the bus) or non-consistent memory. The

	latter can provide performance gains, for instance the CPU cache

	sync/flush operations can be avoided if the buffer is accessed by the

	corresponding device only and the CPU does not read/write to/from that

	buffer. However, this requires extra care from the driver -- it must

	guarantee memory consistency by issuing a cache flush/sync when

	consistency is needed. If this flag is set V4L2 will attempt to

	allocate the buffer in non-consistent memory. The flag takes effect

	only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the

	queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS

	<V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.



.. c:type:: v4l2_memory



enum v4l2_memory

Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst

+1 −5
Original line number Diff line number Diff line
@@ -120,13 +120,9 @@ than the number requested. 
	If you want to just query the capabilities without making any

	other changes, then set ``count`` to 0, ``memory`` to

	``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.

    * - __u32

      - ``flags``

      - Specifies additional buffer management attributes.

	See :ref:`memory-flags`.



    * - __u32

      - ``reserved``\ [6]

      - ``reserved``\ [7]

      - A place holder for future extensions. Drivers and applications

	must set the array to zero.

Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst

+2 −10
Original line number Diff line number Diff line
@@ -112,17 +112,10 @@ aborting or finishing any DMA in progress, an implicit 
	``V4L2_MEMORY_MMAP`` and ``type`` set to the buffer type. This will

	free any previously allocated buffers, so this is typically something

	that will be done at the start of the application.

    * - union {

      - (anonymous)

    * - __u32

      - ``flags``

      - Specifies additional buffer management attributes.

	See :ref:`memory-flags`.

    * - __u32

      - ``reserved``\ [1]

      - Kept for backwards compatibility. Use ``flags`` instead.

    * - }

      -

      - A place holder for future extensions. Drivers and applications

	must set the array to zero.



.. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
@@ -169,7 +162,6 @@ aborting or finishing any DMA in progress, an implicit 
      - This capability is set by the driver to indicate that the queue supports

        cache and memory management hints. However, it's only valid when the

        queue is used for :ref:`memory mapping <mmap>` streaming I/O. See

        :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT <V4L2-FLAG-MEMORY-NON-CONSISTENT>`,

        :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE <V4L2-BUF-FLAG-NO-CACHE-INVALIDATE>` and

        :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN <V4L2-BUF-FLAG-NO-CACHE-CLEAN>`.

drivers/media/common/videobuf2/videobuf2-core.c

+6 −40
Original line number Diff line number Diff line
@@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, 
}

EXPORT_SYMBOL(vb2_verify_memory_type);



static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem)

{

	q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT;



	if (!vb2_queue_allows_cache_hints(q))

		return;

	if (!consistent_mem)

		q->dma_attrs |= DMA_ATTR_NON_CONSISTENT;

}



static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem)

{

	bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT);



	if (consistent_mem != queue_is_consistent) {

		dprintk(q, 1, "memory consistency model mismatch\n");

		return false;

	}

	return true;

}



int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,

		     unsigned int flags, unsigned int *count)

		     unsigned int *count)

{

	unsigned int num_buffers, allocated_buffers, num_planes = 0;

	unsigned plane_sizes[VB2_MAX_PLANES] = { };

	bool consistent_mem = true;

	unsigned int i;

	int ret;



	if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)

		consistent_mem = false;



	if (q->streaming) {

		dprintk(q, 1, "streaming active\n");

		return -EBUSY;
@@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, 
	}



	if (*count == 0 || q->num_buffers != 0 ||

	    (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) ||

	    !verify_consistency_attr(q, consistent_mem)) {

	    (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) {

		/*

		 * We already have buffers allocated, so first check if they

		 * are not in use and can be freed.
@@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, 
	num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME);

	memset(q->alloc_devs, 0, sizeof(q->alloc_devs));

	q->memory = memory;

	set_queue_consistency(q, consistent_mem);



	/*

	 * Ask the driver how many buffers and planes per buffer it requires.
@@ -888,18 +861,14 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, 
EXPORT_SYMBOL_GPL(vb2_core_reqbufs);



int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,

			 unsigned int flags, unsigned int *count,

			 unsigned int *count,

			 unsigned int requested_planes,

			 const unsigned int requested_sizes[])

{

	unsigned int num_planes = 0, num_buffers, allocated_buffers;

	unsigned plane_sizes[VB2_MAX_PLANES] = { };

	bool consistent_mem = true;

	int ret;



	if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)

		consistent_mem = false;



	if (q->num_buffers == VB2_MAX_FRAME) {

		dprintk(q, 1, "maximum number of buffers already allocated\n");

		return -ENOBUFS;
@@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, 
		}

		memset(q->alloc_devs, 0, sizeof(q->alloc_devs));

		q->memory = memory;

		set_queue_consistency(q, consistent_mem);

		q->waiting_for_buffers = !q->is_output;

	} else {

		if (q->memory != memory) {

			dprintk(q, 1, "memory model mismatch\n");

			return -EINVAL;

		}

		if (!verify_consistency_attr(q, consistent_mem))

			return -EINVAL;

	}



	num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
@@ -2581,7 +2547,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) 
	fileio->memory = VB2_MEMORY_MMAP;

	fileio->type = q->type;

	q->fileio = fileio;

	ret = vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);

	ret = vb2_core_reqbufs(q, fileio->memory, &fileio->count);

	if (ret)

		goto err_kfree;
@@ -2638,7 +2604,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) 


err_reqbufs:

	fileio->count = 0;

	vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);

	vb2_core_reqbufs(q, fileio->memory, &fileio->count);



err_kfree:

	q->fileio = NULL;
@@ -2658,7 +2624,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q) 
		vb2_core_streamoff(q, q->type);

		q->fileio = NULL;

		fileio->count = 0;

		vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);

		vb2_core_reqbufs(q, fileio->memory, &fileio->count);

		kfree(fileio);

		dprintk(q, 3, "file io emulator closed\n");

	}

drivers/media/common/videobuf2/videobuf2-dma-contig.c

+0 −19
Original line number Diff line number Diff line
@@ -42,11 +42,6 @@ struct vb2_dc_buf { 
	struct dma_buf_attachment	*db_attach;

};



static inline bool vb2_dc_buffer_consistent(unsigned long attr)

{

	return !(attr & DMA_ATTR_NON_CONSISTENT);

}



/*********************************************/

/*        scatterlist table functions        */

/*********************************************/
@@ -341,13 +336,6 @@ static int 
vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf,

				   enum dma_data_direction direction)

{

	struct vb2_dc_buf *buf = dbuf->priv;

	struct sg_table *sgt = buf->dma_sgt;



	if (vb2_dc_buffer_consistent(buf->attrs))

		return 0;



	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);

	return 0;

}
@@ -355,13 +343,6 @@ static int 
vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf,

				 enum dma_data_direction direction)

{

	struct vb2_dc_buf *buf = dbuf->priv;

	struct sg_table *sgt = buf->dma_sgt;



	if (vb2_dc_buffer_consistent(buf->attrs))

		return 0;



	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);

	return 0;

}