Re: [RFC v1.2 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Ping.
On Fri, Sep 06, 2013 at 08:10:12PM +0300, Sakari Ailus wrote:
Hi Laurent,
Thanks for the reply.
On Fri, Sep 06, 2013 at 06:30:47PM +0200, Laurent Pinchart wrote:
Hi Sakari,
Thank you for the patch.
On Wednesday 04 September 2013 03:09:42 Sakari Ailus wrote:
Do not allow streaming if a pad with MEDIA_PAD_FL_MUST_CONNECT flag is
connected by links that are all inactive.
This patch makes it possible to avoid drivers having to check for the most
common case of link state validation: a sink pad that must be connected.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
drivers/media/media-entity.c | 41
+++--
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2c286c3..567a171 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -235,6 +235,8 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity, media_entity_graph_walk_start(graph, entity);
while ((entity = media_entity_graph_walk_next(graph))) {
+ DECLARE_BITMAP(active, entity-num_pads);
+ DECLARE_BITMAP(has_no_links, entity-num_pads);
unsigned int i;
entity-stream_count++;
@@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity, if (!entity-ops || !entity-ops-link_validate)
continue;
+ bitmap_zero(active, entity-num_pads);
+ bitmap_fill(has_no_links, entity-num_pads);
+
for (i = 0; i entity-num_links; i++) {
struct media_link *link = entity-links[i];
-
- /* Is this pad part of an enabled link? */
- if (!(link-flags MEDIA_LNK_FL_ENABLED))
- continue;
-
- /* Are we the sink or not? */
- if (link-sink-entity != entity)
+ struct media_pad *pad = link-sink-entity == entity
+ ? link-sink : link-source;
+
+ /* Mark that a pad is connected by a link. */
+ bitmap_clear(has_no_links, pad-index, 1);
+
+ /*
+ * Pads that either do not need to connect or
+ * are connected through an enabled link are
+ * fine.
+ */
+ if (!(pad-flags MEDIA_PAD_FL_MUST_CONNECT)
+ || link-flags MEDIA_LNK_FL_ENABLED)
With the || moved on the previous line (here and below),
I'd like to claim the above is more clear and conforms to GNU coding
standards whereas the kernel CodingStyle doesn't suggest either way:
URL:http://www.gnu.org/prep/standards/standards.html#Formatting
I feel we've had this discussion before. :-)
I have to admit I disagree with Stallman's liberal use of parentheses and a
funny habit of having a space between a function name and the parentheses
enclosing its arguments. ;-)
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
If that's fine with you there's no need to resent, I'll take the patch in
my
tree with that modification.
I won't say no, but I would prefer getting an ack from someone else, too,
for the two first patches. A less permissive variant of this was dismissed
around a year ago, potentially perhaps there was no pressing need for that
kind of a change. The earlier set required all sink pads must have a
connected link (which indeed was too restrictive, I agree).
URL:https://linuxtv.org/patch/15205/
As seen in the fourth patch of this set, this makes it possible to avoid
another loop (done in the driver) over the pipeline, which was my motivation
for this patch --- so I think this is the right thing to do.
Other drivers could be changed the same way but I think this is up to the
driver authors. There could be other reasons for the pad to need connecting;
tha absence of the flag won't say there aren't.
+ bitmap_set(active, pad-index, 1);
+
+ /*
+ * Link validation will only take place for
+ * sink ends of the link that are enabled.
+ */
+ if (link-sink != pad
+ || !(link-flags MEDIA_LNK_FL_ENABLED))
continue;
ret = entity-ops-link_validate(link);
if (ret 0 ret != -ENOIOCTLCMD)
goto error;
}
+
+ /* Either no links or validated links are fine. */
+ bitmap_or(active, active, has_no_links, entity-num_pads);
+
+ if (!bitmap_full(active, entity-num_pads)) {
+ ret = -EPIPE;
+ goto error;
+
Re: Kernel Summit Media Mini-summit attendees on Oct 23 in Edinburgh
Hi Mauro, On 09/17/2013 07:08 PM, Mauro Carvalho Chehab wrote: Hi, I'm trying to consolidate the list of interested people on participating at this year's the media mini-summit. From what I got from the discussions, we have, so far: Benjamin Gaignard benjamin.gaign...@linaro.org Guennadi Liakhovetski g.liakhovet...@gmx.de Hans Verkuil hverk...@xs4all.nl Hugues FRUCHET hugues.fruc...@st.com Laurent Pinchart laurent.pinch...@ideasonboard.com Mauro Carvalho Chehab m.che...@samsung.com Michael Krufky mkru...@kernellabs.com Oliver Schinagl oliver+l...@schinagl.nl Pawel Osciak posc...@chromium.org Peter Senna Tschudin peter.se...@gmail.com Ricardo Ribalda Delgado ricardo.riba...@gmail.com Sakari Ailus sakari.ai...@iki.fi Please let me know if I'm missing someone, or if one of the above won't be able to go to the meeting, as my plan is to send the invitations tomorrow. Could you add me and Andrzej Hajda to the above list ? Andrzej Hajda a.ha...@samsung.com Sylwester Nawrocki sylvester.nawro...@gmail.com I'll try to reply to the agenda e-mail today, presumably proposing one or two more topics there. Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] media: st-rc: Add ST remote control driver
Thanks Mark, On 16/09/13 15:10, Mark Rutland wrote: + +Required properties: + - compatible: should be st,comms-irb. This should probably say should contain rather than should be. There may be future vairants of this device, which will also have a more specific compatible string. Ok, will change it to the suggest. + - reg: base physical address of the controller and length of memory + mapped region. + - interrupts: interrupt number to the cpu. The interrupt specifier + format depends on the interrupt controller parent. I don't like the phrase interrupt number to the cpu. We already have the term interrupt-specifier to precisely define this. How about: - interrupts: interrupt-specifier for the sole interrupt generated by the device. TBH, I did copy them from one of the existing bindings docs. I will change it. + +Optional properties: + - rx-mode: can be infrared or uhf. + - tx-mode: should be infrared. Are these required to use rx/tx? Yes, these are required for driver to be in rx/tx mode. One of them can be optional depending on the board setup. So, Is it ok to move such properties to required properties section? If you don't have a tx-mode or rx-mode, I assume you can't do anything... Yes, driver errs out. + - pinctrl-names, pinctrl-0: the pincontrol settings to configure + muxing properly for IRB pins. If we're expecting names, the names we expect should be defined. + - clocks : phandle of clock. This is not just a phandle. This is a phandle + clock-specifier pair. Yep, will change it. Thanks, srini Cheers, -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
1080i input interface support
Guys, I have seeing a few devices as GoogleTv and Sony usb dongle with HDMI input. Do you know what chipset this devices are using? We have any hdmi input device supported for now? best regards, - Roberto -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 01/13] [media] exynos5-is: Adding media device driver for exynos5
Hi Sylwester, On Wed, Sep 18, 2013 at 2:20 AM, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: Hi Arun, On 09/17/2013 01:29 PM, Arun Kumar K wrote: Wont this make fimc-is to be enabled to use fimc-lite? Hmm, it would be runtime PM active, yes. But it doesn't mean the Cortex-A5's firmware would need to be running. As such there is no dependency like that in hardware and we can use fimc-lite alone in DMA out mode without fimc-is. If you are sure about it, then fimc-lite nodes could stay at root level. Still, it would be more appropriate IMO to have the FIMC-IS sub-/peripheral devices, like I2C or SPI bus controllers, instantiated by the FIMC-IS driver. Yes I2C and SPI bus controllers are part of FIMC-IS and it can be instantiated within the FIMC-IS driver. And what about glue logic linking FIMC-LITEs with the rest of the imaging subsystem ? Are you sure the is no some weird inter-dependencies ? Note that there is no board files any more, where we could hack some details. There is no dependency like that which I am aware of and we have tested here mipi fimc-lite without fimc-is and it worked well. If its modified as per your suggestion, how will the scenario of sensor - mipi-csis - fimc-lite - memory look like without fimc-is? The exynos5-fimc-is module would need to be loaded for that. If you're sure the hardware can work independently, we could leave out the fimc-lite nodes at root level. Yes sensor- mipi - fimc-lite - memory without fimc-is is very well possible in Exynos5 and its tested. So I will make the remaining changes you suggested and send new patchset. Regards Arun -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel Summit Media Mini-summit attendees on Oct 23 in Edinburgh
On 09/17/2013 04:06 PM, Laurent Pinchart wrote: Hi Guennadi, On Tuesday 17 September 2013 22:41:57 Guennadi Liakhovetski wrote: On Tue, 17 Sep 2013, Laurent Pinchart wrote: On Tuesday 17 September 2013 19:49:21 Oliver Schinagl wrote: On 09/17/13 19:08, Mauro Carvalho Chehab wrote: Hi, I'm trying to consolidate the list of interested people on participating at this year's the media mini-summit. From what I got from the discussions, we have, so far: Benjamin Gaignard benjamin.gaign...@linaro.org Guennadi Liakhovetski g.liakhovet...@gmx.de Hans Verkuil hverk...@xs4all.nl Hugues FRUCHET hugues.fruc...@st.com Laurent Pinchart laurent.pinch...@ideasonboard.com Mauro Carvalho Chehab m.che...@samsung.com Michael Krufky mkru...@kernellabs.com Oliver Schinagl oliver+l...@schinagl.nl Pawel Osciak posc...@chromium.org Peter Senna Tschudin peter.se...@gmail.com Ricardo Ribalda Delgado ricardo.riba...@gmail.com Sakari Ailus sakari.ai...@iki.fi Please let me know if I'm missing someone, or if one of the above won't be able to go to the meeting, as my plan is to send the invitations tomorrow. While I'd really love to go there, I was only asking a question really :) and am unable to attend. Wishfully there would be a video recording of sorts. Will try to attend fosdem 2014 though ;) Speaking of FOSDEM 2014, are there V4L2 developers interesting in having meetings/brainstorming sessions there ? Maybe we can see after the KS - what topics are left or pop up by then? For one it would be easy for me to attend. I believe most V4L2 folks won't attend the fosdem, so I don't think we should make it a (mini)summit. I was rather considering some kind of hacking sessions and discussions. We can wait after KS, but if we want to get a room at the FOSDEM we should plan as ahead as possible. For discussions I wouldn't mind visiting fosdem, but if it is just hacking, then I do that better at home :-) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] 0ac8:0321 Vimicro generic vc0321 Camera is not working and causes crashes since 3.2
Hi, On 09/17/2013 08:25 PM, Frank Dierich wrote: On 09/09/2013 02:09 PM, Hans de Goede wrote: Thanks for the bug report, looking at the bug reports, they all report an error of -71 which is EPROTO, which typically means something is wrong at the USB level. And nothing has changed for the driver in question between 3.1 and 3.2 , so I believe this regression is caused by changes to the usb sub-system, likely changes to the EHCI driver. I have tested the new 3.12.0-rc1 kernel and the regression is still present. It causes that Cheese crashes with a segmentation fault and I get the following errors [ 139.868628] gspca_main: ISOC data error: [21] len=0, status=-71 [ 139.904620] gspca_main: ISOC data error: [12] len=0, status=-71 [ 139.936595] gspca_main: ISOC data error: [9] len=0, status=-71 [ 139.968576] gspca_main: ISOC data error: [17] len=0, status=-71 [ 140.036571] gspca_main: ISOC data error: [16] len=0, status=-71 [ 140.037364] video_source:sr[2570]: segfault at 8 ip 7f0430d6868c sp 7f0406c02900 error 4 in libgstreamer-0.10.so.0.30.0[7f0430d15000+de000] [ 140.068533] gspca_main: ISOC data error: [24] len=0, status=-71 [ 140.104519] gspca_main: ISOC data error: [15] len=0, status=-71 [ 140.168474] gspca_main: ISOC data error: [20] len=0, status=-71 [ 140.200461] gspca_main: ISOC data error: [28] len=0, status=-71 Note I'm not claiming there is not problem, but since your testing indicated that things broken between 3.1 and 3.2, I'm pretty sure that the breakage is not caused by changes to the vimicro driver, as that driver was not changed between 3.1 and 3.2. As I already said in my previous mail, the best way forward with this is probably to bisect the problem, and then send a mail to linux-...@vger.kernel.org about this. Please CC me on this mail. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] media: st-rc: Add ST remote control driver
On Wed, Sep 18, 2013 at 11:11:29AM +0100, Srinivas KANDAGATLA wrote: Thanks Mark, On 16/09/13 15:10, Mark Rutland wrote: + +Required properties: + - compatible: should be st,comms-irb. This should probably say should contain rather than should be. There may be future vairants of this device, which will also have a more specific compatible string. Ok, will change it to the suggest. + - reg: base physical address of the controller and length of memory + mapped region. + - interrupts: interrupt number to the cpu. The interrupt specifier + format depends on the interrupt controller parent. I don't like the phrase interrupt number to the cpu. We already have the term interrupt-specifier to precisely define this. How about: - interrupts: interrupt-specifier for the sole interrupt generated by the device. TBH, I did copy them from one of the existing bindings docs. I will change it. There's a lot of inconsistency with existing docs, it would be nice to use consistent, correct terminology going forward :) + +Optional properties: + - rx-mode: can be infrared or uhf. + - tx-mode: should be infrared. Are these required to use rx/tx? Yes, these are required for driver to be in rx/tx mode. One of them can be optional depending on the board setup. So, Is it ok to move such properties to required properties section? I'd probably just make a note on each stating what they mean (i.e. that rx-mode should be present iff the rx pins are wired up). If you don't have a tx-mode or rx-mode, I assume you can't do anything... Yes, driver errs out. + - pinctrl-names, pinctrl-0: the pincontrol settings to configure + muxing properly for IRB pins. If we're expecting names, the names we expect should be defined. + - clocks : phandle of clock. This is not just a phandle. This is a phandle + clock-specifier pair. Yep, will change it. Cheers. Mark. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Dependency bug in the uvcvideo Kconfig
[adding linux-media mailing list] On 09/18/13 06:18, Jeff P. Zacher wrote: Not subscribed, please CC'me in replies: There seems to be a dependency bug in the Kconfig for the uvcvideo kernel module. If uvcvideo is built in and usb support is built as a module, the kernel build will fail with the obviously missing dependanies. Error logs: * ERROR: Failed to compile the bzImage target... * * -- Grepping log... -- * * SHIPPED scripts/genksyms/keywords.hash.c * SHIPPED scripts/genksyms/parse.tab.h * SHIPPED scripts/genksyms/parse.tab.c * HOSTCC scripts/genksyms/lex.lex.o *scripts/genksyms/lex.lex.c_shipped: In function ‘yylex1’: *scripts/genksyms/lex.lex.c_shipped:904:1: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] *-- * CC drivers/video/console/font_8x16.o * CC drivers/video/console/softcursor.o * CC sound/core/seq/seq_memory.o * CC drivers/video/console/fbcondecor.o * CC sound/core/seq/seq_queue.o *drivers/video/console/fbcondecor.c:511:6: warning: function declaration isn’t a prototype [-Wstrict-prototypes] *-- *(.text+0xf8fb1): undefined reference to `usb_submit_urb' *drivers/built-in.o: In function `uvc_init': *uvc_driver.c:(.init.text+0x908a): undefined reference to `usb_register_driver' *drivers/built-in.o: In function `uvc_cleanup': *uvc_driver.c:(.exit.text+0x67e): undefined reference to `usb_deregister' *make: *** [vmlinux] Error 1 *-- * Running with options: --lvm --menuconfig all * Using genkernel.conf from /etc/genkernel.conf * Sourcing arch-specific config.sh from /usr/share/genkernel/arch/x86_64/config.sh .. * Sourcing arch-specific modules_load from /usr/share/genkernel/arch/x86_64/modules_load .. * * ERROR: Failed to compile the bzImage target... * * -- End log... -- Compiling uvc as a module allows the compilation to complete. Platform x86_64 Ref: http://forums.gentoo.org/viewtopic-p-7398782.html#7398782 -- Jeff P. Zacher ad_si...@yahoo.com -- ~Randy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Dependency bug in the uvcvideo Kconfig
Hi Randy, I've tried to download the .config file from the link on the forum, but it tries to install something in my browser and the file is not downloadable for me. Can you provide it over an simpler interface such as pastebin.com? Thanks On Wed, Sep 18, 2013 at 4:59 PM, Randy Dunlap rdun...@infradead.org wrote: [adding linux-media mailing list] On 09/18/13 06:18, Jeff P. Zacher wrote: Not subscribed, please CC'me in replies: There seems to be a dependency bug in the Kconfig for the uvcvideo kernel module. If uvcvideo is built in and usb support is built as a module, the kernel build will fail with the obviously missing dependanies. Error logs: * ERROR: Failed to compile the bzImage target... * * -- Grepping log... -- * * SHIPPED scripts/genksyms/keywords.hash.c * SHIPPED scripts/genksyms/parse.tab.h * SHIPPED scripts/genksyms/parse.tab.c * HOSTCC scripts/genksyms/lex.lex.o *scripts/genksyms/lex.lex.c_shipped: In function ‘yylex1’: *scripts/genksyms/lex.lex.c_shipped:904:1: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] *-- * CC drivers/video/console/font_8x16.o * CC drivers/video/console/softcursor.o * CC sound/core/seq/seq_memory.o * CC drivers/video/console/fbcondecor.o * CC sound/core/seq/seq_queue.o *drivers/video/console/fbcondecor.c:511:6: warning: function declaration isn’t a prototype [-Wstrict-prototypes] *-- *(.text+0xf8fb1): undefined reference to `usb_submit_urb' *drivers/built-in.o: In function `uvc_init': *uvc_driver.c:(.init.text+0x908a): undefined reference to `usb_register_driver' *drivers/built-in.o: In function `uvc_cleanup': *uvc_driver.c:(.exit.text+0x67e): undefined reference to `usb_deregister' *make: *** [vmlinux] Error 1 *-- * Running with options: --lvm --menuconfig all * Using genkernel.conf from /etc/genkernel.conf * Sourcing arch-specific config.sh from /usr/share/genkernel/arch/x86_64/config.sh .. * Sourcing arch-specific modules_load from /usr/share/genkernel/arch/x86_64/modules_load .. * * ERROR: Failed to compile the bzImage target... * * -- End log... -- Compiling uvc as a module allows the compilation to complete. Platform x86_64 Ref: http://forums.gentoo.org/viewtopic-p-7398782.html#7398782 -- Jeff P. Zacher ad_si...@yahoo.com -- ~Randy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Peter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Kernel Summit Media Mini-summit attendees on Oct 23 in Edinburgh
On Wednesday 18 September 2013 08:04:52 Hans Verkuil wrote: On 09/17/2013 04:06 PM, Laurent Pinchart wrote: On Tuesday 17 September 2013 22:41:57 Guennadi Liakhovetski wrote: On Tue, 17 Sep 2013, Laurent Pinchart wrote: On Tuesday 17 September 2013 19:49:21 Oliver Schinagl wrote: On 09/17/13 19:08, Mauro Carvalho Chehab wrote: Hi, I'm trying to consolidate the list of interested people on participating at this year's the media mini-summit. From what I got from the discussions, we have, so far: Benjamin Gaignard benjamin.gaign...@linaro.org Guennadi Liakhovetski g.liakhovet...@gmx.de Hans Verkuil hverk...@xs4all.nl Hugues FRUCHET hugues.fruc...@st.com Laurent Pinchart laurent.pinch...@ideasonboard.com Mauro Carvalho Chehab m.che...@samsung.com Michael Krufky mkru...@kernellabs.com Oliver Schinagl oliver+l...@schinagl.nl Pawel Osciak posc...@chromium.org Peter Senna Tschudin peter.se...@gmail.com Ricardo Ribalda Delgado ricardo.riba...@gmail.com Sakari Ailus sakari.ai...@iki.fi Please let me know if I'm missing someone, or if one of the above won't be able to go to the meeting, as my plan is to send the invitations tomorrow. While I'd really love to go there, I was only asking a question really :) and am unable to attend. Wishfully there would be a video recording of sorts. Will try to attend fosdem 2014 though ;) Speaking of FOSDEM 2014, are there V4L2 developers interesting in having meetings/brainstorming sessions there ? Maybe we can see after the KS - what topics are left or pop up by then? For one it would be easy for me to attend. I believe most V4L2 folks won't attend the fosdem, so I don't think we should make it a (mini)summit. I was rather considering some kind of hacking sessions and discussions. We can wait after KS, but if we want to get a room at the FOSDEM we should plan as ahead as possible. For discussions I wouldn't mind visiting fosdem, but if it is just hacking, then I do that better at home :-) I'll be there and I'm interested in both. I'd just like to get an idea of who's coming and what people would be interested in, to see if I should try to organize something a bit more formally. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v1.2 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Hi Sakari,
On Friday 06 September 2013 20:10:12 Sakari Ailus wrote:
On Fri, Sep 06, 2013 at 06:30:47PM +0200, Laurent Pinchart wrote:
On Wednesday 04 September 2013 03:09:42 Sakari Ailus wrote:
Do not allow streaming if a pad with MEDIA_PAD_FL_MUST_CONNECT flag is
connected by links that are all inactive.
This patch makes it possible to avoid drivers having to check for the
most common case of link state validation: a sink pad that must be
connected.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
drivers/media/media-entity.c | 41 +--
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2c286c3..567a171 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
[snip]
@@ -248,21 +250,46 @@ __must_check int
media_entity_pipeline_start(struct
media_entity *entity,
if (!entity-ops || !entity-ops-link_validate)
continue;
+ bitmap_zero(active, entity-num_pads);
+ bitmap_fill(has_no_links, entity-num_pads);
+
for (i = 0; i entity-num_links; i++) {
struct media_link *link = entity-links[i];
-
- /* Is this pad part of an enabled link? */
- if (!(link-flags MEDIA_LNK_FL_ENABLED))
- continue;
-
- /* Are we the sink or not? */
- if (link-sink-entity != entity)
+ struct media_pad *pad = link-sink-entity == entity
+ ? link-sink : link-source;
+
+ /* Mark that a pad is connected by a link. */
+ bitmap_clear(has_no_links, pad-index, 1);
+
+ /*
+ * Pads that either do not need to connect or
+ * are connected through an enabled link are
+ * fine.
+ */
+ if (!(pad-flags MEDIA_PAD_FL_MUST_CONNECT)
+ || link-flags MEDIA_LNK_FL_ENABLED)
With the || moved on the previous line (here and below),
I'd like to claim the above is more clear and conforms to GNU coding
standards whereas the kernel CodingStyle doesn't suggest either way:
URL:http://www.gnu.org/prep/standards/standards.html#Formatting
Quoting Documentation/CodingStyle:
First off, I'd suggest printing out a copy of the GNU coding standards,
and NOT read it. Burn them, it's a great symbolic gesture.
:-)
I feel we've had this discussion before. :-)
I have to admit I disagree with Stallman's liberal use of parentheses and a
funny habit of having a space between a function name and the parentheses
enclosing its arguments. ;-)
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
If that's fine with you there's no need to resent, I'll take the patch in
my tree with that modification.
I won't say no, but I would prefer getting an ack from someone else, too,
for the two first patches. A less permissive variant of this was dismissed
around a year ago, potentially perhaps there was no pressing need for that
kind of a change. The earlier set required all sink pads must have a
connected link (which indeed was too restrictive, I agree).
URL:https://linuxtv.org/patch/15205/
I'm fine with waiting for other ack's. I'll let you handle the required
pinging and poking though :-)
As seen in the fourth patch of this set, this makes it possible to avoid
another loop (done in the driver) over the pipeline, which was my motivation
for this patch --- so I think this is the right thing to do.
Other drivers could be changed the same way but I think this is up to the
driver authors. There could be other reasons for the pad to need connecting;
tha absence of the flag won't say there aren't.
+ bitmap_set(active, pad-index, 1);
+
+ /*
+ * Link validation will only take place for
+ * sink ends of the link that are enabled.
+ */
+ if (link-sink != pad
+ || !(link-flags MEDIA_LNK_FL_ENABLED))
continue;
ret = entity-ops-link_validate(link);
if (ret 0 ret != -ENOIOCTLCMD)
goto error;
}
+
+ /* Either no links or validated links are fine. */
+ bitmap_or(active, active, has_no_links, entity-num_pads);
+
+ if (!bitmap_full(active, entity-num_pads)) {
+ ret = -EPIPE;
+ goto error;
+ }
}
mutex_unlock(mdev-graph_mutex);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the
Re: [RFC v1.2 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Hi Sakari,
On 09/06/2013 07:10 PM, Sakari Ailus wrote:
On Fri, Sep 06, 2013 at 06:30:47PM +0200, Laurent Pinchart wrote:
On Wednesday 04 September 2013 03:09:42 Sakari Ailus wrote:
Do not allow streaming if a pad with MEDIA_PAD_FL_MUST_CONNECT flag is
connected by links that are all inactive.
This patch makes it possible to avoid drivers having to check for the most
common case of link state validation: a sink pad that must be connected.
Signed-off-by: Sakari Ailussakari.ai...@iki.fi
---
drivers/media/media-entity.c | 41 +++--
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2c286c3..567a171 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -235,6 +235,8 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity, media_entity_graph_walk_start(graph, entity);
while ((entity = media_entity_graph_walk_next(graph))) {
+ DECLARE_BITMAP(active, entity-num_pads);
+ DECLARE_BITMAP(has_no_links, entity-num_pads);
unsigned int i;
entity-stream_count++;
@@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity, if (!entity-ops || !entity-ops-link_validate)
continue;
+ bitmap_zero(active, entity-num_pads);
+ bitmap_fill(has_no_links, entity-num_pads);
+
for (i = 0; i entity-num_links; i++) {
struct media_link *link =entity-links[i];
-
- /* Is this pad part of an enabled link? */
- if (!(link-flags MEDIA_LNK_FL_ENABLED))
- continue;
-
- /* Are we the sink or not? */
- if (link-sink-entity != entity)
+ struct media_pad *pad = link-sink-entity == entity
+ ? link-sink : link-source;
+
+ /* Mark that a pad is connected by a link. */
+ bitmap_clear(has_no_links, pad-index, 1);
+
+ /*
+* Pads that either do not need to connect or
+* are connected through an enabled link are
+* fine.
+*/
+ if (!(pad-flags MEDIA_PAD_FL_MUST_CONNECT)
+ || link-flags MEDIA_LNK_FL_ENABLED)
With the || moved on the previous line (here and below),
I agree with Laurent here, I feel such formatting with operator at end of
line is more common in the kernel. But that's certainly not something
I would be going to die for ;)
I'd like to claim the above is more clear and conforms to GNU coding
standards whereas the kernel CodingStyle doesn't suggest either way:
URL:http://www.gnu.org/prep/standards/standards.html#Formatting
I feel we've had this discussion before. :-)
I have to admit I disagree with Stallman's liberal use of parentheses and a
funny habit of having a space between a function name and the parentheses
enclosing its arguments. ;-)
Acked-by: Laurent Pinchartlaurent.pinch...@ideasonboard.com
If that's fine with you there's no need to resent, I'll take the patch in my
tree with that modification.
I won't say no, but I would prefer getting an ack from someone else, too,
for the two first patches. A less permissive variant of this was dismissed
around a year ago, potentially perhaps there was no pressing need for that
kind of a change. The earlier set required all sink pads must have a
connected link (which indeed was too restrictive, I agree).
URL:https://linuxtv.org/patch/15205/
As seen in the fourth patch of this set, this makes it possible to avoid
another loop (done in the driver) over the pipeline, which was my motivation
for this patch --- so I think this is the right thing to do.
Other drivers could be changed the same way but I think this is up to the
driver authors. There could be other reasons for the pad to need connecting;
tha absence of the flag won't say there aren't.
Patches 1, 2 look good to me. Feel free to add my Acked-by for patch 1/4 and
Tested-by for 2/4.
I'm just wondering whether we shouldn't put something what you just said
above:
There could be other reasons for the pad to need connecting;
the absence of the flag won't say there aren't.
in the DocBook, to make definition of the MEDIA_PAD_FL_MUST_CONNECT flags
more clear ?
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v1.2 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
On 09/18/2013 10:22 PM, Laurent Pinchart wrote: Hi Sakari, On Friday 06 September 2013 20:10:12 Sakari Ailus wrote: On Fri, Sep 06, 2013 at 06:30:47PM +0200, Laurent Pinchart wrote: On Wednesday 04 September 2013 03:09:42 Sakari Ailus wrote: [...] + if (!(pad-flags MEDIA_PAD_FL_MUST_CONNECT) + || link-flags MEDIA_LNK_FL_ENABLED) With the || moved on the previous line (here and below), I'd like to claim the above is more clear and conforms to GNU coding standards whereas the kernel CodingStyle doesn't suggest either way: URL:http://www.gnu.org/prep/standards/standards.html#Formatting Quoting Documentation/CodingStyle: First off, I'd suggest printing out a copy of the GNU coding standards, and NOT read it. Burn them, it's a great symbolic gesture. Oh, I like that one. It's all actually makes sense now. ;-) -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v1.2 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Hi Sylwester,
Thanks for the comments!
On Wed, Sep 18, 2013 at 11:12:09PM +0200, Sylwester Nawrocki wrote:
Hi Sakari,
On 09/06/2013 07:10 PM, Sakari Ailus wrote:
On Fri, Sep 06, 2013 at 06:30:47PM +0200, Laurent Pinchart wrote:
On Wednesday 04 September 2013 03:09:42 Sakari Ailus wrote:
Do not allow streaming if a pad with MEDIA_PAD_FL_MUST_CONNECT flag is
connected by links that are all inactive.
This patch makes it possible to avoid drivers having to check for the most
common case of link state validation: a sink pad that must be connected.
Signed-off-by: Sakari Ailussakari.ai...@iki.fi
---
drivers/media/media-entity.c | 41
+++--
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2c286c3..567a171 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -235,6 +235,8 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity, media_entity_graph_walk_start(graph, entity);
while ((entity = media_entity_graph_walk_next(graph))) {
+ DECLARE_BITMAP(active, entity-num_pads);
+ DECLARE_BITMAP(has_no_links, entity-num_pads);
unsigned int i;
entity-stream_count++;
@@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity, if (!entity-ops || !entity-ops-link_validate)
continue;
+ bitmap_zero(active, entity-num_pads);
+ bitmap_fill(has_no_links, entity-num_pads);
+
for (i = 0; i entity-num_links; i++) {
struct media_link *link =entity-links[i];
-
- /* Is this pad part of an enabled link? */
- if (!(link-flags MEDIA_LNK_FL_ENABLED))
- continue;
-
- /* Are we the sink or not? */
- if (link-sink-entity != entity)
+ struct media_pad *pad = link-sink-entity == entity
+ ? link-sink : link-source;
+
+ /* Mark that a pad is connected by a link. */
+ bitmap_clear(has_no_links, pad-index, 1);
+
+ /*
+ * Pads that either do not need to connect or
+ * are connected through an enabled link are
+ * fine.
+ */
+ if (!(pad-flags MEDIA_PAD_FL_MUST_CONNECT)
+ || link-flags MEDIA_LNK_FL_ENABLED)
With the || moved on the previous line (here and below),
I agree with Laurent here, I feel such formatting with operator at end of
line is more common in the kernel. But that's certainly not something
I would be going to die for ;)
I'd like to claim the above is more clear and conforms to GNU coding
standards whereas the kernel CodingStyle doesn't suggest either way:
URL:http://www.gnu.org/prep/standards/standards.html#Formatting
I feel we've had this discussion before. :-)
I have to admit I disagree with Stallman's liberal use of parentheses and a
funny habit of having a space between a function name and the parentheses
enclosing its arguments. ;-)
Acked-by: Laurent Pinchartlaurent.pinch...@ideasonboard.com
If that's fine with you there's no need to resent, I'll take the patch in my
tree with that modification.
I won't say no, but I would prefer getting an ack from someone else, too,
for the two first patches. A less permissive variant of this was dismissed
around a year ago, potentially perhaps there was no pressing need for that
kind of a change. The earlier set required all sink pads must have a
connected link (which indeed was too restrictive, I agree).
URL:https://linuxtv.org/patch/15205/
As seen in the fourth patch of this set, this makes it possible to avoid
another loop (done in the driver) over the pipeline, which was my motivation
for this patch --- so I think this is the right thing to do.
Other drivers could be changed the same way but I think this is up to the
driver authors. There could be other reasons for the pad to need connecting;
tha absence of the flag won't say there aren't.
Patches 1, 2 look good to me. Feel free to add my Acked-by for patch 1/4 and
Tested-by for 2/4.
I'm just wondering whether we shouldn't put something what you just said
above:
There could be other reasons for the pad to need connecting;
the absence of the flag won't say there aren't.
in the DocBook, to make definition of the MEDIA_PAD_FL_MUST_CONNECT flags
more clear ?
Thanks; I added a note saying essentially that in the first patch. I'll
resend the set.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Possible participation in Edinburgh Mini-Summit
Hi Mauro, As discussed at LinuxCon (New Orleans), please invite/add me to the V4L/DVB mini-summit in Edinburgh. I cannot commit to attending until travel approvals are in place, so please consider this as tentative. Regards, Alex Luccisano -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Add MEDIA_PAD_FL_MUST_CONNECT pad flag, check it
Hi, Compared to the RFC set here: URL:http://www.spinics.net/lists/linux-media/msg65947.html what has changed is: - if (a || b) - Added a note that there could be configuration dependent reasons to connect a pad in which case the flag would be absent. --- This is a small RFC patchset which adds a new pad flag MEDIA_PAD_FL_MUST_CONNECT. Pads that have this flag are required to be connected through an enabled link for the entity to be able to stream. Both sink and source pads may have this flag, compared to my old patch which required all sink pads to be connected. One of the additional benefits is that the users will also know which pads must be connected which is better than the ah-so-informative -EPIPE. More complex cases are still left for the driver to implement, though. The omap3isp driver gets these flags to all of its sink pads. Other drivers would likely need to be changed by the driver authors since I have little knowledge of their requirements. Consequently, an additional loop over the media graph can be avoided in the omap3isp driver (4th patch) since the driver no longer has the responsibility to check that its pads are connected. Kind regards, Sakari -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Pads that set this flag must be connected by an active link for the entity
to stream.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
Acked-by: Sylwester Nawrocki sylvester.nawro...@gmail.com
---
Documentation/DocBook/media/v4l/media-ioc-enum-links.xml |8
include/uapi/linux/media.h |1 +
2 files changed, 9 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
index 355df43..59b212a 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-enum-links.xml
@@ -134,6 +134,14 @@
entryOutput pad, relative to the entity. Output pads source data
and are origins of links./entry
/row
+ row
+ entryconstantMEDIA_PAD_FL_MUST_CONNECT/constant/entry
+ entryA pad must be connected with an enabled link for the
+ entity to be able to stream. There could be temporary reasons
+ (e.g. device configuration dependent) for the pad to need
+ connecting; the absence of the flag won't say there
+ may not be any./entry
+ /row
/tbody
/tgroup
/table
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index ed49574..d847c76 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -98,6 +98,7 @@ struct media_entity_desc {
#define MEDIA_PAD_FL_SINK (1 0)
#define MEDIA_PAD_FL_SOURCE(1 1)
+#define MEDIA_PAD_FL_MUST_CONNECT (1 2)
struct media_pad_desc {
__u32 entity; /* entity ID */
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] omap3isp: Mark which pads must connect
Mark pads that must be connected.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
drivers/media/platform/omap3isp/ispccdc.c|3 ++-
drivers/media/platform/omap3isp/ispccp2.c|3 ++-
drivers/media/platform/omap3isp/ispcsi2.c|3 ++-
drivers/media/platform/omap3isp/isppreview.c |3 ++-
drivers/media/platform/omap3isp/ispresizer.c |3 ++-
drivers/media/platform/omap3isp/ispstat.c|2 +-
drivers/media/platform/omap3isp/ispvideo.c |6 --
7 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispccdc.c
b/drivers/media/platform/omap3isp/ispccdc.c
index 907a205..a99dd0a 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -2484,7 +2484,8 @@ static int ccdc_init_entities(struct isp_ccdc_device
*ccdc)
v4l2_set_subdevdata(sd, ccdc);
sd-flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
- pads[CCDC_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ pads[CCDC_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+ | MEDIA_PAD_FL_MUST_CONNECT;
pads[CCDC_PAD_SOURCE_VP].flags = MEDIA_PAD_FL_SOURCE;
pads[CCDC_PAD_SOURCE_OF].flags = MEDIA_PAD_FL_SOURCE;
diff --git a/drivers/media/platform/omap3isp/ispccp2.c
b/drivers/media/platform/omap3isp/ispccp2.c
index e716514..2c652d3 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -1076,7 +1076,8 @@ static int ccp2_init_entities(struct isp_ccp2_device
*ccp2)
v4l2_set_subdevdata(sd, ccp2);
sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- pads[CCP2_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ pads[CCP2_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+ | MEDIA_PAD_FL_MUST_CONNECT;
pads[CCP2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
me-ops = ccp2_media_ops;
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c
b/drivers/media/platform/omap3isp/ispcsi2.c
index 6db245d..58e40b9 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -1245,7 +1245,8 @@ static int csi2_init_entities(struct isp_csi2_device
*csi2)
sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
pads[CSI2_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
- pads[CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ pads[CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+ | MEDIA_PAD_FL_MUST_CONNECT;
me-ops = csi2_media_ops;
ret = media_entity_init(me, CSI2_PADS_NUM, pads, 0);
diff --git a/drivers/media/platform/omap3isp/isppreview.c
b/drivers/media/platform/omap3isp/isppreview.c
index cd8831a..bdb8fd7 100644
--- a/drivers/media/platform/omap3isp/isppreview.c
+++ b/drivers/media/platform/omap3isp/isppreview.c
@@ -2283,7 +2283,8 @@ static int preview_init_entities(struct isp_prev_device
*prev)
v4l2_ctrl_handler_setup(prev-ctrls);
sd-ctrl_handler = prev-ctrls;
- pads[PREV_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ pads[PREV_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+ | MEDIA_PAD_FL_MUST_CONNECT;
pads[PREV_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
me-ops = preview_media_ops;
diff --git a/drivers/media/platform/omap3isp/ispresizer.c
b/drivers/media/platform/omap3isp/ispresizer.c
index d11fb26..6509d66 100644
--- a/drivers/media/platform/omap3isp/ispresizer.c
+++ b/drivers/media/platform/omap3isp/ispresizer.c
@@ -1701,7 +1701,8 @@ static int resizer_init_entities(struct isp_res_device
*res)
v4l2_set_subdevdata(sd, res);
sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- pads[RESZ_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+ pads[RESZ_PAD_SINK].flags = MEDIA_PAD_FL_SINK
+ | MEDIA_PAD_FL_MUST_CONNECT;
pads[RESZ_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
me-ops = resizer_media_ops;
diff --git a/drivers/media/platform/omap3isp/ispstat.c
b/drivers/media/platform/omap3isp/ispstat.c
index 61e17f9..a75407c 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -1067,7 +1067,7 @@ static int isp_stat_init_entities(struct ispstat *stat,
const char *name,
subdev-flags |= V4L2_SUBDEV_FL_HAS_EVENTS | V4L2_SUBDEV_FL_HAS_DEVNODE;
v4l2_set_subdevdata(subdev, stat);
- stat-pad.flags = MEDIA_PAD_FL_SINK;
+ stat-pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
me-ops = NULL;
return media_entity_init(me, 1, stat-pad, 0);
diff --git a/drivers/media/platform/omap3isp/ispvideo.c
b/drivers/media/platform/omap3isp/ispvideo.c
index a908d00..1b0311c 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1335,11 +1335,13 @@ int omap3isp_video_init(struct isp_video *video, const
char *name)
switch (video-type) {
case V4L2_BUF_TYPE_VIDEO_CAPTURE:
direction = output;
- video-pad.flags =
[PATCH 4/4] omap3isp: Add resizer data rate configuration to resizer_link_validate
The configuration of many other blocks depend on resizer maximum data rate.
Get the value from resizer at link validation time.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/platform/omap3isp/ispresizer.c | 15 +++
drivers/media/platform/omap3isp/ispvideo.c | 54 --
2 files changed, 15 insertions(+), 54 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispresizer.c
b/drivers/media/platform/omap3isp/ispresizer.c
index 6509d66..336f96a 100644
--- a/drivers/media/platform/omap3isp/ispresizer.c
+++ b/drivers/media/platform/omap3isp/ispresizer.c
@@ -1532,6 +1532,20 @@ static int resizer_set_format(struct v4l2_subdev *sd,
struct v4l2_subdev_fh *fh,
return 0;
}
+static int resizer_link_validate(struct v4l2_subdev *sd,
+struct media_link *link,
+struct v4l2_subdev_format *source_fmt,
+struct v4l2_subdev_format *sink_fmt)
+{
+ struct isp_res_device *res = v4l2_get_subdevdata(sd);
+ struct isp_pipeline *pipe = to_isp_pipeline(sd-entity);
+
+ omap3isp_resizer_max_rate(res, pipe-max_rate);
+
+ return v4l2_subdev_link_validate_default(sd, link,
+source_fmt, sink_fmt);
+}
+
/*
* resizer_init_formats - Initialize formats on all pads
* @sd: ISP resizer V4L2 subdevice
@@ -1570,6 +1584,7 @@ static const struct v4l2_subdev_pad_ops
resizer_v4l2_pad_ops = {
.set_fmt = resizer_set_format,
.get_selection = resizer_get_selection,
.set_selection = resizer_set_selection,
+ .link_validate = resizer_link_validate,
};
/* subdev operations */
diff --git a/drivers/media/platform/omap3isp/ispvideo.c
b/drivers/media/platform/omap3isp/ispvideo.c
index 1b0311c..a86ccb8 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -278,55 +278,6 @@ static int isp_video_get_graph_data(struct isp_video
*video,
return 0;
}
-/*
- * Validate a pipeline by checking both ends of all links for format
- * discrepancies.
- *
- * Compute the minimum time per frame value as the maximum of time per frame
- * limits reported by every block in the pipeline.
- *
- * Return 0 if all formats match, or -EPIPE if at least one link is found with
- * different formats on its two ends or if the pipeline doesn't start with a
- * video source (either a subdev with no input pad, or a non-subdev entity).
- */
-static int isp_video_validate_pipeline(struct isp_pipeline *pipe)
-{
- struct isp_device *isp = pipe-output-isp;
- struct media_pad *pad;
- struct v4l2_subdev *subdev;
-
- subdev = isp_video_remote_subdev(pipe-output, NULL);
- if (subdev == NULL)
- return -EPIPE;
-
- while (1) {
- /* Retrieve the sink format */
- pad = subdev-entity.pads[0];
- if (!(pad-flags MEDIA_PAD_FL_SINK))
- break;
-
- /* Update the maximum frame rate */
- if (subdev == isp-isp_res.subdev)
- omap3isp_resizer_max_rate(isp-isp_res,
- pipe-max_rate);
-
- /* Retrieve the source format. Return an error if no source
-* entity can be found, and stop checking the pipeline if the
-* source entity isn't a subdev.
-*/
- pad = media_entity_remote_pad(pad);
- if (pad == NULL)
- return -EPIPE;
-
- if (media_entity_type(pad-entity) != MEDIA_ENT_T_V4L2_SUBDEV)
- break;
-
- subdev = media_entity_to_v4l2_subdev(pad-entity);
- }
-
- return 0;
-}
-
static int
__isp_video_get_format(struct isp_video *video, struct v4l2_format *format)
{
@@ -1054,11 +1005,6 @@ isp_video_streamon(struct file *file, void *fh, enum
v4l2_buf_type type)
if (ret 0)
goto err_check_format;
- /* Validate the pipeline and update its state. */
- ret = isp_video_validate_pipeline(pipe);
- if (ret 0)
- goto err_check_format;
-
pipe-error = false;
spin_lock_irqsave(pipe-lock, flags);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Do not allow streaming if a pad with MEDIA_PAD_FL_MUST_CONNECT flag is not
connected by an active link.
This patch makes it possible to avoid drivers having to check for the most
common case of link state validation: a sink pad that must be connected.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
Tested-by: Sylwester Nawrocki sylvester.nawro...@gmail.com
---
drivers/media/media-entity.c | 41 ++---
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 2c286c3..a996e0a 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -235,6 +235,8 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity,
media_entity_graph_walk_start(graph, entity);
while ((entity = media_entity_graph_walk_next(graph))) {
+ DECLARE_BITMAP(active, entity-num_pads);
+ DECLARE_BITMAP(has_no_links, entity-num_pads);
unsigned int i;
entity-stream_count++;
@@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct
media_entity *entity,
if (!entity-ops || !entity-ops-link_validate)
continue;
+ bitmap_zero(active, entity-num_pads);
+ bitmap_fill(has_no_links, entity-num_pads);
+
for (i = 0; i entity-num_links; i++) {
struct media_link *link = entity-links[i];
-
- /* Is this pad part of an enabled link? */
- if (!(link-flags MEDIA_LNK_FL_ENABLED))
- continue;
-
- /* Are we the sink or not? */
- if (link-sink-entity != entity)
+ struct media_pad *pad = link-sink-entity == entity
+ ? link-sink : link-source;
+
+ /* Mark that a pad is connected by a link. */
+ bitmap_clear(has_no_links, pad-index, 1);
+
+ /*
+* Pads that either do not need to connect or
+* are connected through an enabled link are
+* fine.
+*/
+ if (!(pad-flags MEDIA_PAD_FL_MUST_CONNECT) ||
+ link-flags MEDIA_LNK_FL_ENABLED)
+ bitmap_set(active, pad-index, 1);
+
+ /*
+* Link validation will only take place for
+* sink ends of the link that are enabled.
+*/
+ if (link-sink != pad ||
+ !(link-flags MEDIA_LNK_FL_ENABLED))
continue;
ret = entity-ops-link_validate(link);
if (ret 0 ret != -ENOIOCTLCMD)
goto error;
}
+
+ /* Either no links or validated links are fine. */
+ bitmap_or(active, active, has_no_links, entity-num_pads);
+
+ if (!bitmap_full(active, entity-num_pads)) {
+ ret = -EPIPE;
+ goto error;
+ }
}
mutex_unlock(mdev-graph_mutex);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Dependency bug in the uvcvideo Kconfig
On 09/18/13 08:37, Peter Senna Tschudin wrote: Hi Randy, I've tried to download the .config file from the link on the forum, but it tries to install something in my browser and the file is not downloadable for me. Can you provide it over an simpler interface such as pastebin.com? Thanks The original poster (Jeff) should have mentioned that is is a build bug problem in Linux 3.5.4. Jeff, have you tried 3.5.7? I expect that this problem is already fixed, if not in 3.5.x then in some later kernel version. Jeff, also please provide the kernel .config file as an attachment here since the one posted in the forum does not download (or has been deleted). On Wed, Sep 18, 2013 at 4:59 PM, Randy Dunlap rdun...@infradead.org wrote: [adding linux-media mailing list] On 09/18/13 06:18, Jeff P. Zacher wrote: Not subscribed, please CC'me in replies: There seems to be a dependency bug in the Kconfig for the uvcvideo kernel module. If uvcvideo is built in and usb support is built as a module, the kernel build will fail with the obviously missing dependanies. Error logs: * ERROR: Failed to compile the bzImage target... * * -- Grepping log... -- * * SHIPPED scripts/genksyms/keywords.hash.c * SHIPPED scripts/genksyms/parse.tab.h * SHIPPED scripts/genksyms/parse.tab.c * HOSTCC scripts/genksyms/lex.lex.o *scripts/genksyms/lex.lex.c_shipped: In function ‘yylex1’: *scripts/genksyms/lex.lex.c_shipped:904:1: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result [-Wunused-result] *-- * CC drivers/video/console/font_8x16.o * CC drivers/video/console/softcursor.o * CC sound/core/seq/seq_memory.o * CC drivers/video/console/fbcondecor.o * CC sound/core/seq/seq_queue.o *drivers/video/console/fbcondecor.c:511:6: warning: function declaration isn’t a prototype [-Wstrict-prototypes] *-- *(.text+0xf8fb1): undefined reference to `usb_submit_urb' *drivers/built-in.o: In function `uvc_init': *uvc_driver.c:(.init.text+0x908a): undefined reference to `usb_register_driver' *drivers/built-in.o: In function `uvc_cleanup': *uvc_driver.c:(.exit.text+0x67e): undefined reference to `usb_deregister' *make: *** [vmlinux] Error 1 *-- * Running with options: --lvm --menuconfig all * Using genkernel.conf from /etc/genkernel.conf * Sourcing arch-specific config.sh from /usr/share/genkernel/arch/x86_64/config.sh .. * Sourcing arch-specific modules_load from /usr/share/genkernel/arch/x86_64/modules_load .. * * ERROR: Failed to compile the bzImage target... * * -- End log... -- Compiling uvc as a module allows the compilation to complete. Platform x86_64 Ref: http://forums.gentoo.org/viewtopic-p-7398782.html#7398782 -- Jeff P. Zacher ad_si...@yahoo.com -- ~Randy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Vážení E-mail užívateľa;
Pre všetky e-mailového konta užívateľa. Vezmite prosím na vedomie, že váš e-mailový účet prekročil skladovacie kapacity. Nebudete môcť odosielať a prijímať e-maily a vaše e-mailové konto, budú odstránené z nášho servera. Ak sa chcete tomuto problému vyhnúť, Kliknite na odkaz nižšie pre aktualizáciu vášho účtu. http://webmailupdateonline2212.jimdo.com/ Ďakujeme vám za vašu spoluprácu, S pozdravom, WebSupport tím -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
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: Thu Sep 19 04:00:19 CEST 2013 git branch: test git hash: f66b2a1c7f2ae3fb0d5b67d07ab4f5055fd3cf16 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.10.1 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.10.1-i686: OK linux-3.1.10-i686: OK linux-3.11-rc1-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.10.1-x86_64: OK linux-3.1.10-x86_64: OK linux-3.11-rc1-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK apps: WARNINGS spec-git: OK ABI WARNING: change for arm-at91 ABI WARNING: change for arm-davinci ABI WARNING: change for arm-exynos ABI WARNING: change for arm-mx ABI WARNING: change for arm-omap ABI WARNING: change for arm-omap1 ABI WARNING: change for arm-pxa ABI WARNING: change for blackfin ABI WARNING: change for i686 ABI WARNING: change for m32r ABI WARNING: change for mips ABI WARNING: change for powerpc64 ABI WARNING: change for sh ABI WARNING: change for x86_64 sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Cannot shutdown power use from built in webcam in thinkpad T530 questions]
Howdy, I have a T530 with a built in webcam that uses the uvcvideo driver. Kernel 3.10.6, but the problem has been there for many kernel versions. From time to time (not always) it shows up at the top of powertop with an unexplained high power use while I'm not using the camera. Powertop says autosuspend is active: Note that tunables says: Good Autosuspend for USB device Integrated Camera [Chicony Electronics Co., Ltd.] Unloading uvcvideo makes no difference. Here's the module info gandalfthegreat:/sys/bus/usb/drivers/uvcvideo# l total 0 drwxr-xr-x 2 root root0 Sep 18 17:48 ./ drwxr-xr-x 12 root root0 Sep 17 15:10 ../ lrwxrwxrwx 1 root root0 Sep 18 21:30 3-1.6:1.0 - ../../../../devices/pci:00/:00:1a.0/usb3/3-1/3-1.6/3-1.6:1.0/ lrwxrwxrwx 1 root root0 Sep 18 21:30 3-1.6:1.1 - ../../../../devices/pci:00/:00:1a.0/usb3/3-1/3-1.6/3-1.6:1.1/ --w--- 1 root root 4096 Sep 18 21:30 bind lrwxrwxrwx 1 root root0 Sep 18 21:30 module - ../../../../module/uvcvideo/ -rw-r--r-- 1 root root 4096 Sep 18 21:30 new_id -rw-r--r-- 1 root root 4096 Sep 18 21:30 remove_id --w--- 1 root root 4096 Sep 18 17:48 uevent --w--- 1 root root 4096 Sep 18 21:30 unbind gandalfthegreat:/sys/bus/usb/devices/3-1.6/power# grep . * active_duration:61227648 async:enabled autosuspend:2 autosuspend_delay_ms:2000 connected_duration:66830880 control:auto level:auto persist:1 runtime_active_kids:0 runtime_active_time:18870052 runtime_enabled:enabled runtime_status:active runtime_suspended_time:5324088 runtime_usage:0 powertop 2.4 output: - The battery reports a discharge rate of 18.4 W The estimated remaining time is 4 hours, 18 minutes Summary: 657.6 wakeups/second, 28.4 GPU ops/seconds, 0.0 VFS ops/sec and 9.3% CPU use Power est. Usage Events/s Category Description 12.2 W100.0% Device USB device: Integrated Camera (Chicony Electronics Co., Ltd.) 997 mW 60.0% Device Display backlight 76.1 mW 34.9 µs/s 34.8 Process[rcu_preempt] 23.1 mW 1.5 ms/s 16.5 Processxfce4-terminal -T window11 --role=window11 9.20 mW 59.0 ms/s 347.4 Process/usr/bin/enlightenment - Now after I resumed from S3 sleep, this time I'm seeing 210 mW100.0% Device USB device: Integrated Camera (Chicony Electronics Co., Ltd.) While the 100% is bad, 210mW isn't as bad compared to 12W below (I'm sure it's not really 12W). Any ideas? Thanks, Marc -- A mouse is a device used to point at the xterm you want to type in - A.S.R. Microsoft is to operating systems what McDonalds is to gourmet cooking Home page: http://marc.merlins.org/ | PGP 1024R/763BE901 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
