cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Sep 5 04:17:40 CEST 2018 media-tree git hash:d842a7cf938b6e0f8a1aa9f1aec0476c9a599310 media_build git hash: ed1d887e2c18299383c7258615130197c8ce4946 v4l-utils git hash: f44f00e8b4ac6e9aa05bac8953e3fcc89e1fe198 edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.17.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.102-i686: OK linux-3.2.102-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.119-i686: OK linux-3.18.119-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.152-i686: OK linux-4.4.152-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.124-i686: OK linux-4.9.124-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.67-i686: OK linux-4.14.67-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.5-i686: OK linux-4.18.5-x86_64: OK linux-4.19-rc1-i686: OK linux-4.19-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
[RFP] Automated testing on the media subsystem
There is a lot of discussion going on around testing, so it's a good opportunity for us to talk about our current testing infrastructure. We are already doing a good job with v4l2-compliance. Can we do more? I expect this discussion can span 30-40 minutes. Regards, Ezequiel
Re: [PATCH v3 1/2] media: ov5640: Re-work MIPI startup sequence
Hi Jacopo, > - ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5), > -on ? 0 : BIT(5)); > - if (ret) > - return ret; > - ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00, > - on ? 0x00 : 0x70); > + /* > +* Enable/disable the MIPI interface > +* > +* 0x300e = on ? 0x45 : 0x40 > +* [7:5] = 001 : 2 data lanes mode Does 2-Lanes work with this config? AFAIU, if 2-Lanes is bit 5, value should be 0x25 and 0x20. > +* [4] = 0 : Power up MIPI HS Tx > +* [3] = 0 : Power up MIPI LS Rx > +* [2] = 1/0: MIPI interface enable/disable > +* [1:0] = 01/00: FIXME: 'debug' > +*/ > + ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00, > + on ? 0x45 : 0x40); Regards, Loic
Re: [RFCv2 PATCH 1/3] uapi/linux/media.h: add property support
On Tue, Sep 04, 2018 at 06:43:20PM +0300, Sakari Ailus wrote: > media_v2_prop, called e.g. payload_length. I also think we should have the > size (and length) of the property in a specific unit, such as bytes, so the > parser does not have to know a given property type to determine it > correctly. Ah, please ignore this. I see you ended up to the same conclusion yourself already. -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [RFCv2 PATCH 1/3] uapi/linux/media.h: add property support
Hi Hans,
On Tue, Sep 04, 2018 at 03:50:33PM +0200, Hans Verkuil wrote:
> On 09/04/18 15:01, Sakari Ailus wrote:
> > Hi Hans,
> >
> > Thanks for the set.
> >
> > On Tue, Aug 07, 2018 at 12:28:45PM +0200, Hans Verkuil wrote:
> >> From: Hans Verkuil
> >>
> >> Add a new topology struct that includes properties.
> >>
> >> Signed-off-by: Hans Verkuil
> >> ---
> >> include/uapi/linux/media.h | 40 ++
> >> 1 file changed, 40 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> >> index 36f76e777ef9..1910c091601e 100644
> >> --- a/include/uapi/linux/media.h
> >> +++ b/include/uapi/linux/media.h
> >> @@ -342,6 +342,40 @@ struct media_v2_link {
> >>__u32 reserved[6];
> >> } __attribute__ ((packed));
> >>
> >> +#define MEDIA_PROP_TYPE_U64 1
> >> +#define MEDIA_PROP_TYPE_S64 2
> >> +#define MEDIA_PROP_TYPE_STRING3
> >> +
> >> +/**
> >> + * struct media_v2_prop - A media property
> >> + *
> >> + * @id: The unique non-zero ID of this property
> >> + * @owner_id: The ID of the object this property belongs to
> >
> > This assumes everything has a graph object ID. Speaking of "everything",
> > one of the use cases for this could be telling the user whether the media
> > device registration is finished.
> >
> > One approach could be to create a special graph object with a constant ID
> > for that purpose. As the type is a part of the ID field, we could simply
> > create a new type of constant objects.
>
> Why would you use properties for that? Why not just allow polling for
> exceptions (i.e. new events) on the media device node, similar to events
> for V4L2? It's very easy to use with select() et al.
Yes, that's needed; there was actually a support for media events on an
older version of my take of the request API.
But events are not all that's needed: how does the user know registration
has finished if it has already happened when the user opens a device?
Event-wise, I think a "topology change" event is all that's needed.
>
> > On a sidenote, the proposed API effectively prohibits conveying structured
> > data. One use case for that would be to tell about camera modules. We don't
> > have that right now as part of the uAPI as there's nothing the kernel
> > currently knows about camera modules --- because there's nothing to control
> > there. The user space would still be interested in knowing quite a few
> > parameters of these devices which means the concept of the camera module
> > would be good to have in precisely this kind of an interface. What should
> > be there could in practice be what does the module contain (some of the
> > components are sub-devices whereas some would not be, such as an IR filter
> > or the lens) and their parameters (e.g. focal length).
>
> You can easily (but indeed not yet implemented in this initial version) create
> hierarchical properties by simply setting the owner_id to a parent property.
Good point. For arrays we could e.g. add a new field to struct
media_v2_prop, called e.g. payload_length. I also think we should have the
size (and length) of the property in a specific unit, such as bytes, so the
parser does not have to know a given property type to determine it
correctly.
>
> And you obviously need to carefully define and document how data is
> represented in properties.
>
> If people feel strongly enough about this, then I can implement it in the
> next RFC. But I want feedback on this RFC first.
Feel free to go ahead with that but I'm fine with the current scope as
well. Just remember it was one of the use cases. :-)
>
> > I'm not demanding this must be a part of the API in the beginning, however
> > this was one of the reasons why I originally proposed using JSON:
> >
> > https://www.spinics.net/lists/linux-media/msg90160.html>
> >
> > The downside with JSON would be that it does not fit very well with the
> > rest of the MC API.
> >
> >> + * @type: Property type
> >> + * @flags:Property flags
> >> + * @payload_size: Property payload size, 0 for U64/S64
> >> + * @payload_offset: Property payload starts at this offset from
> >> + *This is 0 for U64/S64.
> >> + * @reserved: Property reserved field, will be zeroed.
> >> + * @name: Property name
> >> + * @uval: Property value (unsigned)
> >> + * @sval: Property value (signed)
> >> + */
> >> +struct media_v2_prop {
> >> + __u32 id;
> >> + __u32 owner_id;
> >> + __u32 type;
> >> + __u32 flags;
> >> + __u32 payload_size;
> >> + __u32 payload_offset;
> >> + __u32 reserved[18];
> >
> > That's plenty. What is the typical size of the properties array for a media
> > device with, say, a few hundred entities with some four pads each, linked
> > with a single link on average?
> >
> >> + char name[32];
> >
> > Is 32 enough? For controls and formats it's been sometimes a bit limiting.
> >
> > Another approach could be to make this variable size, but that would make
> >
Re: [RFCv2 PATCH 1/3] uapi/linux/media.h: add property support
On 09/04/18 15:01, Sakari Ailus wrote:
> Hi Hans,
>
> Thanks for the set.
>
> On Tue, Aug 07, 2018 at 12:28:45PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> Add a new topology struct that includes properties.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> include/uapi/linux/media.h | 40 ++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
>> index 36f76e777ef9..1910c091601e 100644
>> --- a/include/uapi/linux/media.h
>> +++ b/include/uapi/linux/media.h
>> @@ -342,6 +342,40 @@ struct media_v2_link {
>> __u32 reserved[6];
>> } __attribute__ ((packed));
>>
>> +#define MEDIA_PROP_TYPE_U64 1
>> +#define MEDIA_PROP_TYPE_S64 2
>> +#define MEDIA_PROP_TYPE_STRING 3
>> +
>> +/**
>> + * struct media_v2_prop - A media property
>> + *
>> + * @id: The unique non-zero ID of this property
>> + * @owner_id: The ID of the object this property belongs to
>
> This assumes everything has a graph object ID. Speaking of "everything",
> one of the use cases for this could be telling the user whether the media
> device registration is finished.
>
> One approach could be to create a special graph object with a constant ID
> for that purpose. As the type is a part of the ID field, we could simply
> create a new type of constant objects.
Why would you use properties for that? Why not just allow polling for
exceptions (i.e. new events) on the media device node, similar to events
for V4L2? It's very easy to use with select() et al.
> On a sidenote, the proposed API effectively prohibits conveying structured
> data. One use case for that would be to tell about camera modules. We don't
> have that right now as part of the uAPI as there's nothing the kernel
> currently knows about camera modules --- because there's nothing to control
> there. The user space would still be interested in knowing quite a few
> parameters of these devices which means the concept of the camera module
> would be good to have in precisely this kind of an interface. What should
> be there could in practice be what does the module contain (some of the
> components are sub-devices whereas some would not be, such as an IR filter
> or the lens) and their parameters (e.g. focal length).
You can easily (but indeed not yet implemented in this initial version) create
hierarchical properties by simply setting the owner_id to a parent property.
And you obviously need to carefully define and document how data is
represented in properties.
If people feel strongly enough about this, then I can implement it in the
next RFC. But I want feedback on this RFC first.
> I'm not demanding this must be a part of the API in the beginning, however
> this was one of the reasons why I originally proposed using JSON:
>
> https://www.spinics.net/lists/linux-media/msg90160.html>
>
> The downside with JSON would be that it does not fit very well with the
> rest of the MC API.
>
>> + * @type: Property type
>> + * @flags: Property flags
>> + * @payload_size: Property payload size, 0 for U64/S64
>> + * @payload_offset: Property payload starts at this offset from
>> + * This is 0 for U64/S64.
>> + * @reserved: Property reserved field, will be zeroed.
>> + * @name: Property name
>> + * @uval: Property value (unsigned)
>> + * @sval: Property value (signed)
>> + */
>> +struct media_v2_prop {
>> +__u32 id;
>> +__u32 owner_id;
>> +__u32 type;
>> +__u32 flags;
>> +__u32 payload_size;
>> +__u32 payload_offset;
>> +__u32 reserved[18];
>
> That's plenty. What is the typical size of the properties array for a media
> device with, say, a few hundred entities with some four pads each, linked
> with a single link on average?
>
>> +char name[32];
>
> Is 32 enough? For controls and formats it's been sometimes a bit limiting.
>
> Another approach could be to make this variable size, but that would make
> parsing a little bit more complicated. As you already have separated the
> property descriptor and the payload, that wouldn't be a major change
> anymore.
I'll have to experiment a bit with that.
Thanks for looking at this!
Regards,
Hans
>
>> +union {
>> +__u64 uval;
>> +__s64 sval;
>> +};
>> +} __attribute__ ((packed));
>> +
>> struct media_v2_topology {
>> __u64 topology_version;
>>
>> @@ -360,6 +394,10 @@ struct media_v2_topology {
>> __u32 num_links;
>> __u32 reserved4;
>> __u64 ptr_links;
>> +
>> +__u32 num_props;
>> +__u32 props_payload_size;
>> +__u64 ptr_props;
>> } __attribute__ ((packed));
>>
>> /* ioctls */
>> @@ -368,6 +406,8 @@ struct media_v2_topology {
>> #define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct
>> media_entity_desc)
>> #define MEDIA_IOC_ENUM_LINKS_IOWR('|', 0x02, struct
>> media_links_enum)
>> #define MEDIA_IOC_SETUP_LINK_IOWR('|', 0x03, struct media_link_desc)
>>
Re: [RFCv2 PATCH 1/3] uapi/linux/media.h: add property support
Hi Hans,
Thanks for the set.
On Tue, Aug 07, 2018 at 12:28:45PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Add a new topology struct that includes properties.
>
> Signed-off-by: Hans Verkuil
> ---
> include/uapi/linux/media.h | 40 ++
> 1 file changed, 40 insertions(+)
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 36f76e777ef9..1910c091601e 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -342,6 +342,40 @@ struct media_v2_link {
> __u32 reserved[6];
> } __attribute__ ((packed));
>
> +#define MEDIA_PROP_TYPE_U64 1
> +#define MEDIA_PROP_TYPE_S64 2
> +#define MEDIA_PROP_TYPE_STRING 3
> +
> +/**
> + * struct media_v2_prop - A media property
> + *
> + * @id: The unique non-zero ID of this property
> + * @owner_id:The ID of the object this property belongs to
This assumes everything has a graph object ID. Speaking of "everything",
one of the use cases for this could be telling the user whether the media
device registration is finished.
One approach could be to create a special graph object with a constant ID
for that purpose. As the type is a part of the ID field, we could simply
create a new type of constant objects.
On a sidenote, the proposed API effectively prohibits conveying structured
data. One use case for that would be to tell about camera modules. We don't
have that right now as part of the uAPI as there's nothing the kernel
currently knows about camera modules --- because there's nothing to control
there. The user space would still be interested in knowing quite a few
parameters of these devices which means the concept of the camera module
would be good to have in precisely this kind of an interface. What should
be there could in practice be what does the module contain (some of the
components are sub-devices whereas some would not be, such as an IR filter
or the lens) and their parameters (e.g. focal length).
I'm not demanding this must be a part of the API in the beginning, however
this was one of the reasons why I originally proposed using JSON:
https://www.spinics.net/lists/linux-media/msg90160.html>
The downside with JSON would be that it does not fit very well with the
rest of the MC API.
> + * @type:Property type
> + * @flags: Property flags
> + * @payload_size: Property payload size, 0 for U64/S64
> + * @payload_offset: Property payload starts at this offset from
> + * This is 0 for U64/S64.
> + * @reserved:Property reserved field, will be zeroed.
> + * @name:Property name
> + * @uval:Property value (unsigned)
> + * @sval:Property value (signed)
> + */
> +struct media_v2_prop {
> + __u32 id;
> + __u32 owner_id;
> + __u32 type;
> + __u32 flags;
> + __u32 payload_size;
> + __u32 payload_offset;
> + __u32 reserved[18];
That's plenty. What is the typical size of the properties array for a media
device with, say, a few hundred entities with some four pads each, linked
with a single link on average?
> + char name[32];
Is 32 enough? For controls and formats it's been sometimes a bit limiting.
Another approach could be to make this variable size, but that would make
parsing a little bit more complicated. As you already have separated the
property descriptor and the payload, that wouldn't be a major change
anymore.
> + union {
> + __u64 uval;
> + __s64 sval;
> + };
> +} __attribute__ ((packed));
> +
> struct media_v2_topology {
> __u64 topology_version;
>
> @@ -360,6 +394,10 @@ struct media_v2_topology {
> __u32 num_links;
> __u32 reserved4;
> __u64 ptr_links;
> +
> + __u32 num_props;
> + __u32 props_payload_size;
> + __u64 ptr_props;
> } __attribute__ ((packed));
>
> /* ioctls */
> @@ -368,6 +406,8 @@ struct media_v2_topology {
> #define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct
> media_entity_desc)
> #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
> #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
> +/* Old MEDIA_IOC_G_TOPOLOGY ioctl without props support */
> +#define MEDIA_IOC_G_TOPOLOGY_OLD 0xc0487c04
> #define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
>
> #ifndef __KERNEL__
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCHv4 09/10] media-request: EPERM -> EACCES/EBUSY
On 09/04/18 10:21, Tomasz Figa wrote: > On Tue, Sep 4, 2018 at 4:59 PM Hans Verkuil wrote: >> >> From: Hans Verkuil >> >> If requests are not supported by the driver, then return EACCES, not >> EPERM. >> >> If you attempt to mix queueing buffers directly and using requests, >> then EBUSY is returned instead of EPERM: once a specific queueing mode >> has been chosen the queue is 'busy' if you attempt the other mode >> (i.e. direct queueing vs via a request). >> >> Signed-off-by: Hans Verkuil >> --- >> .../uapi/mediactl/media-request-ioc-queue.rst | 9 - >> .../media/uapi/mediactl/request-api.rst| 4 ++-- >> Documentation/media/uapi/v4l/buffer.rst| 2 +- >> .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 9 - >> Documentation/media/uapi/v4l/vidioc-qbuf.rst | 18 ++ >> .../media/common/videobuf2/videobuf2-core.c| 2 +- >> .../media/common/videobuf2/videobuf2-v4l2.c| 9 ++--- >> drivers/media/media-request.c | 4 ++-- >> include/media/media-request.h | 6 +++--- >> 9 files changed, 33 insertions(+), 30 deletions(-) > [snip] >> diff --git a/include/media/media-request.h b/include/media/media-request.h >> index d8c8db89dbde..0ce75c35131f 100644 >> --- a/include/media/media-request.h >> +++ b/include/media/media-request.h >> @@ -198,8 +198,8 @@ void media_request_put(struct media_request *req); >> * Get the request represented by @request_fd that is owned >> * by the media device. >> * >> - * Return a -EPERM error pointer if requests are not supported >> - * by this driver. Return -ENOENT if the request was not found. >> + * Return a -EACCES error pointer if requests are not supported >> + * by this driver. Return -EINVAL if the request was not found. > > I think the bottom-most line belongs to patch 1/10. With that fixed > (possibly when applying): Correct, I moved that to patch 1. > > Reviewed-by: Tomasz Figa Thanks! Hans > > Best regards, > Tomasz >
cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Sep 4 09:37:57 CEST 2018 media-tree git hash:d842a7cf938b6e0f8a1aa9f1aec0476c9a599310 media_build git hash: ed1d887e2c18299383c7258615130197c8ce4946 v4l-utils git hash: f44f00e8b4ac6e9aa05bac8953e3fcc89e1fe198 edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.17.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.102-i686: OK linux-3.2.102-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.119-i686: OK linux-3.18.119-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.152-i686: OK linux-4.4.152-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.124-i686: OK linux-4.9.124-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.67-i686: OK linux-4.14.67-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.5-i686: OK linux-4.18.5-x86_64: OK linux-4.19-rc1-i686: OK linux-4.19-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH 3/3] media: imx-pxp: add i.MX Pixel Pipeline driver
Hi Jacopo,
thank you for the review!
On Tue, 2018-09-04 at 10:04 +0200, jacopo mondi wrote:
> Hi Philipp,
>
> On Fri, Aug 10, 2018 at 05:18:22PM +0200, Philipp Zabel wrote:
[...]
> > +static struct pxp_fmt formats[] = {
> > + {
> > + .fourcc = V4L2_PIX_FMT_XBGR32,
> > + .depth = 32,
> > + /* Both capture and output format */
> > + .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
[...]
> > + }, {
> > + .fourcc = V4L2_PIX_FMT_YUV32,
> > + .depth = 16,
>
> According to:
> https://www.linuxtv.org/downloads/v4l-dvb-apis-old/packed-yuv.html
> shouldn't this be 32?
Yes, I'll change depth to 32.
[...]
> > +}
> > +
>
> Multiple blank lines (in a few other places too)
>
> > +
Found them, will fix them.
[...]
> > + /* Enable clocks and dump registers */
> > + clk_prepare_enable(dev->clk);
> > + pxp_soft_reset(dev);
> > +
> > + spin_lock_init(>irqlock);
> > +
> > + ret = v4l2_device_register(>dev, >v4l2_dev);
> > + if (ret)
> > + return ret;
> > +
> > + atomic_set(>num_inst, 0);
> > + mutex_init(>dev_mutex);
> > +
> > + dev->vfd = pxp_videodev;
>
> The name of the video device is the same for all instances of this
> driver. Is this ok?
I expect that there is only ever going to be one instance on the SoC.
[...]
> > + v4l2_device_unregister(>v4l2_dev);
>
> Disable clock?
Yes, I'll fix the clock handling.
[...]
> > +MODULE_DESCRIPTION("i.MX PXP mem2mem scaler/CSC/rotator");
> > +MODULE_AUTHOR("Philipp Zabel ");
> > +MODULE_LICENSE("GPL");
>
> SPDX header says GPL2.0+
See include/linux/module.h:
/*
* The following license idents are currently accepted as indicating free
* software modules
*
* "GPL" [GNU Public License v2 or later]
* [...]
*/
This already seems to be the right choice.
regards
Philipp
Re: [PATCHv4 09/10] media-request: EPERM -> EACCES/EBUSY
On Tue, Sep 4, 2018 at 4:59 PM Hans Verkuil wrote: > > From: Hans Verkuil > > If requests are not supported by the driver, then return EACCES, not > EPERM. > > If you attempt to mix queueing buffers directly and using requests, > then EBUSY is returned instead of EPERM: once a specific queueing mode > has been chosen the queue is 'busy' if you attempt the other mode > (i.e. direct queueing vs via a request). > > Signed-off-by: Hans Verkuil > --- > .../uapi/mediactl/media-request-ioc-queue.rst | 9 - > .../media/uapi/mediactl/request-api.rst| 4 ++-- > Documentation/media/uapi/v4l/buffer.rst| 2 +- > .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 9 - > Documentation/media/uapi/v4l/vidioc-qbuf.rst | 18 ++ > .../media/common/videobuf2/videobuf2-core.c| 2 +- > .../media/common/videobuf2/videobuf2-v4l2.c| 9 ++--- > drivers/media/media-request.c | 4 ++-- > include/media/media-request.h | 6 +++--- > 9 files changed, 33 insertions(+), 30 deletions(-) [snip] > diff --git a/include/media/media-request.h b/include/media/media-request.h > index d8c8db89dbde..0ce75c35131f 100644 > --- a/include/media/media-request.h > +++ b/include/media/media-request.h > @@ -198,8 +198,8 @@ void media_request_put(struct media_request *req); > * Get the request represented by @request_fd that is owned > * by the media device. > * > - * Return a -EPERM error pointer if requests are not supported > - * by this driver. Return -ENOENT if the request was not found. > + * Return a -EACCES error pointer if requests are not supported > + * by this driver. Return -EINVAL if the request was not found. I think the bottom-most line belongs to patch 1/10. With that fixed (possibly when applying): Reviewed-by: Tomasz Figa Best regards, Tomasz
Re: [PATCH 3/3] media: imx-pxp: add i.MX Pixel Pipeline driver
Hi Philipp,
On Fri, Aug 10, 2018 at 05:18:22PM +0200, Philipp Zabel wrote:
> Add a V4L2 mem-to-mem scaler/CSC driver for the Pixel Pipeline (PXP)
> version found on i.MX6ULL SoCs. A similar variant is used on i.MX7D.
>
> Since this driver only uses the legacy pipeline, it should be reasonably
> easy to extend it to work with the older PXP versions found on i.MX6UL,
> i.MX6SX, i.MX6SL, i.MX28, and i.MX23.
>
> The driver supports scaling and colorspace conversion. There is
> currently no support for rotation, alpha-blending, and the LUTs.
>
> Signed-off-by: Philipp Zabel
> ---
> drivers/media/platform/Kconfig |9 +
> drivers/media/platform/Makefile |2 +
> drivers/media/platform/imx-pxp.c | 1455 ++
> drivers/media/platform/imx-pxp.h | 1685 ++
> 4 files changed, 3151 insertions(+)
> create mode 100644 drivers/media/platform/imx-pxp.c
> create mode 100644 drivers/media/platform/imx-pxp.h
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index b25c8d3c1c31..ae1c025c14de 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -181,6 +181,15 @@ config VIDEO_CODA
> config VIDEO_IMX_VDOA
> def_tristate VIDEO_CODA if SOC_IMX6Q || COMPILE_TEST
>
> +config VIDEO_IMX_PXP
> + tristate "i.MX Pixel Pipeline (PXP)"
> + depends on VIDEO_DEV && VIDEO_V4L2 && (ARCH_MXC || COMPILE_TEST)
> + select VIDEOBUF2_DMA_CONTIG
> + select V4L2_MEM2MEM_DEV
> + help
> + The i.MX Pixel Pipeline is a memory-to-memory engine for scaling,
> + color space conversion, and rotation.
> +
> config VIDEO_MEDIATEK_JPEG
> tristate "Mediatek JPEG Codec driver"
> depends on MTK_IOMMU_V1 || COMPILE_TEST
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 08640ba87fc2..0c2714c2bb05 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -25,6 +25,8 @@ obj-$(CONFIG_VIDEO_TI_CAL) += ti-vpe/
> obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
> obj-$(CONFIG_VIDEO_CODA) += coda/
>
> +obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
> +
> obj-$(CONFIG_VIDEO_SH_VEU) += sh_veu.o
>
> obj-$(CONFIG_CEC_GPIO) += cec-gpio/
> diff --git a/drivers/media/platform/imx-pxp.c
> b/drivers/media/platform/imx-pxp.c
> new file mode 100644
> index ..c9e3ef0f92b4
> --- /dev/null
> +++ b/drivers/media/platform/imx-pxp.c
> @@ -0,0 +1,1455 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * i.MX Pixel Pipeline (PXP) mem-to-mem scaler/CSC/rotator driver
> + *
> + * Copyright (c) 2018 Pengutronix, Philipp Zabel
> + *
> + * based on vim2m
> + *
> + * Copyright (c) 2009-2010 Samsung Electronics Co., Ltd.
> + * Pawel Osciak,
> + * Marek Szyprowski,
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "imx-pxp.h"
> +
> +static unsigned int debug;
> +module_param(debug, uint, 0644);
> +MODULE_PARM_DESC(debug, "activates debug info");
> +
> +#define MIN_W 8
> +#define MIN_H 8
> +#define MAX_W 4096
> +#define MAX_H 4096
> +#define ALIGN_W 3 /* 8x8 pixel blocks */
> +#define ALIGN_H 3
> +
> +/* Flags that indicate a format can be used for capture/output */
> +#define MEM2MEM_CAPTURE (1 << 0)
> +#define MEM2MEM_OUTPUT (1 << 1)
> +
> +#define MEM2MEM_NAME "pxp"
> +
> +/* Per queue */
> +#define MEM2MEM_DEF_NUM_BUFS VIDEO_MAX_FRAME
> +/* In bytes, per queue */
> +#define MEM2MEM_VID_MEM_LIMIT(16 * 1024 * 1024)
> +
> +/* Flags that indicate processing mode */
> +#define MEM2MEM_HFLIP(1 << 0)
> +#define MEM2MEM_VFLIP(1 << 1)
> +
> +#define dprintk(dev, fmt, arg...) \
> + v4l2_dbg(1, debug, >v4l2_dev, "%s: " fmt, __func__, ## arg)
> +
> +struct pxp_fmt {
> + u32 fourcc;
> + int depth;
> + /* Types the format can be used for */
> + u32 types;
> +};
> +
> +static struct pxp_fmt formats[] = {
> + {
> + .fourcc = V4L2_PIX_FMT_XBGR32,
> + .depth = 32,
> + /* Both capture and output format */
> + .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
> + }, {
> + .fourcc = V4L2_PIX_FMT_ABGR32,
> + .depth = 32,
> + /* Capture-only format */
> + .types = MEM2MEM_CAPTURE,
> + }, {
> + .fourcc = V4L2_PIX_FMT_BGR24,
> + .depth = 24,
> + .types = MEM2MEM_CAPTURE,
> + }, {
> + .fourcc = V4L2_PIX_FMT_RGB565,
> + .depth = 16,
> + .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
> + }, {
> + .fourcc = V4L2_PIX_FMT_RGB555,
> + .depth = 16,
> + .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
> +
[PATCHv4 00/10] Post-v18: Request API updates
From: Hans Verkuil Hi all, This patch series sits on top of the request_api topic branch in the media_tree. It makes some final (?) changes as discussed in: https://www.mail-archive.com/linux-media@vger.kernel.org/msg134419.html and: https://www.spinics.net/lists/linux-media/msg138596.html The combined v18 patches + this series is available here: https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=reqv18-1 Updated v4l-utils for this is available here: https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=request Userspace visible changes: - Invalid request_fd values now return -EINVAL instead of -ENOENT. - It is no longer possible to use VIDIOC_G_EXT_CTRLS for requests that are not completed. -EACCES is returned in that case. - Attempting to use requests if requests are not supported by the driver will result in -EACCES instead of -EPERM. - Attempting to mix direct QBUF with queueing buffers via a request will result in -EBUSY instead of -EPERM. Driver visible changes (important for the cedrus driver!): Drivers should set the new vb2_queue 'supports_request' bitfield to 1 if a vb2_queue can support requests. Otherwise the queue cannot be used with requests. This bitfield is also used to fill in the new capabilities field in struct v4l2_requestbuffers and v4l2_create_buffers. Changes since v3: - I missed a few EPERM references in comments, documentation and even a stub functions. They have all been replaced by EBUSY or EACCES. Thanks to Sakari 'Eagle-Eye' Sailus for finding these. This only effects patch 09/10. Changes since v2: - Fix typo in commit message of patch 07/10: why -> when - Attempting to mix direct QBUF with queueing buffers via a request will result in -EBUSY instead of -EPERM. - Replaced -EPERM with -EBUSY in videobuf2-core.c that was missed in v1. Changes since v1: - Updated patch 4/10 to explain how to query the capabilities with REQBUFS/CREATE_BUFS with a minimum of side-effects (requested by Tomasz). - Added patches 6-10: 6: Sakari found a corner case: when accessing a request the request has to be protected from being re-inited. New media_request_(un)lock_for_access helpers are added for this. 7: use these helpers in g_ext_ctrls. 8: make s/try_ext_ctrls more robust by keeping the request references until we're fully done setting/trying the controls. 9: Change two more EPERM's to EACCES. EPERM suggests that you can fix it by changing permissions somehow, but in this case the driver simply doesn't support requests at all. 10: Update the request documentation based on Laurent's comments: https://www.spinics.net/lists/linux-media/msg139152.html To do: split off the V4L2 specifics into a V4L2 specific rst file. But this will take more time and is for later. Regards, Hans Hans Verkuil (10): media-request: return -EINVAL for invalid request_fds v4l2-ctrls: return -EACCES if request wasn't completed buffer.rst: only set V4L2_BUF_FLAG_REQUEST_FD for QBUF videodev2.h: add new capabilities for buffer types vb2: set reqbufs/create_bufs capabilities media-request: add media_request_(un)lock_for_access v4l2-ctrls: use media_request_(un)lock_for_access v4l2-ctrls: improve media_request_(un)lock_for_update media-request: EPERM -> EACCES/EBUSY media-request: update documentation .../uapi/mediactl/media-ioc-request-alloc.rst | 3 +- .../uapi/mediactl/media-request-ioc-queue.rst | 16 ++--- .../media/uapi/mediactl/request-api.rst | 55 +--- .../uapi/mediactl/request-func-close.rst | 1 + .../media/uapi/mediactl/request-func-poll.rst | 2 +- Documentation/media/uapi/v4l/buffer.rst | 22 --- .../media/uapi/v4l/vidioc-create-bufs.rst | 14 - .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 48 +++--- Documentation/media/uapi/v4l/vidioc-qbuf.rst | 35 ++- .../media/uapi/v4l/vidioc-reqbufs.rst | 42 - .../media/common/videobuf2/videobuf2-core.c | 2 +- .../media/common/videobuf2/videobuf2-v4l2.c | 24 ++- drivers/media/media-request.c | 20 -- drivers/media/platform/vim2m.c| 1 + drivers/media/platform/vivid/vivid-core.c | 5 ++ drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 +- drivers/media/v4l2-core/v4l2-ctrls.c | 32 ++ drivers/media/v4l2-core/v4l2-ioctl.c | 4 +- include/media/media-request.h | 62 ++- include/media/videobuf2-core.h| 2 + include/uapi/linux/videodev2.h| 13 +++- 21 files changed, 294 insertions(+), 113 deletions(-) -- 2.18.0
[PATCHv4 02/10] v4l2-ctrls: return -EACCES if request wasn't completed
From: Hans Verkuil
For now (this might be relaxed in the future) we do not allow getting
controls from a request that isn't completed. In that case we return
-EACCES. Update the documentation accordingly.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
.../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 18 +-
drivers/media/v4l2-core/v4l2-ctrls.c | 5 ++---
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index 9c56a9b6e98a..ad8908ce3095 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -107,13 +107,12 @@ then ``EINVAL`` will be returned.
An attempt to call :ref:`VIDIOC_S_EXT_CTRLS ` for a
request that has already been queued will result in an ``EBUSY`` error.
-If ``request_fd`` is specified and ``which`` is set to
``V4L2_CTRL_WHICH_REQUEST_VAL``
-during a call to :ref:`VIDIOC_G_EXT_CTRLS `, then the
-returned values will be the values currently set for the request (or the
-hardware value if none is set) if the request has not yet been queued, or the
-values of the controls at the time of request completion if it has already
-completed. Attempting to get controls while the request has been queued but
-not yet completed will result in an ``EBUSY`` error.
+If ``request_fd`` is specified and ``which`` is set to
+``V4L2_CTRL_WHICH_REQUEST_VAL`` during a call to
+:ref:`VIDIOC_G_EXT_CTRLS `, then it will return the
+values of the controls at the time of request completion.
+If the request is not yet completed, then this will result in an
+``EACCES`` error.
The driver will only set/get these controls if all control values are
correct. This prevents the situation where only some of the controls
@@ -405,8 +404,9 @@ ENOSPC
and this error code is returned.
EACCES
-Attempt to try or set a read-only control or to get a write-only
-control.
+Attempt to try or set a read-only control, or to get a write-only
+control, or to get a control from a request that has not yet been
+completed.
EPERM
The ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` but the
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index a197b60183f5..ccaf3068de6d 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3301,10 +3301,9 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
struct media_device *mdev,
if (IS_ERR(req))
return PTR_ERR(req);
- if (req->state != MEDIA_REQUEST_STATE_IDLE &&
- req->state != MEDIA_REQUEST_STATE_COMPLETE) {
+ if (req->state != MEDIA_REQUEST_STATE_COMPLETE) {
media_request_put(req);
- return -EBUSY;
+ return -EACCES;
}
obj = v4l2_ctrls_find_req_obj(hdl, req, false);
--
2.18.0
[PATCHv4 05/10] vb2: set reqbufs/create_bufs capabilities
From: Hans Verkuil
Set the capabilities field of v4l2_requestbuffers and v4l2_create_buffers.
The various mapping modes were easy, but for signaling the request capability
a new 'supports_requests' bitfield was added to videobuf2-core.h (and set in
vim2m and vivid). Drivers have to set this bitfield for any queue where
requests are supported.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
.../media/common/videobuf2/videobuf2-v4l2.c | 19 ++-
drivers/media/platform/vim2m.c| 1 +
drivers/media/platform/vivid/vivid-core.c | 5 +
drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 +++-
drivers/media/v4l2-core/v4l2-ioctl.c | 4 ++--
include/media/videobuf2-core.h| 2 ++
6 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index a70df16d68f1..2caaabd50532 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -384,7 +384,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q,
struct media_device *md
return -EPERM;
}
return 0;
- } else if (q->uses_qbuf) {
+ } else if (q->uses_qbuf || !q->supports_requests) {
dprintk(1, "%s: queue does not use requests\n", opname);
return -EPERM;
}
@@ -619,10 +619,24 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer
*b)
}
EXPORT_SYMBOL(vb2_querybuf);
+static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
+{
+ *caps = 0;
+ if (q->io_modes & VB2_MMAP)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP;
+ if (q->io_modes & VB2_USERPTR)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_USERPTR;
+ if (q->io_modes & VB2_DMABUF)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_DMABUF;
+ if (q->supports_requests)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+}
+
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, >count);
}
EXPORT_SYMBOL_GPL(vb2_reqbufs);
@@ -654,6 +668,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct
v4l2_create_buffers *create)
int ret = vb2_verify_memory_type(q, create->memory, f->type);
unsigned i;
+ fill_buf_caps(q, >capabilities);
create->index = q->num_buffers;
if (create->count == 0)
return ret != -EBUSY ? ret : 0;
@@ -861,6 +876,7 @@ 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);
+ fill_buf_caps(vdev->queue, >capabilities);
if (res)
return res;
if (vb2_queue_is_busy(vdev, file))
@@ -882,6 +898,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
p->format.type);
p->index = vdev->queue->num_buffers;
+ fill_buf_caps(vdev->queue, >capabilities);
/*
* If count == 0, then just check if memory and type are valid.
* Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 5423f0dd0821..40fbb1e429af 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -855,6 +855,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
struct vb2_queue *ds
src_vq->mem_ops = _vmalloc_memops;
src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
src_vq->lock = >dev->dev_mutex;
+ src_vq->supports_requests = true;
ret = vb2_queue_init(src_vq);
if (ret)
diff --git a/drivers/media/platform/vivid/vivid-core.c
b/drivers/media/platform/vivid/vivid-core.c
index 3f6f5cbe1b60..e7f1394832fe 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -1077,6 +1077,7 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->min_buffers_needed = 2;
q->lock = >mutex;
q->dev = dev->v4l2_dev.dev;
+ q->supports_requests = true;
ret = vb2_queue_init(q);
if (ret)
@@ -1097,6 +1098,7 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->min_buffers_needed = 2;
q->lock = >mutex;
q->dev = dev->v4l2_dev.dev;
+ q->supports_requests = true;
ret = vb2_queue_init(q);
if (ret)
@@ -1117,6 +1119,7 @@ static int vivid_create_instance(struct platform_device
*pdev, int inst)
q->min_buffers_needed = 2;
[PATCHv4 07/10] v4l2-ctrls: use media_request_(un)lock_for_access
From: Hans Verkuil
When getting control values from a completed request, we have
to protect the request against being re-inited when it is
being accessed by calling media_request_(un)lock_for_access.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
drivers/media/v4l2-core/v4l2-ctrls.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index ccaf3068de6d..cc266a4a6e88 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3289,11 +3289,10 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
struct media_device *mdev,
struct v4l2_ext_controls *cs)
{
struct media_request_object *obj = NULL;
+ struct media_request *req = NULL;
int ret;
if (cs->which == V4L2_CTRL_WHICH_REQUEST_VAL) {
- struct media_request *req;
-
if (!mdev || cs->request_fd < 0)
return -EINVAL;
@@ -3306,11 +3305,18 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
struct media_device *mdev,
return -EACCES;
}
+ ret = media_request_lock_for_access(req);
+ if (ret) {
+ media_request_put(req);
+ return ret;
+ }
+
obj = v4l2_ctrls_find_req_obj(hdl, req, false);
- /* Reference to the request held through obj */
- media_request_put(req);
- if (IS_ERR(obj))
+ if (IS_ERR(obj)) {
+ media_request_unlock_for_access(req);
+ media_request_put(req);
return PTR_ERR(obj);
+ }
hdl = container_of(obj, struct v4l2_ctrl_handler,
req_obj);
@@ -3318,8 +3324,11 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl,
struct media_device *mdev,
ret = v4l2_g_ext_ctrls_common(hdl, cs);
- if (obj)
+ if (obj) {
+ media_request_unlock_for_access(req);
media_request_object_put(obj);
+ media_request_put(req);
+ }
return ret;
}
EXPORT_SYMBOL(v4l2_g_ext_ctrls);
--
2.18.0
[PATCHv4 09/10] media-request: EPERM -> EACCES/EBUSY
From: Hans Verkuil If requests are not supported by the driver, then return EACCES, not EPERM. If you attempt to mix queueing buffers directly and using requests, then EBUSY is returned instead of EPERM: once a specific queueing mode has been chosen the queue is 'busy' if you attempt the other mode (i.e. direct queueing vs via a request). Signed-off-by: Hans Verkuil --- .../uapi/mediactl/media-request-ioc-queue.rst | 9 - .../media/uapi/mediactl/request-api.rst| 4 ++-- Documentation/media/uapi/v4l/buffer.rst| 2 +- .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 9 - Documentation/media/uapi/v4l/vidioc-qbuf.rst | 18 ++ .../media/common/videobuf2/videobuf2-core.c| 2 +- .../media/common/videobuf2/videobuf2-v4l2.c| 9 ++--- drivers/media/media-request.c | 4 ++-- include/media/media-request.h | 6 +++--- 9 files changed, 33 insertions(+), 30 deletions(-) diff --git a/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst b/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst index d4f8119e0643..dbf635ae9b2b 100644 --- a/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst +++ b/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst @@ -49,7 +49,7 @@ exception is the ``EIO`` error which signals a fatal error that requires the application to stop streaming to reset the hardware state. It is not allowed to mix queuing requests with queuing buffers directly -(without a request). ``EPERM`` will be returned if the first buffer was +(without a request). ``EBUSY`` will be returned if the first buffer was queued directly and you next try to queue a request, or vice versa. A request must contain at least one buffer, otherwise this ioctl will @@ -63,10 +63,9 @@ appropriately. The generic error codes are described at the :ref:`Generic Error Codes ` chapter. EBUSY -The request was already queued. -EPERM -The application queued the first buffer directly, but later attempted -to use a request. It is not permitted to mix the two APIs. +The request was already queued or the application queued the first +buffer directly, but later attempted to use a request. It is not permitted +to mix the two APIs. ENOENT The request did not contain any buffers. All requests are required to have at least one buffer. This can also be returned if required diff --git a/Documentation/media/uapi/mediactl/request-api.rst b/Documentation/media/uapi/mediactl/request-api.rst index 0b9da58b01e3..aeb8d00934a4 100644 --- a/Documentation/media/uapi/mediactl/request-api.rst +++ b/Documentation/media/uapi/mediactl/request-api.rst @@ -64,7 +64,7 @@ request cannot be modified anymore. .. caution:: For :ref:`memory-to-memory devices ` you can use requests only for output buffers, not for capture buffers. Attempting to add a capture buffer - to a request will result in an ``EPERM`` error. + to a request will result in an ``EACCES`` error. If the request contains parameters for multiple entities, individual drivers may synchronize so the requested pipeline's topology is applied before the buffers @@ -77,7 +77,7 @@ perfect atomicity may not be possible due to hardware limitations. whichever method is used first locks this in place until :ref:`VIDIOC_STREAMOFF ` is called or the device is :ref:`closed `. Attempts to directly queue a buffer when earlier - a buffer was queued via a request or vice versa will result in an ``EPERM`` + a buffer was queued via a request or vice versa will result in an ``EBUSY`` error. Controls can still be set without a request and are applied immediately, diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index 1865cd5b9d3c..58a6d7d336e6 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -314,7 +314,7 @@ struct v4l2_buffer :ref:`ioctl VIDIOC_QBUF ` and ignored by other ioctls. Applications should not set ``V4L2_BUF_FLAG_REQUEST_FD`` for any ioctls other than :ref:`VIDIOC_QBUF `. - If the device does not support requests, then ``EPERM`` will be returned. + If the device does not support requests, then ``EACCES`` will be returned. If requests are supported but an invalid request file descriptor is given, then ``EINVAL`` will be returned. diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst index ad8908ce3095..54a999df5aec 100644 --- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst +++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst @@ -100,7 +100,7 @@ file descriptor and ``which`` is set to ``V4L2_CTRL_WHICH_REQUEST_VAL``, then the controls are not applied immediately when calling :ref:`VIDIOC_S_EXT_CTRLS `, but instead are applied by the driver for the buffer associated
[PATCHv4 08/10] v4l2-ctrls: improve media_request_(un)lock_for_update
From: Hans Verkuil
The request reference count was decreased again once a reference to the
request object was taken. Postpone this until we finished using the object.
In theory I think it is possible that the request_fd can be closed by
the application from another thread. In that case when request_put is
called the whole request would be freed.
It's highly unlikely, but let's just be safe and fix this potential
race condition.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
drivers/media/v4l2-core/v4l2-ctrls.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index cc266a4a6e88..95d065d54308 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3657,10 +3657,9 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh,
}
obj = v4l2_ctrls_find_req_obj(hdl, req, set);
- /* Reference to the request held through obj */
- media_request_put(req);
if (IS_ERR(obj)) {
media_request_unlock_for_update(req);
+ media_request_put(req);
return PTR_ERR(obj);
}
hdl = container_of(obj, struct v4l2_ctrl_handler,
@@ -3670,8 +3669,9 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh,
ret = try_set_ext_ctrls_common(fh, hdl, cs, set);
if (obj) {
- media_request_unlock_for_update(obj->req);
+ media_request_unlock_for_update(req);
media_request_object_put(obj);
+ media_request_put(req);
}
return ret;
--
2.18.0
[PATCHv4 04/10] videodev2.h: add new capabilities for buffer types
From: Hans Verkuil
VIDIOC_REQBUFS and VIDIOC_CREATE_BUFFERS will return capabilities
telling userspace what the given buffer type is capable of.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
.../media/uapi/v4l/vidioc-create-bufs.rst | 14 ++-
.../media/uapi/v4l/vidioc-reqbufs.rst | 42 ++-
include/uapi/linux/videodev2.h| 13 +-
3 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
index a39e18d69511..eadf6f757fbf 100644
--- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
@@ -102,7 +102,19 @@ than the number requested.
- ``format``
- Filled in by the application, preserved by the driver.
* - __u32
- - ``reserved``\ [8]
+ - ``capabilities``
+ - Set by the driver. If 0, then the driver doesn't support
+capabilities. In that case all you know is that the driver is
+ guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
+ other :c:type:`v4l2_memory` types. It will not support any others
+ capabilities. See :ref:`here ` for a list of the
+ capabilities.
+
+ 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
+ - ``reserved``\ [7]
- A place holder for future extensions. Drivers and applications
must set the array to zero.
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index 316f52c8a310..d40c60e8 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -88,10 +88,50 @@ any DMA in progress, an implicit
``V4L2_MEMORY_DMABUF`` or ``V4L2_MEMORY_USERPTR``. See
:c:type:`v4l2_memory`.
* - __u32
- - ``reserved``\ [2]
+ - ``capabilities``
+ - Set by the driver. If 0, then the driver doesn't support
+capabilities. In that case all you know is that the driver is
+ guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
+ other :c:type:`v4l2_memory` types. It will not support any others
+ capabilities.
+
+ If you want to query the capabilities with a minimum of side-effects,
+ then this can be called with ``count`` set to 0, ``memory`` set to
+ ``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
+ - ``reserved``\ [1]
- 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}|
+
+.. _v4l2-buf-capabilities:
+.. _V4L2-BUF-CAP-SUPPORTS-MMAP:
+.. _V4L2-BUF-CAP-SUPPORTS-USERPTR:
+.. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
+.. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
+
+.. cssclass:: longtable
+
+.. flat-table:: V4L2 Buffer Capabilities Flags
+:header-rows: 0
+:stub-columns: 0
+:widths: 3 1 4
+
+* - ``V4L2_BUF_CAP_SUPPORTS_MMAP``
+ - 0x0001
+ - This buffer type supports the ``V4L2_MEMORY_MMAP`` streaming mode.
+* - ``V4L2_BUF_CAP_SUPPORTS_USERPTR``
+ - 0x0002
+ - This buffer type supports the ``V4L2_MEMORY_USERPTR`` streaming mode.
+* - ``V4L2_BUF_CAP_SUPPORTS_DMABUF``
+ - 0x0004
+ - This buffer type supports the ``V4L2_MEMORY_DMABUF`` streaming mode.
+* - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
+ - 0x0008
+ - This buffer type supports :ref:`requests `.
Return Value
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2350151ce4ea..55d45a387dd2 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -856,9 +856,16 @@ struct v4l2_requestbuffers {
__u32 count;
__u32 type; /* enum v4l2_buf_type */
__u32 memory; /* enum v4l2_memory */
- __u32 reserved[2];
+ __u32 capabilities;
+ __u32 reserved[1];
};
+/* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
+#define V4L2_BUF_CAP_SUPPORTS_MMAP (1 << 0)
+#define V4L2_BUF_CAP_SUPPORTS_USERPTR (1 << 1)
+#define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2)
+#define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3)
+
/**
* struct v4l2_plane - plane info for multi-planar buffers
* @bytesused: number of bytes occupied by data in the plane (payload)
@@ -2319,6 +2326,7 @@ struct v4l2_dbg_chip_info {
* return: number of created buffers
* @memory:enum v4l2_memory;
[PATCHv4 10/10] media-request: update documentation
From: Hans Verkuil
Various clarifications and readability improvements based on
Laurent Pinchart's review of the documentation.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
.../uapi/mediactl/media-ioc-request-alloc.rst | 3 +-
.../uapi/mediactl/media-request-ioc-queue.rst | 7 +--
.../media/uapi/mediactl/request-api.rst | 51 +++
.../uapi/mediactl/request-func-close.rst | 1 +
.../media/uapi/mediactl/request-func-poll.rst | 2 +-
Documentation/media/uapi/v4l/buffer.rst | 14 +++--
.../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 5 +-
Documentation/media/uapi/v4l/vidioc-qbuf.rst | 5 +-
8 files changed, 52 insertions(+), 36 deletions(-)
diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
index 34434e2b3918..0f8b31874002 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
@@ -52,7 +52,8 @@ for the request to complete.
The request will remain allocated until all the file descriptors associated
with it are closed by :ref:`close() ` and the driver no
-longer uses the request internally.
+longer uses the request internally. See also
+:ref:`here ` for more information.
Return Value
diff --git a/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
b/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
index dbf635ae9b2b..6dd2d7fea714 100644
--- a/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
+++ b/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
@@ -40,9 +40,6 @@ Other errors can be returned if the contents of the request
contained
invalid or inconsistent data, see the next section for a list of
common error codes. On error both the request and driver state are unchanged.
-Typically if you get an error here, then that means that the application
-did something wrong and you have to fix the application.
-
Once a request is queued, then the driver is required to gracefully handle
errors that occur when the request is applied to the hardware. The
exception is the ``EIO`` error which signals a fatal error that requires
@@ -68,8 +65,8 @@ EBUSY
to mix the two APIs.
ENOENT
The request did not contain any buffers. All requests are required
-to have at least one buffer. This can also be returned if required
-controls are missing.
+to have at least one buffer. This can also be returned if some required
+configuration is missing in the request.
ENOMEM
Out of memory when allocating internal data structures for this
request.
diff --git a/Documentation/media/uapi/mediactl/request-api.rst
b/Documentation/media/uapi/mediactl/request-api.rst
index aeb8d00934a4..5f4a23029c48 100644
--- a/Documentation/media/uapi/mediactl/request-api.rst
+++ b/Documentation/media/uapi/mediactl/request-api.rst
@@ -12,6 +12,9 @@ the same pipeline to reconfigure and collaborate closely on a
per-frame basis.
Another is support of stateless codecs, which require controls to be applied
to specific frames (aka 'per-frame controls') in order to be used efficiently.
+While the initial use-case was V4L2, it can be extended to other subsystems
+as well, as long as they use the media controller.
+
Supporting these features without the Request API is not always possible and if
it is, it is terribly inefficient: user-space would have to flush all activity
on the media pipeline, reconfigure it for the next frame, queue the buffers to
@@ -20,19 +23,23 @@ dequeuing before considering the next frame. This defeats
the purpose of having
buffer queues since in practice only one buffer would be queued at a time.
The Request API allows a specific configuration of the pipeline (media
-controller topology + controls for each media entity) to be associated with
-specific buffers. The parameters are applied by each participating device as
-buffers associated to a request flow in. This allows user-space to schedule
-several tasks ("requests") with different parameters in advance, knowing that
-the parameters will be applied when needed to get the expected result. Control
-values at the time of request completion are also available for reading.
+controller topology + configuration for each media entity) to be associated
with
+specific buffers. This allows user-space to schedule several tasks ("requests")
+with different configurations in advance, knowing that the configuration will
be
+applied when needed to get the expected result. Configuration values at the
time
+of request completion are also available for reading.
Usage
=
-The Request API is used on top of standard media controller and V4L2 calls,
-which are augmented with an extra ``request_fd`` parameter. Requests themselves
-are allocated from the supporting media controller node.
+The Request API extends the Media Controller API and cooperates
[PATCHv4 03/10] buffer.rst: only set V4L2_BUF_FLAG_REQUEST_FD for QBUF
From: Hans Verkuil Document that V4L2_BUF_FLAG_REQUEST_FD should only be used with VIDIOC_QBUF and cleared otherwise. Signed-off-by: Hans Verkuil Reviewed-by: Tomasz Figa --- Documentation/media/uapi/v4l/buffer.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index 35c2fadd10de..1865cd5b9d3c 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -312,6 +312,8 @@ struct v4l2_buffer and flag ``V4L2_BUF_FLAG_REQUEST_FD`` is set, then the buffer will be queued to that request. This is set by the user when calling :ref:`ioctl VIDIOC_QBUF ` and ignored by other ioctls. + Applications should not set ``V4L2_BUF_FLAG_REQUEST_FD`` for any ioctls + other than :ref:`VIDIOC_QBUF `. If the device does not support requests, then ``EPERM`` will be returned. If requests are supported but an invalid request file descriptor is given, then ``EINVAL`` will be returned. -- 2.18.0
[PATCHv4 06/10] media-request: add media_request_(un)lock_for_access
From: Hans Verkuil
Add helper functions to prevent a completed request from being
re-inited while it is being accessed.
Signed-off-by: Hans Verkuil
Reviewed-by: Tomasz Figa
---
drivers/media/media-request.c | 10 +++
include/media/media-request.h | 56 +++
2 files changed, 66 insertions(+)
diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
index 4cee67e6657e..414197645e09 100644
--- a/drivers/media/media-request.c
+++ b/drivers/media/media-request.c
@@ -43,6 +43,7 @@ static void media_request_clean(struct media_request *req)
/* Just a sanity check. No other code path is allowed to change this. */
WARN_ON(req->state != MEDIA_REQUEST_STATE_CLEANING);
WARN_ON(req->updating_count);
+ WARN_ON(req->access_count);
list_for_each_entry_safe(obj, obj_safe, >objects, list) {
media_request_object_unbind(obj);
@@ -50,6 +51,7 @@ static void media_request_clean(struct media_request *req)
}
req->updating_count = 0;
+ req->access_count = 0;
WARN_ON(req->num_incomplete_objects);
req->num_incomplete_objects = 0;
wake_up_interruptible_all(>poll_wait);
@@ -198,6 +200,13 @@ static long media_request_ioctl_reinit(struct
media_request *req)
spin_unlock_irqrestore(>lock, flags);
return -EBUSY;
}
+ if (req->access_count) {
+ dev_dbg(mdev->dev,
+ "request: %s is being accessed, cannot reinit\n",
+ req->debug_str);
+ spin_unlock_irqrestore(>lock, flags);
+ return -EBUSY;
+ }
req->state = MEDIA_REQUEST_STATE_CLEANING;
spin_unlock_irqrestore(>lock, flags);
@@ -313,6 +322,7 @@ int media_request_alloc(struct media_device *mdev, int
*alloc_fd)
spin_lock_init(>lock);
init_waitqueue_head(>poll_wait);
req->updating_count = 0;
+ req->access_count = 0;
*alloc_fd = fd;
diff --git a/include/media/media-request.h b/include/media/media-request.h
index ac02019c1d77..d8c8db89dbde 100644
--- a/include/media/media-request.h
+++ b/include/media/media-request.h
@@ -53,6 +53,7 @@ struct media_request_object;
* @debug_str: Prefix for debug messages (process name:fd)
* @state: The state of the request
* @updating_count: count the number of request updates that are in progress
+ * @access_count: count the number of request accesses that are in progress
* @objects: List of @struct media_request_object request objects
* @num_incomplete_objects: The number of incomplete objects in the request
* @poll_wait: Wait queue for poll
@@ -64,6 +65,7 @@ struct media_request {
char debug_str[TASK_COMM_LEN + 11];
enum media_request_state state;
unsigned int updating_count;
+ unsigned int access_count;
struct list_head objects;
unsigned int num_incomplete_objects;
struct wait_queue_head poll_wait;
@@ -72,6 +74,50 @@ struct media_request {
#ifdef CONFIG_MEDIA_CONTROLLER
+/**
+ * media_request_lock_for_access - Lock the request to access its objects
+ *
+ * @req: The media request
+ *
+ * Use before accessing a completed request. A reference to the request must
+ * be held during the access. This usually takes place automatically through
+ * a file handle. Use @media_request_unlock_for_access when done.
+ */
+static inline int __must_check
+media_request_lock_for_access(struct media_request *req)
+{
+ unsigned long flags;
+ int ret = -EBUSY;
+
+ spin_lock_irqsave(>lock, flags);
+ if (req->state == MEDIA_REQUEST_STATE_COMPLETE) {
+ req->access_count++;
+ ret = 0;
+ }
+ spin_unlock_irqrestore(>lock, flags);
+
+ return ret;
+}
+
+/**
+ * media_request_unlock_for_access - Unlock a request previously locked for
+ * access
+ *
+ * @req: The media request
+ *
+ * Unlock a request that has previously been locked using
+ * @media_request_lock_for_access.
+ */
+static inline void media_request_unlock_for_access(struct media_request *req)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(>lock, flags);
+ if (!WARN_ON(!req->access_count))
+ req->access_count--;
+ spin_unlock_irqrestore(>lock, flags);
+}
+
/**
* media_request_lock_for_update - Lock the request for updating its objects
*
@@ -333,6 +379,16 @@ void media_request_object_complete(struct
media_request_object *obj);
#else
+static inline int __must_check
+media_request_lock_for_access(struct media_request *req)
+{
+ return -EINVAL;
+}
+
+static inline void media_request_unlock_for_access(struct media_request *req)
+{
+}
+
static inline int __must_check
media_request_lock_for_update(struct media_request *req)
{
--
2.18.0
[PATCHv4 01/10] media-request: return -EINVAL for invalid request_fds
From: Hans Verkuil Instead of returning -ENOENT when a request_fd was not found (VIDIOC_QBUF and VIDIOC_G/S/TRY_EXT_CTRLS), we now return -EINVAL. This is in line with what we do when invalid dmabuf fds are passed to e.g. VIDIOC_QBUF. Also document that EINVAL is returned for invalid m.fd values, we never documented that. Signed-off-by: Hans Verkuil Reviewed-by: Tomasz Figa --- Documentation/media/uapi/v4l/buffer.rst| 4 ++-- .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 18 -- Documentation/media/uapi/v4l/vidioc-qbuf.rst | 12 +--- drivers/media/media-request.c | 6 -- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index dd0065a95ea0..35c2fadd10de 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -313,8 +313,8 @@ struct v4l2_buffer queued to that request. This is set by the user when calling :ref:`ioctl VIDIOC_QBUF ` and ignored by other ioctls. If the device does not support requests, then ``EPERM`` will be returned. - If requests are supported but an invalid request FD is given, then - ``ENOENT`` will be returned. + If requests are supported but an invalid request file descriptor is + given, then ``EINVAL`` will be returned. diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst index 771fd1161277..9c56a9b6e98a 100644 --- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst +++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst @@ -101,8 +101,8 @@ then the controls are not applied immediately when calling :ref:`VIDIOC_S_EXT_CTRLS `, but instead are applied by the driver for the buffer associated with the same request. If the device does not support requests, then ``EPERM`` will be returned. -If requests are supported but an invalid request FD is given, then -``ENOENT`` will be returned. +If requests are supported but an invalid request file descriptor is given, +then ``EINVAL`` will be returned. An attempt to call :ref:`VIDIOC_S_EXT_CTRLS ` for a request that has already been queued will result in an ``EBUSY`` error. @@ -301,8 +301,8 @@ still cause this situation. - File descriptor of the request to be used by this operation. Only valid if ``which`` is set to ``V4L2_CTRL_WHICH_REQUEST_VAL``. If the device does not support requests, then ``EPERM`` will be returned. - If requests are supported but an invalid request FD is given, then - ``ENOENT`` will be returned. + If requests are supported but an invalid request file descriptor is + given, then ``EINVAL`` will be returned. * - __u32 - ``reserved``\ [1] - Reserved for future extensions. @@ -378,11 +378,13 @@ appropriately. The generic error codes are described at the EINVAL The struct :c:type:`v4l2_ext_control` ``id`` is -invalid, the struct :c:type:`v4l2_ext_controls` +invalid, or the struct :c:type:`v4l2_ext_controls` ``which`` is invalid, or the struct :c:type:`v4l2_ext_control` ``value`` was inappropriate (e.g. the given menu index is not supported by the -driver). This error code is also returned by the +driver), or the ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` +but the given ``request_fd`` was invalid. +This error code is also returned by the :ref:`VIDIOC_S_EXT_CTRLS ` and :ref:`VIDIOC_TRY_EXT_CTRLS ` ioctls if two or more control values are in conflict. @@ -409,7 +411,3 @@ EACCES EPERM The ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` but the device does not support requests. - -ENOENT -The ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` but the -the given ``request_fd`` was invalid. diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst b/Documentation/media/uapi/v4l/vidioc-qbuf.rst index 0e415f2551b2..7bff69c15452 100644 --- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst +++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst @@ -105,8 +105,8 @@ until the request itself is queued. Also, the driver will apply any settings associated with the request for this buffer. This field will be ignored unless the ``V4L2_BUF_FLAG_REQUEST_FD`` flag is set. If the device does not support requests, then ``EPERM`` will be returned. -If requests are supported but an invalid request FD is given, then -``ENOENT`` will be returned. +If requests are supported but an invalid request file descriptor is given, +then ``EINVAL`` will be returned. .. caution:: It is not allowed to mix queuing requests with queuing buffers directly. @@ -152,7 +152,9 @@ EAGAIN EINVAL The buffer ``type`` is not supported, or the ``index`` is out of bounds, or no buffers have been allocated yet, or the ``userptr`` or -``length`` are
Re: cron job: media_tree daily build: ERRORS
Hi Jasmin, On 09/04/2018 08:33 AM, Hans Verkuil wrote: > This message is generated daily by a cron job that builds media_tree for > the kernels and architectures in the list below. > > Results of the daily build of media_tree: Thank you for all your work, it looks much better now. It seems a lot of the remaining errors are due to a missing dev_warn_once. I've added compat code for that and will do another build run, hopefully the result will be posted here later today. Regards, Hans > > date: Tue Sep 4 04:00:14 CEST 2018 > media-tree git hash: d842a7cf938b6e0f8a1aa9f1aec0476c9a599310 > media_build git hash: 1cd94ce3d513f211dffc576698a5be347352e3cb > v4l-utils git hash: f44f00e8b4ac6e9aa05bac8953e3fcc89e1fe198 > edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 > gcc version: i686-linux-gcc (GCC) 8.2.0 > sparse version: 0.5.2 (Debian: 0.5.2-1+b1) > smatch version: v0.5.0-3428-gdfe27cf > host hardware:x86_64 > host os: 4.17.0-1-amd64 > > linux-git-arm-at91: OK > linux-git-arm-davinci: OK > linux-git-arm-multi: OK > linux-git-arm-pxa: OK > linux-git-arm-stm32: OK > linux-git-arm64: OK > linux-git-i686: OK > linux-git-mips: OK > linux-git-powerpc64: OK > linux-git-sh: OK > linux-git-x86_64: OK > Check COMPILE_TEST: OK > linux-2.6.36.4-i686: ERRORS > linux-2.6.36.4-x86_64: ERRORS > linux-2.6.37.6-i686: ERRORS > linux-2.6.37.6-x86_64: ERRORS > linux-2.6.38.8-i686: ERRORS > linux-2.6.38.8-x86_64: ERRORS > linux-2.6.39.4-i686: ERRORS > linux-2.6.39.4-x86_64: ERRORS > linux-3.0.101-i686: ERRORS > linux-3.0.101-x86_64: ERRORS > linux-3.1.10-i686: ERRORS > linux-3.1.10-x86_64: ERRORS > linux-3.2.102-i686: ERRORS > linux-3.2.102-x86_64: ERRORS > linux-3.3.8-i686: ERRORS > linux-3.3.8-x86_64: ERRORS > linux-3.4.113-i686: ERRORS > linux-3.4.113-x86_64: ERRORS > linux-3.5.7-i686: ERRORS > linux-3.5.7-x86_64: ERRORS > linux-3.6.11-i686: ERRORS > linux-3.6.11-x86_64: ERRORS > linux-3.7.10-i686: ERRORS > linux-3.7.10-x86_64: ERRORS > linux-3.8.13-i686: ERRORS > linux-3.8.13-x86_64: ERRORS > linux-3.9.11-i686: ERRORS > linux-3.9.11-x86_64: ERRORS > linux-3.10.108-i686: ERRORS > linux-3.10.108-x86_64: ERRORS > linux-3.11.10-i686: ERRORS > linux-3.11.10-x86_64: ERRORS > linux-3.12.74-i686: ERRORS > linux-3.12.74-x86_64: ERRORS > linux-3.13.11-i686: ERRORS > linux-3.13.11-x86_64: ERRORS > linux-3.14.79-i686: ERRORS > linux-3.14.79-x86_64: ERRORS > linux-3.15.10-i686: ERRORS > linux-3.15.10-x86_64: ERRORS > linux-3.16.57-i686: ERRORS > linux-3.16.57-x86_64: ERRORS > linux-3.17.8-i686: ERRORS > linux-3.17.8-x86_64: ERRORS > linux-3.18.119-i686: ERRORS > linux-3.18.119-x86_64: ERRORS > linux-3.19.8-i686: OK > linux-3.19.8-x86_64: OK > linux-4.0.9-i686: OK > linux-4.0.9-x86_64: OK > linux-4.1.52-i686: OK > linux-4.1.52-x86_64: OK > linux-4.2.8-i686: OK > linux-4.2.8-x86_64: OK > linux-4.3.6-i686: OK > linux-4.3.6-x86_64: OK > linux-4.4.152-i686: OK > linux-4.4.152-x86_64: OK > linux-4.5.7-i686: OK > linux-4.5.7-x86_64: OK > linux-4.6.7-i686: OK > linux-4.6.7-x86_64: OK > linux-4.7.10-i686: OK > linux-4.7.10-x86_64: OK > linux-4.8.17-i686: OK > linux-4.8.17-x86_64: OK > linux-4.9.124-i686: OK > linux-4.9.124-x86_64: OK > linux-4.10.17-i686: OK > linux-4.10.17-x86_64: OK > linux-4.11.12-i686: OK > linux-4.11.12-x86_64: OK > linux-4.12.14-i686: OK > linux-4.12.14-x86_64: OK > linux-4.13.16-i686: OK > linux-4.13.16-x86_64: OK > linux-4.14.67-i686: OK > linux-4.14.67-x86_64: OK > linux-4.15.18-i686: OK > linux-4.15.18-x86_64: OK > linux-4.16-rc7-i686: OK > linux-4.16-rc7-x86_64: OK > linux-4.16.18-i686: OK > linux-4.16.18-x86_64: OK > linux-4.17.19-i686: OK > linux-4.17.19-x86_64: OK > linux-4.18-rc1-i686: OK > linux-4.18-rc1-x86_64: OK > linux-4.18.5-i686: OK > linux-4.18.5-x86_64: OK > linux-4.19-rc1-i686: OK > linux-4.19-rc1-x86_64: OK > apps: OK > spec-git: OK > sparse: WARNINGS > > Logs weren't copied as they are too large (580 kB) > > The Media Infrastructure API from this daily build is here: > > http://www.xs4all.nl/~hverkuil/spec/index.html >
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Sep 4 04:00:14 CEST 2018 media-tree git hash:d842a7cf938b6e0f8a1aa9f1aec0476c9a599310 media_build git hash: 1cd94ce3d513f211dffc576698a5be347352e3cb v4l-utils git hash: f44f00e8b4ac6e9aa05bac8953e3fcc89e1fe198 edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 (Debian: 0.5.2-1+b1) smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.17.0-1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK Check COMPILE_TEST: OK linux-2.6.36.4-i686: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-i686: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.101-i686: ERRORS linux-3.0.101-x86_64: ERRORS linux-3.1.10-i686: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.102-i686: ERRORS linux-3.2.102-x86_64: ERRORS linux-3.3.8-i686: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.113-i686: ERRORS linux-3.4.113-x86_64: ERRORS linux-3.5.7-i686: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-i686: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.10-i686: ERRORS linux-3.7.10-x86_64: ERRORS linux-3.8.13-i686: ERRORS linux-3.8.13-x86_64: ERRORS linux-3.9.11-i686: ERRORS linux-3.9.11-x86_64: ERRORS linux-3.10.108-i686: ERRORS linux-3.10.108-x86_64: ERRORS linux-3.11.10-i686: ERRORS linux-3.11.10-x86_64: ERRORS linux-3.12.74-i686: ERRORS linux-3.12.74-x86_64: ERRORS linux-3.13.11-i686: ERRORS linux-3.13.11-x86_64: ERRORS linux-3.14.79-i686: ERRORS linux-3.14.79-x86_64: ERRORS linux-3.15.10-i686: ERRORS linux-3.15.10-x86_64: ERRORS linux-3.16.57-i686: ERRORS linux-3.16.57-x86_64: ERRORS linux-3.17.8-i686: ERRORS linux-3.17.8-x86_64: ERRORS linux-3.18.119-i686: ERRORS linux-3.18.119-x86_64: ERRORS linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.152-i686: OK linux-4.4.152-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.124-i686: OK linux-4.9.124-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.67-i686: OK linux-4.14.67-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16-rc7-i686: OK linux-4.16-rc7-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18-rc1-i686: OK linux-4.18-rc1-x86_64: OK linux-4.18.5-i686: OK linux-4.18.5-x86_64: OK linux-4.19-rc1-i686: OK linux-4.19-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Logs weren't copied as they are too large (580 kB) The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
