Re: [PATCH 7/8] videobuf2: handle V4L2_FLAG_MEMORY_NON_COHERENT flag
On 02/03/2021 01:46, Sergey Senozhatsky wrote:
> This patch lets user-space to request a non-coherent memory
> allocation during CREATE_BUFS and REQBUFS ioctl calls.
>
> = CREATE_BUFS
>
> struct v4l2_create_buffers has seven 4-byte reserved areas,
> so reserved[0] is renamed to ->flags. The struct, thus, now
> has six reserved 4-byte regions.
>
> = CREATE_BUFS32
>
> struct v4l2_create_buffers32 has seven 4-byte reserved areas,
> so reserved[0] is renamed to ->flags. The struct, thus, now
> has six reserved 4-byte regions.
>
> = REQBUFS
>
> We use one bit of a ->reserved[1] member of struct v4l2_requestbuffers,
> which is now renamed to ->flags. Unlike v4l2_create_buffers, struct
> v4l2_requestbuffers does not have enough reserved room. Therefore for
> backward compatibility ->reserved and ->flags were put into anonymous
> union.
>
> Signed-off-by: Sergey Senozhatsky
> ---
> .../media/v4l/vidioc-create-bufs.rst | 7 ++-
> .../media/v4l/vidioc-reqbufs.rst | 11 +--
> .../media/common/videobuf2/videobuf2-core.c | 6 ++
> .../media/common/videobuf2/videobuf2-v4l2.c | 19 ---
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 -
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 +
> include/uapi/linux/videodev2.h| 11 +--
> 7 files changed, 55 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> index b06e5b528e11..132c8b612a94 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> @@ -113,7 +113,12 @@ than the number requested.
> ``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.
>
> * - __u32
> - - ``reserved``\ [7]
> + - ``flags``
> + - Specifies additional buffer management attributes.
> + See :ref:`memory-flags`.
> +
> +* - __u32
> + - ``reserved``\ [6]
>- A place holder for future extensions. Drivers and applications
> must set the array to zero.
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> index 950e7ec1aac5..80ea48acea84 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> @@ -104,10 +104,17 @@ 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]
> - - A place holder for future extensions. Drivers and applications
> - must set the array to zero.
> + - Kept for backwards compatibility. Use ``flags`` instead.
> +* - }
> + -
>
> .. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 7040b7f47133..5906a48e7757 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -768,6 +768,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> unsigned int i;
> int ret;
>
> + if (flags & V4L2_FLAG_MEMORY_NON_COHERENT)
> + coherent_mem = false;
> +
I think it is better if this is done at the bool coherent_mem declaration:
bool coherent_mem = !(flags & V4L2_FLAG_MEMORY_NON_COHERENT);
> if (q->streaming) {
> dprintk(q, 1, "streaming active\n");
> return -EBUSY;
> @@ -911,6 +914,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum
> vb2_memory memory,
> bool coherent_mem = true;
> int ret;
>
> + if (flags & V4L2_FLAG_MEMORY_NON_COHERENT)
> + coherent_mem = false;
> +
Ditto.
Regards,
Hans
> if (q->num_buffers == VB2_MAX_FRAME) {
> dprintk(q, 1, "maximum number of buffers already allocated\n");
> return -ENOBUFS;
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 1166d5a9291a..f6a8dcc1b5c6 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -692,12 +692,22 @@ static void fill_buf_caps(struct vb2_queue *q, u32
> *caps)
> #endif
> }
>
> +static void validate_coherency_flags(struct vb2_queue *q,
> + int memory,
> + unsigned
Re: [PATCH 7/8] videobuf2: handle V4L2_FLAG_MEMORY_NON_COHERENT flag
On 02/03/2021 01:46, Sergey Senozhatsky wrote:
> This patch lets user-space to request a non-coherent memory
> allocation during CREATE_BUFS and REQBUFS ioctl calls.
>
> = CREATE_BUFS
>
> struct v4l2_create_buffers has seven 4-byte reserved areas,
> so reserved[0] is renamed to ->flags. The struct, thus, now
> has six reserved 4-byte regions.
>
> = CREATE_BUFS32
>
> struct v4l2_create_buffers32 has seven 4-byte reserved areas,
> so reserved[0] is renamed to ->flags. The struct, thus, now
> has six reserved 4-byte regions.
>
> = REQBUFS
>
> We use one bit of a ->reserved[1] member of struct v4l2_requestbuffers,
> which is now renamed to ->flags. Unlike v4l2_create_buffers, struct
> v4l2_requestbuffers does not have enough reserved room. Therefore for
> backward compatibility ->reserved and ->flags were put into anonymous
> union.
>
> Signed-off-by: Sergey Senozhatsky
> ---
> .../media/v4l/vidioc-create-bufs.rst | 7 ++-
> .../media/v4l/vidioc-reqbufs.rst | 11 +--
> .../media/common/videobuf2/videobuf2-core.c | 6 ++
> .../media/common/videobuf2/videobuf2-v4l2.c | 19 ---
> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 -
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 +
> include/uapi/linux/videodev2.h| 11 +--
> 7 files changed, 55 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> index b06e5b528e11..132c8b612a94 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> @@ -113,7 +113,12 @@ than the number requested.
> ``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.
>
> * - __u32
> - - ``reserved``\ [7]
> + - ``flags``
> + - Specifies additional buffer management attributes.
> + See :ref:`memory-flags`.
> +
> +* - __u32
> + - ``reserved``\ [6]
>- A place holder for future extensions. Drivers and applications
> must set the array to zero.
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> index 950e7ec1aac5..80ea48acea84 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
> @@ -104,10 +104,17 @@ 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]
> - - A place holder for future extensions. Drivers and applications
> - must set the array to zero.
> + - Kept for backwards compatibility. Use ``flags`` instead.
> +* - }
> + -
I would do this differently. Replace the __u32 reserved[1] by:
__u8 flags;
__u8 reserved[3];
We only need a single bit for flags, so this way we still have a few reserved
bytes
available.
>
> .. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 7040b7f47133..5906a48e7757 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -768,6 +768,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
> memory,
> unsigned int i;
> int ret;
>
> + if (flags & V4L2_FLAG_MEMORY_NON_COHERENT)
> + coherent_mem = false;
> +
> if (q->streaming) {
> dprintk(q, 1, "streaming active\n");
> return -EBUSY;
> @@ -911,6 +914,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum
> vb2_memory memory,
> bool coherent_mem = true;
> int ret;
>
> + if (flags & V4L2_FLAG_MEMORY_NON_COHERENT)
> + coherent_mem = false;
> +
> if (q->num_buffers == VB2_MAX_FRAME) {
> dprintk(q, 1, "maximum number of buffers already allocated\n");
> return -ENOBUFS;
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 1166d5a9291a..f6a8dcc1b5c6 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -692,12 +692,22 @@ static void fill_buf_caps(struct vb2_queue *q, u32
> *caps)
> #endif
> }
>
> +static void validate_coherency_flags(struct vb2_queue *q,
This should validate flags in general, not just the single
[PATCH 7/8] videobuf2: handle V4L2_FLAG_MEMORY_NON_COHERENT flag
This patch lets user-space to request a non-coherent memory
allocation during CREATE_BUFS and REQBUFS ioctl calls.
= CREATE_BUFS
struct v4l2_create_buffers has seven 4-byte reserved areas,
so reserved[0] is renamed to ->flags. The struct, thus, now
has six reserved 4-byte regions.
= CREATE_BUFS32
struct v4l2_create_buffers32 has seven 4-byte reserved areas,
so reserved[0] is renamed to ->flags. The struct, thus, now
has six reserved 4-byte regions.
= REQBUFS
We use one bit of a ->reserved[1] member of struct v4l2_requestbuffers,
which is now renamed to ->flags. Unlike v4l2_create_buffers, struct
v4l2_requestbuffers does not have enough reserved room. Therefore for
backward compatibility ->reserved and ->flags were put into anonymous
union.
Signed-off-by: Sergey Senozhatsky
---
.../media/v4l/vidioc-create-bufs.rst | 7 ++-
.../media/v4l/vidioc-reqbufs.rst | 11 +--
.../media/common/videobuf2/videobuf2-core.c | 6 ++
.../media/common/videobuf2/videobuf2-v4l2.c | 19 ---
drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 -
drivers/media/v4l2-core/v4l2-ioctl.c | 5 +
include/uapi/linux/videodev2.h| 11 +--
7 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
index b06e5b528e11..132c8b612a94 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
@@ -113,7 +113,12 @@ than the number requested.
``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.
* - __u32
- - ``reserved``\ [7]
+ - ``flags``
+ - Specifies additional buffer management attributes.
+ See :ref:`memory-flags`.
+
+* - __u32
+ - ``reserved``\ [6]
- A place holder for future extensions. Drivers and applications
must set the array to zero.
diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
index 950e7ec1aac5..80ea48acea84 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
@@ -104,10 +104,17 @@ 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]
- - A place holder for future extensions. Drivers and applications
- must set the array to zero.
+ - Kept for backwards compatibility. Use ``flags`` instead.
+* - }
+ -
.. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
b/drivers/media/common/videobuf2/videobuf2-core.c
index 7040b7f47133..5906a48e7757 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -768,6 +768,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory
memory,
unsigned int i;
int ret;
+ if (flags & V4L2_FLAG_MEMORY_NON_COHERENT)
+ coherent_mem = false;
+
if (q->streaming) {
dprintk(q, 1, "streaming active\n");
return -EBUSY;
@@ -911,6 +914,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum
vb2_memory memory,
bool coherent_mem = true;
int ret;
+ if (flags & V4L2_FLAG_MEMORY_NON_COHERENT)
+ coherent_mem = false;
+
if (q->num_buffers == VB2_MAX_FRAME) {
dprintk(q, 1, "maximum number of buffers already allocated\n");
return -ENOBUFS;
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 1166d5a9291a..f6a8dcc1b5c6 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -692,12 +692,22 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
#endif
}
+static void validate_coherency_flags(struct vb2_queue *q,
+int memory,
+unsigned int *flags)
+{
+ if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP)
+ *flags &= ~V4L2_FLAG_MEMORY_NON_COHERENT;
+}
+
int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
{
int ret = vb2_verify_memory_type(q, req->memory, req->type);
fill_buf_caps(q, >capabilities);
- return ret ? ret : vb2_core_reqbufs(q, req->memory, 0, >count);
+
