[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200219085945.GE122464@google.com>
Date: Wed, 19 Feb 2020 17:59:45 +0900
From: Sergey Senozhatsky <senozhatsky@...omium.org>
To: Hans Verkuil <hverkuil@...all.nl>
Cc: Sergey Senozhatsky <senozhatsky@...omium.org>,
Hans Verkuil <hans.verkuil@...co.com>,
Tomasz Figa <tfiga@...omium.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Kyungmin Park <kyungmin.park@...sung.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Sakari Ailus <sakari.ailus@....fi>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Pawel Osciak <posciak@...omium.org>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCHv2 05/12] videobuf2: handle
V4L2_FLAG_MEMORY_NON_CONSISTENT flag
On (20/02/19 09:25), Hans Verkuil wrote:
[..]
> > diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> > index bd08e4f77ae4..68185e94b686 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> > @@ -121,7 +121,14 @@ than the number requested.
> > other changes, then set ``count`` to 0, ``memory`` to
> > ``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.
> > * - __u32
> > - - ``reserved``\ [7]
> > + - ``flags``
> > + - Specifies additional buffer management attributes. Valid only when
> > + queue reports :ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.
> > + Old drivers and applications must set it to zero.
>
> Drop the 'Valid only' sentence. The V4L2_FLAG_MEMORY_NON_CONSISTENT depends
> on that capability, but other flags added in the future may not.
The whole sentence, right?
> Inside add a reference to the memory flags section created in patch 3.
Sorry. Inside?
[..]
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 72efc1c544cd..169a8cf345ed 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -938,7 +938,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];
> > + };
>
> How about this:
>
> __u8 flags;
> __u8 reserved[3];
>
> That avoids the anonymous union and allows some space for future additions.
Hmm. This way old apps, which clear out ->reserved, e.g.
memset(&x.reserved, 0x00, sizeof(x.reserved)), won't clear
out x.flags and can accidentally submit some unintended
garbage. It's not the case with anon union.
-ss
Powered by blists - more mailing lists