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

[Sakari Ailus]

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.

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

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

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx