lkml.org 
[lkml]   [2020]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][PATCH 05/15] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT in REQBUFS
    From
    Date
    On 12/17/19 4:20 AM, Sergey Senozhatsky wrote:
    > This patch lets user-space to request a non-consistent memory
    > allocation during REQBUFS ioctl call. We use one bit of a
    > ->reserved[1] member of struct v4l2_requestbuffers, which is
    > now renamed to ->flags.
    >
    > There is just 1 four-byte reserved area in v4l2_requestbuffers
    > struct, therefore for backward compatibility ->reserved and
    > ->flags were put into anonymous union.
    >
    > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
    > ---
    > Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 14 ++++++++++++--
    > drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++--
    > drivers/media/v4l2-core/v4l2-ioctl.c | 3 ---
    > include/uapi/linux/videodev2.h | 5 ++++-
    > 4 files changed, 28 insertions(+), 8 deletions(-)
    >
    > diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
    > index d0c643db477a..9b69a61d9fd4 100644
    > --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
    > +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
    > @@ -112,10 +112,20 @@ 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.
    > - * - __u32
    > + * - union
    > + - (anonymous)
    > + * -
    > + - __u32
    > + - ``flags``\ [1]
    > + - Specifies additional buffer management attributes. E.g. when
    > + ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` set vb2 backends may be allocated
    > + in non-consistent memory.

    This should link to the table with these memory flags, rather than
    effectively documenting V4L2_FLAG_MEMORY_NON_CONSISTENT again.

    You also probably meant "vb2 buffers" rather than "vb2 backends".

    > + * -
    > + - __u32
    > - ``reserved``\ [1]
    > - A place holder for future extensions. Drivers and applications
    > - must set the array to zero.
    > + must set the array to zero, unless application wants to specify
    > + buffer management ``flags``.

    I think support for this flag should be signaled as a V4L2_BUF_CAP capability.
    If the capability is not set, then vb2 should set 'flags' to 0 to preserve the
    old 'Drivers and applications must set the array to zero' behavior.

    The documentation for 'reserved[1]' should be changed to something like:

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

    Regards,

    Hans
    >
    > .. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
    >
    > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
    > index f1e88c9398c7..0eabb589684f 100644
    > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
    > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
    > @@ -693,9 +693,15 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
    > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
    > {
    > int ret = vb2_verify_memory_type(q, req->memory, req->type);
    > + bool consistent = true;
    > +
    > + if (req->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
    > + consistent = false;
    >
    > fill_buf_caps(q, &req->capabilities);
    > - return ret ? ret : vb2_core_reqbufs(q, req->memory, true, &req->count);
    > + if (ret)
    > + return ret;
    > + return vb2_core_reqbufs(q, req->memory, consistent, &req->count);
    > }
    > EXPORT_SYMBOL_GPL(vb2_reqbufs);
    >
    > @@ -939,13 +945,17 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
    > {
    > struct video_device *vdev = video_devdata(file);
    > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
    > + bool consistent = true;
    >
    > fill_buf_caps(vdev->queue, &p->capabilities);
    > if (res)
    > return res;
    > if (vb2_queue_is_busy(vdev, file))
    > return -EBUSY;
    > - res = vb2_core_reqbufs(vdev->queue, p->memory, true, &p->count);
    > + if (p->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
    > + consistent = false;
    > +
    > + res = vb2_core_reqbufs(vdev->queue, p->memory, consistent, &p->count);
    > /* If count == 0, then the owner has released all buffers and he
    > is no longer owner of the queue. Otherwise we have a new owner. */
    > if (res == 0)
    > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
    > index 003b7422aeef..225d06819bce 100644
    > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
    > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
    > @@ -1973,9 +1973,6 @@ static int v4l_reqbufs(const struct v4l2_ioctl_ops *ops,
    >
    > if (ret)
    > return ret;
    > -
    > - CLEAR_AFTER_FIELD(p, capabilities);
    > -
    > return ops->vidioc_reqbufs(file, fh, p);
    > }
    >
    > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
    > index d352997f2b62..73a4854f71bd 100644
    > --- a/include/uapi/linux/videodev2.h
    > +++ b/include/uapi/linux/videodev2.h
    > @@ -919,7 +919,10 @@ struct v4l2_requestbuffers {
    > __u32 type; /* enum v4l2_buf_type */
    > __u32 memory; /* enum v4l2_memory */
    > __u32 capabilities;
    > - __u32 reserved[1];
    > + union {
    > + __u32 flags;
    > + __u32 reserved[1];
    > + };
    > };
    >
    > /* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
    >

    \
     
     \ /
      Last update: 2020-01-10 10:57    [W:3.210 / U:0.336 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site