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: Sat Aug 4 05:00:10 CEST 2018 media-tree git hash:12f336c88090fb8004736fd4329184326a49673b media_build git hash: e75fbc93fc427f769c3ce5a020bf204e02a45852 v4l-utils git hash: 70b13df426d30ca58c79cf8a366e73463bb22cbb edid-decode git hash: ab18befbcacd6cd4dff63faa82e32700369d6f25 gcc version:i686-linux-gcc (GCC) 8.1.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.16.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: WARNINGS linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS 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: 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.115-i686: ERRORS linux-3.18.115-x86_64: ERRORS linux-3.19.8-i686: ERRORS linux-3.19.8-x86_64: ERRORS linux-4.0.9-i686: ERRORS linux-4.0.9-x86_64: ERRORS linux-4.1.52-i686: ERRORS linux-4.1.52-x86_64: ERRORS linux-4.2.8-i686: ERRORS linux-4.2.8-x86_64: ERRORS linux-4.3.6-i686: ERRORS linux-4.3.6-x86_64: ERRORS linux-4.4.140-i686: ERRORS linux-4.4.140-x86_64: ERRORS linux-4.5.7-i686: ERRORS linux-4.5.7-x86_64: ERRORS linux-4.6.7-i686: ERRORS linux-4.6.7-x86_64: ERRORS linux-4.7.10-i686: ERRORS linux-4.7.10-x86_64: ERRORS linux-4.8.17-i686: ERRORS linux-4.8.17-x86_64: ERRORS linux-4.9.112-i686: OK linux-4.9.112-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.55-i686: OK linux-4.14.55-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.6-i686: OK linux-4.17.6-x86_64: OK linux-4.18-rc4-i686: OK linux-4.18-rc4-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
[GIT PULL for 4.19] Fix for mt9v111 driver
Hi Mauro, Here's one more patch for 4.19. It's a fix, but the driver isn't in the fixes branch so it's on master. Please pull. The following changes since commit 2c3449fb95c318920ca8dc645d918d408db219ac: media: usb: hackrf: Replace GFP_ATOMIC with GFP_KERNEL (2018-08-02 19:16:17 -0400) are available in the git repository at: ssh://linuxtv.org/git/sailus/media_tree.git for-4.19-5 for you to fetch changes up to 86f2c9ac5248b4a4d784a7af95270567bbb959e3: media: i2c: mt9v111: Fix v4l2-ctrl error handling (2018-08-04 01:05:44 +0300) Jacopo Mondi (1): media: i2c: mt9v111: Fix v4l2-ctrl error handling drivers/media/i2c/mt9v111.c | 41 + 1 file changed, 13 insertions(+), 28 deletions(-) -- Sakari Ailus e-mail: sakari.ai...@iki.fi
[RFC] minute notes from DT bindings discussion on Aug, 2
Hi all, In 2016, we had lots of discussions among the top media developers with regards to connectors and DT bindings. On that time, we didn't reach a final conclusion. We recently received a patchset from Marco changing how tvp5150 driver works, adding a new DT binding proposal: https://patchwork.linuxtv.org/patch/50648/ So, yesterday we did another round in order to try to reach some consensus among the core developers. The goal of this RFC is to summarize the consensus we reached, asking the community to provide us feedback, in order for us to improve the subsystem support for connectors. The entire discussion can be seen at: https://linuxtv.org/irc/irclogger_log/media-maint?date=2018-08-02,Thu Exceptionally, this time we did the discussions at the media maintainer's channel, because #v4l was to noisy with lots of spam being flooded there. Thanks, Mauro 1. Description of the problem = In 2015, we had the first media controller workshop and we added support for new stuff at the media controller side. One of the points agreed there is that connectors should be represented at the media graph. In 2016, we had several discussions about how to properly represent connectors, like, for example: - https://linuxtv.org/irc/irclogger_log/v4l?date=2016-02-26,Fri - https://linuxtv.org/irc/irclogger_log/v4l?date=2016-03-03,Thu On that time, the consensus was that connectors should be describing the logical connections, as physically sometimes a single connector (like SCART) can bring several different signals inside, each being routed to a different entity or pad. However, we didn't finish the discussions. On today's meeting, we came with the challenge of answering two questions: (a) should a physical connector that internally comes with two separate electric signals (like S-Video) be mapped as a single connector? (b) what would be the proper way to describe a device like tvp5150, with has internally a MUX that selects between 3 different possible physical connectors (composite 0, composite 1, s-video), mapping them into 2 electrical pins (AIP1A and AIP1B) and the decoder itself can handle just one signal, e. g., physically, the hardware is: "physical" pin input internal connectors terminals input port comp 0 --\ + --> AIP1A > \ +-+ +/ pin \--> | | | luma| | s-video-+ | MUX +---> video decoder | chroma | | +-\ /--> | | + -> AIP1B > / +-+ comp 1 ---/ pin So, depending on how the device is seemed, it could have 3, 2 or 1 media controller pads and 3, 2 or 1 Device tree ports. 2. Answers for the questions question (a): S-Video is a connector? -_--- During the discussion, Hans came with a similar problem on a hardware he works with (SX80 telepresence codec). This hardware has: - 3 HDMI inputs; - 1 DVI-I input; - 2 RCA inputs (either two composite or combines to one S-Video); - 2 HDMI outputs; - 1 DVI-I output. The DVI-I mapping is also a challenge, as it can carry either an analog or a digital signal (or both). On this specific hardware, if one wants to use S-Video, it needs a cable with one miniDin 4-pin in one side and 2 RCA pins at the other side. It was also discussed about the properties API, with would allow adding labels to each connector. It was a consensus that the best way to map S-Video is to be a single connector. So, on a hardware like SX80, for example, the properties API could carry a description like: comp0: -> "RCA Y Plug" comp1 -> "RCA C Plug" s-video -> "RCA Y+C Plugs" question (b): how many MC PADs and DT ports tvp5150 should have? That was the warmer discussion, as we started with 3 different opinions: - Mauro would map at the output of the mux, e. g. 1 pad/port; - Marco proposed on his patchset to use (up to) 3 pad/ports, e. g. at the "physically" visible connectors[1]; - Laurent would map as 2, e. g. at the two physical pins at the tvp5150 chip. [1] That's actually not true physical connectors - as the connectors are device-specific. For example the device I'm using for tests is a Terratec Grabster AV-350. It have one SCART connector with both composite and S-Video signals, a S-Video port and 3 connectors for composite (video, audio R, audio L) and a mechanical switch (labeled Video/Scart) with selects between the SCART connector and the 4 other ones; After some discussions, we reached a consensus that mapping it at the tvp5150 chip pin level - e. g. two
Re: [PATCH 20/22] [media] tvp5150: Add input port connectors DT bindings
Em Fri, 3 Aug 2018 09:29:53 +0200 Marco Felsch escreveu: > Hi Rob, > > first of all, thanks for the review. After some discussion with the > media guys I have a question about the dt-bindings. > > On 18-07-03 17:23, Rob Herring wrote: > > On Thu, Jun 28, 2018 at 06:20:52PM +0200, Marco Felsch wrote: > > > The TVP5150/1 decoders support different video input sources to their > > > AIP1A/B pins. > > > > > > Possible configurations are as follows: > > > - Analog Composite signal connected to AIP1A. > > > - Analog Composite signal connected to AIP1B. > > > - Analog S-Video Y (luminance) and C (chrominance) > > > signals connected to AIP1A and AIP1B respectively. > > > > > > This patch extends the device tree bindings documentation to describe > > > how the input connectors for these devices should be defined in a DT. > > > > > > Signed-off-by: Marco Felsch > > > --- > > > .../devicetree/bindings/media/i2c/tvp5150.txt | 118 +- > > > 1 file changed, 113 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > index 8c0fc1a26bf0..feed8c911c5e 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > @@ -12,11 +12,23 @@ Optional Properties: > > > - pdn-gpios: phandle for the GPIO connected to the PDN pin, if any. > > > - reset-gpios: phandle for the GPIO connected to the RESETB pin, if any. > > > > > > -The device node must contain one 'port' child node for its digital output > > > -video port, in accordance with the video interface bindings defined in > > > -Documentation/devicetree/bindings/media/video-interfaces.txt. > > > +The device node must contain one 'port' child node per device input and > > > output > > > +port, in accordance with the video interface bindings defined in > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port > > > nodes > > > +are numbered as follows > > > > > > -Required Endpoint Properties for parallel synchronization: > > > + Name TypePort > > > + -- > > > + AIP1A sink0 > > > + AIP1B sink1 > > > + S-VIDEO sink2 > > > + Y-OUT src 3 > > Do you think it's correct to have a seperate port for each binding? > Since the S-Video port is a combination of AIP1A and AIP1B. After a > discussion with Mauro [1] the TVP5150 should have only 3 pads. Since the > pads are directly mappped to the dt-ports this will correspond to three > ports (2 in, 1 out). Now the svideo connector will be mapped to a second > endpoint in each port: > > port@0 > endpoint@0 ---> Comp0-Con > endpoint@1 -+-> Svideo-Con > port@1| > endpoint@0 -|-> Comp1-Con > endpoint@1 -+ > port@2 > endpoint For tvp5150, the model is like the above, so just one port at the S-video connector. Yet, for more complex devices that would allow switching the endpoints at the Svideo connector, the model would be: port@0 endpoint@0 (AIP1A) ---> Comp0-Con endpoint@1 (AIP1B) ---> Svideo-Con port@0 (luminance) port@1 endpoint@0 (AIP1A) ---> Comp1-Con endpoint@1 (AIP1B) ---> Svideo-Con port@1 (chrominance) port@2 endpoint (video bitstream output) E. g. the S-Video connector will also have two ports, one for the chrominance signal and another one for the luminance one. > I don't like that solution that much, since the mapping is now signal > based. We also don't map each line of a parallel port. > > A quick grep shows that currently each device using a svideo connector > seperates them in a own port as I did. No. I've no idea about how you did the grep, but this is not how other drivers handle it currently. Right now, on all hardware where connectors are mapped, there is just one input port and multiple connectors linked to it. You can see some examples here: https://www.infradead.org/~mchehab/mc-next-gen/au0828_hvr950q.png https://www.infradead.org/~mchehab/mc-next-gen/cx231xx_hvr930c_hd.png https://www.infradead.org/~mchehab/mc-next-gen/em28xx_hvr950.png https://www.infradead.org/~mchehab/mc-next-gen/playtv_usb.png https://www.infradead.org/~mchehab/mc-next-gen/saa7134-asus-p7131-dual.png https://www.infradead.org/~mchehab/mc-next-gen/wintv_usb2.png The problem with this approach is that it doesn't reflect how the hardware is actually wired. On some hardware similar to tvp5150, it may be possible, for example, to wire the S-Video both ways, e. g., something equivalent to (using tvp5150 terminology): Luminance -> AIP1A Chrominance -> AIP1B or Luminance
Re: [RFC PATCH 2/3] media: add support for properties
Em Fri, 3 Aug 2018 16:36:25 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Initially support u64/s64 and string properties.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 98 +++-
> drivers/media/media-entity.c | 65 +
> include/media/media-device.h | 6 ++
> include/media/media-entity.h | 172 +++
> 4 files changed, 338 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index fcdf3d5dc4b6..6fa9555a669f 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -241,10 +241,15 @@ static long media_device_get_topology(struct
> media_device *mdev, void *arg)
> struct media_interface *intf;
> struct media_pad *pad;
> struct media_link *link;
> + struct media_prop *prop;
> struct media_v2_entity kentity, __user *uentity;
> struct media_v2_interface kintf, __user *uintf;
> struct media_v2_pad kpad, __user *upad;
> struct media_v2_link klink, __user *ulink;
> + struct media_v2_prop kprop, __user *uprop;
> + void __user *uprop_payload;
> + unsigned int payload_size = 0;
> + unsigned int payload_offset = 0;
> unsigned int i;
> int ret = 0;
>
> @@ -374,6 +379,73 @@ static long media_device_get_topology(struct
> media_device *mdev, void *arg)
> topo->num_links = i;
> topo->reserved4 = 0;
>
> + /* Get properties and number of properties */
> + i = 0;
> + uprop = media_get_uptr(topo->ptr_props);
> + payload_offset = topo->num_props * sizeof(*uprop);
> + media_device_for_each_prop(prop, mdev) {
> + payload_size += prop->payload_size;
> + i++;
> +
> + if (ret || !uprop)
> + continue;
I would add here filter capabilities to to the objects requested by
the ioctl, e. g. something similar to this:
bool return_prop = false;
enum media_gobj_type type = media_type(prop->graph_obj.id);
/*
* If only props are required, don't filter them.
* Otherwise, filter properties according with the types requested
* by G_TOPOLOGY call
*/
if (!(uentity || uintf || upad || ulink))
return_prop = true;
else if (uentity && type == MEDIA_GRAPH_ENTITY)
return_prop = true;
else if (uintf && type == MEDIA_GRAPH_INTF_DEVNODE)
return_prop = true;
else if (upad && type == MEDIA_GRAPH_PAD)
return_prop = true;
else if (link && type == MEDIA_GRAPH_LINK)
return_prop = true;
if (return_prop) {
/* the code that copy properties below */
}
> +
> + if (i > topo->num_props) {
> + ret = -ENOSPC;
> + continue;
> + }
> +
> + memset(, 0, sizeof(kprop));
> +
> + /* Copy prop fields to userspace struct */
> + kprop.id = prop->graph_obj.id;
> + kprop.owner_id = prop->owner->id;
> + kprop.type = prop->type;
> + kprop.flags = 0;
> + kprop.payload_size = prop->payload_size;
> + if (kprop.payload_size)
> + kprop.payload_offset = payload_offset +
> + payload_size - prop->payload_size;
> + else
> + kprop.payload_offset = 0;
> + payload_offset -= sizeof(*uprop);
> + memcpy(kprop.name, prop->name, sizeof(kprop.name));
> + kprop.uval = prop->uval;
> +
> + if (copy_to_user(uprop, , sizeof(kprop)))
> + ret = -EFAULT;
> + uprop++;
This way, properties will be filtered according with userspace's needs.
> + }
> + topo->num_props = i;
> + if (uprop && topo->props_payload_size < payload_size)
> + ret = -ENOSPC;
> + topo->props_payload_size = payload_size;
> + if (!uprop || ret)
> + return ret;
> +
> + uprop_payload = uprop;
> + media_device_for_each_prop(prop, mdev) {
> + i++;
> +
> + if (!prop->payload_size)
> + continue;
You would need a similar test here too.
> +
> + if (copy_to_user(uprop_payload, prop->string,
> prop->payload_size))
> + return -EFAULT;
> + uprop_payload += prop->payload_size;
> + }
> +
> + return 0;
> +}
> +
> +static long media_device_get_topology_1(struct media_device *mdev, void *arg)
> +{
> + struct media_v2_topology topo = {};
> + long ret;
> +
> + memcpy(, arg, sizeof(struct media_v2_topology_1));
> + ret = media_device_get_topology(mdev, );
> + memcpy(arg, , sizeof(struct media_v2_topology_1));
> return ret;
> }
Nah, no need that.
>
> @@ -424,6 +496,7 @@ static const struct media_ioctl_info
Re: [RFC PATCH 1/3] uapi/linux/media.h: add property support
Em Fri, 3 Aug 2018 16:36:24 +0200
Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Add a new topology struct that includes properties.
>
> Signed-off-by: Hans Verkuil
> ---
> include/uapi/linux/media.h | 62 +-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 36f76e777ef9..dd8c96a17020 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -342,6 +342,61 @@ 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
> + * @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];
> + char name[32];
> + union {
> + __u64 uval;
> + __s64 sval;
> + };
> +} __attribute__ ((packed));
> +
> +/* Old version 1 of this struct */
> +struct media_v2_topology_1 {
> + __u64 topology_version;
> +
> + __u32 num_entities;
> + __u32 reserved1;
> + __u64 ptr_entities;
> +
> + __u32 num_interfaces;
> + __u32 reserved2;
> + __u64 ptr_interfaces;
> +
> + __u32 num_pads;
> + __u32 reserved3;
> + __u64 ptr_pads;
> +
> + __u32 num_links;
> + __u32 reserved4;
> + __u64 ptr_links;
> +} __attribute__ ((packed));
> +
As I said at patch 0/3, no need to keep it at the public header. you'll
need this only at media-device.c, just in order to do:
sizeof(old_struct)
> struct media_v2_topology {
> __u64 topology_version;
>
> @@ -360,6 +415,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,7 +427,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)
> -#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
> +#define MEDIA_IOC_G_TOPOLOGY_1 _IOWR('|', 0x04, struct
> media_v2_topology_1)
> +#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x05, struct media_v2_topology)
Why renaming it? No need at all. Just keep the original definition,
and let media-device.c to handle the different ioctl sizes.
Thanks,
Mauro
Re: [RFC PATCH 0/3] Media Controller Properties
Em Fri, 3 Aug 2018 17:03:20 +0200 Hans Verkuil escreveu: > On 08/03/2018 04:36 PM, Hans Verkuil wrote: > > From: Hans Verkuil > > > > This RFC patch series implements properties for the media controller. > > > > This is not finished, but I wanted to post this so people can discuss > > this further. > > > > No documentation yet (too early for that). That's my first complain :-D Anyway, just 3 patches should be good enough to review without docs (famous last words). > > > > An updated v4l2-ctl and v4l2-compliance that can report properties > > is available here: > > > > https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=props > > > > There is one main missing piece: currently the properties are effectively > > laid out in random order. My plan is to change that so they are grouped > > by object type and object owner. So first all properties for each entity, > > then for each pad, etc. I started to work on that, but it's a bit more > > work than expected and I wanted to post this before the weekend. IMO, the best is to use some tree struct. The Kernel has some types that could help setting it, although we never used on media. See, for example Documentation/rbtree.txt for one such type. > > > > While it is possible to have nested properties, this is not currently > > implemented. Only properties for entities and pads are supported in this > > code, but that's easy to extend to interfaces and links. With a tree-based data structure, this would likely come for free. > > > > I'm not sure about the G_TOPOLOGY ioctl handling: I went with the quickest > > option by renaming the old ioctl and adding a new one with property support. Why? No need for that at the public header. Just add the needed fields at the end of the code and check for struct size at the ioctl handler. It could make sense to have the old struct inside media-device.c, just to allow using sizeof() there. > > > > I think this needs to change (at the very least the old and new should > > share the same ioctl NR), but that's something for the future. You don't need that. Just be sure to mask the size when checking for the ioctl, and then the code will check for the struct size, comparing if it matches _v1 and the latest version (with should keep the same name). > > > > Currently I support u64, s64 and const char * property types. But it > > can be anything including binary data if needed. No array support (as we > > have for controls), but there are enough reserved fields in media_v2_prop > > to add this if needed. That's u64/s64/char* are probably enough for now. > > > > I added properties for entities and pads to vimc, so I could test this. > > I forgot to mention that there are known issues with the initialization > of the entity props list (it happens in two places, I need to look into > that) and that mdev is expected to be valid when adding properties, but > I don't think that is necessarily the case. > > So just be aware of that. > > Regards, > > Hans > > > > > Regards, > > > > Hans > > > > Hans Verkuil (3): > > uapi/linux/media.h: add property support > > media: add support for properties > > vimc: add test properties > > > > drivers/media/media-device.c | 98 +++- > > drivers/media/media-entity.c | 65 > > drivers/media/platform/vimc/vimc-common.c | 18 +++ > > drivers/media/platform/vimc/vimc-core.c | 6 +- > > include/media/media-device.h | 6 + > > include/media/media-entity.h | 172 ++ > > include/uapi/linux/media.h| 62 +++- > > 7 files changed, 420 insertions(+), 7 deletions(-) > > > Thanks, Mauro
Re: [RFC PATCH 0/3] Media Controller Properties
On 08/03/2018 04:36 PM, Hans Verkuil wrote: > From: Hans Verkuil > > This RFC patch series implements properties for the media controller. > > This is not finished, but I wanted to post this so people can discuss > this further. > > No documentation yet (too early for that). > > An updated v4l2-ctl and v4l2-compliance that can report properties > is available here: > > https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=props > > There is one main missing piece: currently the properties are effectively > laid out in random order. My plan is to change that so they are grouped > by object type and object owner. So first all properties for each entity, > then for each pad, etc. I started to work on that, but it's a bit more > work than expected and I wanted to post this before the weekend. > > While it is possible to have nested properties, this is not currently > implemented. Only properties for entities and pads are supported in this > code, but that's easy to extend to interfaces and links. > > I'm not sure about the G_TOPOLOGY ioctl handling: I went with the quickest > option by renaming the old ioctl and adding a new one with property support. > > I think this needs to change (at the very least the old and new should > share the same ioctl NR), but that's something for the future. > > Currently I support u64, s64 and const char * property types. But it > can be anything including binary data if needed. No array support (as we > have for controls), but there are enough reserved fields in media_v2_prop > to add this if needed. > > I added properties for entities and pads to vimc, so I could test this. I forgot to mention that there are known issues with the initialization of the entity props list (it happens in two places, I need to look into that) and that mdev is expected to be valid when adding properties, but I don't think that is necessarily the case. So just be aware of that. Regards, Hans > > Regards, > > Hans > > Hans Verkuil (3): > uapi/linux/media.h: add property support > media: add support for properties > vimc: add test properties > > drivers/media/media-device.c | 98 +++- > drivers/media/media-entity.c | 65 > drivers/media/platform/vimc/vimc-common.c | 18 +++ > drivers/media/platform/vimc/vimc-core.c | 6 +- > include/media/media-device.h | 6 + > include/media/media-entity.h | 172 ++ > include/uapi/linux/media.h| 62 +++- > 7 files changed, 420 insertions(+), 7 deletions(-) >
Re: [PATCHv16 01/34] Documentation: v4l: document request API
Hi Hans, On Fri, Jul 6, 2018 at 1:04 AM Hans Verkuil wrote: > > From: Alexandre Courbot > > Document the request API for V4L2 devices, and amend the documentation > of system calls influenced by it. > > Signed-off-by: Alexandre Courbot > Signed-off-by: Hans Verkuil > --- > .../media/uapi/mediactl/media-controller.rst | 1 + > .../media/uapi/mediactl/media-funcs.rst | 6 + > .../uapi/mediactl/media-ioc-request-alloc.rst | 77 ++ > .../uapi/mediactl/media-request-ioc-queue.rst | 81 ++ > .../mediactl/media-request-ioc-reinit.rst | 51 > .../media/uapi/mediactl/request-api.rst | 247 ++ > .../uapi/mediactl/request-func-close.rst | 49 > .../uapi/mediactl/request-func-ioctl.rst | 68 + > .../media/uapi/mediactl/request-func-poll.rst | 74 ++ > Documentation/media/uapi/v4l/buffer.rst | 21 +- > .../media/uapi/v4l/vidioc-g-ext-ctrls.rst | 94 --- > Documentation/media/uapi/v4l/vidioc-qbuf.rst | 32 ++- > .../media/videodev2.h.rst.exceptions | 1 + > 13 files changed, 766 insertions(+), 36 deletions(-) > create mode 100644 > Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst > create mode 100644 > Documentation/media/uapi/mediactl/media-request-ioc-queue.rst > create mode 100644 > Documentation/media/uapi/mediactl/media-request-ioc-reinit.rst > create mode 100644 Documentation/media/uapi/mediactl/request-api.rst > create mode 100644 Documentation/media/uapi/mediactl/request-func-close.rst > create mode 100644 Documentation/media/uapi/mediactl/request-func-ioctl.rst > create mode 100644 Documentation/media/uapi/mediactl/request-func-poll.rst > Thanks a lot for working on this and sorry for being late to the party. Please see some comments inline. (Mostly wording nits, though.) > diff --git a/Documentation/media/uapi/mediactl/media-controller.rst > b/Documentation/media/uapi/mediactl/media-controller.rst > index 0eea4f9a07d5..66aff38cd499 100644 > --- a/Documentation/media/uapi/mediactl/media-controller.rst > +++ b/Documentation/media/uapi/mediactl/media-controller.rst > @@ -21,6 +21,7 @@ Part IV - Media Controller API > media-controller-intro > media-controller-model > media-types > +request-api > media-funcs > media-header > > diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst > b/Documentation/media/uapi/mediactl/media-funcs.rst > index 076856501cdb..260f9dcadcde 100644 > --- a/Documentation/media/uapi/mediactl/media-funcs.rst > +++ b/Documentation/media/uapi/mediactl/media-funcs.rst > @@ -16,3 +16,9 @@ Function Reference > media-ioc-enum-entities > media-ioc-enum-links > media-ioc-setup-link > +media-ioc-request-alloc > +request-func-close > +request-func-ioctl > +request-func-poll > +media-request-ioc-queue > +media-request-ioc-reinit > diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst > b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst > new file mode 100644 > index ..8f8ecf6c63b6 > --- /dev/null > +++ b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst > @@ -0,0 +1,77 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +.. _media_ioc_request_alloc: > + > +* > +ioctl MEDIA_IOC_REQUEST_ALLOC > +* > + > +Name > + > + > +MEDIA_IOC_REQUEST_ALLOC - Allocate a request > + > + > +Synopsis > + > + > +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_ALLOC, struct > media_request_alloc *argp ) > +:name: MEDIA_IOC_REQUEST_ALLOC > + > + > +Arguments > += > + > +``fd`` > +File descriptor returned by :ref:`open() `. > + > +``argp`` > + Missing description. > + > +Description > +=== > + > +If the media device supports :ref:`requests `, then > +this ioctl can be used to allocate a request. If it is not supported, then > +``errno`` is set to ``ENOTTY``. A request is accessed through a file > descriptor > +that is returned in struct :c:type:`media_request_alloc`. > + > +If the request was successfully allocated, then the request file descriptor > +can be passed to the :ref:`VIDIOC_QBUF `, > +:ref:`VIDIOC_G_EXT_CTRLS `, > +:ref:`VIDIOC_S_EXT_CTRLS ` and > +:ref:`VIDIOC_TRY_EXT_CTRLS ` ioctls. > + > +In addition, the request can be queued by calling > +:ref:`MEDIA_REQUEST_IOC_QUEUE` and re-initialized by calling > +:ref:`MEDIA_REQUEST_IOC_REINIT`. > + > +Finally, the file descriptor can be :ref:`polled ` to wait > +for the request to complete. > + > +The request will remain allocated until the associated file descriptor is > +closed by :ref:`close() ` and the driver no longer uses > +the request internally. I suppose that would be "until all the file descriptors associated with it are closed and", since one could dup() the original file descriptor and even close it, keeping just the copy. > + > +.. c:type:: media_request_alloc > + > +.. tabularcolumns::
Re: [PATCHv5 01/12] media: add 'index' to struct media_v2_pad
Em Fri, 3 Aug 2018 15:34:26 +0300 Sakari Ailus escreveu: > > Still thinking out loud, the fact that we can't change the size of the > > structures pointed to by media_v2_topology bothers me. We could add a > > version > > field to media_v2_topology that would be set by applications to tell the > > kernel which version of the API they expect. On the other hand, maybe we'll > > just do a media_v3_topology when the need arises... > > What you could to is to allocate another field for the new struct; we're > entirely free to put whatever we want there, and the kernel would simply > need to convert between the two. Not ideal though. Another option would be to > explicitly convey the struct size as for the IOCTL argument itself. Both > should probably be used sparingly. The idea we had at the MC workshop is that anything else would be mapped via the properties API. So, the per-type structs will be kept on a minimal format. Thanks, Mauro
[PATCH] Documentation/media: uapi: Explicitly say there are no Invariant Sections
The GNU Free Documentation License allows for a work to specify Invariant Sections that are not allowed to be modified. (Debian considers that this makes such works non-free.) The Linux Media Infrastructure userspace API documentation does not specify any such sections, but it also doesn't say there are none (as is recommended by the license text). Make it explicit that there are none. References: https://bugs.debian.org/698668 Signed-off-by: Ben Hutchings --- Documentation/media/media_uapi.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/media/media_uapi.rst b/Documentation/media/media_uapi.rst index 28eb35a1f965..5198ff24a094 100644 --- a/Documentation/media/media_uapi.rst +++ b/Documentation/media/media_uapi.rst @@ -10,9 +10,9 @@ Linux Media Infrastructure userspace API Permission is granted to copy, distribute and/or modify this document under the terms of the GNU Free Documentation License, Version 1.1 or -any later version published by the Free Software Foundation. A copy of -the license is included in the chapter entitled "GNU Free Documentation -License". +any later version published by the Free Software Foundation, with no +Invariant Sections. A copy of the license is included in the chapter +entitled "GNU Free Documentation License". .. only:: html signature.asc Description: Digital signature
Enhancing your images
Do you have photos for editing? We are image team and we can process 500+ images each day. We edit ecommerce photo, jewelry photos, and beauty model photos. also wedding photos. We do cut out and clipping path for photos, also retouching. You may send us a test photo to check our quality. Thanks, Sam Parker
[RFC PATCH 1/3] uapi/linux/media.h: add property support
From: Hans Verkuil
Add a new topology struct that includes properties.
Signed-off-by: Hans Verkuil
---
include/uapi/linux/media.h | 62 +-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 36f76e777ef9..dd8c96a17020 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -342,6 +342,61 @@ struct media_v2_link {
__u32 reserved[6];
} __attribute__ ((packed));
+#define MEDIA_PROP_TYPE_U641
+#define MEDIA_PROP_TYPE_S642
+#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
+ * @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];
+ char name[32];
+ union {
+ __u64 uval;
+ __s64 sval;
+ };
+} __attribute__ ((packed));
+
+/* Old version 1 of this struct */
+struct media_v2_topology_1 {
+ __u64 topology_version;
+
+ __u32 num_entities;
+ __u32 reserved1;
+ __u64 ptr_entities;
+
+ __u32 num_interfaces;
+ __u32 reserved2;
+ __u64 ptr_interfaces;
+
+ __u32 num_pads;
+ __u32 reserved3;
+ __u64 ptr_pads;
+
+ __u32 num_links;
+ __u32 reserved4;
+ __u64 ptr_links;
+} __attribute__ ((packed));
+
struct media_v2_topology {
__u64 topology_version;
@@ -360,6 +415,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,7 +427,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)
-#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
+#define MEDIA_IOC_G_TOPOLOGY_1 _IOWR('|', 0x04, struct media_v2_topology_1)
+#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x05, struct media_v2_topology)
#ifndef __KERNEL__
--
2.18.0
[RFC PATCH 2/3] media: add support for properties
From: Hans Verkuil
Initially support u64/s64 and string properties.
Signed-off-by: Hans Verkuil
---
drivers/media/media-device.c | 98 +++-
drivers/media/media-entity.c | 65 +
include/media/media-device.h | 6 ++
include/media/media-entity.h | 172 +++
4 files changed, 338 insertions(+), 3 deletions(-)
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index fcdf3d5dc4b6..6fa9555a669f 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -241,10 +241,15 @@ static long media_device_get_topology(struct media_device
*mdev, void *arg)
struct media_interface *intf;
struct media_pad *pad;
struct media_link *link;
+ struct media_prop *prop;
struct media_v2_entity kentity, __user *uentity;
struct media_v2_interface kintf, __user *uintf;
struct media_v2_pad kpad, __user *upad;
struct media_v2_link klink, __user *ulink;
+ struct media_v2_prop kprop, __user *uprop;
+ void __user *uprop_payload;
+ unsigned int payload_size = 0;
+ unsigned int payload_offset = 0;
unsigned int i;
int ret = 0;
@@ -374,6 +379,73 @@ static long media_device_get_topology(struct media_device
*mdev, void *arg)
topo->num_links = i;
topo->reserved4 = 0;
+ /* Get properties and number of properties */
+ i = 0;
+ uprop = media_get_uptr(topo->ptr_props);
+ payload_offset = topo->num_props * sizeof(*uprop);
+ media_device_for_each_prop(prop, mdev) {
+ payload_size += prop->payload_size;
+ i++;
+
+ if (ret || !uprop)
+ continue;
+
+ if (i > topo->num_props) {
+ ret = -ENOSPC;
+ continue;
+ }
+
+ memset(, 0, sizeof(kprop));
+
+ /* Copy prop fields to userspace struct */
+ kprop.id = prop->graph_obj.id;
+ kprop.owner_id = prop->owner->id;
+ kprop.type = prop->type;
+ kprop.flags = 0;
+ kprop.payload_size = prop->payload_size;
+ if (kprop.payload_size)
+ kprop.payload_offset = payload_offset +
+ payload_size - prop->payload_size;
+ else
+ kprop.payload_offset = 0;
+ payload_offset -= sizeof(*uprop);
+ memcpy(kprop.name, prop->name, sizeof(kprop.name));
+ kprop.uval = prop->uval;
+
+ if (copy_to_user(uprop, , sizeof(kprop)))
+ ret = -EFAULT;
+ uprop++;
+ }
+ topo->num_props = i;
+ if (uprop && topo->props_payload_size < payload_size)
+ ret = -ENOSPC;
+ topo->props_payload_size = payload_size;
+ if (!uprop || ret)
+ return ret;
+
+ uprop_payload = uprop;
+ media_device_for_each_prop(prop, mdev) {
+ i++;
+
+ if (!prop->payload_size)
+ continue;
+
+ if (copy_to_user(uprop_payload, prop->string,
prop->payload_size))
+ return -EFAULT;
+ uprop_payload += prop->payload_size;
+ }
+
+ return 0;
+}
+
+static long media_device_get_topology_1(struct media_device *mdev, void *arg)
+{
+ struct media_v2_topology topo = {};
+ long ret;
+
+ memcpy(, arg, sizeof(struct media_v2_topology_1));
+ ret = media_device_get_topology(mdev, );
+ memcpy(arg, , sizeof(struct media_v2_topology_1));
return ret;
}
@@ -424,6 +496,7 @@ static const struct media_ioctl_info ioctl_info[] = {
MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities,
MEDIA_IOC_FL_GRAPH_MUTEX),
MEDIA_IOC(ENUM_LINKS, media_device_enum_links,
MEDIA_IOC_FL_GRAPH_MUTEX),
MEDIA_IOC(SETUP_LINK, media_device_setup_link,
MEDIA_IOC_FL_GRAPH_MUTEX),
+ MEDIA_IOC(G_TOPOLOGY_1, media_device_get_topology_1,
MEDIA_IOC_FL_GRAPH_MUTEX),
MEDIA_IOC(G_TOPOLOGY, media_device_get_topology,
MEDIA_IOC_FL_GRAPH_MUTEX),
};
@@ -438,12 +511,12 @@ static long media_device_ioctl(struct file *filp,
unsigned int cmd,
long ret;
if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
- || ioctl_info[_IOC_NR(cmd)].cmd != cmd)
+ || (ioctl_info[_IOC_NR(cmd)].cmd != cmd))
return -ENOIOCTLCMD;
info = _info[_IOC_NR(cmd)];
- if (_IOC_SIZE(info->cmd) > sizeof(__karg)) {
+ if (_IOC_SIZE(cmd) > sizeof(__karg)) {
karg = kmalloc(_IOC_SIZE(info->cmd), GFP_KERNEL);
if (!karg)
return -ENOMEM;
@@ -582,6 +655,7 @@ int __must_check media_device_register_entity(struct
media_device *mdev,
WARN_ON(entity->graph_obj.mdev != NULL);
entity->graph_obj.mdev = mdev;
[RFC PATCH 0/3] Media Controller Properties
From: Hans Verkuil This RFC patch series implements properties for the media controller. This is not finished, but I wanted to post this so people can discuss this further. No documentation yet (too early for that). An updated v4l2-ctl and v4l2-compliance that can report properties is available here: https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=props There is one main missing piece: currently the properties are effectively laid out in random order. My plan is to change that so they are grouped by object type and object owner. So first all properties for each entity, then for each pad, etc. I started to work on that, but it's a bit more work than expected and I wanted to post this before the weekend. While it is possible to have nested properties, this is not currently implemented. Only properties for entities and pads are supported in this code, but that's easy to extend to interfaces and links. I'm not sure about the G_TOPOLOGY ioctl handling: I went with the quickest option by renaming the old ioctl and adding a new one with property support. I think this needs to change (at the very least the old and new should share the same ioctl NR), but that's something for the future. Currently I support u64, s64 and const char * property types. But it can be anything including binary data if needed. No array support (as we have for controls), but there are enough reserved fields in media_v2_prop to add this if needed. I added properties for entities and pads to vimc, so I could test this. Regards, Hans Hans Verkuil (3): uapi/linux/media.h: add property support media: add support for properties vimc: add test properties drivers/media/media-device.c | 98 +++- drivers/media/media-entity.c | 65 drivers/media/platform/vimc/vimc-common.c | 18 +++ drivers/media/platform/vimc/vimc-core.c | 6 +- include/media/media-device.h | 6 + include/media/media-entity.h | 172 ++ include/uapi/linux/media.h| 62 +++- 7 files changed, 420 insertions(+), 7 deletions(-) -- 2.18.0
[RFC PATCH 3/3] vimc: add test properties
From: Hans Verkuil Signed-off-by: Hans Verkuil --- drivers/media/platform/vimc/vimc-common.c | 18 ++ drivers/media/platform/vimc/vimc-core.c | 6 +++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c index 617415c224fe..db8a8d1eca54 100644 --- a/drivers/media/platform/vimc/vimc-common.c +++ b/drivers/media/platform/vimc/vimc-common.c @@ -452,6 +452,24 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved, goto err_clean_m_ent; } + ret = media_entity_add_prop_u64(>entity, "u64", ~1); + if (!ret) + ret = media_entity_add_prop_s64(>entity, "s64", -5); + if (!ret) + ret = media_entity_add_prop_string(>entity, "string", + sd->name); + if (!ret) + ret = media_pad_add_prop_u64(>entity.pads[num_pads - 1], +"u64", ~1); + if (!ret) + ret = media_pad_add_prop_s64(>entity.pads[num_pads - 1], +"s64", -5); + if (!ret) + ret = media_pad_add_prop_string(>entity.pads[0], + "string", sd->name); + if (ret) + goto err_clean_m_ent; + return 0; err_clean_m_ent: diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c index 9246f265de31..d8d3803a47f9 100644 --- a/drivers/media/platform/vimc/vimc-core.c +++ b/drivers/media/platform/vimc/vimc-core.c @@ -309,13 +309,13 @@ static int vimc_probe(struct platform_device *pdev) if (!vimc->subdevs) return -ENOMEM; + /* Link the media device within the v4l2_device */ + vimc->v4l2_dev.mdev = >mdev; + match = vimc_add_subdevs(vimc); if (IS_ERR(match)) return PTR_ERR(match); - /* Link the media device within the v4l2_device */ - vimc->v4l2_dev.mdev = >mdev; - /* Initialize media device */ strlcpy(vimc->mdev.model, VIMC_MDEV_MODEL_NAME, sizeof(vimc->mdev.model)); -- 2.18.0
[PATCH 3/4] media: dvb-usb-v2: fix spelling mistake: "completition" -> "completion"
From: Colin Ian King Trivial fix to spelling mistake in dev_dbg and dev_err messages Signed-off-by: Colin Ian King --- drivers/media/usb/dvb-usb-v2/usb_urb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/usb_urb.c b/drivers/media/usb/dvb-usb-v2/usb_urb.c index b0499f95ec45..024c751eb165 100644 --- a/drivers/media/usb/dvb-usb-v2/usb_urb.c +++ b/drivers/media/usb/dvb-usb-v2/usb_urb.c @@ -40,7 +40,7 @@ static void usb_urb_complete(struct urb *urb) return; default:/* error */ dev_dbg_ratelimited(>udev->dev, - "%s: urb completition failed=%d\n", + "%s: urb completion failed=%d\n", __func__, urb->status); break; } @@ -69,7 +69,7 @@ static void usb_urb_complete(struct urb *urb) break; default: dev_err(>udev->dev, - "%s: unknown endpoint type in completition handler\n", + "%s: unknown endpoint type in completion handler\n", KBUILD_MODNAME); return; } -- 2.17.1
about the photos and
Do you have photos for editing? We are image team and we can process 500+ images each day. We edit ecommerce photo, jewelry photos, and beauty model photos. also wedding photos. We do cut out and clipping path for photos, also retouching. You may send us a test photo to check our quality. Thanks, Sam Parker
Re: [RESEND PATCH v4] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung, On Fri, Aug 03, 2018 at 11:04:57AM +0800, Ping-chung Chen wrote: > From: "Chen, Ping-chung" > > Add a V4L2 sub-device driver for the Sony IMX208 image sensor. > This is a camera sensor using the I2C bus for control and the > CSI-2 bus for data. > > Signed-off-by: Ping-Chung Chen > --- > since v1: > -- Update the function media_entity_pads_init for upstreaming. > -- Change the structure name mutex as imx208_mx. > -- Refine the control flow of test pattern function. > -- vflip/hflip control support (will impact the output bayer order) > -- support 4 bayer orders output (via change v/hflip) > - SRGGB10(default), SGRBG10, SGBRG10, SBGGR10 > -- Simplify error handling in the set_stream function. > since v2: > -- Refine coding style. > -- Fix the if statement to use pm_runtime_get_if_in_use(). > -- Print more error log during error handling. > -- Remove mutex_destroy() from imx208_free_controls(). > -- Add more comments. > since v3: > -- Set explicit indices to link frequencies. Could you add support for obtaining the link frequencies from firmware, please? -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
On Fri, Aug 03, 2018 at 03:46:32PM +0200, Philippe De Muyter wrote: > Hi Sakari, > > On Fri, Aug 03, 2018 at 03:43:15PM +0300, Sakari Ailus wrote: > > Hi Philippe, > > > > On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote: > > > When v4l2_i2c_subdev_init is called, dev_name(>dev) has already > > > been set. Use it to generate subdev's name instead of recreating it > > > with "%d-%04x". This improves the similarity in subdev's name creation > > > between v4l2_i2c_subdev_init and v4l2_spi_subdev_init. > > > > > > Signed-off-by: Philippe De Muyter > > > --- > > > drivers/media/v4l2-core/v4l2-common.c | 5 ++--- > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c > > > b/drivers/media/v4l2-core/v4l2-common.c > > > index 5471c6d..b062111 100644 > > > --- a/drivers/media/v4l2-core/v4l2-common.c > > > +++ b/drivers/media/v4l2-core/v4l2-common.c > > > @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, > > > struct i2c_client *client, > > > v4l2_set_subdevdata(sd, client); > > > i2c_set_clientdata(client, sd); > > > /* initialize name */ > > > - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x", > > > - client->dev.driver->name, i2c_adapter_id(client->adapter), > > > - client->addr); > > > + snprintf(sd->name, sizeof(sd->name), "%s %s", > > > + client->dev.driver->name, dev_name(>dev)); > > > } > > > EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init); > > > > > > > I like the patch in principle. But what's the effect of this on the actual > > sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be > > different. We can't change the existing entity naming in drivers, this will > > break applications that expect them to be named in a certain way. > > Yeah. I am 10 years too late. > > Maybe adding for a long transition period a kernel message giving the new > name and the old one if they are different ? I think this information would be usable for the property API if we ever get around defining it precisely and implementing it. That's long term stuff in any case. https://spinics.net/lists/linux-media/msg90160.html> -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
Hi Sakari, On Fri, Aug 03, 2018 at 03:43:15PM +0300, Sakari Ailus wrote: > Hi Philippe, > > On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote: > > When v4l2_i2c_subdev_init is called, dev_name(>dev) has already > > been set. Use it to generate subdev's name instead of recreating it > > with "%d-%04x". This improves the similarity in subdev's name creation > > between v4l2_i2c_subdev_init and v4l2_spi_subdev_init. > > > > Signed-off-by: Philippe De Muyter > > --- > > drivers/media/v4l2-core/v4l2-common.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-common.c > > b/drivers/media/v4l2-core/v4l2-common.c > > index 5471c6d..b062111 100644 > > --- a/drivers/media/v4l2-core/v4l2-common.c > > +++ b/drivers/media/v4l2-core/v4l2-common.c > > @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, > > struct i2c_client *client, > > v4l2_set_subdevdata(sd, client); > > i2c_set_clientdata(client, sd); > > /* initialize name */ > > - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x", > > - client->dev.driver->name, i2c_adapter_id(client->adapter), > > - client->addr); > > + snprintf(sd->name, sizeof(sd->name), "%s %s", > > + client->dev.driver->name, dev_name(>dev)); > > } > > EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init); > > > > I like the patch in principle. But what's the effect of this on the actual > sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be > different. We can't change the existing entity naming in drivers, this will > break applications that expect them to be named in a certain way. Yeah. I am 10 years too late. Maybe adding for a long transition period a kernel message giving the new name and the old one if they are different ? Thank you Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
[GIT PULL FOR v4.19] SPDX headers for Renesas media drivers
Hi Mauro, The following changes since commit 2c3449fb95c318920ca8dc645d918d408db219ac: media: usb: hackrf: Replace GFP_ATOMIC with GFP_KERNEL (2018-08-02 19:16:17 -0400) are available in the Git repository at: git://linuxtv.org/pinchartl/media.git v4l2/renesas/spdx for you to fetch changes up to e4df82294fcd03c22522457c51117092a688805f: media: sh_mobile_ceu: convert to SPDX identifiers (2018-08-03 16:17:26 +0300) Kuninori Morimoto (9): media: soc_camera_platform: convert to SPDX identifiers media: rcar-vin: convert to SPDX identifiers media: rcar-fcp: convert to SPDX identifiers media: rcar_drif: convert to SPDX identifiers media: rcar_fdp1: convert to SPDX identifiers media: rcar_jpu: convert to SPDX identifiers media: sh_veu: convert to SPDX identifiers media: sh_vou: convert to SPDX identifiers media: sh_mobile_ceu: convert to SPDX identifiers drivers/media/platform/rcar-fcp.c| 6 +- drivers/media/platform/rcar-vin/Kconfig | 1 + drivers/media/platform/rcar-vin/Makefile | 1 + drivers/media/platform/rcar-vin/rcar-core.c | 8 ++-- drivers/media/platform/rcar-vin/rcar-dma.c | 6 +- drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 +- drivers/media/platform/rcar-vin/rcar-vin.h | 6 +- drivers/media/platform/rcar_drif.c | 8 ++-- drivers/media/platform/rcar_fdp1.c | 6 +- drivers/media/platform/rcar_jpu.c| 5 + drivers/media/platform/sh_veu.c | 5 + drivers/media/platform/sh_vou.c | 5 + drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c | 6 +- drivers/media/platform/soc_camera/soc_camera_platform.c | 5 + 14 files changed, 16 insertions(+), 58 deletions(-) -- Regards, Laurent Pinchart
[GIT PULL FOR v4.19] VSP1 changes
Hi Mauro, The following changes since commit 2c3449fb95c318920ca8dc645d918d408db219ac: media: usb: hackrf: Replace GFP_ATOMIC with GFP_KERNEL (2018-08-02 19:16:17 -0400) are available in the Git repository at: git://linuxtv.org/pinchartl/media.git v4l2/vsp1/interlace for you to fetch changes up to 7e4432c469bda8dc09ff4f78b998bca22a131588: media: vsp1: Support Interlaced display pipelines (2018-08-03 16:12:07 +0300) Please note that the last patch from the corresponding series, which enables interlaced support on the display side, is missing from this pull request and will be submitted for v4.20 through the DRM tree. As we're late in the release cycle I thought it would be too difficult to coordinate with Dave in a timely manner. This series will cause no harm whatsoever even without the change on the DRM side, as the newly introduced interlaced flag in the VSP1 API exposed to the DU driver is part of a structure that is initialized to 0. Kieran Bingham (10): media: vsp1: drm: Fix minor grammar error media: vsp1: use kernel __packed for structures media: vsp1: Rename dl_child to dl_next media: vsp1: Remove unused display list structure field media: vsp1: Clean up DLM objects on error media: vsp1: Provide VSP1 feature helper macro media: vsp1: Use header display lists for all WPF outputs linked to the DU media: vsp1: Add support for extended display list headers media: vsp1: Provide support for extended command pools media: vsp1: Support Interlaced display pipelines drivers/media/platform/vsp1/vsp1.h | 3 + drivers/media/platform/vsp1/vsp1_dl.c | 432 +++--- drivers/media/platform/vsp1/vsp1_dl.h | 28 +++ drivers/media/platform/vsp1/vsp1_drm.c | 8 +- drivers/media/platform/vsp1/vsp1_drv.c | 20 +- drivers/media/platform/vsp1/vsp1_pipe.h | 2 + drivers/media/platform/vsp1/vsp1_regs.h | 5 +- drivers/media/platform/vsp1/vsp1_rpf.c | 72 ++- drivers/media/platform/vsp1/vsp1_wpf.c | 6 +- include/media/vsp1.h| 2 + 10 files changed, 468 insertions(+), 110 deletions(-) -- Regards, Laurent Pinchart
we look after your photos
Do you have photos for editing? We are image team and we can process 500+ images each day. We edit ecommerce photo, jewelry photos, and beauty model photos. also wedding photos. We do cut out and clipping path for photos, also retouching. You may send us a test photo to check our quality. Thanks, Sam Parker
Re: [PATCH 2/2] media: v4l2-common: simplify v4l2_i2c_subdev_init name generation
Hi Philippe, On Wed, Aug 01, 2018 at 11:20:57PM +0200, Philippe De Muyter wrote: > When v4l2_i2c_subdev_init is called, dev_name(>dev) has already > been set. Use it to generate subdev's name instead of recreating it > with "%d-%04x". This improves the similarity in subdev's name creation > between v4l2_i2c_subdev_init and v4l2_spi_subdev_init. > > Signed-off-by: Philippe De Muyter > --- > drivers/media/v4l2-core/v4l2-common.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c > b/drivers/media/v4l2-core/v4l2-common.c > index 5471c6d..b062111 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -121,9 +121,8 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct > i2c_client *client, > v4l2_set_subdevdata(sd, client); > i2c_set_clientdata(client, sd); > /* initialize name */ > - snprintf(sd->name, sizeof(sd->name), "%s %d-%04x", > - client->dev.driver->name, i2c_adapter_id(client->adapter), > - client->addr); > + snprintf(sd->name, sizeof(sd->name), "%s %s", > + client->dev.driver->name, dev_name(>dev)); > } > EXPORT_SYMBOL_GPL(v4l2_i2c_subdev_init); > I like the patch in principle. But what's the effect of this on the actual sub-device (and entity) names? Looking at i2c_dev_set_name(), this will be different. We can't change the existing entity naming in drivers, this will break applications that expect them to be named in a certain way. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCHv5 01/12] media: add 'index' to struct media_v2_pad
Hi Laurent,
On Wed, Jul 11, 2018 at 02:33:47PM +0300, Laurent Pinchart wrote:
> Hi Hans,
>
> On Monday, 9 July 2018 16:40:51 EEST Hans Verkuil wrote:
> > On 09/07/18 14:55, Laurent Pinchart wrote:
> > > On Friday, 29 June 2018 14:43:20 EEST Hans Verkuil wrote:
> > >> From: Hans Verkuil
> > >>
> > >> The v2 pad structure never exposed the pad index, which made it
> > >> impossible
> > >> to call the MEDIA_IOC_SETUP_LINK ioctl, which needs that information.
> > >>
> > >> It is really trivial to just expose this information, so implement this.
> > >>
> > >> Signed-off-by: Hans Verkuil
> > >> Acked-by: Sakari Ailus
> > >> ---
> > >>
> > >> drivers/media/media-device.c | 1 +
> > >> include/uapi/linux/media.h | 12 +++-
> > >> 2 files changed, 12 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > >> index 47bb2254fbfd..047d38372a27 100644
> > >> --- a/drivers/media/media-device.c
> > >> +++ b/drivers/media/media-device.c
> > >> @@ -331,6 +331,7 @@ static long media_device_get_topology(struct
> > >> media_device *mdev, void *arg)
> > >> kpad.id = pad->graph_obj.id;
> > >> kpad.entity_id = pad->entity->graph_obj.id;
> > >> kpad.flags = pad->flags;
> > >> +kpad.index = pad->index;
> > >>
> > >> if (copy_to_user(upad, , sizeof(kpad)))
> > >> ret = -EFAULT;
> > >> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > >> index 86c7dcc9cba3..f6338bd57929 100644
> > >> --- a/include/uapi/linux/media.h
> > >> +++ b/include/uapi/linux/media.h
> > >> @@ -305,11 +305,21 @@ struct media_v2_interface {
> > >> };
> > >> } __attribute__ ((packed));
> > >>
> > >> +/*
> > >> + * Appeared in 4.19.0.
> > >> + *
> > >> + * The media_version argument comes from the media_version field in
> > >> + * struct media_device_info.
> > >> + */
> > >> +#define MEDIA_V2_PAD_HAS_INDEX(media_version) \
> > >> +((media_version) >= ((4 << 16) | (19 << 8) | 0))
> > >
> > > I agree that we need tn index field, but I don't think we need to care
> > > about backward compatibility. The lack of an index field makes it clear
> > > that the API has never been properly used, as it was impossible to do so.
> >
> > We do need to care: there is no reason why a v4l2 application can't be used
> > on an older kernel. Most v4l2 applications copy the V4L2 headers to the
> > application (in fact, that's what v4l-utils does) and so they need to know
> > if a field is actually filled in by whatever kernel is used. In most cases
> > they can just check against 0, but that happens to be a valid index :-(
> >
> > So this is really needed. Same for the flags field.
>
> You're right. I was thinking we could detect this on the kernel side by
> checking the ioctl argument size if we added the index field to the
> media_v2_pad structure instead of replacing one of the reserved fields, but
> media_v2_pad is not passed directly to the G_TOPOLOGY ioctl, so that won't
> help.
>
> I wonder whether we shouldn't just define
>
> #define MEDIA_V2_IS_BROKEN(media_version) \
> ((media_version) < ((4 << 16) | (19 << 8) | 0))
>
> as in practice applications should really avoid the G_TOPOLOGY ioctl without
> this patch series. Having multiple version-based macros to check for features
> won't be very helpful, and could be counter-productive as applications might
> incorrectly decide to still use the API to retrieve some information when
> they
> should really avoid it.
>
> And, while at it, should we use KERNEL_VERSION() instead of hardcoding it ?
>
> #define MEDIA_V2_IS_BROKEN(media_version) \
> ((media_version) < KERNEL_VERSION(4, 19, 0))
>
> Still thinking out loud, the fact that we can't change the size of the
> structures pointed to by media_v2_topology bothers me. We could add a version
> field to media_v2_topology that would be set by applications to tell the
> kernel which version of the API they expect. On the other hand, maybe we'll
> just do a media_v3_topology when the need arises...
What you could to is to allocate another field for the new struct; we're
entirely free to put whatever we want there, and the kernel would simply
need to convert between the two. Not ideal though. Another option would be to
explicitly convey the struct size as for the IOCTL argument itself. Both
should probably be used sparingly.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
[PATCH v2 1/2] uvcvideo: rename UVC_QUIRK_INFO to UVC_INFO_QUIRK
This macro defines "information about quirks," not "quirks for
information."
Signed-off-by: Guennadi Liakhovetski
---
drivers/media/usb/uvc/uvc_driver.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index d46dc43..699984b 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2344,7 +2344,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.quirks = UVC_QUIRK_FORCE_Y8,
};
-#define UVC_QUIRK_INFO(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks =
q}
+#define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks =
q}
/*
* The Logitech cameras listed below have their interface class set to
@@ -2453,7 +2453,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info =
UVC_QUIRK_INFO(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
+ .driver_info =
UVC_INFO_QUIRK(UVC_QUIRK_RESTORE_CTRLS_ON_INIT) },
/* Chicony CNF7129 (Asus EEE 100HE) */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -2462,7 +2462,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_INFO(UVC_QUIRK_RESTRICT_FRAME_RATE)
},
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_RESTRICT_FRAME_RATE)
},
/* Alcor Micro AU3820 (Future Boy PC USB Webcam) */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -2525,7 +2525,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_BUILTIN_ISIGHT) },
/* Apple Built-In iSight via iBridge */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
@@ -2607,7 +2607,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_PROBE_DEF) },
/* IMC Networks (Medion Akoya) */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
@@ -2707,7 +2707,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_MINMAX
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_PROBE_EXTRAFIELDS) },
/* Aveo Technology USB 2.0 Camera (Tasco USB Microscope) */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
@@ -2725,7 +2725,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_INFO(UVC_QUIRK_PROBE_EXTRAFIELDS) },
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_PROBE_EXTRAFIELDS) },
/* Manta MM-353 Plako */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -2771,7 +2771,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info = UVC_QUIRK_INFO(UVC_QUIRK_STATUS_INTERVAL) },
+ .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_STATUS_INTERVAL) },
/* MSI StarCam 370i */
{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE
| USB_DEVICE_ID_MATCH_INT_INFO,
@@ -2798,7 +2798,7 @@ static int uvc_clock_param_set(const char *val, const
struct kernel_param *kp)
.bInterfaceClass = USB_CLASS_VIDEO,
.bInterfaceSubClass = 1,
.bInterfaceProtocol = 0,
- .driver_info =
[PATCH v2 2/2] uvcvideo: add a D4M camera description
D4M is a mobile model from the D4XX family of Intel RealSense cameras.
This patch adds a descriptor for it, which enables reading per-frame
metadata from it.
Signed-off-by: Guennadi Liakhovetski
---
Documentation/media/uapi/v4l/meta-formats.rst | 1 +
Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst | 204 ++
drivers/media/usb/uvc/uvc_driver.c| 11 ++
include/uapi/linux/videodev2.h| 1 +
4 files changed, 217 insertions(+)
create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
b/Documentation/media/uapi/v4l/meta-formats.rst
index 0c4e1ec..cf971d5 100644
--- a/Documentation/media/uapi/v4l/meta-formats.rst
+++ b/Documentation/media/uapi/v4l/meta-formats.rst
@@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface only.
.. toctree::
:maxdepth: 1
+pixfmt-meta-d4xx
pixfmt-meta-uvc
pixfmt-meta-vsp1-hgo
pixfmt-meta-vsp1-hgt
diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
b/Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
new file mode 100644
index 000..57ecfd9
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
@@ -0,0 +1,204 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _v4l2-meta-fmt-d4xx:
+
+***
+V4L2_META_FMT_D4XX ('D4XX')
+***
+
+Intel D4xx UVC Cameras Metadata
+
+
+Description
+===
+
+Intel D4xx (D435 and other) cameras include per-frame metadata in their UVC
+payload headers, following the Microsoft(R) UVC extension proposal [1_]. That
+means, that the private D4XX metadata, following the standard UVC header, is
+organised in blocks. D4XX cameras implement several standard block types,
+proposed by Microsoft, and several proprietary ones. Supported standard
metadata
+types are MetadataId_CaptureStats (ID 3), MetadataId_CameraExtrinsics (ID 4),
+and MetadataId_CameraIntrinsics (ID 5). For their description see [1_]. This
+document describes proprietary metadata types, used by D4xx cameras.
+
+V4L2_META_FMT_D4XX buffers follow the metadata buffer layout of
+V4L2_META_FMT_UVC with the only difference, that it also includes proprietary
+payload header data. D4xx cameras use bulk transfers and only send one payload
+per frame, therefore their headers cannot be larger than 255 bytes.
+
+Below are proprietary Microsoft style metadata types, used by D4xx cameras,
+where all fields are in little endian order:
+
+.. flat-table:: D4xx metadata
+:widths: 1 4
+:header-rows: 1
+:stub-columns: 0
+
+* - Field
+ - Description
+* - :cspan:`1` *Depth Control*
+* - __u32 ID
+ - 0x8000
+* - __u32 Size
+ - Size in bytes (currently 56)
+* - __u32 Version
+ - Version of the struct
+* - __u32 Flags
+ - A bitmask of flags: see [2_] below
+* - __u32 Gain
+ - Gain value in internal units, same as the V4L2_CID_GAIN control, used
to
+ capture the frame
+* - __u32 Exposure
+ - Exposure time (in microseconds) used to capture the frame
+* - __u32 Laser power
+ - Power of the laser LED 0-360, used for depth measurement
+* - __u32 AE mode
+ - 0: manual; 1: automatic exposure
+* - __u32 Exposure priority
+ - Exposure priority value: 0 - constant frame rate
+* - __u32 AE ROI left
+ - Left border of the AE Region of Interest (all ROI values are in pixels
+ and lie between 0 and maximum width or height respectively)
+* - __u32 AE ROI right
+ - Right border of the AE Region of Interest
+* - __u32 AE ROI top
+ - Top border of the AE Region of Interest
+* - __u32 AE ROI bottom
+ - Bottom border of the AE Region of Interest
+* - __u32 Preset
+ - Preset selector value, default: 0, unless changed by the user
+* - __u32 Laser mode
+ - 0: off, 1: on
+* - :cspan:`1` *Capture Timing*
+* - __u32 ID
+ - 0x8001
+* - __u32 Size
+ - Size in bytes (currently 40)
+* - __u32 Version
+ - Version of the struct
+* - __u32 Flags
+ - A bitmask of flags: see [3_] below
+* - __u32 Frame counter
+ - Monotonically increasing counter
+* - __u32 Optical time
+ - Time in microseconds from the beginning of a frame till its middle
+* - __u32 Readout time
+ - Time, used to read out a frame in microseconds
+* - __u32 Exposure time
+ - Frame exposure time in microseconds
+* - __u32 Frame interval
+ - In microseconds = 100 / framerate
+* - __u32 Pipe latency
+ - Time in microseconds from start of frame to data in USB buffer
+* - :cspan:`1` *Configuration*
+* - __u32 ID
+ - 0x8002
+* - __u32 Size
+ - Size in bytes (currently 40)
+* - __u32 Version
+ - Version of the struct
+* - __u32 Flags
+ - A bitmask of flags: see [4_] below
+* - __u8 Hardware type
+ - Camera
Re: [PATCH v3 4/4] selftests: media_tests: Add a memory-to-memory concurrent stress test
Hi Ezequiel,
On 01/08/18 22:50, Ezequiel Garcia wrote:
Add a test for the memory-to-memory framework, to exercise the
scheduling of concurrent jobs, using multiple contexts.
This test needs to be run using the vim2m virtual driver,
and so needs no hardware.
While here, rework the media_tests suite in order to make it
useful for automatic tools. Those tests that need human intervention
are now separated from those that can run automatically, needing
only virtual drivers to work.
Signed-off-by: Ezequiel Garcia
---
.../testing/selftests/media_tests/.gitignore | 1 +
tools/testing/selftests/media_tests/Makefile | 5 +-
.../selftests/media_tests/m2m_job_test.c | 287 ++
.../selftests/media_tests/m2m_job_test.sh | 32 ++
4 files changed, 324 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/media_tests/m2m_job_test.c
create mode 100755 tools/testing/selftests/media_tests/m2m_job_test.sh
diff --git a/tools/testing/selftests/media_tests/.gitignore
b/tools/testing/selftests/media_tests/.gitignore
index 8745eba39012..71c6508348ce 100644
--- a/tools/testing/selftests/media_tests/.gitignore
+++ b/tools/testing/selftests/media_tests/.gitignore
@@ -1,3 +1,4 @@
media_device_test
media_device_open
video_device_test
+m2m_job_test
diff --git a/tools/testing/selftests/media_tests/Makefile
b/tools/testing/selftests/media_tests/Makefile
index 60826d7d37d4..d25d4c3eb7d2 100644
--- a/tools/testing/selftests/media_tests/Makefile
+++ b/tools/testing/selftests/media_tests/Makefile
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
#
CFLAGS += -I../ -I../../../../usr/include/
-TEST_GEN_PROGS := media_device_test media_device_open video_device_test
+TEST_GEN_PROGS_EXTENDED := media_device_test media_device_open
video_device_test m2m_job_test
+TEST_PROGS := m2m_job_test.sh
include ../lib.mk
+
+LDLIBS += -lpthread
diff --git a/tools/testing/selftests/media_tests/m2m_job_test.c
b/tools/testing/selftests/media_tests/m2m_job_test.c
new file mode 100644
index ..5800269567e6
--- /dev/null
+++ b/tools/testing/selftests/media_tests/m2m_job_test.c
@@ -0,0 +1,287 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) Collabora, Ltd.
Add the year (2018), and authors (you).
+
+/*
+ * This file adds a test for the memory-to-memory
+ * framework.
+ *
+ * This test opens a user specified video device and then
+ * queues concurrent jobs. The jobs are totally dummy,
+ * its purpose is only to verify that each of the queued
+ * jobs is run, and is run only once.
+ *
+ * The vim2m driver is needed in order to run the test.
+ *
+ * Usage:
+ * ./m2m-job-test -d /dev/videoX
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "../kselftest.h"
+
+#define V4L2_CID_TRANS_TIME_MSEC(V4L2_CID_USER_BASE + 0x1000)
+#define V4L2_CID_TRANS_NUM_BUFS (V4L2_CID_USER_BASE + 0x1001)
+
+#define MAX_TRANS_TIME_MSEC 500
+#define MAX_COUNT 50
+#define MAX_BUFFERS 5
+#define W 10
+#define H 10
I like short names, but W and H might be a little bit too short
esp for a macro.
+#ifndef DEBUG
+#define dprintf(fmt, arg...) \
+ do {\
+ } while (0)
+#else
+#define dprintf(fmt, arg...) printf(fmt, ## arg)
+#endif
+
+static char video_device[256];
+static int thread_count;
+
+struct context {
+ int vfd;
+ unsigned int width;
+ unsigned int height;
+ int buffers;
+};
+
+static int req_src_buf(struct context *ctx, int buffers)
+{
+ struct v4l2_requestbuffers reqbuf;
+ struct v4l2_buffer buf;
+ int i, ret;
+
+ memset(, 0, sizeof(reqbuf));
+ memset(, 0, sizeof(buf));
+
+ reqbuf.count= buffers;
+ reqbuf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ reqbuf.memory = V4L2_MEMORY_MMAP;
+ ret = ioctl(ctx->vfd, VIDIOC_REQBUFS, );
+ if (ret)
+ return ret;
+
+ for (i = 0; i < buffers; i++) {
+ buf.type= V4L2_BUF_TYPE_VIDEO_OUTPUT;
+ buf.memory = V4L2_MEMORY_MMAP;
+ buf.index = i;
+ ret = ioctl(ctx->vfd, VIDIOC_QUERYBUF, );
+ if (ret)
+ return ret;
+ buf.bytesused = W*H*2;
Shouldn't you be using ->width and ->height here rather than W
and H?
In fact, maybe these can actually be set as "static const
unsigned int WIDTH = 10;" in the main function rather than global
macros, since you're parring the context around at runtime.
+ ret = ioctl(ctx->vfd, VIDIOC_QBUF, );
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int req_dst_buf(struct context *ctx, int buffers)
+{
+ struct v4l2_requestbuffers reqbuf;
+ struct v4l2_buffer buf;
+ int i, ret;
+
+ memset(, 0,
Re: [PATCH] uvcvideo: add a D4M camera description
Hi Laurent,
Thanks for the review. A general note: I think you're requesting a rather
detailed information about many parameters. That isn't a problem by
itself, however, it is difficult to obtain some of that information. I'll
address whatever comments I can in an updated version, just answering some
questions here. I directed youor questions, that I couldn't answer myself
to respective people, but I have no idea if and when I get replies. So,
it's up to you whether to wait for that additional information or to take
at least what we have now.
On Sun, 29 Jul 2018, Laurent Pinchart wrote:
> Hi Guennadi,
>
> Thank you for the patch.
>
> On Saturday, 23 December 2017 13:11:00 EEST Guennadi Liakhovetski wrote:
> > From: Guennadi Liakhovetski
> >
> > D4M is a mobile model from the D4XX family of Intel RealSense cameras.
> > This patch adds a descriptor for it, which enables reading per-frame
> > metadata from it.
> >
> > Signed-off-by: Guennadi Liakhovetski
> > ---
> > Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst | 202 +++
> > drivers/media/usb/uvc/uvc_driver.c| 11 ++
> > include/uapi/linux/videodev2.h| 1 +
> > 3 files changed, 214 insertions(+)
> > create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
> >
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
> > b/Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst new file mode 100644
> > index 000..950780d
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-meta-d4xx.rst
> > @@ -0,0 +1,202 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _v4l2-meta-fmt-d4xx:
> > +
> > +***
> > +V4L2_META_FMT_D4XX ('D4XX')
> > +***
> > +
> > +D4XX Metadata
>
> How about "Intel D4xx UVC Cameras Metadata" ?
>
> > +
> > +
> > +Description
> > +===
> > +
> > +D4XX (D435 and other) cameras include per-frame metadata in their UVC
> > payload
>
> Should this be "Intel D4XX" ?
>
> > +headers, following the Microsoft(R) UVC extension proposal [1_]. That
> > means,
> > +that the private D4XX metadata, following the standard UVC header, is
> > organised
> > +in blocks. D4XX cameras implement several standard block types, proposed by
> > +Microsoft, and several proprietary ones. Supported standard metadata types
> > +include MetadataId_CaptureStats (ID 3), MetadataId_CameraExtrinsics (ID 4),
> > and
> > +MetadataId_CameraIntrinsics (ID 5). For their description see [1_].
>
> Does "including" mean that the list isn't exhaustive and that other standard
> types could be returned too ? If so, would it be possible to get an
> exhaustive
> list ? And if the list is exhaustive, could you word this paragraph to make
> that clear ?
>
> > This
> > +document describes proprietary metadata types, used by DS4XX cameras.
>
> Is it D4XX or DS4XX ?
>
> > +V4L2_META_FMT_D4XX buffers follow the metadata buffer layout of
> > +V4L2_META_FMT_UVC with the only difference, that it also includes
> > proprietary
> > +payload header data. D4XX cameras use bulk transfers and only send one
> > payload
> > +per frame, therefore their headers cannot be larger than 255 bytes.
> > +
> > +Below are proprietary Microsoft style metadata types, used by D4XX cameras,
> > +where all fields are in little endian order:
> > +
> > +.. flat-table:: D4XX metadata
> > +:widths: 1 4
> > +:header-rows: 1
> > +:stub-columns: 0
> > +
> > +* - Field
> > + - Description
> > +* - :cspan:`1` *Depth Control*
> > +* - __u32 ID
> > + - 0x8000
> > +* - __u32 Size
> > + - Size in bytes (currently 56)
> > +* - __u32 Version
> > + - Version of the struct
>
> What is this field used for ?
For future changes to this (and all other) struct(s). If in the future a
new field is added to this struct, the version will be incremented to
inform the user.
> > +* - __u32 Flags
> > + - A bitmask of flags: see [2_] below
> > +* - __u32 Gain
> > + - Manual gain value
>
> What is the gain unit ?
It's in internal units. I guess librealsense has formulas to convert them
to ISO or something else standard. It's the same units as the
V4L2_CID_GAIN control.
> > +* - __u32 Exposure
> > + - Manual exposure time in microseconds
>
> When auto-exposure is enabled, does this reflect the actual exposure time
> used
> to capture the image ? If so I'd name the field just "exposure time", and
> expand the document to explain this. Maybe something like
>
> "Exposure time (in microseconds) that was used to capture the frame."
>
> It would also be useful to explain what happens when auto-exposure is
> disabled.
>
> This comment applies to the gain as well.
>
> > +* - __u32 Laser power
> > + - Power of the laser LED 0-360, used for depth measurement
> > +* - __u32 AE mode
> > + - 0: manual; 1: automatic exposure
> > +* - __u32 Exposure priority
> > +
Re: [PATCH v6 00/13] media: staging/imx7: add i.MX7 media driver
On 08/02/18 18:45, Rui Miguel Silva wrote: > Hi Hans, > On Thu 02 Aug 2018 at 13:37, Hans Verkuil wrote: >> Hi Rui, >> >> On 05/22/18 16:52, Rui Miguel Silva wrote: >>> Hi, >>> This series introduces the Media driver to work with the i.MX7 >>> SoC. it uses the >>> already existing imx media core drivers but since the i.MX7, >>> contrary to >>> i.MX5/6, do not have an IPU and because of that some changes in >>> the imx media >>> core are made along this series to make it support that case. >>> >>> This patches adds CSI and MIPI-CSI2 drivers for i.MX7, along >>> with several >>> configurations changes for this to work as a capture subsystem. >>> Some bugs are >>> also fixed along the line. And necessary documentation. >>> >>> For a more detailed view of the capture paths, pads links in >>> the i.MX7 please >>> take a look at the documentation in PATCH 14. >>> >>> The system used to test and develop this was the Warp7 board >>> with an OV2680 >>> sensor, which output format is 10-bit bayer. So, only MIPI >>> interface was >>> tested, a scenario with an parallel input would nice to have. >>> >>> *Important note*, this code depends on Steve Longerbeam series >>> [0]: >>> [PATCH v4 00/13] media: imx: Switch to subdev notifiers >>> which the merging status is not clear to me, but the changes in >>> there make >>> senses to this series >>> >>> Bellow goes an example of the output of the pads and links and >>> the output of >>> v4l2-compliance testing. >>> >>> The v4l-utils version used is: >>> v4l2-compliance SHA : >>> 47d43b130dc6e9e0edc900759fb37649208371e4 from Apr 4th. >>> >>> The Media Driver fail some tests but this failures are coming >>> from code out of >>> scope of this series (video-mux, imx-capture), and some from >>> the sensor OV2680 >>> but that I think not related with the sensor driver but with >>> the testing and >>> core. >>> >>> The csi and mipi-csi entities pass all compliance tests. >>> >>> Cheers, >>> Rui >>> >>> [0]: >>> https://www.mail-archive.com/linux-media@vger.kernel.org/msg131186.html >> >> This patch series was delayed quite a bit since the patch series >> above >> it depends on is still not merged. >> >> But the v6 version of that series will be merged once the 4.20 >> cycle opens: >> https://www.mail-archive.com/linux-media@vger.kernel.org/msg133391.html > > Good news. > >> >> Sakari has a branch with that series on top of the latest >> media_tree master: >> https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-fwnode >> >> Can you rebase this imx7 series on top of that? And test it >> again with the >> *latest* v4l2-compliance? (I've added new checks recently, so >> you need to >> update this utility) >> >> Please post the output of the v4l2-compliance test (after fixing >> any issues >> it raises of course), either as a reply to this post or in the >> cover letter >> of a v7 version of this series if you had to make changes. > > Sure, I will rebase on top of Sakari tree and will update the > compliance > tests and run them again. Oops, forgot to mention: you will likely need to update the include/linux/media.h header: Look for MEDIA_V2_ENTITY_HAS_FLAGS and MEDIA_V2_PAD_HAS_INDEX and change 19 to 18 in those defines. This is added for kernel 4.19, but since in our master tree the kernel version is still 4.18 the v4l2-compliance utility thinks these features are not present when in fact they are. Without this change (temporarily since this will automatically be fixed after the 4.19 merge window closes) v4l2-compliance will fail with an error. Regards, Hans
[PATCH] media: i2c: mt9v111: Fix v4l2-ctrl error handling
Fix error handling of v4l2_ctrl creation by inspecting the ctrl.error flag
instead of testing for each returned value correctness.
As reported by Dan Carpenter returning PTR_ERR() on the v4l2_ctrl_new_std()
return value is also wrong, as that function return NULL on error.
While at there re-order the cleanup path to respect the operation inverse
order.
Fixes: aab7ed1c "media: i2c: Add driver for Aptina MT9V111"
Reported-by: Dan Carpenter
Signed-off-by: Jacopo Mondi
---
drivers/media/i2c/mt9v111.c | 41 +
1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/drivers/media/i2c/mt9v111.c b/drivers/media/i2c/mt9v111.c
index da8f6ab..2b87bf9 100644
--- a/drivers/media/i2c/mt9v111.c
+++ b/drivers/media/i2c/mt9v111.c
@@ -1159,41 +1159,21 @@ static int mt9v111_probe(struct i2c_client *client)
V4L2_CID_AUTO_WHITE_BALANCE,
0, 1, 1,
V4L2_WHITE_BALANCE_AUTO);
- if (IS_ERR_OR_NULL(mt9v111->auto_awb)) {
- ret = PTR_ERR(mt9v111->auto_awb);
- goto error_free_ctrls;
- }
-
mt9v111->auto_exp = v4l2_ctrl_new_std_menu(>ctrls,
_ctrl_ops,
V4L2_CID_EXPOSURE_AUTO,
V4L2_EXPOSURE_MANUAL,
0, V4L2_EXPOSURE_AUTO);
- if (IS_ERR_OR_NULL(mt9v111->auto_exp)) {
- ret = PTR_ERR(mt9v111->auto_exp);
- goto error_free_ctrls;
- }
-
- /* Initialize timings */
mt9v111->hblank = v4l2_ctrl_new_std(>ctrls, _ctrl_ops,
V4L2_CID_HBLANK,
MT9V111_CORE_R05_MIN_HBLANK,
MT9V111_CORE_R05_MAX_HBLANK, 1,
MT9V111_CORE_R05_DEF_HBLANK);
- if (IS_ERR_OR_NULL(mt9v111->hblank)) {
- ret = PTR_ERR(mt9v111->hblank);
- goto error_free_ctrls;
- }
-
mt9v111->vblank = v4l2_ctrl_new_std(>ctrls, _ctrl_ops,
V4L2_CID_VBLANK,
MT9V111_CORE_R06_MIN_VBLANK,
MT9V111_CORE_R06_MAX_VBLANK, 1,
MT9V111_CORE_R06_DEF_VBLANK);
- if (IS_ERR_OR_NULL(mt9v111->vblank)) {
- ret = PTR_ERR(mt9v111->vblank);
- goto error_free_ctrls;
- }
/* PIXEL_RATE is fixed: just expose it to user space. */
v4l2_ctrl_new_std(>ctrls, _ctrl_ops,
@@ -1201,6 +1181,10 @@ static int mt9v111_probe(struct i2c_client *client)
DIV_ROUND_CLOSEST(mt9v111->sysclk, 2), 1,
DIV_ROUND_CLOSEST(mt9v111->sysclk, 2));
+ if (mt9v111->ctrls.error) {
+ ret = mt9v111->ctrls.error;
+ goto error_free_ctrls;
+ }
mt9v111->sd.ctrl_handler = >ctrls;
/* Start with default configuration: 640x480 UYVY. */
@@ -1226,26 +1210,27 @@ static int mt9v111_probe(struct i2c_client *client)
mt9v111->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>sd.entity, 1, >pad);
if (ret)
- goto error_free_ctrls;
+ goto error_free_entity;
#endif
ret = mt9v111_chip_probe(mt9v111);
if (ret)
- goto error_free_ctrls;
+ goto error_free_entity;
ret = v4l2_async_register_subdev(>sd);
if (ret)
- goto error_free_ctrls;
+ goto error_free_entity;
return 0;
-error_free_ctrls:
- v4l2_ctrl_handler_free(>ctrls);
-
+error_free_entity:
#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
#endif
+error_free_ctrls:
+ v4l2_ctrl_handler_free(>ctrls);
+
mutex_destroy(>pwr_mutex);
mutex_destroy(>stream_mutex);
@@ -1259,12 +1244,12 @@ static int mt9v111_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
- v4l2_ctrl_handler_free(>ctrls);
-
#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>entity);
#endif
+ v4l2_ctrl_handler_free(>ctrls);
+
mutex_destroy(>pwr_mutex);
mutex_destroy(>stream_mutex);
--
2.7.4
Re: [bug report] media: i2c: Add driver for Aptina MT9V111
Hi Dan,
thanks for noticing this,
On Tue, Jul 31, 2018 at 09:35:54PM +0300, Dan Carpenter wrote:
> Hello Jacopo Mondi,
>
> The patch aab7ed1c3927: "media: i2c: Add driver for Aptina MT9V111"
> from Jul 25, 2018, leads to the following static checker warning:
>
> drivers/media/i2c/mt9v111.c:1163 mt9v111_probe() warn: passing zero to
> 'PTR_ERR'
> drivers/media/i2c/mt9v111.c:1173 mt9v111_probe() warn: passing zero to
> 'PTR_ERR'
> drivers/media/i2c/mt9v111.c:1184 mt9v111_probe() warn: passing zero to
> 'PTR_ERR'
> drivers/media/i2c/mt9v111.c:1194 mt9v111_probe() warn: passing zero to
> 'PTR_ERR'
>
> drivers/media/i2c/mt9v111.c
> 1155 v4l2_ctrl_handler_init(>ctrls, 5);
> 1156
> 1157 mt9v111->auto_awb = v4l2_ctrl_new_std(>ctrls,
> 1158_ctrl_ops,
> 1159
> V4L2_CID_AUTO_WHITE_BALANCE,
> 11600, 1, 1,
> 1161
> V4L2_WHITE_BALANCE_AUTO);
> 1162 if (IS_ERR_OR_NULL(mt9v111->auto_awb)) {
> 1163 ret = PTR_ERR(mt9v111->auto_awb);
>
> This just returns success because v4l2_ctrl_new_std() only return NULL
Correct, sorry, I didn't notice that.
> on error, it never returns error pointers. I guess we should set ret to
> EINVAL?
>
> if (!mt9v111->auto_awb) {
> ret = -EINVAL;
> goto error_free_ctrls;
> }
>
We can do even better than that.
The v4l2 control handler retains errors in a flag I can inspect after
having added/created all controls here and here below.
I can return that error flag if something goes wrong.
Thanks
j
> 1164 goto error_free_ctrls;
> 1165 }
> 1166
> 1167 mt9v111->auto_exp = v4l2_ctrl_new_std_menu(>ctrls,
> 1168 _ctrl_ops,
> 1169
> V4L2_CID_EXPOSURE_AUTO,
> 1170
> V4L2_EXPOSURE_MANUAL,
> 1171 0,
> V4L2_EXPOSURE_AUTO);
> 1172 if (IS_ERR_OR_NULL(mt9v111->auto_exp)) {
> 1173 ret = PTR_ERR(mt9v111->auto_exp);
> 1174 goto error_free_ctrls;
> 1175 }
> 1176
> 1177 /* Initialize timings */
> 1178 mt9v111->hblank = v4l2_ctrl_new_std(>ctrls,
> _ctrl_ops,
> 1179 V4L2_CID_HBLANK,
> 1180
> MT9V111_CORE_R05_MIN_HBLANK,
> 1181
> MT9V111_CORE_R05_MAX_HBLANK, 1,
> 1182
> MT9V111_CORE_R05_DEF_HBLANK);
> 1183 if (IS_ERR_OR_NULL(mt9v111->hblank)) {
> 1184 ret = PTR_ERR(mt9v111->hblank);
> 1185 goto error_free_ctrls;
> 1186 }
> 1187
> 1188 mt9v111->vblank = v4l2_ctrl_new_std(>ctrls,
> _ctrl_ops,
> 1189 V4L2_CID_VBLANK,
> 1190
> MT9V111_CORE_R06_MIN_VBLANK,
> 1191
> MT9V111_CORE_R06_MAX_VBLANK, 1,
> 1192
> MT9V111_CORE_R06_DEF_VBLANK);
> 1193 if (IS_ERR_OR_NULL(mt9v111->vblank)) {
> 1194 ret = PTR_ERR(mt9v111->vblank);
> 1195 goto error_free_ctrls;
> 1196 }
> 1197
> 1198 /* PIXEL_RATE is fixed: just expose it to user space. */
> 1199 v4l2_ctrl_new_std(>ctrls, _ctrl_ops,
>
> regards,
> dan carpenter
signature.asc
Description: PGP signature
Re: [PATCH 20/22] [media] tvp5150: Add input port connectors DT bindings
Hi Rob,
first of all, thanks for the review. After some discussion with the
media guys I have a question about the dt-bindings.
On 18-07-03 17:23, Rob Herring wrote:
> On Thu, Jun 28, 2018 at 06:20:52PM +0200, Marco Felsch wrote:
> > The TVP5150/1 decoders support different video input sources to their
> > AIP1A/B pins.
> >
> > Possible configurations are as follows:
> > - Analog Composite signal connected to AIP1A.
> > - Analog Composite signal connected to AIP1B.
> > - Analog S-Video Y (luminance) and C (chrominance)
> > signals connected to AIP1A and AIP1B respectively.
> >
> > This patch extends the device tree bindings documentation to describe
> > how the input connectors for these devices should be defined in a DT.
> >
> > Signed-off-by: Marco Felsch
> > ---
> > .../devicetree/bindings/media/i2c/tvp5150.txt | 118 +-
> > 1 file changed, 113 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> > b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> > index 8c0fc1a26bf0..feed8c911c5e 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> > +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt
> > @@ -12,11 +12,23 @@ Optional Properties:
> > - pdn-gpios: phandle for the GPIO connected to the PDN pin, if any.
> > - reset-gpios: phandle for the GPIO connected to the RESETB pin, if any.
> >
> > -The device node must contain one 'port' child node for its digital output
> > -video port, in accordance with the video interface bindings defined in
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +The device node must contain one 'port' child node per device input and
> > output
> > +port, in accordance with the video interface bindings defined in
> > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port
> > nodes
> > +are numbered as follows
> >
> > -Required Endpoint Properties for parallel synchronization:
> > + Name TypePort
> > + --
> > + AIP1A sink0
> > + AIP1B sink1
> > + S-VIDEO sink2
> > + Y-OUT src 3
Do you think it's correct to have a seperate port for each binding?
Since the S-Video port is a combination of AIP1A and AIP1B. After a
discussion with Mauro [1] the TVP5150 should have only 3 pads. Since the
pads are directly mappped to the dt-ports this will correspond to three
ports (2 in, 1 out). Now the svideo connector will be mapped to a second
endpoint in each port:
port@0
endpoint@0 ---> Comp0-Con
endpoint@1 -+-> Svideo-Con
port@1 |
endpoint@0 -|-> Comp1-Con
endpoint@1 -+
port@2
endpoint
I don't like that solution that much, since the mapping is now signal
based. We also don't map each line of a parallel port.
A quick grep shows that currently each device using a svideo connector
seperates them in a own port as I did. IMHO this is a uncomplicate
solution, but don't abstract the HW correctly. What is your opinion
about that? Is it correct to have seperate (virtual) port or should I
map the svideo connector as shown above?
[1] https://www.spinics.net/lists/devicetree/msg242825.html
Regards,
Marco
> > +
> > +The device node must contain at least the Y-OUT port. Each input port must
> > be
> > +linked to an endpoint defined in
> > +Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt.
> > +
> > +Required Endpoint Properties for parallel synchronization on output port:
> >
> > - hsync-active: active state of the HSYNC signal. Must be <1> (HIGH).
> > - vsync-active: active state of the VSYNC signal. Must be <1> (HIGH).
> > @@ -26,7 +38,9 @@ Required Endpoint Properties for parallel synchronization:
> > If none of hsync-active, vsync-active and field-even-active is specified,
> > the endpoint is assumed to use embedded BT.656 synchronization.
> >
> > -Example:
> > +Examples:
> > +
> > +Only Output:
> >
> > {
> > ...
> > @@ -37,6 +51,100 @@ Example:
> > reset-gpios = < 7 GPIO_ACTIVE_LOW>;
> >
> > port {
> > + reg = <3>;
> > + tvp5150_1: endpoint {
> > + remote-endpoint = <_ep>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +One Input:
> > +
> > +connector@0 {
>
> Drop the unit-address as there is no reg property.
>
> > + compatible = "composite-video-connector";
> > + label = "Composite0";
> > +
> > + port {
> > + comp0_out: endpoint {
> > + remote-endpoint = <_comp0_in>;
> > + };
> > + };
> > +};
> > +
> > + {
> > + ...
> > + tvp5150@5c {
> > + compatible = "ti,tvp5150";
> > + reg = <0x5c>;
> > + pdn-gpios = < 30 GPIO_ACTIVE_LOW>;
> > +
