Re: [PATCH RFC] media: open.rst: document devnode-centric and mc-centric types

[Mauro Carvalho Chehab]

Em Thu, 24 Aug 2017 12:18:56 +0300
Sakari Ailus <sakari.ailus@xxxxxx> escreveu:

> Hi Mauro,
> 
> 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 <sakari.ailus@xxxxxx> escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > Thanks for the patch! Something like this was long due.
> > > 
> > > I cc'd Hans and Laurent to get their attention, too.
> > > 
> > > 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@xxxxxxxxxxxxxxxx>
> > > > ---
> > > > 
> > > > 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.

OK.

> 
> >   
> > > 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.

OK.

> >   
> > > 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.

I guess we can call it as "V4L2 device centric", or simply "device-centric",
in opposite to sub-device/mc-centric.

> > > > +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.
> 
> > 
> > (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?

When sub-devices are exposed via subdev API, MC support is needed.

In thesis, it would still be possible to use V4L2 on device-centric way
and export both MC and subdev. The above note is to forbid that.
 
> > 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.

>From software PoV, if those are used on regular PC, the should work 
with standard V4L2 applications. So, they should support device-centric
control.

> 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.


That's why we need that notice: to prevent people to implement it
wrong. Yet, we should not limit ourselves to the possibility of
changing it if needed in the future.

I'll submit a version 2 of this RFC later today to address your
comments.

Cheers,
Mauro