Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1591273 > unrolled thread

Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

Started bySakari Ailus <sakari.ailus@iki.fi>
First post2017-03-02 17:50 +0100
Last post2017-03-11 22:40 +0100
Articles 20 on this page of 52 — 6 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-02 17:50 +0100
    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-03 01:00 +0100
      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-03 02:00 +0100
      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-03 03:30 +0100
        Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-03 20:20 +0100
          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-03 23:50 +0100
    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-04 00:20 +0100
      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-04 01:40 +0100
      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-04 14:20 +0100
        Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 14:00 +0100
          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-10 14:10 +0100
            Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 14:30 +0100
              Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-10 15:10 +0100
                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 15:30 +0100
                  Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-10 17:00 +0100
                    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-10 23:40 +0100
                      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-11 12:30 +0100
                        Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-03-11 23:00 +0100
                        Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 00:20 +0100
                          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 01:30 +0100
                          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-03-12 22:30 +0100
                            Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-12 23:40 +0100
                        Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-13 13:50 +0100
            Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-10 16:30 +0100
              Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-10 17:00 +0100
                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-10 18:10 +0100
                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-10 21:50 +0100
                  Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-03-10 23:00 +0100
          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-10 16:10 +0100
            Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-11 12:40 +0100
              Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-11 14:20 +0100
                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-11 16:40 +0100
                  Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-11 18:40 +0100
                  Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 19:10 +0100
                    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-11 19:50 +0100
                      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 20:00 +0100
                        Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-11 20:10 +0100
                          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 20:10 +0100
                            Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-11 21:50 +0100
                          Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 04:40 +0100
                            Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 08:40 +0100
                              Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 19:00 +0100
                                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-12 23:00 +0100
                                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 11:50 +0100
                                  Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 12:00 +0100
                                    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 12:10 +0100
                                    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-13 12:50 +0100
                                      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 13:40 +0100
                              Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-03-12 19:20 +0100
                    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-03-11 21:30 +0100
                      Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 21:40 +0100
                Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-03-11 22:40 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


#1598765

FromPavel Machek <pavel@ucw.cz>
Date2017-03-12 22:30 +0100
Message-ID<tkjdU-7cP-7@gated-at.bofh.it>
In reply to#1598505

[Multipart message — attachments visible in raw view] — view raw

On Sat 2017-03-11 23:14:56, Russell King - ARM Linux wrote:
> On Sat, Mar 11, 2017 at 08:25:49AM -0300, Mauro Carvalho Chehab wrote:
> > This situation is there since 2009. If I remember well, you tried to write
> > such generic plugin in the past, but never finished it, apparently because
> > it is too complex. Others tried too over the years. 
> > 
> > The last trial was done by Jacek, trying to cover just the exynos4 driver. 
> > Yet, even such limited scope plugin was not good enough, as it was never
> > merged upstream. Currently, there's no such plugins upstream.
> > 
> > If we can't even merge a plugin that solves it for just *one* driver,
> > I have no hope that we'll be able to do it for the generic case.
> 
> This is what really worries me right now about the current proposal for
> iMX6.  What's being proposed is to make the driver exclusively MC-based.
> 
> What that means is that existing applications are _not_ going to work
> until we have some answer for libv4l2, and from what you've said above,
> it seems that this has been attempted multiple times over the last _8_
> years, and each time it's failed.

Yeah. We need a mid-layer between legacy applications and MC
devices. Such layer does not exist in userspace or in kernel.

> Loading the problem onto the user in the hope that the user knows
> enough to properly configure it also doesn't work - who is going to
> educate the user about the various quirks of the hardware they're
> dealing with?

We have docs. Users can write shell scripts. Still, mid-layer would be
nice.

> So, the problem space we have here is absolutely huge, and merely
> having a plugin that activates when you open a /dev/video* node
> really doesn't solve it.
> 
> All in all, I really don't think "lets hope someone writes a v4l2
> plugin to solve it" is ever going to be successful.  I don't even
> see that there will ever be a userspace application that is anything
> more than a representation of the dot graphs that users can use to
> manually configure the capture system with system knowledge.
> 
> I think everyone needs to take a step back and think long and hard
> about this from the system usability perspective - I seriously
> doubt that we will ever see any kind of solution to this if we
> continue to progress with "we'll sort it in userspace some day."

Mid-layer is difficult... there are _hundreds_ of possible
pipeline setups. If it should live in kernel or in userspace is a
question... but I don't think having it in kernel helps in any way.

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]


#1598788

FromMauro Carvalho Chehab <mchehab@s-opensource.com>
Date2017-03-12 23:40 +0100
Message-ID<tkkjE-7Sg-3@gated-at.bofh.it>
In reply to#1598765
Em Sun, 12 Mar 2017 22:29:04 +0100
Pavel Machek <pavel@ucw.cz> escreveu:

> Mid-layer is difficult... there are _hundreds_ of possible
> pipeline setups. If it should live in kernel or in userspace is a
> question... but I don't think having it in kernel helps in any way.

Mid-layer is difficult, because we either need to feed some
library with knowledge for all kernel drivers or we need to improve
the MC API to provide more details.

For example, several drivers used to expose entities via the
generic MEDIA_ENT_T_DEVNODE to represent entities of different
types. See, for example, entities 1, 5 and 7 (and others) at:
	https://mchehab.fedorapeople.org/mc-next-gen/igepv2_omap3isp.png

A device-specific code could either be hardcoding the entity number
or checking for the entity strings to add some logic to setup
controls on those "unknown" entities, a generic app won't be able 
to do anything with them, as it doesn't know what function(s) such
entity provide.

Also, on some devices, like the analog TV decoder at:
	https://mchehab.fedorapeople.org/mc-next-gen/au0828_test/mc_nextgen_test-output.png

May have pads with different signals on their output. In such
case, pads 1 and 2 provide video, while pad 3 provides audio using a
different type of output.

The application needs to know such kind of things in order to be able
to properly setup the pipeline [1].

[1] this specific device has a fixed pipeline, but I'm aware of SoC with
flexible pipelines that have this kind of issue (nobody upstreamed the
V4L part of those devices yet).

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]


#1599303

FromSakari Ailus <sakari.ailus@iki.fi>
Date2017-03-13 13:50 +0100
Message-ID<tkxAd-ys-13@gated-at.bofh.it>
In reply to#1598303
Hi Mauro,

On Sat, Mar 11, 2017 at 08:25:49AM -0300, Mauro Carvalho Chehab wrote:
> Em Sat, 11 Mar 2017 00:37:14 +0200
> Sakari Ailus <sakari.ailus@iki.fi> escreveu:
> 
> > Hi Mauro (and others),
> > 
> > On Fri, Mar 10, 2017 at 12:53:42PM -0300, Mauro Carvalho Chehab wrote:
> > > Em Fri, 10 Mar 2017 15:20:48 +0100
> > > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > >   
> > > >   
> > > > > As I've already mentioned, from talking about this with Mauro, it seems
> > > > > Mauro is in agreement with permitting the control inheritence... I wish
> > > > > Mauro would comment for himself, as I can't quote our private discussion
> > > > > on the subject.    
> > > > 
> > > > I can't comment either, not having seen his mail and reasoning.  
> > > 
> > > The rationale is that we should support the simplest use cases first.
> > > 
> > > In the case of the first MC-based driver (and several subsequent
> > > ones), the simplest use case required MC, as it was meant to suport
> > > a custom-made sophisticated application that required fine control
> > > on each component of the pipeline and to allow their advanced
> > > proprietary AAA userspace-based algorithms to work.  
> > 
> > The first MC based driver (omap3isp) supports what the hardware can do, it
> > does not support applications as such.
> 
> All media drivers support a subset of what the hardware can do. The
> question is if such subset covers the use cases or not.

Can you name a feature in the OMAP 3 ISP that is not and can not be
supported using the current driver model (MC + V4L2 sub-device + V4L2) that
could be even remotely useful?

> 
> The current MC-based drivers (except for uvc) took a patch to offer a
> more advanced API, to allow direct control to each IP module, as it was
> said, by the time we merged the OMAP3 driver, that, for the N9/N900 camera
> to work, it was mandatory to access the pipeline's individual components.
> 
> Such approach require that some userspace software will have knowledge
> about some hardware details, in order to setup pipelines and send controls
> to the right components. That makes really hard to have a generic user
> friendly application to use such devices.

The effect you described above is true, but I disagree with the cause. The
cause is the hardware is more complex and variable than what has been
supported previously, and providing a generic interface for accessing such
hardware will require more complex interface.

The hardware we have today and the user cases we have today are more --- not
less --- complex and nuanced than when the Media controller was merged back
in 2010. Arguably there is thus more need for the functionality it provides,
not less.

> 
> Non-MC based drivers control the hardware via a portable interface with
> doesn't require any knowledge about the hardware specifics, as either the
> Kernel or some firmware at the device will set any needed pipelines.
> 
> In the case of V4L2 controls, when there's no subdev API, the main
> driver (e. g. the driver that creates the /dev/video nodes) sends a
> multicast message to all bound I2C drivers. The driver(s) that need 
> them handle it. When the same control may be implemented on different
> drivers, the main driver sends a unicast message to just one driver[1].
> 
> [1] There are several non-MC drivers that have multiple ways to
> control some things, like doing scaling or adjust volume levels at
> either the bridge driver or at a subdriver.
> 
> There's nothing wrong with this approach: it works, it is simpler,
> it is generic. So, if it covers most use cases, why not allowing it
> for usecases where a finer control is not a requirement?

Drivers are written to support hardware, not particular use case. Use case
specific knowledge should be present only in applications, not in drivers.
Doing it otherwise will lead to use case specific drivers and more driver
code to maintain for any particular piece of hardware.

An individual could possibly choose the right driver for his / her use case,
but this approach could hardly work for Linux distribution kernels.

The plain V4L2 interface is generic within its own scope: hardware can be
supported within the hardware model assumed by the interface. However, on
some devices this will end up being a small subset of what the hardware can
do. Besides that, when writing the driver, you need to decide at detail
level what kind of subset that might be.

That's not something anyone writing a driver should need to confront.

> 
> > Adding support to drivers for different "operation modes" --- this is
> > essentially what is being asked for --- is not an approach which could serve
> > either purpose (some functionality with simple interface vs. fully support
> > what the hardware can do, with interfaces allowing that) adequately in the
> > short or the long run.
> 
> Why not?

Let's suppose that the omap3isp driver provided an "operation mode" for more
simple applications.

Would you continue to have a V4L2 video device per DMA engine? Without Media
controller it'd be rather confusing for applications since depending on
which format (and level of processing) is requested defines the video node
where the images is captured.

Instead you'd probably want to have a single video node. For the driver to
expose just a single device node, should that be a Kconfig option or a
module parameter, for instance?

I have to say I wouldn't be even particularly interested to know how much
driver changes you'd have to implement to achieve that and how
unmaintainable to end result would be. Consider inflicting the same on all
drivers.

That's just *one* of a large number of things you'd need to change in order
to support plain V4L2 applications from the driver, while still continuing to
support the current interface.

With the help of a user space library, we can show the omap3isp device as a
single video node with a number of inputs (sensors) that can provide some
level of service to the user. I'm using "can", because it's just up to a
missing implementation of such a library.

It may be hardware specific or not. A hardware specific one may produce
better results than best effort since it may use knowledge of the hardware
not available through the kernel interfaces.

> 
> > If we are missing pieces in the puzzle --- in this case the missing pieces
> > in the puzzle are a generic pipeline configuration library and another
> > library that, with the help of pipeline autoconfiguration would implement
> > "best effort" service for regular V4L2 on top of the MC + V4L2 subdev + V4L2
> > --- then these pieces need to be impelemented. The solution is
> > *not* to attempt to support different types of applications in each driver
> > separately. That will make writing drivers painful, error prone and is
> > unlikely ever deliver what either purpose requires.
> > 
> > So let's continue to implement the functionality that the hardware supports.
> > Making a different choice here is bound to create a lasting conflict between
> > having to change kernel interface behaviour and the requirement of
> > supporting new functionality that hasn't been previously thought of, pushing
> > away SoC vendors from V4L2 ecosystem. This is what we all do want to avoid.
> 
> This situation is there since 2009. If I remember well, you tried to write
> such generic plugin in the past, but never finished it, apparently because
> it is too complex. Others tried too over the years. 

I'd argue I know better what happened with that attempt than you do. I had a
prototype of a generic pipeline configuration library but due to various
reasons I haven't been able to continue working on that since around 2012.
The prototype could figure out that the ccdc -> resizer path isn't usable
with raw sensors due to the lack of common formats between the two,
something that was argued to be too complex to implement.

I'm not aware of anyone else who would have tried that. Are you?

> 
> The last trial was done by Jacek, trying to cover just the exynos4 driver. 
> Yet, even such limited scope plugin was not good enough, as it was never
> merged upstream. Currently, there's no such plugins upstream.
> 
> If we can't even merge a plugin that solves it for just *one* driver,
> I have no hope that we'll be able to do it for the generic case.

I believe Jacek ceased to work on that plugin in his day job; other than
that, there are some matters left to be addressed in his latest patchset.

Having provided feedback on that patchset, I don't see additional technical
problems that require solving before the patches can be merged. The
remaining matters seem to be actually fairly trivial.

> 
> That's why I'm saying that I'm OK on merging any patch that would allow
> setting controls via the /dev/video interface on MC-based drivers when
> compiled without subdev API. I may also consider merging patches allowing
> to change the behavior on runtime, when compiled with subdev API.
> 
> > As far as i.MX6 driver goes, it is always possible to implement i.MX6 plugin
> > for libv4l to perform this. This should be much easier than getting the
> > automatic pipe configuration library and the rest working, and as it is
> > custom for i.MX6, the resulting plugin may make informed technical choices
> > for better functionality.
> 
> I wouldn't call "much easier" something that experienced media
> developers failed to do over the last 8 years.
> 
> It is just the opposite: broadcasting a control via I2C is very easy:
> there are several examples about how to do that all over the media
> drivers.

That's one of the things Jacek's plugin actually does. This is still
functionality that *only* some user applications wish to have, and as
implemented in the plugin it works correctly without use case specific
semantics --- please see my comments on the patch picking the controls from
the pipeline.

Drivers can also make use of v4l2_device_for_each_subdev() to distribute
setting controls on different sub-devices. A few drivers actually do that.

> 
> > Jacek has been working on such a plugin for
> > Samsung Exynos hardware, but I don't think he has quite finished it yey.
> 
> As Jacek answered when questioned about the merge status:
> 
> 	Hi Hans,
> 
> 	On 11/03/2016 12:51 PM, Hans Verkuil wrote:
> 	> Hi all,
> 	>
> 	> Is there anything that blocks me from merging this?
> 	>
> 	> This plugin work has been ongoing for years and unless there are serious
> 	> objections I propose that this is merged.
> 	>
> 	> Jacek, is there anything missing that would prevent merging this?  
> 
> 	There were issues raised by Sakari during last review, related to
> 	the way how v4l2 control bindings are defined. That discussion wasn't
> 	finished, so I stayed by my approach. Other than that - I've tested it
> 	and it works fine both with GStreamer and my test app.
> 
> After that, he sent a new version (v7.1), but never got reviews.

There are a number of mutually agreed but unaddressed comments against v7.
Also, v7.1 is just a single patch that does not address issues pointed out
in v7, but something Jacek wanted to fix himself.

In other words, there are comments to address but no patches to review.
Let's see how to best address them. I could possibly fix at least some of
those, but due to lack of hardware I have no ability to test the end result.

> 
> > The original plan was and continues to be sound, it's just that there have
> > always been too few hands to implement it. :-(
> 
> If there are no people to implement a plan, it doesn't matter how good
> the plan is, it won't work.

Do you have other proposals than what we have commonly agreed on? I don't
see other approaches that could satisfactorily address all the requirements
going forward.

That said, I do think we need to reinvigorate the efforts to get things
rolling again on supporting plain V4L2 applications on devices that are
controlled through the MC inteface. These matters have received undeservedly
little attention in recent years.

> 
> > > That's not true, for example, for the UVC driver. There, MC
> > > is optional, as it should be.  
> > 
> > UVC is different. The device simply provides additional information through
> > MC to the user but MC (or V4L2 sub-device interface) is not used for
> > controlling the device.
> 
> It is not different. If the Kernel is compiled without the V4L2
> subdev interface, the i.MX6 driver (or whatever other driver)
> won't receive any control via the subdev interface. So, it has to
> handle the control logic control via the only interface that
> supports it, e. g. via the video devnode.

Looking at the driver code and the Kconfig file, i.MX6 driver depends on
CONFIG_MEDIA_CONTROLLER and uses the Media controller API. So if Media
controller is disabled in kernel configuration, the i.MX6 IPU driver won't
be compiled.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]


#1597916

FromMauro Carvalho Chehab <mchehab@s-opensource.com>
Date2017-03-10 16:30 +0100
Message-ID<tjuEp-5UC-7@gated-at.bofh.it>
In reply to#1597489
Hi Russell,

Em Fri, 10 Mar 2017 13:07:33 +0000
Russell King - ARM Linux <linux@armlinux.org.uk> escreveu:

> The idea that the v4l libraries should intercept the format negotiation
> between the application and kernel is a particularly painful one - the
> default gstreamer build detects the v4l libraries, and links against it.
> That much is fine.
> 
> However, the problem comes when you're trying to use bayer formats. The
> v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> negotiation, and decide that they'll invoke v4lconvert to convert the
> bayer to RGB for you, whether you want them to do that or not.
> 
> v4lconvert may not be the most efficient way to convert, or even what
> is desired (eg, you may want to receive the raw bayer image.)  However,
> since the v4l libraries/v4lconvert gives you no option but to have its
> conversion forced into the pipeline, other options (such as using the
> gstreamer neon accelerated de-bayer plugin) isn't an option 

That's not true. There is an special flag, used only by libv4l2
emulated formats, that indicates when a video format is handled
via v4lconvert:

    * - ``V4L2_FMT_FLAG_EMULATED``
      - 0x0002
      - This format is not native to the device but emulated through
	software (usually libv4l2), where possible try to use a native
	format instead for better performance.

Using this flag, if the application supports a video format directly
supported by the hardware, it can use their own video format decoder.
If not, it is still possible to use the V4L2 hardware, by using
v4lconvert.

Unfortunately, very few applications currently check it.

I wrote a patch for zbar (a multi-format barcode reader) in the past,
adding a logic there that gives a high priority to hardware formats,
and a low priority to emulated ones:
	https://lists.fedoraproject.org/pipermail/scm-commits/2010-December/537428.html

> without
> rebuilding gstreamer _without_ linking against the v4l libraries.

I guess it wouldn't be complex to add a logic similar to that at
gstreamer.

AFAIKT, there's another problem that would prevent to make
libv4l the default on gstreamer: right now, libv4l doesn't support
DMABUF. As gstreamer is being used on embedded hardware, I'd say
that DMABUF support should be default there.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]


#1597936

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-10 17:00 +0100
Message-ID<tjv7r-66F-7@gated-at.bofh.it>
In reply to#1597916
On Fri, Mar 10, 2017 at 12:26:34PM -0300, Mauro Carvalho Chehab wrote:
> Hi Russell,
> 
> Em Fri, 10 Mar 2017 13:07:33 +0000
> Russell King - ARM Linux <linux@armlinux.org.uk> escreveu:
> 
> > The idea that the v4l libraries should intercept the format negotiation
> > between the application and kernel is a particularly painful one - the
> > default gstreamer build detects the v4l libraries, and links against it.
> > That much is fine.
> > 
> > However, the problem comes when you're trying to use bayer formats. The
> > v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> > negotiation, and decide that they'll invoke v4lconvert to convert the
> > bayer to RGB for you, whether you want them to do that or not.
> > 
> > v4lconvert may not be the most efficient way to convert, or even what
> > is desired (eg, you may want to receive the raw bayer image.)  However,
> > since the v4l libraries/v4lconvert gives you no option but to have its
> > conversion forced into the pipeline, other options (such as using the
> > gstreamer neon accelerated de-bayer plugin) isn't an option 
> 
> That's not true. There is an special flag, used only by libv4l2
> emulated formats, that indicates when a video format is handled
> via v4lconvert:

I'm afraid that my statement comes from trying to use gstreamer with
libv4l2 and _not_ being able to use the 8-bit bayer formats there at
all - they are simply not passed across to the application through
libv4l2/v4lconvert.

Instead, the formats that are passed across are the emulated formats.
As I said above, that forces applications to use only the v4lconvert
formats, the raw formats are not available.

So, the presence or absence of the V4L2_FMT_FLAG_EMULATED is quite
meaningless if you can't even enumerate the non-converted formats.

The problem comes from the "always needs conversion" stuff in
v4lconvert coupled with the way this subdev stuff works - since it
requires manual configuration of all the pads within the kernel
media pipeline, the kernel ends up only advertising _one_ format
to userspace - in my case, that's RGGB8.

When v4lconvert_create_with_dev_ops() enumerates the formats from
the kernel, it gets only RGGB8.  That causes always_needs_conversion
in there to remain true, so the special v4l control which enables/
disables conversion gets created with a default value of "true".
The RGGB8 bit is also set in data->supported_src_formats.

This causes v4lconvert_supported_dst_fmt_only() to return true.

What this all means is that v4lconvert_enum_fmt() will _not_ return
any of the kernel formats, only the faked formats.

Ergo, the RGGB8 format from the kernel is completely hidden from the
application, and only the emulated format is made available.  As I
said above, this forces v4lconvert's debayering on the application,
whether you want it or not.

In the gstreamer case, it knows nothing about this special control,
which means that trying to use this gstreamer pipeline:

$ gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink

is completely impossible without first rebuilding gstreamer _without_
libv4l support.  Build gstreamer without libv4l support, and the above
works.

Enabling debug output in gstreamer's v4l2src plugin confirms that
the kernel's bayer format are totally hidden from gstreamer when
linked with libv4l2, but are present when it isn't linked with
libv4l2.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1597961

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-10 18:10 +0100
Message-ID<tjwdb-72n-7@gated-at.bofh.it>
In reply to#1597936
On Fri, Mar 10, 2017 at 03:57:09PM +0000, Russell King - ARM Linux wrote:
> Enabling debug output in gstreamer's v4l2src plugin confirms that
> the kernel's bayer format are totally hidden from gstreamer when
> linked with libv4l2, but are present when it isn't linked with
> libv4l2.

Here's the information to back up my claims:

root@hbi2ex:~# v4l2-ctl -d 6 --list-formats-ext
ioctl: VIDIOC_ENUM_FMT
        Index       : 0
        Type        : Video Capture
        Pixel Format: 'RGGB'
        Name        : 8-bit Bayer RGRG/GBGB

root@hbi2ex:~# DISPLAY=:0 GST_DEBUG_NO_COLOR=1 GST_DEBUG=v4l2:9 gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink > gst-v4l2-1.log 2>&1
root@hbi2ex:~# cut -b65- gst-v4l2-1.log|less
v4l2_calls.c:519:gst_v4l2_open:<v4l2src0> Trying to open device /dev/video6
v4l2_calls.c:69:gst_v4l2_get_capabilities:<v4l2src0> getting capabilities
v4l2_calls.c:77:gst_v4l2_get_capabilities:<v4l2src0> driver:      'imx-media-camif'
v4l2_calls.c:78:gst_v4l2_get_capabilities:<v4l2src0> card:        'imx-media-camif'
v4l2_calls.c:79:gst_v4l2_get_capabilities:<v4l2src0> bus_info:    ''
v4l2_calls.c:80:gst_v4l2_get_capabilities:<v4l2src0> version:     00040a00
v4l2_calls.c:81:gst_v4l2_get_capabilities:<v4l2src0> capabilites: 85200001
...
v4l2_calls.c:258:gst_v4l2_fill_lists:<v4l2src0>   controls+menus
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 00000000
v4l2_calls.c:319:gst_v4l2_fill_lists:<v4l2src0> starting control class 'User Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 00980001
v4l2_calls.c:389:gst_v4l2_fill_lists:<v4l2src0> Adding ControlID white_balance_automatic (98090c)
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 0098090c
v4l2_calls.c:389:gst_v4l2_fill_lists:<v4l2src0> Adding ControlID gamma (980910)
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 00980910
v4l2_calls.c:389:gst_v4l2_fill_lists:<v4l2src0> Adding ControlID gain (980913)
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 00980913
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 00980914
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 00980915
v4l2_calls.c:319:gst_v4l2_fill_lists:<v4l2src0> starting control class 'Camera Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009a0001
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID exposure_time_absolute (9a0902) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009a0902
v4l2_calls.c:319:gst_v4l2_fill_lists:<v4l2src0> starting control class 'Image Source Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0001
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID vertical_blanking (9e0901) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0901
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID horizontal_blanking (9e0902) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0902
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID analogue_gain (9e0903) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0903
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID red_pixel_value (9e0904) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0904
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID green_red_pixel_value (9e0905) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0905
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID blue_pixel_value (9e0906) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0906
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID green_blue_pixel_value (9e0907) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009e0907
v4l2_calls.c:319:gst_v4l2_fill_lists:<v4l2src0> starting control class 'Image Processing Controls'
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009f0001
v4l2_calls.c:340:gst_v4l2_fill_lists:<v4l2src0> Control type for 'Pixel Rate' not suppored for extra controls.
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID Pixel Rate (9f0902) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009f0902
v4l2_calls.c:382:gst_v4l2_fill_lists:<v4l2src0> ControlID test_pattern (9f0903) unhandled, FIXME
v4l2_calls.c:278:gst_v4l2_fill_lists:<v4l2src0> checking control 009f0903
v4l2_calls.c:284:gst_v4l2_fill_lists:<v4l2src0> controls finished
v4l2_calls.c:451:gst_v4l2_fill_lists:<v4l2src0> done
v4l2_calls.c:587:gst_v4l2_open:<v4l2src0> Opened device 'imx-media-camif' (/dev/video6) successfully
gstv4l2object.c:804:gst_v4l2_set_defaults:<v4l2src0> tv_norm=0x0, norm=(nil)
v4l2_calls.c:734:gst_v4l2_get_norm:<v4l2src0> getting norm
v4l2_calls.c:1021:gst_v4l2_get_input:<v4l2src0> trying to get input
v4l2_calls.c:1031:gst_v4l2_get_input:<v4l2src0> input: 0

gstv4l2object.c:1106:gst_v4l2_object_fill_format_list:<v4l2src0> getting src format enumerations
gstv4l2object.c:1124:gst_v4l2_object_fill_format_list:<v4l2src0> index:       0
gstv4l2object.c:1125:gst_v4l2_object_fill_format_list:<v4l2src0> type:        1
gstv4l2object.c:1126:gst_v4l2_object_fill_format_list:<v4l2src0> flags:       00000002
gstv4l2object.c:1128:gst_v4l2_object_fill_format_list:<v4l2src0> description: 'RGB3'
gstv4l2object.c:1130:gst_v4l2_object_fill_format_list:<v4l2src0> pixelformat: RGB3
gstv4l2object.c:1124:gst_v4l2_object_fill_format_list:<v4l2src0> index:       1
gstv4l2object.c:1125:gst_v4l2_object_fill_format_list:<v4l2src0> type:        1
gstv4l2object.c:1126:gst_v4l2_object_fill_format_list:<v4l2src0> flags:       00000002
gstv4l2object.c:1128:gst_v4l2_object_fill_format_list:<v4l2src0> description: 'BGR3'
gstv4l2object.c:1130:gst_v4l2_object_fill_format_list:<v4l2src0> pixelformat: BGR3
gstv4l2object.c:1124:gst_v4l2_object_fill_format_list:<v4l2src0> index:       2
gstv4l2object.c:1125:gst_v4l2_object_fill_format_list:<v4l2src0> type:        1
gstv4l2object.c:1126:gst_v4l2_object_fill_format_list:<v4l2src0> flags:       00000002
gstv4l2object.c:1128:gst_v4l2_object_fill_format_list:<v4l2src0> description: 'YU12'
gstv4l2object.c:1130:gst_v4l2_object_fill_format_list:<v4l2src0> pixelformat: YU12
gstv4l2object.c:1124:gst_v4l2_object_fill_format_list:<v4l2src0> index:       3
gstv4l2object.c:1125:gst_v4l2_object_fill_format_list:<v4l2src0> type:        1
gstv4l2object.c:1126:gst_v4l2_object_fill_format_list:<v4l2src0> flags:       00000002
gstv4l2object.c:1128:gst_v4l2_object_fill_format_list:<v4l2src0> description: 'YV12'

gstv4l2object.c:1130:gst_v4l2_object_fill_format_list:<v4l2src0> pixelformat: YV12
gstv4l2object.c:1143:gst_v4l2_object_fill_format_list:<v4l2src0> got 4 format(s):
gstv4l2object.c:1149:gst_v4l2_object_fill_format_list:<v4l2src0>   YU12 (emulated)
gstv4l2object.c:1149:gst_v4l2_object_fill_format_list:<v4l2src0>   YV12 (emulated)
gstv4l2object.c:1149:gst_v4l2_object_fill_format_list:<v4l2src0>   BGR3 (emulated)
gstv4l2object.c:1149:gst_v4l2_object_fill_format_list:<v4l2src0>   RGB3 (emulated)

As you can see from this, the RGGB bayer format advertised from the
kernel is not listed - only the four emulated formats provided by
v4lconvert are listed, so the application has _no_ choice but to use
v4lconvert's RGGB conversion.

The result is that the above pipeline fails:

0:00:00.345739030  2794   0x3ade60 DEBUG                   v4l2 gstv4l2object.c:3812:gst_v4l2_object_get_caps:<v4l2src0> ret: video/x-raw, format=(string)I420, framerate=(fraction)[ 0/1, 2147483647/1 ], width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, framerate=(fraction)[ 0/1, 2147483647/1 ], width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)BGR, framerate=(fraction)[ 0/1, 2147483647/1 ], width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, framerate=(fraction)[ 0/1, 2147483647/1 ], width=(int)816, height=(int)616, interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1
ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Internal data flow error.
Additional debug info:
gstbasesrc.c(2948): gst_base_src_loop (): /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
streaming task paused, reason not-negotiated (-4)

as the v4l2src element is offering an I420-formatted buffer to the
gstreamer bayer converter, which obviously objects.

Rebuilding without libv4l2 linked results in gstreamer working.
Using a kernel driver which exposes some formats that libv4lconvert
_doesn't_ need to convert in addition to bayer _also_ works.

The only case where this fails is where the kernel device only
advertises formats where libv4lconvert is in "we must always convert"
mode, where upon the unconverted formats are completely hidden from
the application.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1598082

FromMauro Carvalho Chehab <mchehab@s-opensource.com>
Date2017-03-10 21:50 +0100
Message-ID<tjzE6-Jy-13@gated-at.bofh.it>
In reply to#1597936
Em Fri, 10 Mar 2017 15:57:09 +0000
Russell King - ARM Linux <linux@armlinux.org.uk> escreveu:

> On Fri, Mar 10, 2017 at 12:26:34PM -0300, Mauro Carvalho Chehab wrote:
> > Hi Russell,
> > 
> > Em Fri, 10 Mar 2017 13:07:33 +0000
> > Russell King - ARM Linux <linux@armlinux.org.uk> escreveu:
> >   
> > > The idea that the v4l libraries should intercept the format negotiation
> > > between the application and kernel is a particularly painful one - the
> > > default gstreamer build detects the v4l libraries, and links against it.
> > > That much is fine.
> > > 
> > > However, the problem comes when you're trying to use bayer formats. The
> > > v4l libraries "helpfully" (or rather unhelpfully) intercept the format
> > > negotiation, and decide that they'll invoke v4lconvert to convert the
> > > bayer to RGB for you, whether you want them to do that or not.
> > > 
> > > v4lconvert may not be the most efficient way to convert, or even what
> > > is desired (eg, you may want to receive the raw bayer image.)  However,
> > > since the v4l libraries/v4lconvert gives you no option but to have its
> > > conversion forced into the pipeline, other options (such as using the
> > > gstreamer neon accelerated de-bayer plugin) isn't an option   
> > 
> > That's not true. There is an special flag, used only by libv4l2
> > emulated formats, that indicates when a video format is handled
> > via v4lconvert:  
> 
> I'm afraid that my statement comes from trying to use gstreamer with
> libv4l2 and _not_ being able to use the 8-bit bayer formats there at
> all - they are simply not passed across to the application through
> libv4l2/v4lconvert.
> 
> Instead, the formats that are passed across are the emulated formats.
> As I said above, that forces applications to use only the v4lconvert
> formats, the raw formats are not available.
> 
> So, the presence or absence of the V4L2_FMT_FLAG_EMULATED is quite
> meaningless if you can't even enumerate the non-converted formats.
> 
> The problem comes from the "always needs conversion" stuff in
> v4lconvert coupled with the way this subdev stuff works - since it
> requires manual configuration of all the pads within the kernel
> media pipeline, the kernel ends up only advertising _one_ format
> to userspace - in my case, that's RGGB8.
> 
> When v4lconvert_create_with_dev_ops() enumerates the formats from
> the kernel, it gets only RGGB8.  That causes always_needs_conversion
> in there to remain true, so the special v4l control which enables/
> disables conversion gets created with a default value of "true".
> The RGGB8 bit is also set in data->supported_src_formats.
> 
> This causes v4lconvert_supported_dst_fmt_only() to return true.
> 
> What this all means is that v4lconvert_enum_fmt() will _not_ return
> any of the kernel formats, only the faked formats.
> 
> Ergo, the RGGB8 format from the kernel is completely hidden from the
> application, and only the emulated format is made available.  As I
> said above, this forces v4lconvert's debayering on the application,
> whether you want it or not.
> 
> In the gstreamer case, it knows nothing about this special control,
> which means that trying to use this gstreamer pipeline:
> 
> $ gst-launch-1.0 v4l2src device=/dev/video6 ! bayer2rgbneon ! xvimagesink
> 
> is completely impossible without first rebuilding gstreamer _without_
> libv4l support.  Build gstreamer without libv4l support, and the above
> works.
> 
> Enabling debug output in gstreamer's v4l2src plugin confirms that
> the kernel's bayer format are totally hidden from gstreamer when
> linked with libv4l2, but are present when it isn't linked with
> libv4l2.

Argh! that is indeed a bug at libv4l (and maybe at gstreamer).

I guess that the always_needs_conversion logic was meant to be used to
really odd proprietary formats, e. g:
	
/*  Vendor-specific formats   */
#define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
#define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
#define V4L2_PIX_FMT_SN9C10X  v4l2_fourcc('S', '9', '1', '0') /* SN9C10x compression */
...

I suspect that nobody uses libv4l2 with MC-based V4L2 devices. That's
likely why nobody reported this bug before (that I know of).

In any case, for non-proprietary formats, the default should be to
always offer both the emulated format and the original one.

I suspect that the enclosed patch should fix the issue with bayer formats.

> 

Thanks,
Mauro

[PATCH RFC] libv4lconvert: by default, offer the original format to the client

Applications should have the right to decide between using a
libv4lconvert emulated format or to implement the decoding themselves,
as this may have significative performance impact.

So, change the default to always show both formats.

Change also the default for Bayer encoded formats, as userspace
likely will want to handle it directly.

Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>


diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
index da718918b030..2e5458fa420d 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -118,10 +118,10 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
 	{ V4L2_PIX_FMT_OV511,		 0,	 7,	 7,	1 },
 	{ V4L2_PIX_FMT_OV518,		 0,	 7,	 7,	1 },
 	/* uncompressed bayer */
-	{ V4L2_PIX_FMT_SBGGR8,		 8,	 8,	 8,	1 },
-	{ V4L2_PIX_FMT_SGBRG8,		 8,	 8,	 8,	1 },
-	{ V4L2_PIX_FMT_SGRBG8,		 8,	 8,	 8,	1 },
-	{ V4L2_PIX_FMT_SRGGB8,		 8,	 8,	 8,	1 },
+	{ V4L2_PIX_FMT_SBGGR8,		 8,	 8,	 8,	0 },
+	{ V4L2_PIX_FMT_SGBRG8,		 8,	 8,	 8,	0 },
+	{ V4L2_PIX_FMT_SGRBG8,		 8,	 8,	 8,	0 },
+	{ V4L2_PIX_FMT_SRGGB8,		 8,	 8,	 8,	0 },
 	{ V4L2_PIX_FMT_STV0680,		 8,	 8,	 8,	1 },
 	/* compressed bayer */
 	{ V4L2_PIX_FMT_SPCA561,		 0,	 9,	 9,	1 },
@@ -178,7 +178,7 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
 	/* This keeps tracks of devices which have only formats for which apps
 	   most likely will need conversion and we can thus safely add software
 	   processing controls without a performance impact. */
-	int always_needs_conversion = 1;
+	int always_needs_conversion = 0;
 
 	if (!data) {
 		fprintf(stderr, "libv4lconvert: error: out of memory!\n");
@@ -208,8 +208,8 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
 		if (j < ARRAY_SIZE(supported_src_pixfmts)) {
 			data->supported_src_formats |= 1ULL << j;
 			v4lconvert_get_framesizes(data, fmt.pixelformat, j);
-			if (!supported_src_pixfmts[j].needs_conversion)
-				always_needs_conversion = 0;
+			if (supported_src_pixfmts[j].needs_conversion)
+				always_needs_conversion = 1;
 		} else
 			always_needs_conversion = 0;
 	}

[toc] | [prev] | [next] | [standalone]


#1598125

FromPavel Machek <pavel@ucw.cz>
Date2017-03-10 23:00 +0100
Message-ID<tjAJQ-1r3-17@gated-at.bofh.it>
In reply to#1598082

[Multipart message — attachments visible in raw view] — view raw

Hi!

> Argh! that is indeed a bug at libv4l (and maybe at gstreamer).
> 
> I guess that the always_needs_conversion logic was meant to be used to
> really odd proprietary formats, e. g:
> 	
> /*  Vendor-specific formats   */
> #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> #define V4L2_PIX_FMT_WNVA     v4l2_fourcc('W', 'N', 'V', 'A') /* Winnov hw compress */
> #define V4L2_PIX_FMT_SN9C10X  v4l2_fourcc('S', '9', '1', '0') /* SN9C10x compression */
> ...
> 
> I suspect that nobody uses libv4l2 with MC-based V4L2 devices. That's
> likely why nobody reported this bug before (that I know of).
> 
> In any case, for non-proprietary formats, the default should be to
> always offer both the emulated format and the original one.
> 
> I suspect that the enclosed patch should fix the issue with bayer formats.

...
> @@ -178,7 +178,7 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>  	/* This keeps tracks of devices which have only formats for which apps
>  	   most likely will need conversion and we can thus safely add software
>  	   processing controls without a performance impact. */
> -	int always_needs_conversion = 1;
> +	int always_needs_conversion = 0;
>  
>  	if (!data) {
>  		fprintf(stderr, "libv4lconvert: error: out of memory!\n");
> @@ -208,8 +208,8 @@ struct v4lconvert_data *v4lconvert_create_with_dev_ops(int fd, void *dev_ops_pri
>  		if (j < ARRAY_SIZE(supported_src_pixfmts)) {
>  			data->supported_src_formats |= 1ULL << j;
>  			v4lconvert_get_framesizes(data, fmt.pixelformat, j);
> -			if (!supported_src_pixfmts[j].needs_conversion)
> -				always_needs_conversion = 0;
> +			if (supported_src_pixfmts[j].needs_conversion)
> +				always_needs_conversion = 1;
>  		} else
>  			always_needs_conversion = 0;
>  	}

Is the else still needed? You changed default to 0...

										Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]


#1597906

FromMauro Carvalho Chehab <mchehab@s-opensource.com>
Date2017-03-10 16:10 +0100
Message-ID<tjul3-5NC-3@gated-at.bofh.it>
In reply to#1597464
Em Fri, 10 Mar 2017 13:54:28 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> > Devices that have complex pipeline that do essentially require using the
> > Media controller interface to configure them are out of that scope.
> >   
> 
> Way too much of how the MC devices should be used is in the minds of developers.
> There is a major lack for good detailed documentation, utilities, compliance
> test (really needed!) and libv4l plugins.

Unfortunately, we merged an incomplete MC support at the Kernel. We knew
all the problems with MC-based drivers and V4L2 applications by the time
it was developed, and we requested Nokia developers (with was sponsoring MC
develoment, on that time) to work on a solution to allow standard V4L2
applications to work with MC based boards.

Unfortunately, we took the decision to merge MC without that, because 
Nokia was giving up on Linux development, and we didn't want to lose the
2 years of discussions and work around it, as Nokia employers were leaving
the company. Also, on that time, there was already some patches floating
around adding backward support via libv4l. Unfortunately, those patches
were never finished.

The net result is that MC was merged with some huge gaps, including
the lack of a proper solution for a generic V4L2 program to work
with V4L2 devices that use the subdev API.

That was not that bad by then, as MC was used only on cell phones
that run custom-made applications. 

The reallity changed, as now, we have lots of low cost SoC based
boards, used for all sort of purposes. So, we need a quick solution
for it.

In other words, while that would be acceptable support special apps
on really embedded systems, it is *not OK* for general purpose SoC
harware[1].

[1] I'm calling "general purpose SoC harware" those ARM boards
like Raspberry Pi that are shipped to the mass and used by a wide
range of hobbyists and other people that just wants to run Linux on
ARM. It is possible to buy such boards for a very cheap price,
making them to be used not only on special projects, where a custom
made application could be interesting, but also for a lot of
users that just want to run Linux on a low cost ARM board, while
keeping using standard V4L2 apps, like "camorama".

That's perhaps one of the reasons why it took a long time for us to
start receiving drivers upstream for such hardware: it is quite 
intimidating and not logical to require developers to implement
on their drivers 2 complex APIs (MC, subdev) for those
hardware that most users won't care. From user's perspective,
being able to support generic applications like "camorama" and
"zbar" is all they want.

In summary, I'm pretty sure we need to support standard V4L2 
applications on boards like Raspberry Pi and those low-cost 
SoC-based boards that are shipped to end users.

> Anyway, regarding this specific patch and for this MC-aware driver: no, you
> shouldn't inherit controls from subdevs. It defeats the purpose.

Sorry, but I don't agree with that. The subdev API is an optional API
(and even the MC API can be optional).

I see the rationale for using MC and subdev APIs on cell phones,
ISV and other embedded hardware, as it will allow fine-tuning
the driver's support to allow providing the required quality for
certain custom-made applications. but on general SoC hardware,
supporting standard V4L2 applications is a need.

Ok, perhaps supporting both subdev API and V4L2 API at the same
time doesn't make much sense. We could disable one in favor of the
other, either at compilation time or at runtime.

This way, if the subdev API is disabled, the driver will be
functional for V4L2-based applications that don't support neither
MC or subdev APIs.

> As mentioned, I will attempt to try and get some time to work on this
> later this year. Fingers crossed.

That will be good, and, once we have a solution that works, we can
work on cleanup the code, but, until then, drivers for arm-based boards
sold to end consumers should work out of the box with standard V4L2 apps.

While we don't have that, I'm OK to merge patches adding such support
upstream.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]


#1598305

FromHans Verkuil <hverkuil@xs4all.nl>
Date2017-03-11 12:40 +0100
Message-ID<tjNxn-25P-5@gated-at.bofh.it>
In reply to#1597906
On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
> Em Fri, 10 Mar 2017 13:54:28 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>>> Devices that have complex pipeline that do essentially require using the
>>> Media controller interface to configure them are out of that scope.
>>>   
>>
>> Way too much of how the MC devices should be used is in the minds of developers.
>> There is a major lack for good detailed documentation, utilities, compliance
>> test (really needed!) and libv4l plugins.
> 
> Unfortunately, we merged an incomplete MC support at the Kernel. We knew
> all the problems with MC-based drivers and V4L2 applications by the time
> it was developed, and we requested Nokia developers (with was sponsoring MC
> develoment, on that time) to work on a solution to allow standard V4L2
> applications to work with MC based boards.
> 
> Unfortunately, we took the decision to merge MC without that, because 
> Nokia was giving up on Linux development, and we didn't want to lose the
> 2 years of discussions and work around it, as Nokia employers were leaving
> the company. Also, on that time, there was already some patches floating
> around adding backward support via libv4l. Unfortunately, those patches
> were never finished.
> 
> The net result is that MC was merged with some huge gaps, including
> the lack of a proper solution for a generic V4L2 program to work
> with V4L2 devices that use the subdev API.
> 
> That was not that bad by then, as MC was used only on cell phones
> that run custom-made applications. 
> 
> The reallity changed, as now, we have lots of low cost SoC based
> boards, used for all sort of purposes. So, we need a quick solution
> for it.
> 
> In other words, while that would be acceptable support special apps
> on really embedded systems, it is *not OK* for general purpose SoC
> harware[1].
> 
> [1] I'm calling "general purpose SoC harware" those ARM boards
> like Raspberry Pi that are shipped to the mass and used by a wide
> range of hobbyists and other people that just wants to run Linux on
> ARM. It is possible to buy such boards for a very cheap price,
> making them to be used not only on special projects, where a custom
> made application could be interesting, but also for a lot of
> users that just want to run Linux on a low cost ARM board, while
> keeping using standard V4L2 apps, like "camorama".
> 
> That's perhaps one of the reasons why it took a long time for us to
> start receiving drivers upstream for such hardware: it is quite 
> intimidating and not logical to require developers to implement
> on their drivers 2 complex APIs (MC, subdev) for those
> hardware that most users won't care. From user's perspective,
> being able to support generic applications like "camorama" and
> "zbar" is all they want.
> 
> In summary, I'm pretty sure we need to support standard V4L2 
> applications on boards like Raspberry Pi and those low-cost 
> SoC-based boards that are shipped to end users.
> 
>> Anyway, regarding this specific patch and for this MC-aware driver: no, you
>> shouldn't inherit controls from subdevs. It defeats the purpose.
> 
> Sorry, but I don't agree with that. The subdev API is an optional API
> (and even the MC API can be optional).
> 
> I see the rationale for using MC and subdev APIs on cell phones,
> ISV and other embedded hardware, as it will allow fine-tuning
> the driver's support to allow providing the required quality for
> certain custom-made applications. but on general SoC hardware,
> supporting standard V4L2 applications is a need.
> 
> Ok, perhaps supporting both subdev API and V4L2 API at the same
> time doesn't make much sense. We could disable one in favor of the
> other, either at compilation time or at runtime.

Right. If the subdev API is disabled, then you have to inherit the subdev
controls in the bridge driver (how else would you be able to access them?).
And that's the usual case.

If you do have the subdev API enabled, AND you use the MC, then the
intention clearly is to give userspace full control and inheriting controls
no longer makes any sense (and is highly confusing IMHO).

> 
> This way, if the subdev API is disabled, the driver will be
> functional for V4L2-based applications that don't support neither
> MC or subdev APIs.

I'm not sure if it makes sense for the i.MX driver to behave differently
depending on whether the subdev API is enabled or disabled. I don't know
enough of the hardware to tell if it would ever make sense to disable the
subdev API.

Regards,

	Hans

> 
>> As mentioned, I will attempt to try and get some time to work on this
>> later this year. Fingers crossed.
> 
> That will be good, and, once we have a solution that works, we can
> work on cleanup the code, but, until then, drivers for arm-based boards
> sold to end consumers should work out of the box with standard V4L2 apps.
> 
> While we don't have that, I'm OK to merge patches adding such support
> upstream.
> 
> Thanks,
> Mauro
> 

[toc] | [prev] | [next] | [standalone]


#1598317

FromMauro Carvalho Chehab <mchehab@s-opensource.com>
Date2017-03-11 14:20 +0100
Message-ID<tjP69-3gI-17@gated-at.bofh.it>
In reply to#1598305
Em Sat, 11 Mar 2017 12:32:43 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
> > Em Fri, 10 Mar 2017 13:54:28 +0100
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >   
> >>> Devices that have complex pipeline that do essentially require using the
> >>> Media controller interface to configure them are out of that scope.
> >>>     
> >>
> >> Way too much of how the MC devices should be used is in the minds of developers.
> >> There is a major lack for good detailed documentation, utilities, compliance
> >> test (really needed!) and libv4l plugins.  
> > 
> > Unfortunately, we merged an incomplete MC support at the Kernel. We knew
> > all the problems with MC-based drivers and V4L2 applications by the time
> > it was developed, and we requested Nokia developers (with was sponsoring MC
> > develoment, on that time) to work on a solution to allow standard V4L2
> > applications to work with MC based boards.
> > 
> > Unfortunately, we took the decision to merge MC without that, because 
> > Nokia was giving up on Linux development, and we didn't want to lose the
> > 2 years of discussions and work around it, as Nokia employers were leaving
> > the company. Also, on that time, there was already some patches floating
> > around adding backward support via libv4l. Unfortunately, those patches
> > were never finished.
> > 
> > The net result is that MC was merged with some huge gaps, including
> > the lack of a proper solution for a generic V4L2 program to work
> > with V4L2 devices that use the subdev API.
> > 
> > That was not that bad by then, as MC was used only on cell phones
> > that run custom-made applications. 
> > 
> > The reallity changed, as now, we have lots of low cost SoC based
> > boards, used for all sort of purposes. So, we need a quick solution
> > for it.
> > 
> > In other words, while that would be acceptable support special apps
> > on really embedded systems, it is *not OK* for general purpose SoC
> > harware[1].
> > 
> > [1] I'm calling "general purpose SoC harware" those ARM boards
> > like Raspberry Pi that are shipped to the mass and used by a wide
> > range of hobbyists and other people that just wants to run Linux on
> > ARM. It is possible to buy such boards for a very cheap price,
> > making them to be used not only on special projects, where a custom
> > made application could be interesting, but also for a lot of
> > users that just want to run Linux on a low cost ARM board, while
> > keeping using standard V4L2 apps, like "camorama".
> > 
> > That's perhaps one of the reasons why it took a long time for us to
> > start receiving drivers upstream for such hardware: it is quite 
> > intimidating and not logical to require developers to implement
> > on their drivers 2 complex APIs (MC, subdev) for those
> > hardware that most users won't care. From user's perspective,
> > being able to support generic applications like "camorama" and
> > "zbar" is all they want.
> > 
> > In summary, I'm pretty sure we need to support standard V4L2 
> > applications on boards like Raspberry Pi and those low-cost 
> > SoC-based boards that are shipped to end users.
> >   
> >> Anyway, regarding this specific patch and for this MC-aware driver: no, you
> >> shouldn't inherit controls from subdevs. It defeats the purpose.  
> > 
> > Sorry, but I don't agree with that. The subdev API is an optional API
> > (and even the MC API can be optional).
> > 
> > I see the rationale for using MC and subdev APIs on cell phones,
> > ISV and other embedded hardware, as it will allow fine-tuning
> > the driver's support to allow providing the required quality for
> > certain custom-made applications. but on general SoC hardware,
> > supporting standard V4L2 applications is a need.
> > 
> > Ok, perhaps supporting both subdev API and V4L2 API at the same
> > time doesn't make much sense. We could disable one in favor of the
> > other, either at compilation time or at runtime.  
> 
> Right. If the subdev API is disabled, then you have to inherit the subdev
> controls in the bridge driver (how else would you be able to access them?).
> And that's the usual case.
> 
> If you do have the subdev API enabled, AND you use the MC, then the
> intention clearly is to give userspace full control and inheriting controls
> no longer makes any sense (and is highly confusing IMHO).

I tend to agree with that.

> > 
> > This way, if the subdev API is disabled, the driver will be
> > functional for V4L2-based applications that don't support neither
> > MC or subdev APIs.  
> 
> I'm not sure if it makes sense for the i.MX driver to behave differently
> depending on whether the subdev API is enabled or disabled. I don't know
> enough of the hardware to tell if it would ever make sense to disable the
> subdev API.

Yeah, I don't know enough about it either. The point is: this is
something that the driver maintainer and driver users should
decide if it either makes sense or not, based on the expected use cases.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]


#1598358

FromSakari Ailus <sakari.ailus@iki.fi>
Date2017-03-11 16:40 +0100
Message-ID<tjRhD-4Hk-5@gated-at.bofh.it>
In reply to#1598317
Hi Mauro and Hans,

On Sat, Mar 11, 2017 at 10:14:08AM -0300, Mauro Carvalho Chehab wrote:
> Em Sat, 11 Mar 2017 12:32:43 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
> > On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
> > > Em Fri, 10 Mar 2017 13:54:28 +0100
> > > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> > >   
> > >>> Devices that have complex pipeline that do essentially require using the
> > >>> Media controller interface to configure them are out of that scope.
> > >>>     
> > >>
> > >> Way too much of how the MC devices should be used is in the minds of developers.
> > >> There is a major lack for good detailed documentation, utilities, compliance
> > >> test (really needed!) and libv4l plugins.  
> > > 
> > > Unfortunately, we merged an incomplete MC support at the Kernel. We knew
> > > all the problems with MC-based drivers and V4L2 applications by the time
> > > it was developed, and we requested Nokia developers (with was sponsoring MC
> > > develoment, on that time) to work on a solution to allow standard V4L2
> > > applications to work with MC based boards.
> > > 
> > > Unfortunately, we took the decision to merge MC without that, because 
> > > Nokia was giving up on Linux development, and we didn't want to lose the
> > > 2 years of discussions and work around it, as Nokia employers were leaving
> > > the company. Also, on that time, there was already some patches floating
> > > around adding backward support via libv4l. Unfortunately, those patches
> > > were never finished.
> > > 
> > > The net result is that MC was merged with some huge gaps, including
> > > the lack of a proper solution for a generic V4L2 program to work
> > > with V4L2 devices that use the subdev API.
> > > 
> > > That was not that bad by then, as MC was used only on cell phones
> > > that run custom-made applications. 
> > > 
> > > The reallity changed, as now, we have lots of low cost SoC based
> > > boards, used for all sort of purposes. So, we need a quick solution
> > > for it.
> > > 
> > > In other words, while that would be acceptable support special apps
> > > on really embedded systems, it is *not OK* for general purpose SoC
> > > harware[1].
> > > 
> > > [1] I'm calling "general purpose SoC harware" those ARM boards
> > > like Raspberry Pi that are shipped to the mass and used by a wide
> > > range of hobbyists and other people that just wants to run Linux on
> > > ARM. It is possible to buy such boards for a very cheap price,
> > > making them to be used not only on special projects, where a custom
> > > made application could be interesting, but also for a lot of
> > > users that just want to run Linux on a low cost ARM board, while
> > > keeping using standard V4L2 apps, like "camorama".
> > > 
> > > That's perhaps one of the reasons why it took a long time for us to
> > > start receiving drivers upstream for such hardware: it is quite 
> > > intimidating and not logical to require developers to implement
> > > on their drivers 2 complex APIs (MC, subdev) for those
> > > hardware that most users won't care. From user's perspective,
> > > being able to support generic applications like "camorama" and
> > > "zbar" is all they want.
> > > 
> > > In summary, I'm pretty sure we need to support standard V4L2 
> > > applications on boards like Raspberry Pi and those low-cost 
> > > SoC-based boards that are shipped to end users.
> > >   
> > >> Anyway, regarding this specific patch and for this MC-aware driver: no, you
> > >> shouldn't inherit controls from subdevs. It defeats the purpose.  
> > > 
> > > Sorry, but I don't agree with that. The subdev API is an optional API
> > > (and even the MC API can be optional).
> > > 
> > > I see the rationale for using MC and subdev APIs on cell phones,
> > > ISV and other embedded hardware, as it will allow fine-tuning
> > > the driver's support to allow providing the required quality for
> > > certain custom-made applications. but on general SoC hardware,
> > > supporting standard V4L2 applications is a need.
> > > 
> > > Ok, perhaps supporting both subdev API and V4L2 API at the same
> > > time doesn't make much sense. We could disable one in favor of the
> > > other, either at compilation time or at runtime.  
> > 
> > Right. If the subdev API is disabled, then you have to inherit the subdev
> > controls in the bridge driver (how else would you be able to access them?).
> > And that's the usual case.
> > 
> > If you do have the subdev API enabled, AND you use the MC, then the
> > intention clearly is to give userspace full control and inheriting controls
> > no longer makes any sense (and is highly confusing IMHO).
> 
> I tend to agree with that.

I agree as well.

This is in line with how existing drivers behave, too.

> 
> > > 
> > > This way, if the subdev API is disabled, the driver will be
> > > functional for V4L2-based applications that don't support neither
> > > MC or subdev APIs.  
> > 
> > I'm not sure if it makes sense for the i.MX driver to behave differently
> > depending on whether the subdev API is enabled or disabled. I don't know
> > enough of the hardware to tell if it would ever make sense to disable the
> > subdev API.
> 
> Yeah, I don't know enough about it either. The point is: this is
> something that the driver maintainer and driver users should
> decide if it either makes sense or not, based on the expected use cases.

My understanding of the i.MX6 case is the hardware is configurable enough
to warrant the use of the Media controller API. Some patches indicate
there are choices to be made in data routing.

Steve: could you enlighten us on the topic, by e.g. doing media-ctl
--print-dot and sending the results to the list? What kind of different IP
blocks are there and what do they do? A pointer to hardware documentation
wouldn't hurt either (if it's available).

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]


#1598384

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-11 18:40 +0100
Message-ID<tjT9M-61I-3@gated-at.bofh.it>
In reply to#1598358

[Multipart message — attachments visible in raw view] — view raw

On Sat, Mar 11, 2017 at 05:32:29PM +0200, Sakari Ailus wrote:
> My understanding of the i.MX6 case is the hardware is configurable enough
> to warrant the use of the Media controller API. Some patches indicate
> there are choices to be made in data routing.

The iMX6 does have configurable data routing, but in some scenarios
(eg, when receiving bayer data) there's only one possible routing.

> Steve: could you enlighten us on the topic, by e.g. doing media-ctl
> --print-dot and sending the results to the list? What kind of different IP
> blocks are there and what do they do? A pointer to hardware documentation
> wouldn't hurt either (if it's available).

Attached for the imx219 camera.  Note that although the CSI2 block has
four outputs, each output is dedicated to a CSI virtual channel, so
they can not be arbitarily assigned without configuring the sensor.

Since the imx219 only produces bayer, the graph is also showing the
_only_ possible routing for the imx219 configured for CSI virtual
channel 0.

The iMX6 manuals are available on the 'net.

	https://community.nxp.com/docs/DOC-101840

There are several chapters that cover the capture side:

* MIPI CSI2
* IPU CSI2 gasket
* IPU

The IPU not only performs capture, but also display as well.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1598395

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-03-11 19:10 +0100
Message-ID<tjTCO-6sG-5@gated-at.bofh.it>
In reply to#1598358

[Multipart message — attachments visible in raw view] — view raw

On 03/11/2017 07:32 AM, Sakari Ailus wrote:
> Hi Mauro and Hans,
>
> On Sat, Mar 11, 2017 at 10:14:08AM -0300, Mauro Carvalho Chehab wrote:
>> Em Sat, 11 Mar 2017 12:32:43 +0100
>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>
>>> On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
>>>> Em Fri, 10 Mar 2017 13:54:28 +0100
>>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>>
>>>>>> Devices that have complex pipeline that do essentially require using the
>>>>>> Media controller interface to configure them are out of that scope.
>>>>>>
>>>>>
>>>>> Way too much of how the MC devices should be used is in the minds of developers.
>>>>> There is a major lack for good detailed documentation, utilities, compliance
>>>>> test (really needed!) and libv4l plugins.
>>>>
>>>> Unfortunately, we merged an incomplete MC support at the Kernel. We knew
>>>> all the problems with MC-based drivers and V4L2 applications by the time
>>>> it was developed, and we requested Nokia developers (with was sponsoring MC
>>>> develoment, on that time) to work on a solution to allow standard V4L2
>>>> applications to work with MC based boards.
>>>>
>>>> Unfortunately, we took the decision to merge MC without that, because
>>>> Nokia was giving up on Linux development, and we didn't want to lose the
>>>> 2 years of discussions and work around it, as Nokia employers were leaving
>>>> the company. Also, on that time, there was already some patches floating
>>>> around adding backward support via libv4l. Unfortunately, those patches
>>>> were never finished.
>>>>
>>>> The net result is that MC was merged with some huge gaps, including
>>>> the lack of a proper solution for a generic V4L2 program to work
>>>> with V4L2 devices that use the subdev API.
>>>>
>>>> That was not that bad by then, as MC was used only on cell phones
>>>> that run custom-made applications.
>>>>
>>>> The reallity changed, as now, we have lots of low cost SoC based
>>>> boards, used for all sort of purposes. So, we need a quick solution
>>>> for it.
>>>>
>>>> In other words, while that would be acceptable support special apps
>>>> on really embedded systems, it is *not OK* for general purpose SoC
>>>> harware[1].
>>>>
>>>> [1] I'm calling "general purpose SoC harware" those ARM boards
>>>> like Raspberry Pi that are shipped to the mass and used by a wide
>>>> range of hobbyists and other people that just wants to run Linux on
>>>> ARM. It is possible to buy such boards for a very cheap price,
>>>> making them to be used not only on special projects, where a custom
>>>> made application could be interesting, but also for a lot of
>>>> users that just want to run Linux on a low cost ARM board, while
>>>> keeping using standard V4L2 apps, like "camorama".
>>>>
>>>> That's perhaps one of the reasons why it took a long time for us to
>>>> start receiving drivers upstream for such hardware: it is quite
>>>> intimidating and not logical to require developers to implement
>>>> on their drivers 2 complex APIs (MC, subdev) for those
>>>> hardware that most users won't care. From user's perspective,
>>>> being able to support generic applications like "camorama" and
>>>> "zbar" is all they want.
>>>>
>>>> In summary, I'm pretty sure we need to support standard V4L2
>>>> applications on boards like Raspberry Pi and those low-cost
>>>> SoC-based boards that are shipped to end users.
>>>>
>>>>> Anyway, regarding this specific patch and for this MC-aware driver: no, you
>>>>> shouldn't inherit controls from subdevs. It defeats the purpose.
>>>>
>>>> Sorry, but I don't agree with that. The subdev API is an optional API
>>>> (and even the MC API can be optional).
>>>>
>>>> I see the rationale for using MC and subdev APIs on cell phones,
>>>> ISV and other embedded hardware, as it will allow fine-tuning
>>>> the driver's support to allow providing the required quality for
>>>> certain custom-made applications. but on general SoC hardware,
>>>> supporting standard V4L2 applications is a need.
>>>>
>>>> Ok, perhaps supporting both subdev API and V4L2 API at the same
>>>> time doesn't make much sense. We could disable one in favor of the
>>>> other, either at compilation time or at runtime.
>>>
>>> Right. If the subdev API is disabled, then you have to inherit the subdev
>>> controls in the bridge driver (how else would you be able to access them?).
>>> And that's the usual case.
>>>
>>> If you do have the subdev API enabled, AND you use the MC, then the
>>> intention clearly is to give userspace full control and inheriting controls
>>> no longer makes any sense (and is highly confusing IMHO).
>>
>> I tend to agree with that.
>
> I agree as well.
>
> This is in line with how existing drivers behave, too.


Well, sounds like there is consensus on this topic. I guess I'll
go ahead and remove the control inheritance support. I suppose
having a control appear in two places (subdev and video nodes) can
be confusing.

As for the configurability vs. ease-of-use debate, I added the
control inheritance to make it a little easier on the user, but,
as the dot graphs below will show, the user already needs quite
a lot of knowledge of the architecture already, in order to setup
the different pipelines. So perhaps the control inheritance is
rather pointless anyway.



>
>>
>>>>
>>>> This way, if the subdev API is disabled, the driver will be
>>>> functional for V4L2-based applications that don't support neither
>>>> MC or subdev APIs.
>>>
>>> I'm not sure if it makes sense for the i.MX driver to behave differently
>>> depending on whether the subdev API is enabled or disabled. I don't know
>>> enough of the hardware to tell if it would ever make sense to disable the
>>> subdev API.
>>
>> Yeah, I don't know enough about it either. The point is: this is
>> something that the driver maintainer and driver users should
>> decide if it either makes sense or not, based on the expected use cases.
>
> My understanding of the i.MX6 case is the hardware is configurable enough
> to warrant the use of the Media controller API. Some patches indicate
> there are choices to be made in data routing.
>
> Steve: could you enlighten us on the topic, by e.g. doing media-ctl
> --print-dot and sending the results to the list? What kind of different IP
> blocks are there and what do they do? A pointer to hardware documentation
> wouldn't hurt either (if it's available).

Wow, I didn't realize there was so little knowledge of the imx6
IPU capture architecture.

Yes, the imx6 definitely warrants the need for MC, as the dot graphs
will attest.

The graphs follows very closely the actual hardware architecture of
the IPU capture blocks. I.e., all the subdevs and links shown correspond 
to actual hardware connections and sub-blocks.

Russell just provided a link to the imx6 reference manual, and dot
graph for the imx219 based platform.

Also I've added quite a lot of detail to the media doc at
Documentation/media/v4l-drivers/imx.rst.

The dot graphs for SabreSD, SabreLite, and SabreAuto reference platforms 
are attached. This is generated from the most recent
(version 5) imx-media driver.

Steve

[toc] | [prev] | [next] | [standalone]


#1598411

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-11 19:50 +0100
Message-ID<tjUfv-6My-7@gated-at.bofh.it>
In reply to#1598395
On Sat, Mar 11, 2017 at 10:08:23AM -0800, Steve Longerbeam wrote:
> On 03/11/2017 07:32 AM, Sakari Ailus wrote:
> >Hi Mauro and Hans,
> >
> >On Sat, Mar 11, 2017 at 10:14:08AM -0300, Mauro Carvalho Chehab wrote:
> >>Em Sat, 11 Mar 2017 12:32:43 +0100
> >>Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >>
> >>>On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
> >>>>Em Fri, 10 Mar 2017 13:54:28 +0100
> >>>>Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >>>>
> >>>>>>Devices that have complex pipeline that do essentially require using the
> >>>>>>Media controller interface to configure them are out of that scope.
> >>>>>>
> >>>>>
> >>>>>Way too much of how the MC devices should be used is in the minds of developers.
> >>>>>There is a major lack for good detailed documentation, utilities, compliance
> >>>>>test (really needed!) and libv4l plugins.
> >>>>
> >>>>Unfortunately, we merged an incomplete MC support at the Kernel. We knew
> >>>>all the problems with MC-based drivers and V4L2 applications by the time
> >>>>it was developed, and we requested Nokia developers (with was sponsoring MC
> >>>>develoment, on that time) to work on a solution to allow standard V4L2
> >>>>applications to work with MC based boards.
> >>>>
> >>>>Unfortunately, we took the decision to merge MC without that, because
> >>>>Nokia was giving up on Linux development, and we didn't want to lose the
> >>>>2 years of discussions and work around it, as Nokia employers were leaving
> >>>>the company. Also, on that time, there was already some patches floating
> >>>>around adding backward support via libv4l. Unfortunately, those patches
> >>>>were never finished.
> >>>>
> >>>>The net result is that MC was merged with some huge gaps, including
> >>>>the lack of a proper solution for a generic V4L2 program to work
> >>>>with V4L2 devices that use the subdev API.
> >>>>
> >>>>That was not that bad by then, as MC was used only on cell phones
> >>>>that run custom-made applications.
> >>>>
> >>>>The reallity changed, as now, we have lots of low cost SoC based
> >>>>boards, used for all sort of purposes. So, we need a quick solution
> >>>>for it.
> >>>>
> >>>>In other words, while that would be acceptable support special apps
> >>>>on really embedded systems, it is *not OK* for general purpose SoC
> >>>>harware[1].
> >>>>
> >>>>[1] I'm calling "general purpose SoC harware" those ARM boards
> >>>>like Raspberry Pi that are shipped to the mass and used by a wide
> >>>>range of hobbyists and other people that just wants to run Linux on
> >>>>ARM. It is possible to buy such boards for a very cheap price,
> >>>>making them to be used not only on special projects, where a custom
> >>>>made application could be interesting, but also for a lot of
> >>>>users that just want to run Linux on a low cost ARM board, while
> >>>>keeping using standard V4L2 apps, like "camorama".
> >>>>
> >>>>That's perhaps one of the reasons why it took a long time for us to
> >>>>start receiving drivers upstream for such hardware: it is quite
> >>>>intimidating and not logical to require developers to implement
> >>>>on their drivers 2 complex APIs (MC, subdev) for those
> >>>>hardware that most users won't care. From user's perspective,
> >>>>being able to support generic applications like "camorama" and
> >>>>"zbar" is all they want.
> >>>>
> >>>>In summary, I'm pretty sure we need to support standard V4L2
> >>>>applications on boards like Raspberry Pi and those low-cost
> >>>>SoC-based boards that are shipped to end users.
> >>>>
> >>>>>Anyway, regarding this specific patch and for this MC-aware driver: no, you
> >>>>>shouldn't inherit controls from subdevs. It defeats the purpose.
> >>>>
> >>>>Sorry, but I don't agree with that. The subdev API is an optional API
> >>>>(and even the MC API can be optional).
> >>>>
> >>>>I see the rationale for using MC and subdev APIs on cell phones,
> >>>>ISV and other embedded hardware, as it will allow fine-tuning
> >>>>the driver's support to allow providing the required quality for
> >>>>certain custom-made applications. but on general SoC hardware,
> >>>>supporting standard V4L2 applications is a need.
> >>>>
> >>>>Ok, perhaps supporting both subdev API and V4L2 API at the same
> >>>>time doesn't make much sense. We could disable one in favor of the
> >>>>other, either at compilation time or at runtime.
> >>>
> >>>Right. If the subdev API is disabled, then you have to inherit the subdev
> >>>controls in the bridge driver (how else would you be able to access them?).
> >>>And that's the usual case.
> >>>
> >>>If you do have the subdev API enabled, AND you use the MC, then the
> >>>intention clearly is to give userspace full control and inheriting controls
> >>>no longer makes any sense (and is highly confusing IMHO).
> >>
> >>I tend to agree with that.
> >
> >I agree as well.
> >
> >This is in line with how existing drivers behave, too.
> 
> Well, sounds like there is consensus on this topic. I guess I'll
> go ahead and remove the control inheritance support. I suppose
> having a control appear in two places (subdev and video nodes) can
> be confusing.

I would say _don't_ do that until there are tools/libraries in place
that are able to support controlling subdevs, otherwise it's just
going to be another reason for me to walk away from this stuff, and
stick with a version that does work sensibly.

> As for the configurability vs. ease-of-use debate, I added the
> control inheritance to make it a little easier on the user, but,
> as the dot graphs below will show, the user already needs quite
> a lot of knowledge of the architecture already, in order to setup
> the different pipelines. So perhaps the control inheritance is
> rather pointless anyway.

I really don't think expecting the user to understand and configure
the pipeline is a sane way forward.  Think about it - should the
user need to know that, because they have a bayer-only CSI data
source, that there is only one path possible, and if they try to
configure a different path, then things will just error out?

For the case of imx219 connected to iMX6, it really is as simple as
"there is only one possible path" and all the complexity of the media
interfaces/subdevs is completely unnecessary.  Every other block in
the graph is just noise.

The fact is that these dot graphs show a complex picture, but reality
is somewhat different - there's only relatively few paths available
depending on the connected source and the rest of the paths are
completely useless.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1598417

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-03-11 20:00 +0100
Message-ID<tjUpc-6PV-3@gated-at.bofh.it>
In reply to#1598411

On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
> On Sat, Mar 11, 2017 at 10:08:23AM -0800, Steve Longerbeam wrote:
>> On 03/11/2017 07:32 AM, Sakari Ailus wrote:
>>> Hi Mauro and Hans,
>>>
>>> On Sat, Mar 11, 2017 at 10:14:08AM -0300, Mauro Carvalho Chehab wrote:
>>>> Em Sat, 11 Mar 2017 12:32:43 +0100
>>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>>
>>>>> On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
>>>>>> Em Fri, 10 Mar 2017 13:54:28 +0100
>>>>>> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>>>>>>
>>>>>>>> Devices that have complex pipeline that do essentially require using the
>>>>>>>> Media controller interface to configure them are out of that scope.
>>>>>>>>
>>>>>>>
>>>>>>> Way too much of how the MC devices should be used is in the minds of developers.
>>>>>>> There is a major lack for good detailed documentation, utilities, compliance
>>>>>>> test (really needed!) and libv4l plugins.
>>>>>>
>>>>>> Unfortunately, we merged an incomplete MC support at the Kernel. We knew
>>>>>> all the problems with MC-based drivers and V4L2 applications by the time
>>>>>> it was developed, and we requested Nokia developers (with was sponsoring MC
>>>>>> develoment, on that time) to work on a solution to allow standard V4L2
>>>>>> applications to work with MC based boards.
>>>>>>
>>>>>> Unfortunately, we took the decision to merge MC without that, because
>>>>>> Nokia was giving up on Linux development, and we didn't want to lose the
>>>>>> 2 years of discussions and work around it, as Nokia employers were leaving
>>>>>> the company. Also, on that time, there was already some patches floating
>>>>>> around adding backward support via libv4l. Unfortunately, those patches
>>>>>> were never finished.
>>>>>>
>>>>>> The net result is that MC was merged with some huge gaps, including
>>>>>> the lack of a proper solution for a generic V4L2 program to work
>>>>>> with V4L2 devices that use the subdev API.
>>>>>>
>>>>>> That was not that bad by then, as MC was used only on cell phones
>>>>>> that run custom-made applications.
>>>>>>
>>>>>> The reallity changed, as now, we have lots of low cost SoC based
>>>>>> boards, used for all sort of purposes. So, we need a quick solution
>>>>>> for it.
>>>>>>
>>>>>> In other words, while that would be acceptable support special apps
>>>>>> on really embedded systems, it is *not OK* for general purpose SoC
>>>>>> harware[1].
>>>>>>
>>>>>> [1] I'm calling "general purpose SoC harware" those ARM boards
>>>>>> like Raspberry Pi that are shipped to the mass and used by a wide
>>>>>> range of hobbyists and other people that just wants to run Linux on
>>>>>> ARM. It is possible to buy such boards for a very cheap price,
>>>>>> making them to be used not only on special projects, where a custom
>>>>>> made application could be interesting, but also for a lot of
>>>>>> users that just want to run Linux on a low cost ARM board, while
>>>>>> keeping using standard V4L2 apps, like "camorama".
>>>>>>
>>>>>> That's perhaps one of the reasons why it took a long time for us to
>>>>>> start receiving drivers upstream for such hardware: it is quite
>>>>>> intimidating and not logical to require developers to implement
>>>>>> on their drivers 2 complex APIs (MC, subdev) for those
>>>>>> hardware that most users won't care. From user's perspective,
>>>>>> being able to support generic applications like "camorama" and
>>>>>> "zbar" is all they want.
>>>>>>
>>>>>> In summary, I'm pretty sure we need to support standard V4L2
>>>>>> applications on boards like Raspberry Pi and those low-cost
>>>>>> SoC-based boards that are shipped to end users.
>>>>>>
>>>>>>> Anyway, regarding this specific patch and for this MC-aware driver: no, you
>>>>>>> shouldn't inherit controls from subdevs. It defeats the purpose.
>>>>>>
>>>>>> Sorry, but I don't agree with that. The subdev API is an optional API
>>>>>> (and even the MC API can be optional).
>>>>>>
>>>>>> I see the rationale for using MC and subdev APIs on cell phones,
>>>>>> ISV and other embedded hardware, as it will allow fine-tuning
>>>>>> the driver's support to allow providing the required quality for
>>>>>> certain custom-made applications. but on general SoC hardware,
>>>>>> supporting standard V4L2 applications is a need.
>>>>>>
>>>>>> Ok, perhaps supporting both subdev API and V4L2 API at the same
>>>>>> time doesn't make much sense. We could disable one in favor of the
>>>>>> other, either at compilation time or at runtime.
>>>>>
>>>>> Right. If the subdev API is disabled, then you have to inherit the subdev
>>>>> controls in the bridge driver (how else would you be able to access them?).
>>>>> And that's the usual case.
>>>>>
>>>>> If you do have the subdev API enabled, AND you use the MC, then the
>>>>> intention clearly is to give userspace full control and inheriting controls
>>>>> no longer makes any sense (and is highly confusing IMHO).
>>>>
>>>> I tend to agree with that.
>>>
>>> I agree as well.
>>>
>>> This is in line with how existing drivers behave, too.
>>
>> Well, sounds like there is consensus on this topic. I guess I'll
>> go ahead and remove the control inheritance support. I suppose
>> having a control appear in two places (subdev and video nodes) can
>> be confusing.
>
> I would say _don't_ do that until there are tools/libraries in place
> that are able to support controlling subdevs, otherwise it's just
> going to be another reason for me to walk away from this stuff, and
> stick with a version that does work sensibly.
>
>> As for the configurability vs. ease-of-use debate, I added the
>> control inheritance to make it a little easier on the user, but,
>> as the dot graphs below will show, the user already needs quite
>> a lot of knowledge of the architecture already, in order to setup
>> the different pipelines. So perhaps the control inheritance is
>> rather pointless anyway.
>
> I really don't think expecting the user to understand and configure
> the pipeline is a sane way forward.  Think about it - should the
> user need to know that, because they have a bayer-only CSI data
> source, that there is only one path possible, and if they try to
> configure a different path, then things will just error out?
>
> For the case of imx219 connected to iMX6, it really is as simple as
> "there is only one possible path" and all the complexity of the media
> interfaces/subdevs is completely unnecessary.  Every other block in
> the graph is just noise.
>
> The fact is that these dot graphs show a complex picture, but reality
> is somewhat different - there's only relatively few paths available
> depending on the connected source and the rest of the paths are
> completely useless.
>

I totally disagree there. Raw bayer requires passthrough yes, but for
all other media bus formats on a mipi csi-2 bus, and all other media
bus formats on 8-bit parallel buses, the conersion pipelines can be
used for scaling, CSC, rotation, and motion-compensated de-interlacing.

Steve

[toc] | [prev] | [next] | [standalone]


#1598422

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-11 20:10 +0100
Message-ID<tjUyR-78s-11@gated-at.bofh.it>
In reply to#1598417
On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:
> 
> 
> On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
> >I really don't think expecting the user to understand and configure
> >the pipeline is a sane way forward.  Think about it - should the
> >user need to know that, because they have a bayer-only CSI data
> >source, that there is only one path possible, and if they try to
> >configure a different path, then things will just error out?
> >
> >For the case of imx219 connected to iMX6, it really is as simple as
> >"there is only one possible path" and all the complexity of the media
> >interfaces/subdevs is completely unnecessary.  Every other block in
> >the graph is just noise.
> >
> >The fact is that these dot graphs show a complex picture, but reality
> >is somewhat different - there's only relatively few paths available
> >depending on the connected source and the rest of the paths are
> >completely useless.
> >
> 
> I totally disagree there. Raw bayer requires passthrough yes, but for
> all other media bus formats on a mipi csi-2 bus, and all other media
> bus formats on 8-bit parallel buses, the conersion pipelines can be
> used for scaling, CSC, rotation, and motion-compensated de-interlacing.

... which only makes sense _if_ your source can produce those formats.
We don't actually disagree on that.

Let me re-state.  If the source can _only_ produce bayer, then there is
_only_ _one_ possible path, and all the overhead of the media controller
stuff is totally unnecessary.

Or, are you going to tell me that the user should have the right to
configure paths through the iMX6 hardware that are not permitted by the
iMX6 manuals for the data format being produced by the sensor?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1598425

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-03-11 20:10 +0100
Message-ID<tjUyS-78s-21@gated-at.bofh.it>
In reply to#1598422

On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:
> On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:
>>
>>
>> On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
>>> I really don't think expecting the user to understand and configure
>>> the pipeline is a sane way forward.  Think about it - should the
>>> user need to know that, because they have a bayer-only CSI data
>>> source, that there is only one path possible, and if they try to
>>> configure a different path, then things will just error out?
>>>
>>> For the case of imx219 connected to iMX6, it really is as simple as
>>> "there is only one possible path" and all the complexity of the media
>>> interfaces/subdevs is completely unnecessary.  Every other block in
>>> the graph is just noise.
>>>
>>> The fact is that these dot graphs show a complex picture, but reality
>>> is somewhat different - there's only relatively few paths available
>>> depending on the connected source and the rest of the paths are
>>> completely useless.
>>>
>>
>> I totally disagree there. Raw bayer requires passthrough yes, but for
>> all other media bus formats on a mipi csi-2 bus, and all other media
>> bus formats on 8-bit parallel buses, the conersion pipelines can be
>> used for scaling, CSC, rotation, and motion-compensated de-interlacing.
>
> ... which only makes sense _if_ your source can produce those formats.
> We don't actually disagree on that.
>
> Let me re-state.  If the source can _only_ produce bayer, then there is
> _only_ _one_ possible path, and all the overhead of the media controller
> stuff is totally unnecessary.
>
> Or, are you going to tell me that the user should have the right to
> configure paths through the iMX6 hardware that are not permitted by the
> iMX6 manuals for the data format being produced by the sensor?
>

Russell, I'm not following you. The imx6 pipelines allow for many
different sources, not just the inx219 that only outputs bayer. You
seem to be saying that those other pipelines should not be present
because they don't support raw bayer.

Steve

[toc] | [prev] | [next] | [standalone]


#1598459

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-03-11 21:50 +0100
Message-ID<tjW7D-827-1@gated-at.bofh.it>
In reply to#1598425
On Sat, Mar 11, 2017 at 11:06:55AM -0800, Steve Longerbeam wrote:
> On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:
> >On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:
> >>
> >>
> >>On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
> >>>I really don't think expecting the user to understand and configure
> >>>the pipeline is a sane way forward.  Think about it - should the
> >>>user need to know that, because they have a bayer-only CSI data
> >>>source, that there is only one path possible, and if they try to
> >>>configure a different path, then things will just error out?
> >>>
> >>>For the case of imx219 connected to iMX6, it really is as simple as
> >>>"there is only one possible path" and all the complexity of the media
> >>>interfaces/subdevs is completely unnecessary.  Every other block in
> >>>the graph is just noise.
> >>>
> >>>The fact is that these dot graphs show a complex picture, but reality
> >>>is somewhat different - there's only relatively few paths available
> >>>depending on the connected source and the rest of the paths are
> >>>completely useless.
> >>>
> >>
> >>I totally disagree there. Raw bayer requires passthrough yes, but for
> >>all other media bus formats on a mipi csi-2 bus, and all other media
> >>bus formats on 8-bit parallel buses, the conersion pipelines can be
> >>used for scaling, CSC, rotation, and motion-compensated de-interlacing.
> >
> >... which only makes sense _if_ your source can produce those formats.
> >We don't actually disagree on that.
> >
> >Let me re-state.  If the source can _only_ produce bayer, then there is
> >_only_ _one_ possible path, and all the overhead of the media controller
> >stuff is totally unnecessary.
> >
> >Or, are you going to tell me that the user should have the right to
> >configure paths through the iMX6 hardware that are not permitted by the
> >iMX6 manuals for the data format being produced by the sensor?
> >
> 
> Russell, I'm not following you. The imx6 pipelines allow for many
> different sources, not just the inx219 that only outputs bayer. You
> seem to be saying that those other pipelines should not be present
> because they don't support raw bayer.

What I'm saying is this:

_If_ you have a sensor connected that can _only_ produce bayer, _then_
there is only _one_ possible path through the imx6 pipelines that is
legal.  Offering other paths from the source is noise, because every
other path can't be used with a bayer source.

_If_ you have a sensor connected which can produce RGB or YUV formats,
_then_ other paths are available, and pipeline needs to be configured
to select the appropriate path with the desired features.

So, in the case of a bayer source, offering the user the chance to
manually configure the _single_ allowable route through the tree is
needless complexity.  Forcing the user to have to use the subdev
interfaces to configure the camera is needless complexity.  Such a
source can only ever be used with one single /dev/video* node.

Moreover, this requires user education, and this brings me on to much
larger concerns.  We seem to be saying "this is too complicated, the
user can work it out!"

We've been here with VGA devices.  Remember the old days when you had
to put mode lines into the Xorg.conf, or go through a lengthy setup
process to get X running?  It wasn't very user-friendly.  We seem to
be making the same mistake here.

Usability comes first and foremost - throwing complex problems at
users is not a solution.

Now, given that this media control API has been around for several
years, and the userspace side of the story has not really improved
(according to Mauro, several attempts have been made, every single
attempt so far has failed, even for specific hardware) it seems to me
that using the media control API is a very poor choice for the very
simple reason that _no one_ knows how to configure a system using it.
Hans thoughts of getting some funding to look at this aspect is a
good idea, but I really wonder, given the history so far, how long
this will take - and whether it _ever_ will get solved.

If it doesn't get solved, then we're stuck with quite a big problem.

So, I suggest that we don't merge any further media-controller based
kernel code _until_ we have the userspace side sorted out.  Merging
the kernel side drivers when we don't even know that the userspace
API is functionally usable in userspace beyond test programs is
utterly absurd - what if it turns out that no one can write v4l
plugins that sort out the issues that have been highlighted throughout
these discussions.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1598538

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-03-12 04:40 +0100
Message-ID<tk2wp-46p-1@gated-at.bofh.it>
In reply to#1598422

On 03/11/2017 10:59 AM, Russell King - ARM Linux wrote:
> On Sat, Mar 11, 2017 at 10:54:55AM -0800, Steve Longerbeam wrote:
>>
>>
>> On 03/11/2017 10:45 AM, Russell King - ARM Linux wrote:
>>> I really don't think expecting the user to understand and configure
>>> the pipeline is a sane way forward.  Think about it - should the
>>> user need to know that, because they have a bayer-only CSI data
>>> source, that there is only one path possible, and if they try to
>>> configure a different path, then things will just error out?
>>>
>>> For the case of imx219 connected to iMX6, it really is as simple as
>>> "there is only one possible path" and all the complexity of the media
>>> interfaces/subdevs is completely unnecessary.  Every other block in
>>> the graph is just noise.
>>>
>>> The fact is that these dot graphs show a complex picture, but reality
>>> is somewhat different - there's only relatively few paths available
>>> depending on the connected source and the rest of the paths are
>>> completely useless.
>>>
>>
>> I totally disagree there. Raw bayer requires passthrough yes, but for
>> all other media bus formats on a mipi csi-2 bus, and all other media
>> bus formats on 8-bit parallel buses, the conersion pipelines can be
>> used for scaling, CSC, rotation, and motion-compensated de-interlacing.
>
> ... which only makes sense _if_ your source can produce those formats.
> We don't actually disagree on that.

...and there are lots of those sources! You should try getting out of
your imx219 shell some time, and have a look around! :)

>
> Let me re-state.  If the source can _only_ produce bayer, then there is
> _only_ _one_ possible path, and all the overhead of the media controller
> stuff is totally unnecessary.
>
> Or, are you going to tell me that the user should have the right to
> configure paths through the iMX6 hardware that are not permitted by the
> iMX6 manuals for the data format being produced by the sensor?

Anyway, no the user is not allowed to configure a path that is not
allowed by the hardware, such as attempting to pass raw bayer through
an Image Converter path.

I guess you are simply commenting that for users of bayer sensors, the
other pipelines can be "confusing". But I trust you're not saying those
other pipelines should therefore not be present, which would be a
completely nutty argument.

Steve

[toc] | [prev] | [next] | [standalone]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web