Message-ID: <3046850.jg4ctvz9Cj@avalon>
Date:   Fri, 25 Aug 2017 13:27:25 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Sakari Ailus <sakari.ailus@....fi>
Cc:     Mauro Carvalho Chehab <mchehab@...pensource.com>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...radead.org>,
        linux-kernel@...r.kernel.org, Jonathan Corbet <corbet@....net>,
        hverkuil@...all.nl
Subject: Re: [PATCH RFC] media: open.rst: document devnode-centric and mc-centric types

Hi Sakari,

On Thursday, 24 August 2017 12:18:56 EEST Sakari Ailus wrote:
> On Wed, Aug 23, 2017 at 10:42:28AM -0300, Mauro Carvalho Chehab wrote:
> > Em Wed, 23 Aug 2017 12:37:30 +0300 Sakari Ailus escreveu:
> >> On Sat, Aug 19, 2017 at 07:04:40AM -0300, Mauro Carvalho Chehab wrote:
> >>> When we added support for omap3, back in 2010, we added a new
> >>> type of V4L2 devices that aren't fully controlled via the V4L2
> >>> device node. Yet, we never made it clear, at the V4L2 spec,
> >>> about the differences between both types.
> >>> 
> >>> Let's document them with the current implementation.
> >>> 
> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab@...pensource.com>
> >>> ---
> >>> 
> >>> This patch is a result of this week's discussion we had with regards
> >>> to merging a patch series from Sakari documenting the need of
> >>> propagating streaming control between sub-devices on some complex
> >>> mc-centric devices.
> >>> 
> >>> One big gap on our documentation popped up: while we have distinct
> >>> behavior for mc-centric and devnode-centric types of V4L2 devices, we
> >>> never clearly documented about that.
> >>> 
> >>> This RFC patch is a first attempt to have a definition of those types,
> >>> and to standardize the names we use to distinguish between those
> >>> types.
> >>> 
> >>> Comments are welcome.
> >>> 
> >>>  Documentation/media/uapi/v4l/open.rst | 37 ++++++++++++++++++++++++++++
> >>>  1 file changed, 37 insertions(+)
> >>> 
> >>> diff --git a/Documentation/media/uapi/v4l/open.rst
> >>> b/Documentation/media/uapi/v4l/open.rst index
> >>> afd116edb40d..9cf4f74c466a 100644
> >>> --- a/Documentation/media/uapi/v4l/open.rst
> >>> +++ b/Documentation/media/uapi/v4l/open.rst
> >>> @@ -6,6 +6,43 @@
> >>>  Opening and Closing Devices
> >>>  ***************************
> >>> 
> >>> +Types of V4L2 devices
> >>> +=====================
> >>> +
> >>> +V4L2 devices are usually complex: they're implemented via a main
> >>> driver and
> >> 
> >> Not all such as UVC webcams. How about:
> >> 
> >> s/implemented/often implemented/
> >> 
> >>> +several other drivers.
> > 
> > True. uvcvideo and gspca drivers has only a main driver.
> > 
> > What about, instead:
> > 
> > V4L2 devices are usually complex: they're implemented via a main driver
> > and often by several other drivers.
> 
> How about s/other/additional/? That I think would better convey the
> suggestion the main driver's role stays even if there are other drivers.
> 
> >> The main driver always exposes a V4L2 device node
> 
> This should actually say that there's "one or more V4L2 device nodes".
> 
> >>> +(see :ref:`v4l2_device_naming`). The other devices are called
> >>> **sub-devices**.
> >>> +They are usually controlled via a serial bus (I2C or SMBus).
> >> 
> >> The main driver also often exposes sub-devices.
> > 
> > What about:
> > 	(see :ref:`v4l2_device_naming`). The other devices, when present,
> > 	are called **sub-devices**.
> 
> I might use "V4L2 sub-devices" instead of plain "sub-devices". I don't have
> strong opinion either way.
> 
> >> The practice has been that the video nodes are registered by the driver
> >> that generally orchestrates things for the complete device but I don't
> >> think there's anything that would exclude doing this otherwise if there
> >> are two struct devices involved that do DMA.
> > 
> > Yeah. Well, I avoided the discussion if a mc-centric device with just
> > enable a single DMA engine for camera output should orchestrate things
> > for the complete device, as this is something that the current drivers
> > don't usually do (yet, IMHO, they should, at least for those cheap SoC
> > devices with just one camera connector).
> > 
> > Anyway, this is a separate issue. For now, let's focus on documenting
> > the current situation.
> > 
> >>> +
> >>> +When V4L2 started, there was only one type of device, fully
> >>> controlled via
> >>> +V4L2 device nodes. We call those devices as **devnode-centric
> >>> devices**.
> >> 
> >> As device nodes can refer to any types of device nodes (such as
> >> sub-device nodes), how about calling these "video node centric"? To make
> >> it more explicit, I'd use "V4L2 video node centric" here. Perhaps "video
> >> node centric" later to make it shorter.
> > 
> > I'm open to change its name, but "video node" is also wrong. There are
> > some devices that only exposes /dev/radio.
> 
> Good point.
> 
> > main-driver-centric?
> 
> I think most of the user space developers probably see this from the
> interface point of view, they might not know which driver exposes
> particular device nodes, for instance.
> 
> Would it be too clunky to call them non-MC-centric? That would be
> technically correct.
> 
> Or simply "plain". Combined with the explanation above, this could also be
> workable.
> 
> >>> +Since the end of 2010, a new type of V4L2 device was added, in order
> >>> to
> >>> +support complex devices that are common on embedded systems. Those
> >>> devices
> >> 
> >>> +are controlled mainly via the media controller. So, they're called:
> >> 
> >> s/://
> >> 
> >>> +**mc-centric devices**.
> >> 
> >> "Media controller (MC) centric devices" and "MC-centric devices" later?
> > 
> > Works for me. Actually, I did something like that on my very first
> > version (that I didn't submit).
> 
> Ack; agreed.
> 
> >>> +
> >>> +On a **devnode-centric device**, the device and their corresponding
> >>> hardware
> >>> +pipelines are controlled via the V4L2 device node. They may optionally
> >>> +expose the hardware pipelines via the
> >>> +:ref:`media controller API <media_controller>`.
> >>> +
> >>> +On a **mc-centric device**, before using the V4L2 device, it is
> >>> required to +set the hardware pipelines via the
> >>> +:ref:`media controller API <media_controller>`. On those devices, the
> >>> +sub-devices' configuration can be controlled via the
> >>> +:ref:`sub-device API <subdev>`, with creates one device node per sub
> >>> device. +On such devices, the V4L2 device node is mainly responsible
> >>> for controlling +the streaming features, while the media controller
> >>> and the subdevices device +nodes are responsible for configuring the
> >>> hardware.
> >> 
> >> What would you think of using wording that conveys the message that in
> >> this case V4L2 video device nodes essentially are a data interface only
> >> whilst V4L2 sub-device nodes and MC are control interfaces? This is the
> >> grounding difference between the two in my opinion, and makes it easy to
> >> understand for the reader.
> > 
> > The word "control" is too wide. Even on mc-centric, the V4L2 video device
> > nodes are also used to control the device, at least to manage its
> > streaming engine, by sending stream on/off, reqbufs, etc.
> 
> Good point. The buffer operations are about data, but still stream control
> remains. Stream control is indirectly related to buffers but not only the
> buffers.
> 
> We could still mention streaming control as an exception to the rule.
> 
> I wonder what Laurent or Hans think.

I've always been bothered by the fact that MC pipelines are started and 
stopped through the V4L2 device nodes. It's clumsy and unnecessarily difficult 
to implement in drivers. Ideally we should control the pipeline, including 
start/stop operations, separately from the video node.

This being said we can't change existing drivers, so we have to leave with 
this scheme until for the foreseeable future (which doesn't mean we shouldn't 
come up with a better one for new drivers, especially with the request API the 
start/stop stream ioctls on video device nodes won't make any sense). I'm fine 
mentioning stream control explicitly as the only exception.

> > (also, while we don't reach an agreement, even on mc-centric, IMO, it
> > makes sense to use video nodes to send ctrls, propagating it to pipeline,
> > at least on devices like RPi, where we do want normal apps to work with
> > them)
> >  
> >>> +
> >>> +.. note::
> >>> +
> >>> +   Currently, it is forbidden for a **devnode-centric device** to
> >>> expose
> >>> +   subdevices via :ref:`sub-device API <subdev>`, although this might
> >>> +   change in the future.
> >> 
> >> I'd leave out this note.
> > 
> > So, are you proposing to allow video/radio centric devices to also
> > expose subdevs?
> > 
> >> One of the purposes of MC was to find device
> >> nodes, and finding a subdev node related to, say, a video node is a pain
> >> without MC.
> > 
> > I disagree with the above sentence: just video nodes are enough for
> > almost all non-embedded hardware. We implemented MC there only in order to
> 
> How would you find a sub-device node somehow related to a video node
> without MC?
> 
> > solve a conflict when certain sub-devices are busy because they're used by
> > a DVB pipeline while someone tries to stream V4L2 on.
> 
> This matter certainly was as you say, but nowadays e.g. some Intel Core
> SoCs do have IPUs (ISPs) and CSI-2 receivers in them. These chips could be
> used on a regular PC. From software point of view they're not different
> from typical embedded systems.
> 
> If this doesn't mean that there will be universally more need for MC, it
> does still mean that the need for MC is no longer related to embedded
> systems only.
> 
> >> Less variance in interface availability is better for the user
> >> space, and unlike between video node centric vs. MC-centric, there are
> >> hardly technical reasons (or at least I can't remember one) for doing
> >> this.
> >> 
> >> I remember we had a few of these drivers and the agreement was to add MC
> >> interface to those.
> > 
> > Sorry, I'm completely lost what you meant here, as it seems that you're
> > contradicting yourself :-)
> > 
> > Do you want to allow non-mc-centric devices to expose subdev API or not?
> > 
> > If not, we need to add a notice forbidding it (but noticing that it
> > might change in the future, if we ever need it for whatever reason).
> 
> I meant to say that we should constrain ourselves to providing as little
> variance in user space interfaces between different drivers as possible,
> still without limiting ourselves to supporting only a subset of hardware
> features.
> 
> In this case there is no technical reason I'm aware of for providing
> sub-device nodes without Media controller.


-- 
Regards,

Laurent Pinchart

Powered by blists - more mailing lists