Date:   Sat, 11 Mar 2017 10:54:55 -0800
From:   Steve Longerbeam <slongerbeam@...il.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Sakari Ailus <sakari.ailus@....fi>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>,
        mark.rutland@....com, andrew-ct.chen@...iatek.com,
        minghsiu.tsai@...iatek.com, sakari.ailus@...ux.intel.com,
        nick@...anahar.org, songjun.wu@...rochip.com,
        Hans Verkuil <hverkuil@...all.nl>, pavel@....cz,
        robert.jarzmik@...e.fr, devel@...verdev.osuosl.org,
        markus.heiser@...marIT.de,
        laurent.pinchart+renesas@...asonboard.com, shuah@...nel.org,
        geert@...ux-m68k.org, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, kernel@...gutronix.de, arnd@...db.de,
        mchehab@...nel.org, bparrot@...com, robh+dt@...nel.org,
        horms+renesas@...ge.net.au, tiffany.lin@...iatek.com,
        linux-arm-kernel@...ts.infradead.org,
        niklas.soderlund+renesas@...natech.se, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, jean-christophe.trotin@...com,
        p.zabel@...gutronix.de, fabio.estevam@....com, shawnguo@...nel.org,
        sudipm.mukherjee@...il.com
Subject: Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit
 controls from a pipeline



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@...all.nl> escreveu:
>>>>
>>>>> On 10/03/17 16:09, Mauro Carvalho Chehab wrote:
>>>>>> Em Fri, 10 Mar 2017 13:54:28 +0100
>>>>>> Hans Verkuil <hverkuil@...all.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

Powered by blists - more mailing lists