From: Tomasz Figa <tfiga@chromium.org>
To: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Ricardo Ribalda <ribalda@chromium.org>,
Christoph Hellwig <hch@lst.de>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
Collabora Kernel ML <kernel@collabora.com>
Subject: Re: [PATCHv3 7/8] videobuf2: handle V4L2_MEMORY_FLAG_NON_COHERENT flag
Date: Mon, 26 Jul 2021 13:22:12 +0900 [thread overview]
Message-ID: <CAAFQd5D=7Ao_gaxLamf8rv_QWxdemh=KZKdGU-1HFfY7L8A3MQ@mail.gmail.com> (raw)
In-Reply-To: <YP439RUKhVeAm945@google.com>
On Mon, Jul 26, 2021 at 1:20 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (21/07/26 13:05), Sergey Senozhatsky wrote:
> > On (21/07/26 10:40), Sergey Senozhatsky wrote:
> > > On (21/07/22 19:33), Dafna Hirschfeld wrote:
> > > [..]
> > > > > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> > > > > {
> > > > > int ret = vb2_verify_memory_type(q, req->memory, req->type);
> > > > > + u32 flags = req->flags;
> > > > > fill_buf_caps(q, &req->capabilities);
> > > > > - return ret ? ret : vb2_core_reqbufs(q, req->memory, 0, &req->count);
> > > > > + validate_memory_flags(q, req->memory, &flags);
> > > > > + req->flags = flags;
> > > >
> > > > you can do instead
> > > >
> > > > validate_memory_flags(q, req->memory, &req->flags);
> > >
> > > ->flags are u32 for create-bufs and u8 for reqi-bufs. So `*flags = <value>`
> > > can write to ->reserved[] for req-bufs (if the value is huge enough).
> >
> > I guess ->flags can become u8 for both create-bufs and req-bufs.
> > We had ->flags in both structs as u32, but then decided to leave
> > some reserved[] space in req-bufs and switched to u8 there.
>
> Something like this
>
> ---
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> index a048a9f6b7b6..cf633b5a4919 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
> @@ -112,13 +112,13 @@ than the number requested.
> other changes, then set ``count`` to 0, ``memory`` to
> ``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.
>
> - * - __u32
> + * - __u8
> - ``flags``
> - Specifies additional buffer management attributes.
> See :ref:`memory-flags`.
>
> - * - __u32
> - - ``reserved``\ [6]
> + * - __u8
> + - ``reserved``\ [27]
> - A place holder for future extensions. Drivers and applications
> must set the array to zero.
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 6edf4508c636..2ae949ec0afa 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -694,7 +694,7 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
>
> static void validate_memory_flags(struct vb2_queue *q,
> int memory,
> - u32 *flags)
> + u8 *flags)
> {
> if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) {
> /*
> @@ -711,11 +711,9 @@ static void validate_memory_flags(struct vb2_queue *q,
> int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> {
> int ret = vb2_verify_memory_type(q, req->memory, req->type);
> - u32 flags = req->flags;
>
> fill_buf_caps(q, &req->capabilities);
> - validate_memory_flags(q, req->memory, &flags);
> - req->flags = flags;
> + validate_memory_flags(q, req->memory, &req->flags);
> return ret ? ret : vb2_core_reqbufs(q, req->memory,
> req->flags, &req->count);
> }
> @@ -990,11 +988,9 @@ 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);
> - u32 flags = p->flags;
>
> fill_buf_caps(vdev->queue, &p->capabilities);
> - validate_memory_flags(vdev->queue, p->memory, &flags);
> - p->flags = flags;
> + validate_memory_flags(vdev->queue, p->memory, &p->flags);
> if (res)
> return res;
> if (vb2_queue_is_busy(vdev, file))
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 7973aa0465d2..ad4f7cee53f2 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2513,8 +2513,8 @@ struct v4l2_create_buffers {
> __u32 memory;
> struct v4l2_format format;
> __u32 capabilities;
> - __u32 flags;
> - __u32 reserved[6];
> + __u8 flags;
> + __u8 reserved[27];
> };
>
> /*
I don't think we want this UABI churn anymore for a trivial
simplification in the implementation.
Best regards,
Tomasz
next prev parent reply other threads:[~2021-07-26 4:22 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-09 9:20 [PATCHv3 0/8] videobuf2: support new noncontiguous DMA API Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 1/8] videobuf2: rework vb2_mem_ops API Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 2/8] videobuf2: inverse buffer cache_hints flags Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 3/8] videobuf2: split buffer cache_hints initialisation Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 4/8] videobuf2: move cache_hints handling to allocators Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 5/8] videobuf2: add V4L2_MEMORY_FLAG_NON_COHERENT flag Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 6/8] videobuf2: add queue memory coherency parameter Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 7/8] videobuf2: handle V4L2_MEMORY_FLAG_NON_COHERENT flag Sergey Senozhatsky
2021-07-22 17:33 ` Dafna Hirschfeld
2021-07-26 1:40 ` Sergey Senozhatsky
2021-07-26 4:05 ` Sergey Senozhatsky
2021-07-26 4:20 ` Sergey Senozhatsky
2021-07-26 4:22 ` Tomasz Figa [this message]
2021-07-26 4:41 ` Sergey Senozhatsky
2021-07-09 9:20 ` [PATCHv3 8/8] videobuf2: handle non-contiguous DMA allocations Sergey Senozhatsky
2021-07-22 17:26 ` Dafna Hirschfeld
2021-07-26 3:59 ` Sergey Senozhatsky
2021-07-26 4:04 ` Tomasz Figa
2021-07-26 4:10 ` Sergey Senozhatsky
2021-07-27 7:06 ` Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAFQd5D=7Ao_gaxLamf8rv_QWxdemh=KZKdGU-1HFfY7L8A3MQ@mail.gmail.com' \
--to=tfiga@chromium.org \
--cc=dafna.hirschfeld@collabora.com \
--cc=hch@lst.de \
--cc=hverkuil-cisco@xs4all.nl \
--cc=kernel@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=ribalda@chromium.org \
--cc=senozhatsky@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.