2019-06-17 13:33:43

by André Almeida

[permalink] [raw]
Subject: [PATCH v3 1/2] media: vimc: stream: add missing function documentation

Add comments at vimc_streamer_s_stream and vimc_streamer_thread, making
the vimc-stream totally documented.

Signed-off-by: André Almeida <[email protected]>
---
Changes in v3: replace "streaming" by "stream" at vimc_streamer_thread().

Changes in v2: fix typos

drivers/media/platform/vimc/vimc-streamer.c | 22 +++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index 236ade38f1da..1fea6d666c2e 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -122,6 +122,14 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
return -EINVAL;
}

+/*
+ * vimc_streamer_thread - process frames through the pipeline
+ *
+ * @data: vimc_stream struct of the current stream
+ *
+ * From the source to the sink, gets a frame from each subdevice and send to
+ * the next one of the pipeline in a fixed framerate.
+ */
static int vimc_streamer_thread(void *data)
{
struct vimc_stream *stream = data;
@@ -149,6 +157,20 @@ static int vimc_streamer_thread(void *data)
return 0;
}

+/*
+ * vimc_streamer_s_stream - start/stop the streaming on the media pipeline
+ *
+ * @stream: the pointer to the stream structure of the current stream
+ * @ved: pointer to the vimc entity of the entity of the stream
+ * @enable: flag to determine if stream should start/stop
+ *
+ * When starting, check if there is no stream->kthread allocated. This should
+ * indicate that a streaming is already running. Then, it initializes
+ * the pipeline, creates and runs a kthread to consume buffers through the
+ * pipeline.
+ * When stopping, analogously check if there is a stream running, stops
+ * the thread and terminates the pipeline.
+ */
int vimc_streamer_s_stream(struct vimc_stream *stream,
struct vimc_ent_device *ved,
int enable)
--
2.22.0


2019-06-17 13:35:07

by André Almeida

[permalink] [raw]
Subject: [PATCH v3 2/2] media: docs: create vimc documentation

Create vimc documentation file to explain it basics features, it's
topology, how to configure it and to document vimc's subdevices.

Signed-off-by: André Almeida <[email protected]>
Suggested-by: Helen Koike <[email protected]>
---
Changes in v3: none

Changes in v2:
- Fix typos
- Make clear what does means scale factor

Documentation/media/v4l-drivers/index.rst | 1 +
Documentation/media/v4l-drivers/vimc.dot | 22 +++++
Documentation/media/v4l-drivers/vimc.rst | 98 +++++++++++++++++++++++
3 files changed, 121 insertions(+)
create mode 100644 Documentation/media/v4l-drivers/vimc.dot
create mode 100644 Documentation/media/v4l-drivers/vimc.rst

diff --git a/Documentation/media/v4l-drivers/index.rst b/Documentation/media/v4l-drivers/index.rst
index 33a055907258..c4c78a28654c 100644
--- a/Documentation/media/v4l-drivers/index.rst
+++ b/Documentation/media/v4l-drivers/index.rst
@@ -64,5 +64,6 @@ For more details see the file COPYING in the source distribution of Linux.
si476x
soc-camera
uvcvideo
+ vimc
vivid
zr364xx
diff --git a/Documentation/media/v4l-drivers/vimc.dot b/Documentation/media/v4l-drivers/vimc.dot
new file mode 100644
index 000000000000..57863a13fa39
--- /dev/null
+++ b/Documentation/media/v4l-drivers/vimc.dot
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0
+
+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]
+}
diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
new file mode 100644
index 000000000000..e235f806e252
--- /dev/null
+++ b/Documentation/media/v4l-drivers/vimc.rst
@@ -0,0 +1,98 @@
+.. 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.
+
+Topology
+--------
+
+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: vimc.dot
+ :align: center
+
+ Media pipeline graph on vimc
+
+Configuring the topology
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Each subdevice will come with its default configuration (pixelformat, height,
+width, ...). One needs to configure the topology 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 ``v4l2-utils``
+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:
+
+.. 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
+
+Subdevices
+----------
+
+Subdevices define the behavior of an entity in the topology. Depending on the
+subdevice, the entity can have multiple pads of type source or sink.
+
+vimc-sensor:
+ Generates images in several formats using video test pattern generator.
+ Exposes:
+
+ * 1 Pad source
+
+vimc-debayer:
+ Transforms images in bayer format into a non-bayer format.
+ Exposes:
+
+ * 1 Pad sink
+ * 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
+ `Module options`_).
+ Exposes:
+
+ * 1 Pad sink
+ * 1 Pad source
+
+vimc-capture:
+ Exposes node /dev/videoX to allow userspace to capture the stream.
+ Exposes:
+
+ * 1 Pad sink
+ * 1 Pad source
+
+Module options
+---------------
+
+Vimc has a few module parameters to configure the driver. You should pass
+those arguments to each subdevice, not to the vimc module. For example::
+
+ vimc_subdevice.param=value
+
+* ``vimc_scaler.sca_mult=<unsigned int>``
+
+ Image size multiplier factor to be used to multiply both width and
+ height, so the image size will be ``sca_mult^2`` bigger than the
+ original one. Currently, only supports scaling up (the default value
+ is 3).
+
+* ``vimc_debayer.deb_mean_win_size=<unsigned int>``
+
+ Window size to calculate the mean. Note: the window size needs to be an
+ odd number, as the main pixel stays in the center of the window,
+ otherwise the next odd number is considered (the default value is 3).
--
2.22.0

2019-06-21 21:18:50

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] media: vimc: stream: add missing function documentation

Em Mon, 17 Jun 2019 10:32:20 -0300
André Almeida <[email protected]> escreveu:

> Add comments at vimc_streamer_s_stream and vimc_streamer_thread, making
> the vimc-stream totally documented.

I'm applying it right now.

Yet, if this is fully documented, IMO you should add it to
Documentation/media/v4l-drivers, replacing the comments to kernel-doc
markups.

That would make easier for the ones to read the comments and, if someone
changes a function call, warnings will be produced, and the developer
will be warned.

Thanks,
Mauro

2019-06-22 21:52:08

by André Almeida

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] media: vimc: stream: add missing function documentation

Hello Mauro,

On 6/21/19 6:17 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 17 Jun 2019 10:32:20 -0300
> André Almeida <[email protected]> escreveu:
>
>> Add comments at vimc_streamer_s_stream and vimc_streamer_thread, making
>> the vimc-stream totally documented.
> I'm applying it right now.
>
> Yet, if this is fully documented, IMO you should add it to
> Documentation/media/v4l-drivers, replacing the comments to kernel-doc
> markups.

This suggestion is a great improvement and it's simple to apply to the
source. Where do you believe we can place this at[1]? Maybe something like


Source code documentation
-------------------------

vimc-streamer
~~~~~~~~~~~~

.. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.c
:internal:


at the end of the file?

> That would make easier for the ones to read the comments and, if someone
> changes a function call, warnings will be produced, and the developer
> will be warned.
> Thanks,
> Mauro

Thanks,
    André

[1]
https://git.linuxtv.org/media_tree.git/tree/Documentation/media/v4l-drivers/vimc.rst

2019-06-22 21:59:14

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] media: vimc: stream: add missing function documentation

Em Sat, 22 Jun 2019 18:51:06 -0300
André Almeida <[email protected]> escreveu:

> Hello Mauro,
>
> On 6/21/19 6:17 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 17 Jun 2019 10:32:20 -0300
> > André Almeida <[email protected]> escreveu:
> >
> >> Add comments at vimc_streamer_s_stream and vimc_streamer_thread, making
> >> the vimc-stream totally documented.
> > I'm applying it right now.
> >
> > Yet, if this is fully documented, IMO you should add it to
> > Documentation/media/v4l-drivers, replacing the comments to kernel-doc
> > markups.
>
> This suggestion is a great improvement and it's simple to apply to the
> source. Where do you believe we can place this at[1]? Maybe something like
>
>
> Source code documentation
> -------------------------
>
> vimc-streamer
> ~~~~~~~~~~~~
>
> .. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.c
> :internal:
>
>
> at the end of the file?
>
Yeah, this should be enough.

> > That would make easier for the ones to read the comments and, if someone
> > changes a function call, warnings will be produced, and the developer
> > will be warned.
> > Thanks,
> > Mauro
>
> Thanks,
>     André
>
> [1]
> https://git.linuxtv.org/media_tree.git/tree/Documentation/media/v4l-drivers/vimc.rst
>



Thanks,
Mauro