diff mbox series

[2/5] docs: media: vimc: Documenting vimc topology configuration using configfs

Message ID 20190919203208.12515-3-dafna.hirschfeld@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: vimc: use configfs in order to configure devices topologies | expand

Commit Message

Dafna Hirschfeld Sept. 19, 2019, 8:32 p.m. UTC
Add explanation of how to use configfs in order to create a
vimc device with a given topology.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 Documentation/media/v4l-drivers/vimc.dot |  28 ++-
 Documentation/media/v4l-drivers/vimc.rst | 240 ++++++++++++++++++++---
 2 files changed, 220 insertions(+), 48 deletions(-)

Comments

Hans Verkuil Sept. 20, 2019, 1:39 p.m. UTC | #1
On 9/19/19 10:32 PM, Dafna Hirschfeld wrote:
> Add explanation of how to use configfs in order to create a
> vimc device with a given topology.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  Documentation/media/v4l-drivers/vimc.dot |  28 ++-
>  Documentation/media/v4l-drivers/vimc.rst | 240 ++++++++++++++++++++---
>  2 files changed, 220 insertions(+), 48 deletions(-)
> 
> diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
> index 57863a13fa39..e3b41ac2bc46 100644
> --- a/Documentation/media/v4l-drivers/vimc.dot
> +++ b/Documentation/media/v4l-drivers/vimc.dot
> @@ -2,21 +2,15 @@
>  
>  digraph board {
>  	rankdir=TB
> -	n00000001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000001:port0 -> n00000005:port0 [style=bold]
> -	n00000001:port0 -> n0000000b [style=bold]
> -	n00000003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000003:port0 -> n00000008:port0 [style=bold]
> -	n00000003:port0 -> n0000000f [style=bold]
> -	n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000005:port1 -> n00000017:port0
> -	n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000008:port1 -> n00000017:port0 [style=dashed]
> -	n0000000b [label="Raw Capture 0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> -	n0000000f [label="Raw Capture 1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> -	n00000013 [label="RGB/YUV Input\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> -	n00000013 -> n00000017:port0 [style=dashed]
> -	n00000017 [label="{{<port0> 0} | Scaler\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000017:port1 -> n0000001a [style=bold]
> -	n0000001a [label="RGB/YUV Capture\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
> +	n00000001 [label="cap-deb\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> +	n00000005 [label="cap-sen\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> +	n00000009 [label="cap-sca\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> +	n0000000d [label="{{<port0> 0} | sca\n/dev/v4l-subdev0 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n0000000d:port1 -> n00000009 [style=bold]
> +	n00000010 [label="{{<port0> 0} | deb\n/dev/v4l-subdev1 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000010:port1 -> n00000001 [style=bold]
> +	n00000010:port1 -> n0000000d:port0 [style=bold]
> +	n00000013 [label="{{} | sen\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000013:port0 -> n00000005 [style=bold]
> +	n00000013:port0 -> n00000010:port0 [style=bold]
>  }
> diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
> index a582af0509ee..e5636883545f 100644
> --- a/Documentation/media/v4l-drivers/vimc.rst
> +++ b/Documentation/media/v4l-drivers/vimc.rst
> @@ -1,45 +1,225 @@
>  .. SPDX-License-Identifier: GPL-2.0
>  
> +==========================================
>  The Virtual Media Controller Driver (vimc)
>  ==========================================
>  
> -The vimc driver emulates complex video hardware using the V4L2 API and the Media
> -API. It has a capture device and three subdevices: sensor, debayer and scaler.
> +The vimc driver emulates complex video hardware topologies using the V4L2 API
> +and the Media API. It has a capture device and three subdevices:
> +sensor, debayer and scaler. It exposes media devices through /dev/mediaX nodes,
> +video capture devices through /dev/videoX and sub-devices through /dev/v4l-subdevX.
> +
> +
> +To configure a media device of a given topology, a ConfigFS API is provided.
> +
> +Configuring a topology through ConfigFS (Experimental)
> +======================================================
> +
> +.. note:: This API is under development and might change in the future.
> +
> +Mount configfs:
> +::
> +
> +	$ mkdir /configfs
> +	$ mount -t configfs none /configfs
> +
> +When loading the module, you will see a folder named vimc
> +::
> +
> +	$ tree /configfs/
> +	/configfs/
> +	`-- vimc
> +
> +Creating a media device
> +-----------------------
> +
> +To create a media device create a new folder under /configfs/vimc/
> +
> +Example:
> +::
> +
> +	$ mkdir /configfs/vimc/mdev
> +	$ tree /configfs/vimc/mdev
> +	/configfs/
> +	`-- vimc
> +	    `-- mdev
> +	        `-- hotplug
> +
> +Creating entities
> +-----------------
> +
> +To create an entity in the media device's topology, create a folder under
> +``/configfs/vimc/<mdev-name>/`` with the following format:
> +
> +	<entity-type>:<entity-name>

I suspect that there are restrictions to the characters that can be used in
these names. For one the use of ':' in the entity-name is probably a bad idea.
Also the entity name should be unique, i.e. you can't have a sensor and a scaler
with the same entity name.

Such restrictions should be documented here and also tested in the driver!

> +
> +Where <entity-type> is one of the following:
> +
> +- vimc-sensor
> +- vimc-scaler
> +- vimc-debayer
> +- vimc-capture
> +
> +Example:
> +::
> +
> +	$ mkdir /configfs/vimc/mdev/vimc-sensor:sen
> +	$ mkdir /configfs/vimc/mdev/vimc-capture:cap-sen
> +	$ tree /configfs/
> +	/configfs/
> +	`-- vimc
> +	    `-- mdev
> +		|-- hotplug
> +		|-- vimc-capture:cap-sen
> +		|   `-- pad:sink:0
> +		`-- vimc-sensor:sen
> +                    `-- pad:source:0

Do we need the 'pad:' prefix here? Isn't 'sink:' or 'source:' sufficient?
I think pad:sink:0 is rather cumbersome.

> +
> +Default folders are created under the entity directory for each pad of the entity.
> +
> +Creating links
> +--------------
> +
> +To create a link between two entities, you should create a directory for the link
> +under the source pad of the link and then set it to be a symbolic link to the sink pad:
>  
> -Topology
> ---------
> +Example:
> +::
> +
> +	$ mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"

I'd call the directory 'to-cap-sen': it's clearer that this points to the
cap-sen entity. It makes the example easier to understand.

> +	$ ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +	$ tree /configfs
> +	/configfs
> +	`-- vimc
> +	    `-- mdev
> +	        |-- hotplug
> +	        |-- vimc-capture:cap-sen
> +	        |   `-- pad:sink:0
> +	        `-- vimc-sensor:sen
> +	            `-- pad:source:0
> +	                `-- to-cap
> +	                    |-- enabled
> +	                    |-- immutable
> +	                    `-- pad:sink:0 -> ../../../../../vimc/mdev/vimc-capture:cap-sen/pad:sink:0
> +
> +The `enabled` and `immutable` are two boolean attributes of the link corresponding to the link flags
> +
> +
> +Flag values are described in :ref:`Documentation/media/uapi/mediactl/media-types.rst <media-link-flag>`
> +( seek for ``MEDIA_LNK_FL_*``)
> +
> +1 - Enabled
> +	Indicates that the link will be enabled when the media device is created.
> +
> +3 - Enabled and Immutable
> +	Indicates that the link enabled state can't be modified at runtime.
> +
> +Change an attribute of the link by writing "on" or "1" to set it on , and "off" or "0" to set it off
> +
> +Example:
> +::
> +
> +	$ echo "on" > /configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable
> +
> +Activating/Deactivating device
> +------------------------------
> +
> +To activate the device, write one of "plugged", "plug" or "1" to the file
> +``/configfs/vimc/<mdev-name>/hotplug``
> +
> +Example:
> +::
> +
> +	$ echo 1 > /configfs/vimc/mdev/hotplug
> +
> +You should see a new node ``/dev/mediaX`` in your devfs.
> +
> +To deactivate the device, write one of "unplugged", "unplug" or "0" to the file
> +``/configfs/vimc/<ndev-name>/hotplug``
> +
> +Example:
> +::
> +
> +	$ echo unplugged > /configfs/vimc/mdev/hotplug
> +
> +Topology Configuration - Full Example
> +-------------------------------------
> +
> +Here is a full example of a simple topology configuration:
> +
> +.. code-block:: bash
> +
> +    # Creating the entities
> +    mkdir "/configfs/vimc/mdev"
> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen"
> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb"
> +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca"
> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sca" #/dev/video2
> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sen" #/dev/video1
> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-deb" #/dev/video0
> +
> +    # Creating the links
> +    #sen -> deb
> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
> +    ln -s "/configfs/vimc/mdev/vimc-debayer:deb/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/immutable"

If you set immutable to on, then it should automatically set enabled to on as well,
so no need for the next line.

> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/enabled"
> +
> +    #deb -> sca
> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
> +    ln -s "/configfs/vimc/mdev/vimc-scaler:sca/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/enabled"
> +
> +    #sca -> cap-sca
> +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"

Same in this example: use to-cap-sca

> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sca/pad:sink:0" "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
> +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/enabled"
> +
> +    #sen -> cap-sen
> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/enabled"
> +
> +    #deb -> cap-deb
> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-deb/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/enabled"
>  
> -The topology is hardcoded, although you could modify it in vimc-core and
> -recompile the driver to achieve your own topology. This is the default topology:
>  
>  .. _vimc_topology_graph:
>  
>  .. kernel-figure:: vimc.dot
> -    :alt:   Diagram of the default media pipeline topology
> +    :alt:   Diagram of the configured simple topology in the example
>      :align: center
>  
> -    Media pipeline graph on vimc
> +    Simple Media pipeline graph on vimc configured through configfs
>  
> -Configuring the topology
> -~~~~~~~~~~~~~~~~~~~~~~~~
> +Configuring the pipeline formats
> +================================
>  
> -Each subdevice will come with its default configuration (pixelformat, height,
> -width, ...). One needs to configure the topology in order to match the
> +Each subdevice has a default format configuration (pixelformat, height,
> +width, ...). You should configure the formats in order to match the
>  configuration on each linked subdevice to stream frames through the pipeline.
> -If the configuration doesn't match, the stream will fail. The ``v4l-utils``
> +If the configuration doesn't match, streaming will fail. The ``v4l-utils``
>  package is a bundle of user-space applications, that comes with ``media-ctl`` and
> -``v4l2-ctl`` that can be used to configure the vimc configuration. This sequence
> -of commands fits for the default topology:
> +``v4l2-ctl`` that can be used to configure the formats of the entities. This sequence
> +of commands fits the simple topology created in the full example of topology configuration:
>  
>  .. code-block:: bash
>  
> -        media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
> -        media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
> -        media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
> -        media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
> -        v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
> -        v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
> -        v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
> +	media-ctl -d platform:vimc-000 -V '"sen":0[fmt:SBGGR8_1X8/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"deb":0[fmt:SBGGR8_1X8/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"deb":1[fmt:RGB888_1X24/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"sca":0[fmt:RGB888_1X24/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"sca":1[fmt:RGB888_1X24/640x480]'
> +	v4l2-ctl -z platform:vimc-000 -d "cap-sen" -v pixelformat=BA81
> +	v4l2-ctl -z platform:vimc-000 -d "cap-deb" -v pixelformat=RGB3
> +	# The default scaling value of the scaler is 3, so need to set its capture accordingly
> +	v4l2-ctl -z platform:vimc -d "cap-sca" -v pixelformat=RGB3,width=1920,height=1440
>  
>  Subdevices
>  ----------
> @@ -61,8 +241,8 @@ vimc-debayer:
>  	* 1 Pad source
>  
>  vimc-scaler:
> -	Scale up the image by a factor of 3. E.g.: a 640x480 image becomes a
> -        1920x1440 image. (this value can be configured, see at
> +	Scales up the image by a factor of 3. E.g.: a 640x480 image becomes a
> +        1920x1440 image. (this value can be configured, see

Note that this might change with this patch:

https://patchwork.kernel.org/patch/11146175/

Just so you are aware of this.

>          `Module options`_).
>  	Exposes:
>  
> @@ -77,12 +257,10 @@ vimc-capture:
>  	* 1 Pad source
>  
>  
> -        Module options
> ----------------
> -
> -Vimc has a few module parameters to configure the driver.
> +Module options
> +==============
>  
> -        param=value
> +Vimc has 2 module parameters to configure the driver.
>  
>  * ``sca_mult=<unsigned int>``
>  
> @@ -98,10 +276,10 @@ Vimc has a few module parameters to configure the driver.
>          otherwise the next odd number is considered (the default value is 3).
>  
>  Source code documentation
> --------------------------
> +=========================
>  
>  vimc-streamer
> -~~~~~~~~~~~~~
> +-------------
>  
>  .. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.h
>     :internal:
> 

Regards,

	Hans
Dafna Hirschfeld Sept. 23, 2019, 9:29 a.m. UTC | #2
On Fri, 2019-09-20 at 15:39 +0200, Hans Verkuil wrote:
> On 9/19/19 10:32 PM, Dafna Hirschfeld wrote:
> > Add explanation of how to use configfs in order to create a
> > vimc device with a given topology.
> > 
> > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> > ---
> >  Documentation/media/v4l-drivers/vimc.dot |  28 ++-
> >  Documentation/media/v4l-drivers/vimc.rst | 240 ++++++++++++++++++++---
> >  2 files changed, 220 insertions(+), 48 deletions(-)
> > 
> > diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
> > index 57863a13fa39..e3b41ac2bc46 100644
> > --- a/Documentation/media/v4l-drivers/vimc.dot
> > +++ b/Documentation/media/v4l-drivers/vimc.dot
> > @@ -2,21 +2,15 @@
> >  
> >  digraph board {
> >  	rankdir=TB
> > -	n00000001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> > -	n00000001:port0 -> n00000005:port0 [style=bold]
> > -	n00000001:port0 -> n0000000b [style=bold]
> > -	n00000003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> > -	n00000003:port0 -> n00000008:port0 [style=bold]
> > -	n00000003:port0 -> n0000000f [style=bold]
> > -	n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> > -	n00000005:port1 -> n00000017:port0
> > -	n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> > -	n00000008:port1 -> n00000017:port0 [style=dashed]
> > -	n0000000b [label="Raw Capture 0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> > -	n0000000f [label="Raw Capture 1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> > -	n00000013 [label="RGB/YUV Input\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> > -	n00000013 -> n00000017:port0 [style=dashed]
> > -	n00000017 [label="{{<port0> 0} | Scaler\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> > -	n00000017:port1 -> n0000001a [style=bold]
> > -	n0000001a [label="RGB/YUV Capture\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
> > +	n00000001 [label="cap-deb\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> > +	n00000005 [label="cap-sen\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> > +	n00000009 [label="cap-sca\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> > +	n0000000d [label="{{<port0> 0} | sca\n/dev/v4l-subdev0 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> > +	n0000000d:port1 -> n00000009 [style=bold]
> > +	n00000010 [label="{{<port0> 0} | deb\n/dev/v4l-subdev1 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> > +	n00000010:port1 -> n00000001 [style=bold]
> > +	n00000010:port1 -> n0000000d:port0 [style=bold]
> > +	n00000013 [label="{{} | sen\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> > +	n00000013:port0 -> n00000005 [style=bold]
> > +	n00000013:port0 -> n00000010:port0 [style=bold]
> >  }
> > diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
> > index a582af0509ee..e5636883545f 100644
> > --- a/Documentation/media/v4l-drivers/vimc.rst
> > +++ b/Documentation/media/v4l-drivers/vimc.rst
> > @@ -1,45 +1,225 @@
> >  .. SPDX-License-Identifier: GPL-2.0
> >  
> > +==========================================
> >  The Virtual Media Controller Driver (vimc)
> >  ==========================================
> >  
> > -The vimc driver emulates complex video hardware using the V4L2 API and the Media
> > -API. It has a capture device and three subdevices: sensor, debayer and scaler.
> > +The vimc driver emulates complex video hardware topologies using the V4L2 API
> > +and the Media API. It has a capture device and three subdevices:
> > +sensor, debayer and scaler. It exposes media devices through /dev/mediaX nodes,
> > +video capture devices through /dev/videoX and sub-devices through /dev/v4l-subdevX.
> > +
> > +
> > +To configure a media device of a given topology, a ConfigFS API is provided.
> > +
> > +Configuring a topology through ConfigFS (Experimental)
> > +======================================================
> > +
> > +.. note:: This API is under development and might change in the future.
> > +
> > +Mount configfs:
> > +::
> > +
> > +	$ mkdir /configfs
> > +	$ mount -t configfs none /configfs
> > +
> > +When loading the module, you will see a folder named vimc
> > +::
> > +
> > +	$ tree /configfs/
> > +	/configfs/
> > +	`-- vimc
> > +
> > +Creating a media device
> > +-----------------------
> > +
> > +To create a media device create a new folder under /configfs/vimc/
> > +
> > +Example:
> > +::
> > +
> > +	$ mkdir /configfs/vimc/mdev
> > +	$ tree /configfs/vimc/mdev
> > +	/configfs/
> > +	`-- vimc
> > +	    `-- mdev
> > +	        `-- hotplug
> > +
> > +Creating entities
> > +-----------------
> > +
> > +To create an entity in the media device's topology, create a folder under
> > +``/configfs/vimc/<mdev-name>/`` with the following format:
> > +
> > +	<entity-type>:<entity-name>
> 
> I suspect that there are restrictions to the characters that can be used in
> these names. For one the use of ':' in the entity-name is probably a bad idea.
> Also the entity name should be unique, i.e. you can't have a sensor and a scaler
> with the same entity name.
> 
Why is the use of ':' bad idea?
To which char you suggest to replace it?

> Such restrictions should be documented here and also tested in the driver!
> 
> > +
> > +Where <entity-type> is one of the following:
> > +
> > +- vimc-sensor
> > +- vimc-scaler
> > +- vimc-debayer
> > +- vimc-capture
> > +
> > +Example:
> > +::
> > +
> > +	$ mkdir /configfs/vimc/mdev/vimc-sensor:sen
> > +	$ mkdir /configfs/vimc/mdev/vimc-capture:cap-sen
> > +	$ tree /configfs/
> > +	/configfs/
> > +	`-- vimc
> > +	    `-- mdev
> > +		|-- hotplug
> > +		|-- vimc-capture:cap-sen
> > +		|   `-- pad:sink:0
> > +		`-- vimc-sensor:sen
> > +                    `-- pad:source:0
> 
> Do we need the 'pad:' prefix here? Isn't 'sink:' or 'source:' sufficient?
> I think pad:sink:0 is rather cumbersome.
> 
> > +
> > +Default folders are created under the entity directory for each pad of the entity.
> > +
> > +Creating links
> > +--------------
> > +
> > +To create a link between two entities, you should create a directory for the link
> > +under the source pad of the link and then set it to be a symbolic link to the sink pad:
> >  
> > -Topology
> > ---------
> > +Example:
> > +::
> > +
> > +	$ mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> 
> I'd call the directory 'to-cap-sen': it's clearer that this points to the
> cap-sen entity. It makes the example easier to understand.
> 
> > +	$ ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> > +	$ tree /configfs
> > +	/configfs
> > +	`-- vimc
> > +	    `-- mdev
> > +	        |-- hotplug
> > +	        |-- vimc-capture:cap-sen
> > +	        |   `-- pad:sink:0
> > +	        `-- vimc-sensor:sen
> > +	            `-- pad:source:0
> > +	                `-- to-cap
> > +	                    |-- enabled
> > +	                    |-- immutable
> > +	                    `-- pad:sink:0 -> ../../../../../vimc/mdev/vimc-capture:cap-sen/pad:sink:0
> > +
> > +The `enabled` and `immutable` are two boolean attributes of the link corresponding to the link flags
> > +
> > +
> > +Flag values are described in :ref:`Documentation/media/uapi/mediactl/media-types.rst <media-link-flag>`
> > +( seek for ``MEDIA_LNK_FL_*``)
> > +
> > +1 - Enabled
> > +	Indicates that the link will be enabled when the media device is created.
> > +
> > +3 - Enabled and Immutable
> > +	Indicates that the link enabled state can't be modified at runtime.
> > +
> > +Change an attribute of the link by writing "on" or "1" to set it on , and "off" or "0" to set it off
> > +
> > +Example:
> > +::
> > +
> > +	$ echo "on" > /configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable
> > +
> > +Activating/Deactivating device
> > +------------------------------
> > +
> > +To activate the device, write one of "plugged", "plug" or "1" to the file
> > +``/configfs/vimc/<mdev-name>/hotplug``
> > +
> > +Example:
> > +::
> > +
> > +	$ echo 1 > /configfs/vimc/mdev/hotplug
> > +
> > +You should see a new node ``/dev/mediaX`` in your devfs.
> > +
> > +To deactivate the device, write one of "unplugged", "unplug" or "0" to the file
> > +``/configfs/vimc/<ndev-name>/hotplug``
> > +
> > +Example:
> > +::
> > +
> > +	$ echo unplugged > /configfs/vimc/mdev/hotplug
> > +
> > +Topology Configuration - Full Example
> > +-------------------------------------
> > +
> > +Here is a full example of a simple topology configuration:
> > +
> > +.. code-block:: bash
> > +
> > +    # Creating the entities
> > +    mkdir "/configfs/vimc/mdev"
> > +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen"
> > +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb"
> > +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca"
> > +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sca" #/dev/video2
> > +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sen" #/dev/video1
> > +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-deb" #/dev/video0
> > +
> > +    # Creating the links
> > +    #sen -> deb
> > +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
> > +    ln -s "/configfs/vimc/mdev/vimc-debayer:deb/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
> > +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/immutable"
> 
> If you set immutable to on, then it should automatically set enabled to on as well,
> so no need for the next line.

Andrzej Pietrasiewicz suggested not to allow setting the 'immutable' to on if 'enabled' is off
and not to allow setting 'enabled' to off if 'immutable' it on.
He says this is better than an interface where a change in one file cause a change in another file.

> 
> > +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/enabled"
> > +
> > +    #deb -> sca
> > +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
> > +    ln -s "/configfs/vimc/mdev/vimc-scaler:sca/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
> > +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/immutable"
> > +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/enabled"
> > +
> > +    #sca -> cap-sca
> > +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
> 
> Same in this example: use to-cap-sca
> 
> > +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sca/pad:sink:0" "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
> > +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/immutable"
> > +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/enabled"
> > +
> > +    #sen -> cap-sen
> > +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> > +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> > +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable"
> > +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/enabled"
> > +
> > +    #deb -> cap-deb
> > +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
> > +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-deb/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
> > +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/immutable"
> > +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/enabled"
> >  
> > -The topology is hardcoded, although you could modify it in vimc-core and
> > -recompile the driver to achieve your own topology. This is the default topology:
> >  
> >  .. _vimc_topology_graph:
> >  
> >  .. kernel-figure:: vimc.dot
> > -    :alt:   Diagram of the default media pipeline topology
> > +    :alt:   Diagram of the configured simple topology in the example
> >      :align: center
> >  
> > -    Media pipeline graph on vimc
> > +    Simple Media pipeline graph on vimc configured through configfs
> >  
> > -Configuring the topology
> > -~~~~~~~~~~~~~~~~~~~~~~~~
> > +Configuring the pipeline formats
> > +================================
> >  
> > -Each subdevice will come with its default configuration (pixelformat, height,
> > -width, ...). One needs to configure the topology in order to match the
> > +Each subdevice has a default format configuration (pixelformat, height,
> > +width, ...). You should configure the formats in order to match the
> >  configuration on each linked subdevice to stream frames through the pipeline.
> > -If the configuration doesn't match, the stream will fail. The ``v4l-utils``
> > +If the configuration doesn't match, streaming will fail. The ``v4l-utils``
> >  package is a bundle of user-space applications, that comes with ``media-ctl`` and
> > -``v4l2-ctl`` that can be used to configure the vimc configuration. This sequence
> > -of commands fits for the default topology:
> > +``v4l2-ctl`` that can be used to configure the formats of the entities. This sequence
> > +of commands fits the simple topology created in the full example of topology configuration:
> >  
> >  .. code-block:: bash
> >  
> > -        media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
> > -        media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
> > -        media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
> > -        media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
> > -        v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
> > -        v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
> > -        v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
> > +	media-ctl -d platform:vimc-000 -V '"sen":0[fmt:SBGGR8_1X8/640x480]'
> > +	media-ctl -d platform:vimc-000 -V '"deb":0[fmt:SBGGR8_1X8/640x480]'
> > +	media-ctl -d platform:vimc-000 -V '"deb":1[fmt:RGB888_1X24/640x480]'
> > +	media-ctl -d platform:vimc-000 -V '"sca":0[fmt:RGB888_1X24/640x480]'
> > +	media-ctl -d platform:vimc-000 -V '"sca":1[fmt:RGB888_1X24/640x480]'
> > +	v4l2-ctl -z platform:vimc-000 -d "cap-sen" -v pixelformat=BA81
> > +	v4l2-ctl -z platform:vimc-000 -d "cap-deb" -v pixelformat=RGB3
> > +	# The default scaling value of the scaler is 3, so need to set its capture accordingly
> > +	v4l2-ctl -z platform:vimc -d "cap-sca" -v pixelformat=RGB3,width=1920,height=1440
> >  
> >  Subdevices
> >  ----------
> > @@ -61,8 +241,8 @@ vimc-debayer:
> >  	* 1 Pad source
> >  
> >  vimc-scaler:
> > -	Scale up the image by a factor of 3. E.g.: a 640x480 image becomes a
> > -        1920x1440 image. (this value can be configured, see at
> > +	Scales up the image by a factor of 3. E.g.: a 640x480 image becomes a
> > +        1920x1440 image. (this value can be configured, see
> 
> Note that this might change with this patch:
> 
> https://patchwork.kernel.org/patch/11146175/
> 
> Just so you are aware of this.
> 
> >          `Module options`_).
> >  	Exposes:
> >  
> > @@ -77,12 +257,10 @@ vimc-capture:
> >  	* 1 Pad source
> >  
> >  
> > -        Module options
> > ----------------
> > -
> > -Vimc has a few module parameters to configure the driver.
> > +Module options
> > +==============
> >  
> > -        param=value
> > +Vimc has 2 module parameters to configure the driver.
> >  
> >  * ``sca_mult=<unsigned int>``
> >  
> > @@ -98,10 +276,10 @@ Vimc has a few module parameters to configure the driver.
> >          otherwise the next odd number is considered (the default value is 3).
> >  
> >  Source code documentation
> > --------------------------
> > +=========================
> >  
> >  vimc-streamer
> > -~~~~~~~~~~~~~
> > +-------------
> >  
> >  .. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.h
> >     :internal:
> > 
> 
> Regards,
> 
> 	Hans

Thanks,

Dafna
Hans Verkuil Sept. 23, 2019, 9:50 a.m. UTC | #3
On 9/23/19 11:29 AM, Dafna Hirschfeld wrote:
> On Fri, 2019-09-20 at 15:39 +0200, Hans Verkuil wrote:
>> On 9/19/19 10:32 PM, Dafna Hirschfeld wrote:
>>> Add explanation of how to use configfs in order to create a
>>> vimc device with a given topology.
>>>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>> ---
>>>  Documentation/media/v4l-drivers/vimc.dot |  28 ++-
>>>  Documentation/media/v4l-drivers/vimc.rst | 240 ++++++++++++++++++++---
>>>  2 files changed, 220 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
>>> index 57863a13fa39..e3b41ac2bc46 100644
>>> --- a/Documentation/media/v4l-drivers/vimc.dot
>>> +++ b/Documentation/media/v4l-drivers/vimc.dot
>>> @@ -2,21 +2,15 @@
>>>  
>>>  digraph board {
>>>  	rankdir=TB
>>> -	n00000001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
>>> -	n00000001:port0 -> n00000005:port0 [style=bold]
>>> -	n00000001:port0 -> n0000000b [style=bold]
>>> -	n00000003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
>>> -	n00000003:port0 -> n00000008:port0 [style=bold]
>>> -	n00000003:port0 -> n0000000f [style=bold]
>>> -	n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>>> -	n00000005:port1 -> n00000017:port0
>>> -	n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>>> -	n00000008:port1 -> n00000017:port0 [style=dashed]
>>> -	n0000000b [label="Raw Capture 0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
>>> -	n0000000f [label="Raw Capture 1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
>>> -	n00000013 [label="RGB/YUV Input\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
>>> -	n00000013 -> n00000017:port0 [style=dashed]
>>> -	n00000017 [label="{{<port0> 0} | Scaler\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>>> -	n00000017:port1 -> n0000001a [style=bold]
>>> -	n0000001a [label="RGB/YUV Capture\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
>>> +	n00000001 [label="cap-deb\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
>>> +	n00000005 [label="cap-sen\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
>>> +	n00000009 [label="cap-sca\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
>>> +	n0000000d [label="{{<port0> 0} | sca\n/dev/v4l-subdev0 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>>> +	n0000000d:port1 -> n00000009 [style=bold]
>>> +	n00000010 [label="{{<port0> 0} | deb\n/dev/v4l-subdev1 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>>> +	n00000010:port1 -> n00000001 [style=bold]
>>> +	n00000010:port1 -> n0000000d:port0 [style=bold]
>>> +	n00000013 [label="{{} | sen\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
>>> +	n00000013:port0 -> n00000005 [style=bold]
>>> +	n00000013:port0 -> n00000010:port0 [style=bold]
>>>  }
>>> diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
>>> index a582af0509ee..e5636883545f 100644
>>> --- a/Documentation/media/v4l-drivers/vimc.rst
>>> +++ b/Documentation/media/v4l-drivers/vimc.rst
>>> @@ -1,45 +1,225 @@
>>>  .. SPDX-License-Identifier: GPL-2.0
>>>  
>>> +==========================================
>>>  The Virtual Media Controller Driver (vimc)
>>>  ==========================================
>>>  
>>> -The vimc driver emulates complex video hardware using the V4L2 API and the Media
>>> -API. It has a capture device and three subdevices: sensor, debayer and scaler.
>>> +The vimc driver emulates complex video hardware topologies using the V4L2 API
>>> +and the Media API. It has a capture device and three subdevices:
>>> +sensor, debayer and scaler. It exposes media devices through /dev/mediaX nodes,
>>> +video capture devices through /dev/videoX and sub-devices through /dev/v4l-subdevX.
>>> +
>>> +
>>> +To configure a media device of a given topology, a ConfigFS API is provided.
>>> +
>>> +Configuring a topology through ConfigFS (Experimental)
>>> +======================================================
>>> +
>>> +.. note:: This API is under development and might change in the future.
>>> +
>>> +Mount configfs:
>>> +::
>>> +
>>> +	$ mkdir /configfs
>>> +	$ mount -t configfs none /configfs
>>> +
>>> +When loading the module, you will see a folder named vimc
>>> +::
>>> +
>>> +	$ tree /configfs/
>>> +	/configfs/
>>> +	`-- vimc
>>> +
>>> +Creating a media device
>>> +-----------------------
>>> +
>>> +To create a media device create a new folder under /configfs/vimc/
>>> +
>>> +Example:
>>> +::
>>> +
>>> +	$ mkdir /configfs/vimc/mdev
>>> +	$ tree /configfs/vimc/mdev
>>> +	/configfs/
>>> +	`-- vimc
>>> +	    `-- mdev
>>> +	        `-- hotplug
>>> +
>>> +Creating entities
>>> +-----------------
>>> +
>>> +To create an entity in the media device's topology, create a folder under
>>> +``/configfs/vimc/<mdev-name>/`` with the following format:
>>> +
>>> +	<entity-type>:<entity-name>
>>
>> I suspect that there are restrictions to the characters that can be used in
>> these names. For one the use of ':' in the entity-name is probably a bad idea.
>> Also the entity name should be unique, i.e. you can't have a sensor and a scaler
>> with the same entity name.
>>
> Why is the use of ':' bad idea?
> To which char you suggest to replace it?

You are right, ':' is allowed in the entity-name. So ignore that comment.

But entity-name must be unique within the mdev directory. That's a MC requirement.

> 
>> Such restrictions should be documented here and also tested in the driver!
>>
>>> +
>>> +Where <entity-type> is one of the following:
>>> +
>>> +- vimc-sensor
>>> +- vimc-scaler
>>> +- vimc-debayer
>>> +- vimc-capture
>>> +
>>> +Example:
>>> +::
>>> +
>>> +	$ mkdir /configfs/vimc/mdev/vimc-sensor:sen
>>> +	$ mkdir /configfs/vimc/mdev/vimc-capture:cap-sen
>>> +	$ tree /configfs/
>>> +	/configfs/
>>> +	`-- vimc
>>> +	    `-- mdev
>>> +		|-- hotplug
>>> +		|-- vimc-capture:cap-sen
>>> +		|   `-- pad:sink:0
>>> +		`-- vimc-sensor:sen
>>> +                    `-- pad:source:0
>>
>> Do we need the 'pad:' prefix here? Isn't 'sink:' or 'source:' sufficient?
>> I think pad:sink:0 is rather cumbersome.
>>
>>> +
>>> +Default folders are created under the entity directory for each pad of the entity.
>>> +
>>> +Creating links
>>> +--------------
>>> +
>>> +To create a link between two entities, you should create a directory for the link
>>> +under the source pad of the link and then set it to be a symbolic link to the sink pad:
>>>  
>>> -Topology
>>> ---------
>>> +Example:
>>> +::
>>> +
>>> +	$ mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
>>
>> I'd call the directory 'to-cap-sen': it's clearer that this points to the
>> cap-sen entity. It makes the example easier to understand.
>>
>>> +	$ ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
>>> +	$ tree /configfs
>>> +	/configfs
>>> +	`-- vimc
>>> +	    `-- mdev
>>> +	        |-- hotplug
>>> +	        |-- vimc-capture:cap-sen
>>> +	        |   `-- pad:sink:0
>>> +	        `-- vimc-sensor:sen
>>> +	            `-- pad:source:0
>>> +	                `-- to-cap
>>> +	                    |-- enabled
>>> +	                    |-- immutable
>>> +	                    `-- pad:sink:0 -> ../../../../../vimc/mdev/vimc-capture:cap-sen/pad:sink:0
>>> +
>>> +The `enabled` and `immutable` are two boolean attributes of the link corresponding to the link flags
>>> +
>>> +
>>> +Flag values are described in :ref:`Documentation/media/uapi/mediactl/media-types.rst <media-link-flag>`
>>> +( seek for ``MEDIA_LNK_FL_*``)
>>> +
>>> +1 - Enabled
>>> +	Indicates that the link will be enabled when the media device is created.
>>> +
>>> +3 - Enabled and Immutable
>>> +	Indicates that the link enabled state can't be modified at runtime.
>>> +
>>> +Change an attribute of the link by writing "on" or "1" to set it on , and "off" or "0" to set it off
>>> +
>>> +Example:
>>> +::
>>> +
>>> +	$ echo "on" > /configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable
>>> +
>>> +Activating/Deactivating device
>>> +------------------------------
>>> +
>>> +To activate the device, write one of "plugged", "plug" or "1" to the file
>>> +``/configfs/vimc/<mdev-name>/hotplug``
>>> +
>>> +Example:
>>> +::
>>> +
>>> +	$ echo 1 > /configfs/vimc/mdev/hotplug
>>> +
>>> +You should see a new node ``/dev/mediaX`` in your devfs.
>>> +
>>> +To deactivate the device, write one of "unplugged", "unplug" or "0" to the file
>>> +``/configfs/vimc/<ndev-name>/hotplug``
>>> +
>>> +Example:
>>> +::
>>> +
>>> +	$ echo unplugged > /configfs/vimc/mdev/hotplug
>>> +
>>> +Topology Configuration - Full Example
>>> +-------------------------------------
>>> +
>>> +Here is a full example of a simple topology configuration:
>>> +
>>> +.. code-block:: bash
>>> +
>>> +    # Creating the entities
>>> +    mkdir "/configfs/vimc/mdev"
>>> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen"
>>> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb"
>>> +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca"
>>> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sca" #/dev/video2
>>> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sen" #/dev/video1
>>> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-deb" #/dev/video0
>>> +
>>> +    # Creating the links
>>> +    #sen -> deb
>>> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
>>> +    ln -s "/configfs/vimc/mdev/vimc-debayer:deb/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
>>> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/immutable"
>>
>> If you set immutable to on, then it should automatically set enabled to on as well,
>> so no need for the next line.
> 
> Andrzej Pietrasiewicz suggested not to allow setting the 'immutable' to on if 'enabled' is off
> and not to allow setting 'enabled' to off if 'immutable' it on.
> He says this is better than an interface where a change in one file cause a change in another file.

I wonder if this shouldn't be changed to just a single file called 'type'. And it
can be set to 'immutable', 'enabled'/'on' and 'disabled'/'off'. It defaults to
'off'.

That way you don't have this dependency between two properties.

It feels cleaner that way.

> 
>>
>>> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/enabled"
>>> +
>>> +    #deb -> sca
>>> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
>>> +    ln -s "/configfs/vimc/mdev/vimc-scaler:sca/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
>>> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/immutable"
>>> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/enabled"
>>> +
>>> +    #sca -> cap-sca
>>> +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
>>
>> Same in this example: use to-cap-sca
>>
>>> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sca/pad:sink:0" "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
>>> +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/immutable"
>>> +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/enabled"
>>> +
>>> +    #sen -> cap-sen
>>> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
>>> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
>>> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable"
>>> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/enabled"
>>> +
>>> +    #deb -> cap-deb
>>> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
>>> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-deb/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
>>> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/immutable"
>>> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/enabled"
>>>  
>>> -The topology is hardcoded, although you could modify it in vimc-core and
>>> -recompile the driver to achieve your own topology. This is the default topology:
>>>  
>>>  .. _vimc_topology_graph:
>>>  
>>>  .. kernel-figure:: vimc.dot
>>> -    :alt:   Diagram of the default media pipeline topology
>>> +    :alt:   Diagram of the configured simple topology in the example
>>>      :align: center
>>>  
>>> -    Media pipeline graph on vimc
>>> +    Simple Media pipeline graph on vimc configured through configfs
>>>  
>>> -Configuring the topology
>>> -~~~~~~~~~~~~~~~~~~~~~~~~
>>> +Configuring the pipeline formats
>>> +================================
>>>  
>>> -Each subdevice will come with its default configuration (pixelformat, height,
>>> -width, ...). One needs to configure the topology in order to match the
>>> +Each subdevice has a default format configuration (pixelformat, height,
>>> +width, ...). You should configure the formats in order to match the
>>>  configuration on each linked subdevice to stream frames through the pipeline.
>>> -If the configuration doesn't match, the stream will fail. The ``v4l-utils``
>>> +If the configuration doesn't match, streaming will fail. The ``v4l-utils``
>>>  package is a bundle of user-space applications, that comes with ``media-ctl`` and
>>> -``v4l2-ctl`` that can be used to configure the vimc configuration. This sequence
>>> -of commands fits for the default topology:
>>> +``v4l2-ctl`` that can be used to configure the formats of the entities. This sequence
>>> +of commands fits the simple topology created in the full example of topology configuration:
>>>  
>>>  .. code-block:: bash
>>>  
>>> -        media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
>>> -        media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
>>> -        media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
>>> -        media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
>>> -        v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
>>> -        v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
>>> -        v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
>>> +	media-ctl -d platform:vimc-000 -V '"sen":0[fmt:SBGGR8_1X8/640x480]'
>>> +	media-ctl -d platform:vimc-000 -V '"deb":0[fmt:SBGGR8_1X8/640x480]'
>>> +	media-ctl -d platform:vimc-000 -V '"deb":1[fmt:RGB888_1X24/640x480]'
>>> +	media-ctl -d platform:vimc-000 -V '"sca":0[fmt:RGB888_1X24/640x480]'
>>> +	media-ctl -d platform:vimc-000 -V '"sca":1[fmt:RGB888_1X24/640x480]'
>>> +	v4l2-ctl -z platform:vimc-000 -d "cap-sen" -v pixelformat=BA81
>>> +	v4l2-ctl -z platform:vimc-000 -d "cap-deb" -v pixelformat=RGB3
>>> +	# The default scaling value of the scaler is 3, so need to set its capture accordingly
>>> +	v4l2-ctl -z platform:vimc -d "cap-sca" -v pixelformat=RGB3,width=1920,height=1440
>>>  
>>>  Subdevices
>>>  ----------
>>> @@ -61,8 +241,8 @@ vimc-debayer:
>>>  	* 1 Pad source
>>>  
>>>  vimc-scaler:
>>> -	Scale up the image by a factor of 3. E.g.: a 640x480 image becomes a
>>> -        1920x1440 image. (this value can be configured, see at
>>> +	Scales up the image by a factor of 3. E.g.: a 640x480 image becomes a
>>> +        1920x1440 image. (this value can be configured, see
>>
>> Note that this might change with this patch:
>>
>> https://patchwork.kernel.org/patch/11146175/
>>
>> Just so you are aware of this.
>>
>>>          `Module options`_).
>>>  	Exposes:
>>>  
>>> @@ -77,12 +257,10 @@ vimc-capture:
>>>  	* 1 Pad source
>>>  
>>>  
>>> -        Module options
>>> ----------------
>>> -
>>> -Vimc has a few module parameters to configure the driver.
>>> +Module options
>>> +==============
>>>  
>>> -        param=value
>>> +Vimc has 2 module parameters to configure the driver.
>>>  
>>>  * ``sca_mult=<unsigned int>``
>>>  
>>> @@ -98,10 +276,10 @@ Vimc has a few module parameters to configure the driver.
>>>          otherwise the next odd number is considered (the default value is 3).
>>>  
>>>  Source code documentation
>>> --------------------------
>>> +=========================
>>>  
>>>  vimc-streamer
>>> -~~~~~~~~~~~~~
>>> +-------------
>>>  
>>>  .. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.h
>>>     :internal:
>>>
>>
>> Regards,
>>
>> 	Hans
> 
> Thanks,
> 
> Dafna
> 

Regards,

	Hans
Shuah Khan Sept. 26, 2019, 6:59 p.m. UTC | #4
On 9/19/19 2:32 PM, Dafna Hirschfeld wrote:
> Add explanation of how to use configfs in order to create a
> vimc device with a given topology.
> 

Can you add more detail on the problem configfs solves and what
value it adds.

> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>   Documentation/media/v4l-drivers/vimc.dot |  28 ++-
>   Documentation/media/v4l-drivers/vimc.rst | 240 ++++++++++++++++++++---
>   2 files changed, 220 insertions(+), 48 deletions(-)
> 
> diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
> index 57863a13fa39..e3b41ac2bc46 100644
> --- a/Documentation/media/v4l-drivers/vimc.dot
> +++ b/Documentation/media/v4l-drivers/vimc.dot
> @@ -2,21 +2,15 @@
>   
>   digraph board {
>   	rankdir=TB
> -	n00000001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000001:port0 -> n00000005:port0 [style=bold]
> -	n00000001:port0 -> n0000000b [style=bold]
> -	n00000003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000003:port0 -> n00000008:port0 [style=bold]
> -	n00000003:port0 -> n0000000f [style=bold]
> -	n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000005:port1 -> n00000017:port0
> -	n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000008:port1 -> n00000017:port0 [style=dashed]
> -	n0000000b [label="Raw Capture 0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> -	n0000000f [label="Raw Capture 1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> -	n00000013 [label="RGB/YUV Input\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> -	n00000013 -> n00000017:port0 [style=dashed]
> -	n00000017 [label="{{<port0> 0} | Scaler\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> -	n00000017:port1 -> n0000001a [style=bold]
> -	n0000001a [label="RGB/YUV Capture\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
> +	n00000001 [label="cap-deb\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> +	n00000005 [label="cap-sen\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> +	n00000009 [label="cap-sca\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> +	n0000000d [label="{{<port0> 0} | sca\n/dev/v4l-subdev0 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n0000000d:port1 -> n00000009 [style=bold]
> +	n00000010 [label="{{<port0> 0} | deb\n/dev/v4l-subdev1 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000010:port1 -> n00000001 [style=bold]
> +	n00000010:port1 -> n0000000d:port0 [style=bold]
> +	n00000013 [label="{{} | sen\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000013:port0 -> n00000005 [style=bold]
> +	n00000013:port0 -> n00000010:port0 [style=bold]
>   }
> diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
> index a582af0509ee..e5636883545f 100644
> --- a/Documentation/media/v4l-drivers/vimc.rst
> +++ b/Documentation/media/v4l-drivers/vimc.rst
> @@ -1,45 +1,225 @@
>   .. SPDX-License-Identifier: GPL-2.0
>   
> +==========================================
>   The Virtual Media Controller Driver (vimc)
>   ==========================================
>   
> -The vimc driver emulates complex video hardware using the V4L2 API and the Media
> -API. It has a capture device and three subdevices: sensor, debayer and scaler.
> +The vimc driver emulates complex video hardware topologies using the V4L2 API
> +and the Media API. It has a capture device and three subdevices:

Add blank line in between - helps with readability

> +sensor, debayer and scaler. It exposes media devices through /dev/mediaX nodes,
> +video capture devices through /dev/videoX and sub-devices through /dev/v4l-subdevX.
> +
> +
> +To configure a media device of a given topology, a ConfigFS API is provided.

ConfigFS API enables the ability dynamically configure a media device
and its topology. This will help customize topology for specific testing
needs (?)

Assuming that is the goal for this work.

> +
> +Configuring a topology through ConfigFS (Experimental)
> +======================================================
> +
> +.. note:: This API is under development and might change in the future. > +
> +Mount configfs:
> +::
> +
> +	$ mkdir /configfs
> +	$ mount -t configfs none /configfs
> +
> +When loading the module, you will see a folder named vimc
> +::
> +
> +	$ tree /configfs/
> +	/configfs/
> +	`-- vimc
> +
> +Creating a media device
> +-----------------------
> +
> +To create a media device create a new folder under /configfs/vimc/
Change this to:

How to create a media device or flip the sentence around:

Create a folder under /configfs/vimc/ to create media device.

> +
> +Example:
> +::
> +
> +	$ mkdir /configfs/vimc/mdev
> +	$ tree /configfs/vimc/mdev
> +	/configfs/
> +	`-- vimc
> +	    `-- mdev
> +	        `-- hotplug
> +
> +Creating entities
> +-----------------
> +
> +To create an entity in the media device's topology, create a folder under

Same comment as above.

> +``/configfs/vimc/<mdev-name>/`` with the following format:
> +
> +	<entity-type>:<entity-name>
> +
> +Where <entity-type> is one of the following:
> +
> +- vimc-sensor
> +- vimc-scaler
> +- vimc-debayer
> +- vimc-capture
> +
> +Example:
> +::
> +
> +	$ mkdir /configfs/vimc/mdev/vimc-sensor:sen
> +	$ mkdir /configfs/vimc/mdev/vimc-capture:cap-sen
> +	$ tree /configfs/
> +	/configfs/
> +	`-- vimc
> +	    `-- mdev
> +		|-- hotplug
> +		|-- vimc-capture:cap-sen
> +		|   `-- pad:sink:0
> +		`-- vimc-sensor:sen
> +                    `-- pad:source:0
> +
> +Default folders are created under the entity directory for each pad of the entity.
> +
> +Creating links
> +--------------
> +
> +To create a link between two entities, you should create a directory for the link
> +under the source pad of the link and then set it to be a symbolic link to the sink pad:

Same comment as above.

>   
> -Topology
> ---------
> +Example:
> +::
> +
> +	$ mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +	$ ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +	$ tree /configfs
> +	/configfs
> +	`-- vimc
> +	    `-- mdev
> +	        |-- hotplug
> +	        |-- vimc-capture:cap-sen
> +	        |   `-- pad:sink:0
> +	        `-- vimc-sensor:sen
> +	            `-- pad:source:0
> +	                `-- to-cap
> +	                    |-- enabled
> +	                    |-- immutable
> +	                    `-- pad:sink:0 -> ../../../../../vimc/mdev/vimc-capture:cap-sen/pad:sink:0
> +
> +The `enabled` and `immutable` are two boolean attributes of the link corresponding to the link flags
> +
> +
> +Flag values are described in :ref:`Documentation/media/uapi/mediactl/media-types.rst <media-link-flag>`
> +( seek for ``MEDIA_LNK_FL_*``)
> +
> +1 - Enabled
> +	Indicates that the link will be enabled when the media device is created.
> +
> +3 - Enabled and Immutable
> +	Indicates that the link enabled state can't be modified at runtime.
> +
> +Change an attribute of the link by writing "on" or "1" to set it on , and "off" or "0" to set it off

You can change

> +
> +Example:
> +::
> +
> +	$ echo "on" > /configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable
> +
> +Activating/Deactivating device
> +------------------------------
> +
> +To activate the device, write one of "plugged", "plug" or "1" to the file

Same comment as above.

> +``/configfs/vimc/<mdev-name>/hotplug``
> +
> +Example:
> +::
> +
> +	$ echo 1 > /configfs/vimc/mdev/hotplug
> +
> +You should see a new node ``/dev/mediaX`` in your devfs.
> +
> +To deactivate the device, write one of "unplugged", "unplug" or "0" to the file
> +``/configfs/vimc/<ndev-name>/hotplug``

Same comment as above. In general don't start sentence with "To"

> +
> +Example:
> +::
> +
> +	$ echo unplugged > /configfs/vimc/mdev/hotplug
> +
> +Topology Configuration - Full Example
> +-------------------------------------
> +
> +Here is a full example of a simple topology configuration:
> +
> +.. code-block:: bash
> +
> +    # Creating the entities
> +    mkdir "/configfs/vimc/mdev"
> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen"
> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb"
> +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca"
> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sca" #/dev/video2
> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sen" #/dev/video1
> +    mkdir "/configfs/vimc/mdev/vimc-capture:cap-deb" #/dev/video0
> +
> +    # Creating the links
> +    #sen -> deb
> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
> +    ln -s "/configfs/vimc/mdev/vimc-debayer:deb/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/enabled"
> +
> +    #deb -> sca
> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
> +    ln -s "/configfs/vimc/mdev/vimc-scaler:sca/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/enabled"
> +
> +    #sca -> cap-sca
> +    mkdir "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sca/pad:sink:0" "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
> +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/enabled"
> +
> +    #sen -> cap-sen
> +    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/enabled"
> +
> +    #deb -> cap-deb
> +    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
> +    ln -s "/configfs/vimc/mdev/vimc-capture:cap-deb/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/immutable"
> +    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/enabled"
>   
> -The topology is hardcoded, although you could modify it in vimc-core and
> -recompile the driver to achieve your own topology. This is the default topology:

Does this mean, there is no default topology. I think default topology
should still be an option even if it is a module option.

>   
>   .. _vimc_topology_graph:
>   
>   .. kernel-figure:: vimc.dot
> -    :alt:   Diagram of the default media pipeline topology
> +    :alt:   Diagram of the configured simple topology in the example
>       :align: center
>   
> -    Media pipeline graph on vimc
> +    Simple Media pipeline graph on vimc configured through configfs
>   
> -Configuring the topology
> -~~~~~~~~~~~~~~~~~~~~~~~~
> +Configuring the pipeline formats
> +================================
>   
> -Each subdevice will come with its default configuration (pixelformat, height,
> -width, ...). One needs to configure the topology in order to match the
> +Each subdevice has a default format configuration (pixelformat, height,
> +width, ...). You should configure the formats in order to match the
>   configuration on each linked subdevice to stream frames through the pipeline.
> -If the configuration doesn't match, the stream will fail. The ``v4l-utils``
> +If the configuration doesn't match, streaming will fail. The ``v4l-utils``
>   package is a bundle of user-space applications, that comes with ``media-ctl`` and

replace "that comes with" with "which includes"

> -``v4l2-ctl`` that can be used to configure the vimc configuration. This sequence
> -of commands fits for the default topology:
> +``v4l2-ctl`` that can be used to configure the formats of the entities. This sequence
> +of commands fits the simple topology created in the full example of topology configuration:
>   
>   .. code-block:: bash
>   
> -        media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
> -        media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
> -        media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
> -        media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
> -        v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
> -        v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
> -        v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
> +	media-ctl -d platform:vimc-000 -V '"sen":0[fmt:SBGGR8_1X8/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"deb":0[fmt:SBGGR8_1X8/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"deb":1[fmt:RGB888_1X24/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"sca":0[fmt:RGB888_1X24/640x480]'
> +	media-ctl -d platform:vimc-000 -V '"sca":1[fmt:RGB888_1X24/640x480]'
> +	v4l2-ctl -z platform:vimc-000 -d "cap-sen" -v pixelformat=BA81
> +	v4l2-ctl -z platform:vimc-000 -d "cap-deb" -v pixelformat=RGB3
> +	# The default scaling value of the scaler is 3, so need to set its capture accordingly
> +	v4l2-ctl -z platform:vimc -d "cap-sca" -v pixelformat=RGB3,width=1920,height=1440
>   
>   Subdevices
>   ----------
> @@ -61,8 +241,8 @@ vimc-debayer:
>   	* 1 Pad source
>   
>   vimc-scaler:
> -	Scale up the image by a factor of 3. E.g.: a 640x480 image becomes a
> -        1920x1440 image. (this value can be configured, see at
> +	Scales up the image by a factor of 3. E.g.: a 640x480 image becomes a
> +        1920x1440 image. (this value can be configured, see
>           `Module options`_).
>   	Exposes:
>   
> @@ -77,12 +257,10 @@ vimc-capture:
>   	* 1 Pad source
>   
>   
> -        Module options
> ----------------
> -
> -Vimc has a few module parameters to configure the driver.
> +Module options
> +==============
>   
> -        param=value
> +Vimc has 2 module parameters to configure the driver.
>   
>   * ``sca_mult=<unsigned int>``
>   
> @@ -98,10 +276,10 @@ Vimc has a few module parameters to configure the driver.
>           otherwise the next odd number is considered (the default value is 3).
>   

Keep this close to the top of the file and then add the configfs stuff.

>   Source code documentation
> --------------------------
> +=========================
>   
>   vimc-streamer
> -~~~~~~~~~~~~~
> +-------------
>   
>   .. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.h
>      :internal:
> 

thanks,
-- Shuah
Andrzej Pietrasiewicz Sept. 27, 2019, 5:56 a.m. UTC | #5
Hello All,

W dniu 23.09.2019 o 11:50, Hans Verkuil pisze:
> On 9/23/19 11:29 AM, Dafna Hirschfeld wrote:
>> On Fri, 2019-09-20 at 15:39 +0200, Hans Verkuil wrote:
>>> On 9/19/19 10:32 PM, Dafna Hirschfeld wrote:
>>>> Add explanation of how to use configfs in order to create a
>>>> vimc device with a given topology.
>>>>
>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>> ---
>>>>   Documentation/media/v4l-drivers/vimc.dot |  28 ++-
>>>>   Documentation/media/v4l-drivers/vimc.rst | 240 ++++++++++++++++++++---
>>>>   2 files changed, 220 insertions(+), 48 deletions(-)
>>>>

When adding new sysfs or configfs directories/files one should add
appropriate entries to Documentation/ABI/*.

My feeling is that this patch should be merged with the patch actually
enabling the configfs interface, which is patch 4/5. The reason is twofold.

The first is consistency: if a documentation for an interface is added
in a different patch than the interface itself, then at the first of the
two commits in question we either have a documentation for a non-existing
interface, or an undocumented interface.

The second is that the documentation is not only for kernel developers
but also for users who don't have the faintest idea of what code is
inside the kernel, so even though patch 3/5 of this series adds
the code implementing the interface, it is not available to users
until patch 4/5.

Regards,

Andrzej
diff mbox series

Patch

diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
index 57863a13fa39..e3b41ac2bc46 100644
--- a/Documentation/media/v4l-drivers/vimc.dot
+++ b/Documentation/media/v4l-drivers/vimc.dot
@@ -2,21 +2,15 @@ 
 
 digraph board {
 	rankdir=TB
-	n00000001 [label="{{} | Sensor A\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
-	n00000001:port0 -> n00000005:port0 [style=bold]
-	n00000001:port0 -> n0000000b [style=bold]
-	n00000003 [label="{{} | Sensor B\n/dev/v4l-subdev1 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
-	n00000003:port0 -> n00000008:port0 [style=bold]
-	n00000003:port0 -> n0000000f [style=bold]
-	n00000005 [label="{{<port0> 0} | Debayer A\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
-	n00000005:port1 -> n00000017:port0
-	n00000008 [label="{{<port0> 0} | Debayer B\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
-	n00000008:port1 -> n00000017:port0 [style=dashed]
-	n0000000b [label="Raw Capture 0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
-	n0000000f [label="Raw Capture 1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
-	n00000013 [label="RGB/YUV Input\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
-	n00000013 -> n00000017:port0 [style=dashed]
-	n00000017 [label="{{<port0> 0} | Scaler\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
-	n00000017:port1 -> n0000001a [style=bold]
-	n0000001a [label="RGB/YUV Capture\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
+	n00000001 [label="cap-deb\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
+	n00000005 [label="cap-sen\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
+	n00000009 [label="cap-sca\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
+	n0000000d [label="{{<port0> 0} | sca\n/dev/v4l-subdev0 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
+	n0000000d:port1 -> n00000009 [style=bold]
+	n00000010 [label="{{<port0> 0} | deb\n/dev/v4l-subdev1 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
+	n00000010:port1 -> n00000001 [style=bold]
+	n00000010:port1 -> n0000000d:port0 [style=bold]
+	n00000013 [label="{{} | sen\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
+	n00000013:port0 -> n00000005 [style=bold]
+	n00000013:port0 -> n00000010:port0 [style=bold]
 }
diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
index a582af0509ee..e5636883545f 100644
--- a/Documentation/media/v4l-drivers/vimc.rst
+++ b/Documentation/media/v4l-drivers/vimc.rst
@@ -1,45 +1,225 @@ 
 .. SPDX-License-Identifier: GPL-2.0
 
+==========================================
 The Virtual Media Controller Driver (vimc)
 ==========================================
 
-The vimc driver emulates complex video hardware using the V4L2 API and the Media
-API. It has a capture device and three subdevices: sensor, debayer and scaler.
+The vimc driver emulates complex video hardware topologies using the V4L2 API
+and the Media API. It has a capture device and three subdevices:
+sensor, debayer and scaler. It exposes media devices through /dev/mediaX nodes,
+video capture devices through /dev/videoX and sub-devices through /dev/v4l-subdevX.
+
+
+To configure a media device of a given topology, a ConfigFS API is provided.
+
+Configuring a topology through ConfigFS (Experimental)
+======================================================
+
+.. note:: This API is under development and might change in the future.
+
+Mount configfs:
+::
+
+	$ mkdir /configfs
+	$ mount -t configfs none /configfs
+
+When loading the module, you will see a folder named vimc
+::
+
+	$ tree /configfs/
+	/configfs/
+	`-- vimc
+
+Creating a media device
+-----------------------
+
+To create a media device create a new folder under /configfs/vimc/
+
+Example:
+::
+
+	$ mkdir /configfs/vimc/mdev
+	$ tree /configfs/vimc/mdev
+	/configfs/
+	`-- vimc
+	    `-- mdev
+	        `-- hotplug
+
+Creating entities
+-----------------
+
+To create an entity in the media device's topology, create a folder under
+``/configfs/vimc/<mdev-name>/`` with the following format:
+
+	<entity-type>:<entity-name>
+
+Where <entity-type> is one of the following:
+
+- vimc-sensor
+- vimc-scaler
+- vimc-debayer
+- vimc-capture
+
+Example:
+::
+
+	$ mkdir /configfs/vimc/mdev/vimc-sensor:sen
+	$ mkdir /configfs/vimc/mdev/vimc-capture:cap-sen
+	$ tree /configfs/
+	/configfs/
+	`-- vimc
+	    `-- mdev
+		|-- hotplug
+		|-- vimc-capture:cap-sen
+		|   `-- pad:sink:0
+		`-- vimc-sensor:sen
+                    `-- pad:source:0
+
+Default folders are created under the entity directory for each pad of the entity.
+
+Creating links
+--------------
+
+To create a link between two entities, you should create a directory for the link
+under the source pad of the link and then set it to be a symbolic link to the sink pad:
 
-Topology
---------
+Example:
+::
+
+	$ mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
+	$ ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
+	$ tree /configfs
+	/configfs
+	`-- vimc
+	    `-- mdev
+	        |-- hotplug
+	        |-- vimc-capture:cap-sen
+	        |   `-- pad:sink:0
+	        `-- vimc-sensor:sen
+	            `-- pad:source:0
+	                `-- to-cap
+	                    |-- enabled
+	                    |-- immutable
+	                    `-- pad:sink:0 -> ../../../../../vimc/mdev/vimc-capture:cap-sen/pad:sink:0
+
+The `enabled` and `immutable` are two boolean attributes of the link corresponding to the link flags
+
+
+Flag values are described in :ref:`Documentation/media/uapi/mediactl/media-types.rst <media-link-flag>`
+( seek for ``MEDIA_LNK_FL_*``)
+
+1 - Enabled
+	Indicates that the link will be enabled when the media device is created.
+
+3 - Enabled and Immutable
+	Indicates that the link enabled state can't be modified at runtime.
+
+Change an attribute of the link by writing "on" or "1" to set it on , and "off" or "0" to set it off
+
+Example:
+::
+
+	$ echo "on" > /configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable
+
+Activating/Deactivating device
+------------------------------
+
+To activate the device, write one of "plugged", "plug" or "1" to the file
+``/configfs/vimc/<mdev-name>/hotplug``
+
+Example:
+::
+
+	$ echo 1 > /configfs/vimc/mdev/hotplug
+
+You should see a new node ``/dev/mediaX`` in your devfs.
+
+To deactivate the device, write one of "unplugged", "unplug" or "0" to the file
+``/configfs/vimc/<ndev-name>/hotplug``
+
+Example:
+::
+
+	$ echo unplugged > /configfs/vimc/mdev/hotplug
+
+Topology Configuration - Full Example
+-------------------------------------
+
+Here is a full example of a simple topology configuration:
+
+.. code-block:: bash
+
+    # Creating the entities
+    mkdir "/configfs/vimc/mdev"
+    mkdir "/configfs/vimc/mdev/vimc-sensor:sen"
+    mkdir "/configfs/vimc/mdev/vimc-debayer:deb"
+    mkdir "/configfs/vimc/mdev/vimc-scaler:sca"
+    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sca" #/dev/video2
+    mkdir "/configfs/vimc/mdev/vimc-capture:cap-sen" #/dev/video1
+    mkdir "/configfs/vimc/mdev/vimc-capture:cap-deb" #/dev/video0
+
+    # Creating the links
+    #sen -> deb
+    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
+    ln -s "/configfs/vimc/mdev/vimc-debayer:deb/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb"
+    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/immutable"
+    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-deb/enabled"
+
+    #deb -> sca
+    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
+    ln -s "/configfs/vimc/mdev/vimc-scaler:sca/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca"
+    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/immutable"
+    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-sca/enabled"
+
+    #sca -> cap-sca
+    mkdir "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
+    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sca/pad:sink:0" "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap"
+    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/immutable"
+    echo on > "/configfs/vimc/mdev/vimc-scaler:sca/pad:source:1/to-cap/enabled"
+
+    #sen -> cap-sen
+    mkdir "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
+    ln -s "/configfs/vimc/mdev/vimc-capture:cap-sen/pad:sink:0" "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap"
+    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/immutable"
+    echo on > "/configfs/vimc/mdev/vimc-sensor:sen/pad:source:0/to-cap/enabled"
+
+    #deb -> cap-deb
+    mkdir "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
+    ln -s "/configfs/vimc/mdev/vimc-capture:cap-deb/pad:sink:0" "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap"
+    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/immutable"
+    echo on > "/configfs/vimc/mdev/vimc-debayer:deb/pad:source:1/to-cap/enabled"
 
-The topology is hardcoded, although you could modify it in vimc-core and
-recompile the driver to achieve your own topology. This is the default topology:
 
 .. _vimc_topology_graph:
 
 .. kernel-figure:: vimc.dot
-    :alt:   Diagram of the default media pipeline topology
+    :alt:   Diagram of the configured simple topology in the example
     :align: center
 
-    Media pipeline graph on vimc
+    Simple Media pipeline graph on vimc configured through configfs
 
-Configuring the topology
-~~~~~~~~~~~~~~~~~~~~~~~~
+Configuring the pipeline formats
+================================
 
-Each subdevice will come with its default configuration (pixelformat, height,
-width, ...). One needs to configure the topology in order to match the
+Each subdevice has a default format configuration (pixelformat, height,
+width, ...). You should configure the formats in order to match the
 configuration on each linked subdevice to stream frames through the pipeline.
-If the configuration doesn't match, the stream will fail. The ``v4l-utils``
+If the configuration doesn't match, streaming will fail. The ``v4l-utils``
 package is a bundle of user-space applications, that comes with ``media-ctl`` and
-``v4l2-ctl`` that can be used to configure the vimc configuration. This sequence
-of commands fits for the default topology:
+``v4l2-ctl`` that can be used to configure the formats of the entities. This sequence
+of commands fits the simple topology created in the full example of topology configuration:
 
 .. code-block:: bash
 
-        media-ctl -d platform:vimc -V '"Sensor A":0[fmt:SBGGR8_1X8/640x480]'
-        media-ctl -d platform:vimc -V '"Debayer A":0[fmt:SBGGR8_1X8/640x480]'
-        media-ctl -d platform:vimc -V '"Sensor B":0[fmt:SBGGR8_1X8/640x480]'
-        media-ctl -d platform:vimc -V '"Debayer B":0[fmt:SBGGR8_1X8/640x480]'
-        v4l2-ctl -z platform:vimc -d "RGB/YUV Capture" -v width=1920,height=1440
-        v4l2-ctl -z platform:vimc -d "Raw Capture 0" -v pixelformat=BA81
-        v4l2-ctl -z platform:vimc -d "Raw Capture 1" -v pixelformat=BA81
+	media-ctl -d platform:vimc-000 -V '"sen":0[fmt:SBGGR8_1X8/640x480]'
+	media-ctl -d platform:vimc-000 -V '"deb":0[fmt:SBGGR8_1X8/640x480]'
+	media-ctl -d platform:vimc-000 -V '"deb":1[fmt:RGB888_1X24/640x480]'
+	media-ctl -d platform:vimc-000 -V '"sca":0[fmt:RGB888_1X24/640x480]'
+	media-ctl -d platform:vimc-000 -V '"sca":1[fmt:RGB888_1X24/640x480]'
+	v4l2-ctl -z platform:vimc-000 -d "cap-sen" -v pixelformat=BA81
+	v4l2-ctl -z platform:vimc-000 -d "cap-deb" -v pixelformat=RGB3
+	# The default scaling value of the scaler is 3, so need to set its capture accordingly
+	v4l2-ctl -z platform:vimc -d "cap-sca" -v pixelformat=RGB3,width=1920,height=1440
 
 Subdevices
 ----------
@@ -61,8 +241,8 @@  vimc-debayer:
 	* 1 Pad source
 
 vimc-scaler:
-	Scale up the image by a factor of 3. E.g.: a 640x480 image becomes a
-        1920x1440 image. (this value can be configured, see at
+	Scales up the image by a factor of 3. E.g.: a 640x480 image becomes a
+        1920x1440 image. (this value can be configured, see
         `Module options`_).
 	Exposes:
 
@@ -77,12 +257,10 @@  vimc-capture:
 	* 1 Pad source
 
 
-        Module options
----------------
-
-Vimc has a few module parameters to configure the driver.
+Module options
+==============
 
-        param=value
+Vimc has 2 module parameters to configure the driver.
 
 * ``sca_mult=<unsigned int>``
 
@@ -98,10 +276,10 @@  Vimc has a few module parameters to configure the driver.
         otherwise the next odd number is considered (the default value is 3).
 
 Source code documentation
--------------------------
+=========================
 
 vimc-streamer
-~~~~~~~~~~~~~
+-------------
 
 .. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.h
    :internal: