[v3,9/9] Documentation: media: add documentation file c3-isp.rst

From: Keke Li <keke.li@amlogic.com>

Add the file 'c3-isp.rst' that documents the c3-isp driver.

Signed-off-by: Keke Li <keke.li@amlogic.com>
---
 Documentation/admin-guide/media/c3-isp.dot      | 26 +++++++
 Documentation/admin-guide/media/c3-isp.rst      | 96 +++++++++++++++++++++++++
 Documentation/admin-guide/media/v4l-drivers.rst |  1 +
 MAINTAINERS                                     | 10 +++
 4 files changed, 133 insertions(+)

Hi Keke

On Wed, Sep 18, 2024 at 02:07:20PM +0800, Keke Li via B4 Relay wrote:
> From: Keke Li <keke.li@amlogic.com>
>
> Add the file 'c3-isp.rst' that documents the c3-isp driver.
>
> Signed-off-by: Keke Li <keke.li@amlogic.com>
> ---
>  Documentation/admin-guide/media/c3-isp.dot      | 26 +++++++
>  Documentation/admin-guide/media/c3-isp.rst      | 96 +++++++++++++++++++++++++
>  Documentation/admin-guide/media/v4l-drivers.rst |  1 +
>  MAINTAINERS                                     | 10 +++
>  4 files changed, 133 insertions(+)
>
> diff --git a/Documentation/admin-guide/media/c3-isp.dot b/Documentation/admin-guide/media/c3-isp.dot
> new file mode 100644
> index 000000000000..0cc1b8b96404
> --- /dev/null
> +++ b/Documentation/admin-guide/media/c3-isp.dot
> @@ -0,0 +1,26 @@
> +digraph board {
> +	rankdir=TB
> +	n00000001 [label="{{<port0> 0 | <port1> 1} | isp-core\n/dev/v4l-subdev0 | {<port2> 2 | <port3> 3}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000001:port3 -> n00000006:port0 [style=bold]
> +	n00000001:port3 -> n00000009:port0 [style=bold]
> +	n00000001:port3 -> n0000000c:port0 [style=bold]
> +	n00000001:port2 -> n00000020 [style=bold]
> +	n00000006 [label="{{<port0> 0} | isp-resizer0\n/dev/v4l-subdev1 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000006:port1 -> n00000014 [style=bold]
> +	n00000009 [label="{{<port0> 0} | isp-resizer1\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000009:port1 -> n00000018 [style=bold]
> +	n0000000c [label="{{<port0> 0} | isp-resizer2\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n0000000c:port1 -> n0000001c [style=bold]
> +	n0000000f [label="{{<port0> 0} | mipi-adapter\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n0000000f:port1 -> n00000001:port0 [style=bold]
> +	n00000014 [label="isp-video0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> +	n00000018 [label="isp-video1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> +	n0000001c [label="isp-video2\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> +	n00000020 [label="isp-stats\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
> +	n00000024 [label="isp-params\n/dev/video4", shape=box, style=filled, fillcolor=yellow]
> +	n00000024 -> n00000001:port1 [style=bold]
> +	n00000038 [label="{{<port0> 0} | mipi-csi2\n/dev/v4l-subdev5 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n00000038:port1 -> n0000000f:port0 [style=bold]
> +	n0000003d [label="{{} | imx290 2-001a\n/dev/v4l-subdev6 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> +	n0000003d:port0 -> n00000038:port0 [style=bold]
> +}
> diff --git a/Documentation/admin-guide/media/c3-isp.rst b/Documentation/admin-guide/media/c3-isp.rst
> new file mode 100644
> index 000000000000..fab10c962465
> --- /dev/null
> +++ b/Documentation/admin-guide/media/c3-isp.rst
> @@ -0,0 +1,96 @@
> +.. SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +
> +.. include:: <isonum.txt>
> +
> +=================================================
> +Amlogic C3 Image Signal Processing (C3ISP) driver
> +=================================================
> +
> +Introduction
> +============
> +
> +This file documents the Amlogic C3ISP driver located under
> +drivers/media/platform/amlogic/c3-isp.
> +
> +The current version of the driver supports the C3ISP found on
> +Amlogic C308L processor.
> +
> +The driver implements V4L2, Media controller and V4L2 subdev interfaces.
> +Camera sensor using V4L2 subdev interface in the kernel is supported.

I would drop this last statement

> +
> +The driver has been tested on AW419-C308L-Socket platform.
> +
> +Anlogic Camera hardware

Amlogic

> +=======================
> +
> +The Camera hardware found on C308L processors and supported by
> +the driver consists of:
> +
> +- 1 MIPI-CSI2 module. It handle the Physical layer of the CSI2 receivers and
> +  receive MIPI data.

Do not break lines if you do not break it to new paragraphs

     What I mean is not to.
     Do this but instead:

     Either do this on multiple lines if you go beyond 80 columns in
     writing your text without breaking earlier.

     Or break it.

     To separate paragraphs.

> +  A separate camera sensor can be connected to MIPI-CSi2 module.

I would drop "separate" and mention that both RAW and YUV/RGB sensors
are supported

> +- 1 MIPI-ADAPTER module. Organize MIPI data to meet ISP input requirements and
> +  send MIPI data to ISP
> +- 1 ISP (Image Signal Processing) module. Contain a pipeline of image processing
> +  hardware blocks.
> +  The ISP pipeline contains three scalers at the end.
> +  The ISP also contains the DMA interface which writes the output data to memory.
> +
> +Supported functionality
> +=======================
> +
> +The current version of the driver supports:
> +
> +- Input from camera sensor via MIPI-CSI2;
> +
> +- Pixel output interface of ISP
> +
> +  - Scaling support. Configuration of the scaler module
> +    for downscalling with ratio up to 8x.

No need to break line before 80-cols

> +
> +Driver Architecture and Design
> +==============================
> +
> +The driver implements the V4L2 subdev interface. With the goal to model the
> +hardware links between the modules and to expose a clean, logical and usable
> +interface, the driver is split into V4L2 sub-devices as follows:
> +
> +- 1 mipi-csi2 sub-device - mipi-csi2 is represented by a single sub-device.
> +- 1 mipi-adapter sub-device - mipi-adapter is represented by a single sub-devices.
> +- 1 isp-core sub-device - isp-core is represented by a single sub-devices.
> +- 3 isp-resizer sub-devices - isp-resizer is represented by a number of sub-devices
> +  equal to the number of capture device.
> +
> +isp-core sub-device is linked to 2 separate video device nodes and
> +3 isp-resizer sub-devices nodes.
> +
> +- 1 capture statistics video device node.
> +- 1 output parameters video device node.
> +- 3 isp-resizer sub-device nodes.
> +
> +isp-resizer sub-device is linked to capture video device node.
> +
> +- isp-resizer0 is linked to isp-cap0
> +- isp-resizer1 is linked to isp-cap1
> +- isp-resizer2 is linked to isp-cap2
> +
> +The media controller pipeline graph is as follows (with connected a
> +IMX290 camera sensor):
> +
> +.. _isp_topology_graph:
> +
> +.. kernel-figure:: c3-isp.dot
> +    :alt:   c3-isp.dot
> +    :align: center
> +
> +    Media pipeline topology
> +
> +Implementation
> +==============
> +
> +Runtime configuration of the hardware via 'isp-params' video device node.
> +Acquiring statistics of ISP hardware via 'isp-stats' video device node.
> +Acquiring output image of ISP hardware via 'isp-video[0, 2]' video device node.

The text above names them 'isp-cap[0, 2]'

> +
> +The output size of the scaler module in the ISP is configured with
> +the pixel format of 'isp-video[0, 2]' video device node.

I haven't looked yet at how this is implemented, but I presume the
correct sizes should be configured on the isp-resizerX sub-device as
well. I would drop this last pharse as it might be misleading ?

> diff --git a/Documentation/admin-guide/media/v4l-drivers.rst b/Documentation/admin-guide/media/v4l-drivers.rst
> index b6af448b9fe9..be0a8a860f39 100644
> --- a/Documentation/admin-guide/media/v4l-drivers.rst
> +++ b/Documentation/admin-guide/media/v4l-drivers.rst
> @@ -10,6 +10,7 @@ Video4Linux (V4L) driver-specific documentation
>  	:maxdepth: 2
>
>  	bttv
> +	c3-isp
>  	cafe_ccic
>  	cx88
>  	fimc
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 31168c05f304..954dd9bdf77e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1209,6 +1209,16 @@ F:	Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
>  F:	drivers/perf/amlogic/
>  F:	include/soc/amlogic/
>
> +AMLOGIC ISP DRIVER
> +M:	Keke Li <keke.li@amlogic.com>
> +L:	linux-media@vger.kernel.org
> +S:	Maintained

I would add the entry to MAINTAINERS when introducing the ISP driver
and only add

> +F:	Documentation/admin-guide/media/c3-isp.dot
> +F:	Documentation/admin-guide/media/c3-isp.rst

These two files in this patch

> +F:	Documentation/devicetree/bindings/media/amlogic,c3-isp.yaml
> +F:	Documentation/userspace-api/media/v4l/metafmt-c3-isp.rst
> +F:	drivers/media/platform/amlogic/c3-isp/

And also the uAPI header is missing from the files list it seems

Thanks
  j

> +
>  AMLOGIC MIPI ADAPTER DRIVER
>  M:	Keke Li <keke.li@amlogic.com>
>  L:	linux-media@vger.kernel.org
>
> --
> 2.46.1
>
>
>