[PATCH v2 0/5] Add Arm Mali-C55 Image Signal Processor Driver

[PATCH v2 0/5] Add Arm Mali-C55 Image Signal Processor Driver

[Daniel Scally]

Hello all

This patchset introduces a driver for Arm's Mali-C55 Image Signal Processor.
The driver uses the media controller API and in this initial support implements
both of the ISP's capture pipelines allowing a range of output formats plus
downscaling and cropping. The capture pipelines are named "Full resolution" and
"Downscale" and so abbreviated FR and DS throughout the driver.

The driver exposes 4 V4L2 subdevices:

- mali-c55 isp: input data formatting
- mali-c55 tpg: test pattern generator (modeled as a camera sensor entity)
- mali-c55 resizer fr: downscale / crop and format setting for the FR pipe
- mali-c55 resizer ds: downscale / crop and format setting for the DS pipe

Conspicuously missing from the list are subdevices for the ISP's statistics and
parameters; work is progressing in these areas and we plan on introducing them
in later series on top of this one.

Thanks
Dan

Daniel Scally (5):
  media: uapi: Add MEDIA_BUS_FMT_RGB202020_1X60 format code
  dt-bindings: media: Add bindings for ARM mali-c55
  media: mali-c55: Add Mali-C55 ISP driver
  media: Documentation: Add Mali-C55 ISP Documentation
  MAINTAINERS: Add entry for mali-c55 driver

 .../admin-guide/media/mali-c55-graph.dot      |   19 +
 Documentation/admin-guide/media/mali-c55.rst  |  318 +++++
 .../admin-guide/media/v4l-drivers.rst         |    1 +
 .../bindings/media/arm,mali-c55.yaml          |   77 ++
 .../media/v4l/subdev-formats.rst              |  168 +++
 MAINTAINERS                                   |   10 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/arm/Kconfig            |    5 +
 drivers/media/platform/arm/Makefile           |    2 +
 drivers/media/platform/arm/mali-c55/Kconfig   |   18 +
 drivers/media/platform/arm/mali-c55/Makefile  |    9 +
 .../platform/arm/mali-c55/mali-c55-capture.c  | 1021 +++++++++++++++++
 .../platform/arm/mali-c55/mali-c55-common.h   |  271 +++++
 .../platform/arm/mali-c55/mali-c55-core.c     |  767 +++++++++++++
 .../platform/arm/mali-c55/mali-c55-isp.c      |  682 +++++++++++
 .../arm/mali-c55/mali-c55-registers.h         |  180 +++
 .../arm/mali-c55/mali-c55-resizer-coefs.h     |  382 ++++++
 .../platform/arm/mali-c55/mali-c55-resizer.c  |  678 +++++++++++
 .../platform/arm/mali-c55/mali-c55-tpg.c      |  425 +++++++
 include/uapi/linux/media-bus-format.h         |    3 +-
 21 files changed, 5037 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/admin-guide/media/mali-c55-graph.dot
 create mode 100644 Documentation/admin-guide/media/mali-c55.rst
 create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
 create mode 100644 drivers/media/platform/arm/Kconfig
 create mode 100644 drivers/media/platform/arm/Makefile
 create mode 100644 drivers/media/platform/arm/mali-c55/Kconfig
 create mode 100644 drivers/media/platform/arm/mali-c55/Makefile
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-capture.c
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-common.h
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-core.c
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-isp.c
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-registers.h
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-resizer-coefs.h
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-resizer.c
 create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-tpg.c

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/5] media: uapi: Add MEDIA_BUS_FMT_RGB202020_1X60 format code

[Daniel Scally]

The Mali-C55 ISP by ARM requires 20-bits per colour channel input on
the bus. Add a new media bus format code to represent it.

Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

	- none

 .../media/v4l/subdev-formats.rst              | 168 ++++++++++++++++++
 include/uapi/linux/media-bus-format.h         |   3 +-
 2 files changed, 170 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index eb3cd20b0cf2..269b61926855 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -2223,6 +2223,174 @@ The following table list existing packed 48bit wide RGB formats.
 
     \endgroup
 
+The following table list existing packed 60bit wide RGB formats.
+
+.. tabularcolumns:: |p{4.0cm}|p{0.7cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|p{0.22cm}|
+
+.. _v4l2-mbus-pixelcode-rgb-60:
+
+.. raw:: latex
+
+    \begingroup
+    \tiny
+    \setlength{\tabcolsep}{2pt}
+
+.. flat-table:: 60bit RGB formats
+    :header-rows:  3
+    :stub-columns: 0
+    :widths: 36 7 3 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2
+
+    * - Identifier
+      - Code
+      -
+      - :cspan:`31` Data organization
+    * -
+      -
+      - Bit
+      -
+      -
+      -
+      -
+      - 59
+      - 58
+      - 57
+      - 56
+      - 55
+      - 54
+      - 53
+      - 52
+      - 51
+      - 50
+      - 49
+      - 48
+      - 47
+      - 46
+      - 45
+      - 44
+      - 43
+      - 42
+      - 41
+      - 40
+      - 39
+      - 38
+      - 37
+      - 36
+      - 35
+      - 34
+      - 33
+      - 32
+    * -
+      -
+      -
+      - 31
+      - 30
+      - 29
+      - 28
+      - 27
+      - 26
+      - 25
+      - 24
+      - 23
+      - 22
+      - 21
+      - 20
+      - 19
+      - 18
+      - 17
+      - 16
+      - 15
+      - 14
+      - 13
+      - 12
+      - 11
+      - 10
+      - 9
+      - 8
+      - 7
+      - 6
+      - 5
+      - 4
+      - 3
+      - 2
+      - 1
+      - 0
+    * .. _MEDIA-BUS-FMT-RGB202020-1X60:
+
+      - MEDIA_BUS_FMT_RGB202020_1X60
+      - 0x1026
+      -
+      -
+      -
+      -
+      -
+      - r\ :sub:`19`
+      - r\ :sub:`18`
+      - r\ :sub:`17`
+      - r\ :sub:`16`
+      - r\ :sub:`15`
+      - r\ :sub:`14`
+      - r\ :sub:`13`
+      - r\ :sub:`12`
+      - r\ :sub:`11`
+      - r\ :sub:`10`
+      - r\ :sub:`8`
+      - r\ :sub:`8`
+      - r\ :sub:`7`
+      - r\ :sub:`6`
+      - r\ :sub:`5`
+      - r\ :sub:`4`
+      - r\ :sub:`3`
+      - r\ :sub:`2`
+      - r\ :sub:`1`
+      - r\ :sub:`0`
+      - g\ :sub:`19`
+      - g\ :sub:`18`
+      - g\ :sub:`17`
+      - g\ :sub:`16`
+      - g\ :sub:`15`
+      - g\ :sub:`14`
+      - g\ :sub:`13`
+      - g\ :sub:`12`
+    * -
+      -
+      -
+      - g\ :sub:`11`
+      - g\ :sub:`10`
+      - g\ :sub:`9`
+      - g\ :sub:`8`
+      - g\ :sub:`7`
+      - g\ :sub:`6`
+      - g\ :sub:`5`
+      - g\ :sub:`4`
+      - g\ :sub:`3`
+      - g\ :sub:`2`
+      - g\ :sub:`1`
+      - g\ :sub:`0`
+      - b\ :sub:`19`
+      - b\ :sub:`18`
+      - b\ :sub:`17`
+      - b\ :sub:`16`
+      - b\ :sub:`15`
+      - b\ :sub:`14`
+      - b\ :sub:`13`
+      - b\ :sub:`12`
+      - b\ :sub:`11`
+      - b\ :sub:`10`
+      - b\ :sub:`9`
+      - b\ :sub:`8`
+      - b\ :sub:`7`
+      - b\ :sub:`6`
+      - b\ :sub:`5`
+      - b\ :sub:`4`
+      - b\ :sub:`3`
+      - b\ :sub:`2`
+      - b\ :sub:`1`
+      - b\ :sub:`0`
+
+.. raw:: latex
+
+    \endgroup
+
 On LVDS buses, usually each sample is transferred serialized in seven
 time slots per pixel clock, on three (18-bit) or four (24-bit)
 differential data pairs at the same time. The remaining bits are used
diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
index f05f747e444d..8633818cc33c 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -34,7 +34,7 @@
 
 #define MEDIA_BUS_FMT_FIXED			0x0001
 
-/* RGB - next is	0x1026 */
+/* RGB - next is	0x1027 */
 #define MEDIA_BUS_FMT_RGB444_1X12		0x1016
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE	0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE	0x1002
@@ -72,6 +72,7 @@
 #define MEDIA_BUS_FMT_RGB888_1X36_CPADLO	0x1021
 #define MEDIA_BUS_FMT_RGB121212_1X36		0x1019
 #define MEDIA_BUS_FMT_RGB161616_1X48		0x101a
+#define MEDIA_BUS_FMT_RGB202020_1X60		0x1026
 
 /* YUV (including grey) - next is	0x202f */
 #define MEDIA_BUS_FMT_Y8_1X8			0x2001
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Daniel Scally]

Add the yaml binding for ARM's Mali-C55 Image Signal Processor.

Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

	- Added clocks information
	- Fixed the warnings raised by Rob

 .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml

diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
new file mode 100644
index 000000000000..30038cfec3a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Mali-C55 Image Signal Processor
+
+maintainers:
+  - Daniel Scally <dan.scally@ideasonboard.com>
+  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
+
+properties:
+  compatible:
+    const: arm,mali-c55
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: ISP video clock
+      - description: ISP AXI clock
+      - description: ISP AHB-lite clock
+
+  clock-names:
+    items:
+      - const: vclk
+      - const: aclk
+      - const: hclk
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+
+        properties:
+          endpoint:
+            $ref: /schemas/graph.yaml#/properties/endpoint
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    mali_c55: isp@400000 {
+      compatible = "arm,mali-c55";
+      reg = <0x400000 0x200000>;
+      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
+      clock-names = "vclk", "aclk", "hclk";
+      interrupts = <0>;
+
+      ports {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        port@0 {
+          reg = <0>;
+          isp_in: endpoint {
+              remote-endpoint = <&mipi_out>;
+          };
+        };
+      };
+    };
+...
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/5] media: Documentation: Add Mali-C55 ISP Documentation

[Daniel Scally]

Add a documentation page for the mali-c55 driver, which gives a brief
overview of the hardware and explains how to use the driver's capture
devices and the crop/scaler functions.

Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

	- none

 .../admin-guide/media/mali-c55-graph.dot      |  19 ++
 Documentation/admin-guide/media/mali-c55.rst  | 318 ++++++++++++++++++
 .../admin-guide/media/v4l-drivers.rst         |   1 +
 3 files changed, 338 insertions(+)
 create mode 100644 Documentation/admin-guide/media/mali-c55-graph.dot
 create mode 100644 Documentation/admin-guide/media/mali-c55.rst

diff --git a/Documentation/admin-guide/media/mali-c55-graph.dot b/Documentation/admin-guide/media/mali-c55-graph.dot
new file mode 100644
index 000000000000..0775ba42bf4c
--- /dev/null
+++ b/Documentation/admin-guide/media/mali-c55-graph.dot
@@ -0,0 +1,19 @@
+digraph board {
+        rankdir=TB
+        n00000001 [label="{{} | mali-c55 tpg\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
+        n00000001:port0 -> n00000003:port0 [style=dashed]
+        n00000003 [label="{{<port0> 0} | mali-c55 isp\n/dev/v4l-subdev1 | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
+        n00000003:port1 -> n00000007:port0 [style=bold]
+        n00000003:port2 -> n00000007:port2 [style=bold]
+        n00000003:port1 -> n0000000b:port0 [style=bold]
+        n00000007 [label="{{<port0> 0 | <port2> 2} | mali-c55 resizer fr\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
+        n00000007:port1 -> n0000000e [style=bold]
+        n0000000b [label="{{<port0> 0} | mali-c55 resizer ds\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
+        n0000000b:port1 -> n00000012 [style=bold]
+        n0000000e [label="mali-c55 fr\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
+        n00000012 [label="mali-c55 ds\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
+        n00000022 [label="{{<port0> 0} | csi2-rx\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
+        n00000022:port1 -> n00000003:port0
+        n00000027 [label="{{} | imx415 1-001a\n/dev/v4l-subdev5 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
+        n00000027:port0 -> n00000022:port0 [style=bold]
+}
\ No newline at end of file
diff --git a/Documentation/admin-guide/media/mali-c55.rst b/Documentation/admin-guide/media/mali-c55.rst
new file mode 100644
index 000000000000..83f630c3bd9d
--- /dev/null
+++ b/Documentation/admin-guide/media/mali-c55.rst
@@ -0,0 +1,318 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==========================================
+ARM Mali-C55 Image Signal Processor driver
+==========================================
+
+Introduction
+============
+
+This file documents the driver for ARM's Mali-C55 Image Signal Processor. The
+driver is located under drivers/media/platform/arm/mali-c55.
+
+The Mali-C55 ISP receives data in either raw Bayer format or RGB/YUV format from
+sensors through either a parallel interface or a memory bus before processing it
+and outputting it through an internal DMA engine. Two output pipelines are
+possible (though one may not be fitted, depending on the implementation). These
+are referred to as "Full resolution" and "Downscale", but the naming is historic
+and both pipes are capable of cropping/scaling operations. An integrated test
+pattern generator can be used to drive the ISP and produce image data in the
+absence of a connected camera sensor. The driver module is named mali_c55, and
+is enabled through the CONFIG_VIDEO_MALI_C55 config option.
+
+The driver implements V4L2, Media Controller and V4L2 Subdevice interfaces and
+expects camera sensors connected to the ISP to have V4L2 subdevice interfaces.
+
+Mali-C55 ISP hardware
+=====================
+
+A high level functional view of the Mali-C55 ISP is presented below. The ISP
+takes input from either a live source or through a DMA engine for memory input,
+depending on the SoC integration.::
+
+  +---------+    +----------+                                     +--------+
+  | Sensor  |--->| CSI-2 Rx |                "Full Resolution"    |  DMA   |
+  +---------+    +----------+   |\                 Output    +--->| Writer |
+                       |        | \                          |    +--------+
+                       |        |  \    +----------+  +------+---> Streaming I/O
+  +------------+       +------->|   |   |          |  |
+  |            |                |   |-->| Mali-C55 |--+
+  | DMA Reader |--------------->|   |   |    ISP   |  |
+  |            |                |  /    |          |  |      +---> Streaming I/O
+  +------------+                | /     +----------+  |      |
+                                |/                    +------+
+				                             |    +--------+
+                                                             +--->|  DMA   |
+                                               "Downscaled"       | Writer |
+					          Output          +--------+
+
+Media Controller Topology
+=========================
+
+An example of the ISP's topology (as implemented in a system with an IMX415
+camera sensor and generic CSI-2 receiver) is below:
+
+
+.. kernel-figure:: mali-c55-graph.dot
+    :alt:   mali-c55-graph.dot
+    :align: center
+
+The driver has 4 V4L2 subdevices:
+
+- `mali_c55 isp`: Responsible for configuring input crop and color space
+                  conversion
+- `mali_c55 tpg`: The test pattern generator, emulating a camera sensor.
+- `mali_c55 resizer fr`: The Full-Resolution pipe resizer
+- `mali_c55 resizer ds`: The Downscale pipe resizer
+
+The driver has 2 V4L2 video devices:
+
+- `mali-c55 fr`: The full-resolution pipe's capture device
+- `mali-c55 ds`: The downscale pipe's capture device
+
+Idiosyncrasies
+--------------
+
+**mali-c55 isp**
+The `mali-c55 isp` subdevice has a single sink pad to which all sources of data
+should be connected. The active source is selected by enabling the appropriate
+media link and disabling all others. The ISP has two source pads, reflecting the
+different paths through which it can internally route data. Tap points within
+the ISP allow users to divert data to avoid processing by some or all of the
+hardware's processing steps. The diagram below is intended only to highlight how
+the bypassing works and is not a true reflection of those processing steps; for
+a high-level functional block diagram see ARM's developer page for the
+ISP [3]_::
+
+  +--------------------------------------------------------------+
+  |                Possible Internal ISP Data Routes             |
+  |          +------------+  +----------+  +------------+        |
+  +---+      |            |  |          |  |  Colour    |    +---+
+  | 0 |--+-->| Processing |->| Demosaic |->|   Space    |--->| 1 |
+  +---+  |   |            |  |          |  | Conversion |    +---+
+  |      |   +------------+  +----------+  +------------+        |
+  |      |                                                   +---+
+  |      +---------------------------------------------------| 2 |
+  |                                                          +---+
+  |                                                              |
+  +--------------------------------------------------------------+
+
+
+.. flat-table::
+    :header-rows: 1
+
+    * - Pad
+      - Direction
+      - Purpose
+
+    * - 0
+      - sink
+      - Data input, connected to the TPG and camera sensors
+
+    * - 1
+      - source
+      - RGB/YUV data, connected to the FR and DS V4L2 subdevices
+
+    * - 2
+      - source
+      - RAW bayer data, connected to the FR V4L2 subdevices
+
+**mali-c55 resizer fr**
+The `mali-c55 resizer fr` subdevice has two _sink_ pads to reflect the different
+insertion points in the hardware (either RAW or demosaiced data):
+
+.. flat-table::
+    :header-rows: 1
+
+    * - Pad
+      - Direction
+      - Purpose
+
+    * - 0
+      - sink
+      - Data input connected to the ISP's demosaiced stream.
+
+    * - 1
+      - source
+      - Data output connected to the capture video device
+
+    * - 2
+      - sink
+      - Data input connected to the ISP's raw data stream
+
+The data source in use is selected through the routing API; two routes each of a
+single stream are available:
+
+.. flat-table::
+    :header-rows: 1
+
+    * - Sink Pad
+      - Source Pad
+      - Purpose
+
+    * - 0
+      - 1
+      - Demosaiced data route
+
+    * - 2
+      - 1
+      - Raw data route
+
+
+If the demosaiced route is active then the FR pipe is only capable of output
+in RGB/YUV formats. If the raw route is active then the output reflects the
+input (which may be either Bayer or RGB/YUV data).
+
+Using the driver to capture video
+=================================
+
+Using the media controller APIs we can configure the input source and ISP to
+capture images in a variety of formats. In the examples below, configuring the
+media graph is done with the v4l-utils [1]_ package's media-ctl utility.
+Capturing the images is done with yavta [2]_.
+
+Configuring the input source
+----------------------------
+
+The first step is to set the input source that we wish by enabling the correct
+media link. Using the example topology above, we can select the TPG as follows:
+
+.. code-block:: none
+
+    media-ctl -l "'lte-csi2-rx':1->'mali-c55 isp':0[0]"
+    media-ctl -l "'mali-c55 tpg':0->'mali-c55 isp':0[1]"
+
+Capturing bayer data from the source and processing to RGB/YUV
+--------------------------------------------------------------
+To capture 1920x1080 bayer data from the source and push it through the ISP's
+full processing pipeline, we configure the data formats appropriately on the
+source, ISP and resizer subdevices and set the FR resizer's routing to select
+processed data. The media bus format on the resizer's source pad will be either
+RGB121212_1X36 or YUV10_1X30, depending on whether you want to capture RGB or
+YUV. The ISP's debayering block outputs RGB data natively, setting the source
+pad format to YUV10_1X30 enables the colour space conversion block.
+
+In this example we target RGB565 output, so select RGB121212_1X36 as the resizer
+source pad's format:
+
+.. code-block:: none
+
+    # Set formats on the TPG and ISP
+    media-ctl -V "'mali-c55 tpg':0[fmt:SRGGB16_1X16/1920x1080]"
+    media-ctl -V "'mali-c55 isp':0[fmt:SRGGB16_1X16/1920x1080]"
+    media-ctl -V "'mali-c55 isp':1[fmt:SRGGB16_1X16/1920x1080]"
+
+    # Set routing on the FR resizer
+    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[1],2/0->1/0[0]]"
+
+    # Set format on the resizer, must be done AFTER the routing.
+    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/1920x1080]"
+
+The downscale output can also be used to stream data at the same time. In this
+case only processed data can be captured and so no routing need be set:
+
+.. code-block:: none
+
+    # Set format on the resizer
+    media-ctl -V "'mali-c55 resizer ds':1[fmt:RGB121212_1X36/1920x1080]"
+
+Following which images can be captured from both the FR and DS output's video
+devices (simultaneously, if desired):
+
+.. code-block:: none
+
+    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
+    yavta -f RGB565 -s 1920x1080 -c10 /dev/video1
+
+Cropping the image
+~~~~~~~~~~~~~~~~~~
+
+Both the full resolution and downscale pipes can crop to a minimum resolution of
+640x480. To crop the image simply configure the resizer's sink pad's crop and
+compose rectangles and set the format on the video device:
+
+.. code-block:: none
+
+    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(480,270)/640x480 compose:(0,0)/640x480]"
+    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
+    yavta -f RGB565 -s 640x480 -c10 /dev/video0
+
+Downscaling the image
+~~~~~~~~~~~~~~~~~~~~~
+
+Both the full resolution and downscale pipes can downscale the image by up to 8x
+provided the minimum 640x480 resolution is adhered to. For the best image result
+the scaling ratio for each dimension should be the same. To configure scaling we
+use the compose rectangle on the resizer's sink pad:
+
+.. code-block:: none
+
+    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(0,0)/1920x1080 compose:(0,0)/640x480]"
+    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
+    yavta -f RGB565 -s 640x480 -c10 /dev/video0
+
+Capturing images in YUV formats
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If we need to output YUV data rather than RGB the color space conversion block
+needs to be active, which is achieved by setting MEDIA_BUS_FMT_YUV10_1X30 on the
+resizer's source pad. We can then configure a capture format like NV12 (here in
+its multi-planar variant)
+
+.. code-block:: none
+
+    media-ctl -V "'mali-c55 resizer fr':1[fmt:YUV10_1X30/1920x1080]"
+    yavta -f NV12M -s 1920x1080 -c10 /dev/video0
+
+Capturing RGB data from the source and processing it with the resizers
+----------------------------------------------------------------------
+
+The Mali-C55 ISP can work with sensors capable of outputting RGB data. In this
+case although none of the image quality blocks would be used it can still
+crop/scale the data in the usual way.
+
+To achieve this, the ISP's sink pad's format is set to
+MEDIA_BUS_FMT_RGB202020_1X60 - this reflects the format that data must be in to
+work with the ISP. Converting the camera sensor's output to that format is the
+responsibility of external hardware.
+
+In this example we ask the test pattern generator to give us RGB data instead of
+bayer.
+
+.. code-block:: none
+
+    media-ctl -V "'mali-c55 tpg':0[fmt:RGB202020_1X60/1920x1080]"
+    media-ctl -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080]"
+
+Cropping or scaling the data can be done in exactly the same way as outlined
+earlier.
+
+Capturing raw data from the source and outputting it unmodified
+-----------------------------------------------------------------
+
+The ISP can additionally capture raw data from the source and output it on the
+full resolution pipe only, completely unmodified. In this case the downscale
+pipe can still process the data normally and be used at the same time.
+
+To configure raw bypass the FR resizer's subdevice's routing table needs to be
+configured, followed by formats in the appropriate places:
+
+.. code-block:: none
+
+    # We need to configure the routing table for the resizer to use the bypass
+    # path along with set formats on the resizer's bypass sink pad. Doing this
+    # necessitates a single media-ctl command, as multiple calls to the program
+    # reset the routing table.
+    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[0],2/0->1/0[1]]"\
+    -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080],"\
+       "'mali-c55 resizer fr':2[fmt:RGB202020_1X60/1920x1080],"\
+       "'mali-c55 resizer fr':1[fmt:RGB202020_1X60/1920x1080]"
+
+    # Set format on the video device and stream
+    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
+
+References
+==========
+.. [1] https://git.linuxtv.org/v4l-utils.git/
+.. [2] https://git.ideasonboard.org/yavta.git
+.. [3] https://developer.arm.com/Processors/Mali-C55
diff --git a/Documentation/admin-guide/media/v4l-drivers.rst b/Documentation/admin-guide/media/v4l-drivers.rst
index f4bb2605f07e..af033c892808 100644
--- a/Documentation/admin-guide/media/v4l-drivers.rst
+++ b/Documentation/admin-guide/media/v4l-drivers.rst
@@ -17,6 +17,7 @@ Video4Linux (V4L) driver-specific documentation
 	imx7
 	ipu3
 	ivtv
+	mali-c55
 	mgb4
 	omap3isp
 	omap4_camera
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/5] MAINTAINERS: Add entry for mali-c55 driver

[Daniel Scally]

Add a MAINTAINERS entry for the mali-c55 driver and its associated
documentation.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

	- none

 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6a69..bec50bfb09ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1668,6 +1668,16 @@ F:	Documentation/gpu/panfrost.rst
 F:	drivers/gpu/drm/panfrost/
 F:	include/uapi/drm/panfrost_drm.h
 
+ARM MALI-C55 ISP DRIVER
+M:	Daniel Scally <dan.scally@ideasonboard.com>
+M:	Jacopo Mondi <jacopo.mondi@ideasonboard.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/admin-guide/media/mali-c55.rst
+F:	Documentation/devicetree/bindings/media/arm,mali-c55.yaml
+F:	drivers/media/platform/arm/mali-c55/
+
 ARM MALI-DP DRM DRIVER
 M:	Liviu Dudau <liviu.dudau@arm.com>
 S:	Supported
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Laurent Pinchart]

Hi Dan,

Thank you for the patch.

On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> 
> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
> 	- Added clocks information
> 	- Fixed the warnings raised by Rob
> 
>  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> new file mode 100644
> index 000000000000..30038cfec3a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Mali-C55 Image Signal Processor
> +
> +maintainers:
> +  - Daniel Scally <dan.scally@ideasonboard.com>
> +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> +
> +properties:
> +  compatible:
> +    const: arm,mali-c55
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: ISP video clock

I wonder if we need this clock. Granted, it's an input clock to the ISP,
but it's part of the input video bus. I don't expect anyone would ever
need to control it manually, it should be provided by the video source
automatically.

> +      - description: ISP AXI clock
> +      - description: ISP AHB-lite clock

These two other clocks look good to me.

> +
> +  clock-names:
> +    items:
> +      - const: vclk
> +      - const: aclk
> +      - const: hclk
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/graph.yaml#/properties/endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    mali_c55: isp@400000 {
> +      compatible = "arm,mali-c55";
> +      reg = <0x400000 0x200000>;
> +      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> +      clock-names = "vclk", "aclk", "hclk";
> +      interrupts = <0>;
> +
> +      ports {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        port@0 {
> +          reg = <0>;
> +          isp_in: endpoint {
> +              remote-endpoint = <&mipi_out>;
> +          };
> +        };
> +      };
> +    };
> +...

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2615 bytes --]

On Wed, Feb 14, 2024 at 04:28:25PM +0200, Laurent Pinchart wrote:
> Hi Dan,
> 
> Thank you for the patch.
> 
> On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> > Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> > 
> > Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> > Changes in v2:
> > 
> > 	- Added clocks information
> > 	- Fixed the warnings raised by Rob
> > 
> >  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > new file mode 100644
> > index 000000000000..30038cfec3a4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM Mali-C55 Image Signal Processor
> > +
> > +maintainers:
> > +  - Daniel Scally <dan.scally@ideasonboard.com>
> > +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: arm,mali-c55
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: ISP video clock
> 
> I wonder if we need this clock. Granted, it's an input clock to the ISP,
> but it's part of the input video bus. I don't expect anyone would ever
> need to control it manually, it should be provided by the video source
> automatically.

I'd say that if there's a clock controller providing this clock, even if
it is implicit in the video feed it's good to have here. Being able to
increment the refcount on that clock would be good, even if you don't
actually control it manually?

> 
> > +      - description: ISP AXI clock
> > +      - description: ISP AHB-lite clock
> 
> These two other clocks look good to me.
> 
> > +
> > +  clock-names:
> > +    items:
> > +      - const: vclk
> > +      - const: aclk
> > +      - const: hclk

Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
tree clock names you've provided - they're all clocks, so having "clk"
in them is just noise :)

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Laurent Pinchart]

On Wed, Feb 14, 2024 at 05:37:17PM +0000, Conor Dooley wrote:
> On Wed, Feb 14, 2024 at 04:28:25PM +0200, Laurent Pinchart wrote:
> > On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> > > Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> > > 
> > > Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > ---
> > > Changes in v2:
> > > 
> > > 	- Added clocks information
> > > 	- Fixed the warnings raised by Rob
> > > 
> > >  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
> > >  1 file changed, 77 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > new file mode 100644
> > > index 000000000000..30038cfec3a4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > @@ -0,0 +1,77 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ARM Mali-C55 Image Signal Processor
> > > +
> > > +maintainers:
> > > +  - Daniel Scally <dan.scally@ideasonboard.com>
> > > +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: arm,mali-c55
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: ISP video clock
> > 
> > I wonder if we need this clock. Granted, it's an input clock to the ISP,
> > but it's part of the input video bus. I don't expect anyone would ever
> > need to control it manually, it should be provided by the video source
> > automatically.
> 
> I'd say that if there's a clock controller providing this clock, even if
> it is implicit in the video feed it's good to have here. Being able to
> increment the refcount on that clock would be good, even if you don't
> actually control it manually?

I don't expect there would be a clock controller to directly reference
in most cases. It depends a bit on where the clock domain crossing
between the source (often a CSI-2 receiver) and the ISP is. If it's
implemented in glue logic bundled with the ISP, which wouldn't be
described in DT as a separate node, then there's a higher chance we'll
have a clock controller for vclk. If it's implemented in the source,
vclk will just come from the source, which won't be listed as a clock
controller.

One option would be to make this clock optional, by moving it to the end
of the clocks list, and setting

	minItems: 2
	maxItems: 3

> > > +      - description: ISP AXI clock
> > > +      - description: ISP AHB-lite clock
> > 
> > These two other clocks look good to me.
> > 
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: vclk
> > > +      - const: aclk
> > > +      - const: hclk
> 
> Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
> tree clock names you've provided - they're all clocks, so having "clk"
> in them is just noise :)

As far as I understand, the names proposed by Dan come directly from the
IP core documentation.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Dan Scally]

Hi both

On 15/02/2024 11:02, Laurent Pinchart wrote:
> On Wed, Feb 14, 2024 at 05:37:17PM +0000, Conor Dooley wrote:
>> On Wed, Feb 14, 2024 at 04:28:25PM +0200, Laurent Pinchart wrote:
>>> On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
>>>> Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
>>>>
>>>> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
>>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>>>> ---
>>>> Changes in v2:
>>>>
>>>> 	- Added clocks information
>>>> 	- Fixed the warnings raised by Rob
>>>>
>>>>   .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
>>>>   1 file changed, 77 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>>>> new file mode 100644
>>>> index 000000000000..30038cfec3a4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>>>> @@ -0,0 +1,77 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: ARM Mali-C55 Image Signal Processor
>>>> +
>>>> +maintainers:
>>>> +  - Daniel Scally <dan.scally@ideasonboard.com>
>>>> +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: arm,mali-c55
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +
>>>> +  clocks:
>>>> +    items:
>>>> +      - description: ISP video clock
>>> I wonder if we need this clock. Granted, it's an input clock to the ISP,
>>> but it's part of the input video bus. I don't expect anyone would ever
>>> need to control it manually, it should be provided by the video source
>>> automatically.
>> I'd say that if there's a clock controller providing this clock, even if
>> it is implicit in the video feed it's good to have here. Being able to
>> increment the refcount on that clock would be good, even if you don't
>> actually control it manually?
> I don't expect there would be a clock controller to directly reference
> in most cases. It depends a bit on where the clock domain crossing
> between the source (often a CSI-2 receiver) and the ISP is. If it's
> implemented in glue logic bundled with the ISP, which wouldn't be
> described in DT as a separate node, then there's a higher chance we'll
> have a clock controller for vclk. If it's implemented in the source,
> vclk will just come from the source, which won't be listed as a clock
> controller.
>
> One option would be to make this clock optional, by moving it to the end
> of the clocks list, and setting
>
> 	minItems: 2
> 	maxItems: 3
>
>>>> +      - description: ISP AXI clock
>>>> +      - description: ISP AHB-lite clock
>>> These two other clocks look good to me.
>>>
>>>> +
>>>> +  clock-names:
>>>> +    items:
>>>> +      - const: vclk
>>>> +      - const: aclk
>>>> +      - const: hclk
>> Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
>> tree clock names you've provided - they're all clocks, so having "clk"
>> in them is just noise :)
> As far as I understand, the names proposed by Dan come directly from the
> IP core documentation.


This is the case, but I do take Conor's point that more descriptive names might be nicer - if I'm 
honest I just didn't think about it particularly given "Xclk" is such a common name for them 
already, but having been poked into thinking about it I do agree.

>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Laurent Pinchart]

On Fri, Feb 16, 2024 at 01:09:15PM +0000, Daniel Scally wrote:
> On 15/02/2024 11:02, Laurent Pinchart wrote:
> > On Wed, Feb 14, 2024 at 05:37:17PM +0000, Conor Dooley wrote:
> >> On Wed, Feb 14, 2024 at 04:28:25PM +0200, Laurent Pinchart wrote:
> >>> On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> >>>> Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> >>>>
> >>>> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> >>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> >>>> ---
> >>>> Changes in v2:
> >>>>
> >>>> 	- Added clocks information
> >>>> 	- Fixed the warnings raised by Rob
> >>>>
> >>>>   .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
> >>>>   1 file changed, 77 insertions(+)
> >>>>   create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..30038cfec3a4
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> >>>> @@ -0,0 +1,77 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: ARM Mali-C55 Image Signal Processor
> >>>> +
> >>>> +maintainers:
> >>>> +  - Daniel Scally <dan.scally@ideasonboard.com>
> >>>> +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >>>> +
> >>>> +properties:
> >>>> +  compatible:
> >>>> +    const: arm,mali-c55
> >>>> +
> >>>> +  reg:
> >>>> +    maxItems: 1
> >>>> +
> >>>> +  interrupts:
> >>>> +    maxItems: 1
> >>>> +
> >>>> +  clocks:
> >>>> +    items:
> >>>> +      - description: ISP video clock
> >>>
> >>> I wonder if we need this clock. Granted, it's an input clock to the ISP,
> >>> but it's part of the input video bus. I don't expect anyone would ever
> >>> need to control it manually, it should be provided by the video source
> >>> automatically.
> >>
> >> I'd say that if there's a clock controller providing this clock, even if
> >> it is implicit in the video feed it's good to have here. Being able to
> >> increment the refcount on that clock would be good, even if you don't
> >> actually control it manually?
> >
> > I don't expect there would be a clock controller to directly reference
> > in most cases. It depends a bit on where the clock domain crossing
> > between the source (often a CSI-2 receiver) and the ISP is. If it's
> > implemented in glue logic bundled with the ISP, which wouldn't be
> > described in DT as a separate node, then there's a higher chance we'll
> > have a clock controller for vclk. If it's implemented in the source,
> > vclk will just come from the source, which won't be listed as a clock
> > controller.
> >
> > One option would be to make this clock optional, by moving it to the end
> > of the clocks list, and setting
> >
> > 	minItems: 2
> > 	maxItems: 3
> >
> >>>> +      - description: ISP AXI clock
> >>>> +      - description: ISP AHB-lite clock
> >>>
> >>> These two other clocks look good to me.
> >>>
> >>>> +
> >>>> +  clock-names:
> >>>> +    items:
> >>>> +      - const: vclk
> >>>> +      - const: aclk
> >>>> +      - const: hclk
> >>
> >> Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
> >> tree clock names you've provided - they're all clocks, so having "clk"
> >> in them is just noise :)
> >
> > As far as I understand, the names proposed by Dan come directly from the
> > IP core documentation.
> 
> This is the case, but I do take Conor's point that more descriptive names might be nicer - if I'm 
> honest I just didn't think about it particularly given "Xclk" is such a common name for them 
> already, but having been poked into thinking about it I do agree.

Isn't the usual practice in DT bindings is to name GPIOs, clocks and reset
signals based on the hardware documentation ?

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Dan Scally]

On 16/02/2024 13:27, Laurent Pinchart wrote:
> On Fri, Feb 16, 2024 at 01:09:15PM +0000, Daniel Scally wrote:
>> On 15/02/2024 11:02, Laurent Pinchart wrote:
>>> On Wed, Feb 14, 2024 at 05:37:17PM +0000, Conor Dooley wrote:
>>>> On Wed, Feb 14, 2024 at 04:28:25PM +0200, Laurent Pinchart wrote:
>>>>> On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
>>>>>> Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
>>>>>>
>>>>>> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
>>>>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>>
>>>>>> 	- Added clocks information
>>>>>> 	- Fixed the warnings raised by Rob
>>>>>>
>>>>>>    .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
>>>>>>    1 file changed, 77 insertions(+)
>>>>>>    create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..30038cfec3a4
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
>>>>>> @@ -0,0 +1,77 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: ARM Mali-C55 Image Signal Processor
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Daniel Scally <dan.scally@ideasonboard.com>
>>>>>> +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: arm,mali-c55
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  interrupts:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  clocks:
>>>>>> +    items:
>>>>>> +      - description: ISP video clock
>>>>> I wonder if we need this clock. Granted, it's an input clock to the ISP,
>>>>> but it's part of the input video bus. I don't expect anyone would ever
>>>>> need to control it manually, it should be provided by the video source
>>>>> automatically.
>>>> I'd say that if there's a clock controller providing this clock, even if
>>>> it is implicit in the video feed it's good to have here. Being able to
>>>> increment the refcount on that clock would be good, even if you don't
>>>> actually control it manually?
>>> I don't expect there would be a clock controller to directly reference
>>> in most cases. It depends a bit on where the clock domain crossing
>>> between the source (often a CSI-2 receiver) and the ISP is. If it's
>>> implemented in glue logic bundled with the ISP, which wouldn't be
>>> described in DT as a separate node, then there's a higher chance we'll
>>> have a clock controller for vclk. If it's implemented in the source,
>>> vclk will just come from the source, which won't be listed as a clock
>>> controller.
>>>
>>> One option would be to make this clock optional, by moving it to the end
>>> of the clocks list, and setting
>>>
>>> 	minItems: 2
>>> 	maxItems: 3
>>>
>>>>>> +      - description: ISP AXI clock
>>>>>> +      - description: ISP AHB-lite clock
>>>>> These two other clocks look good to me.
>>>>>
>>>>>> +
>>>>>> +  clock-names:
>>>>>> +    items:
>>>>>> +      - const: vclk
>>>>>> +      - const: aclk
>>>>>> +      - const: hclk
>>>> Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
>>>> tree clock names you've provided - they're all clocks, so having "clk"
>>>> in them is just noise :)
>>> As far as I understand, the names proposed by Dan come directly from the
>>> IP core documentation.
>> This is the case, but I do take Conor's point that more descriptive names might be nicer - if I'm
>> honest I just didn't think about it particularly given "Xclk" is such a common name for them
>> already, but having been poked into thinking about it I do agree.
> Isn't the usual practice in DT bindings is to name GPIOs, clocks and reset
> signals based on the hardware documentation ?


Ah - I don't know honestly. If that's so then yeah - these are the names the documentation prescribes.

>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1328 bytes --]

On Fri, Feb 16, 2024 at 02:45:31PM +0000, Dan Scally wrote:

> > > > > > > +      - description: ISP AXI clock
> > > > > > > +      - description: ISP AHB-lite clock
> > > > > > These two other clocks look good to me.
> > > > > > 
> > > > > > > +
> > > > > > > +  clock-names:
> > > > > > > +    items:
> > > > > > > +      - const: vclk
> > > > > > > +      - const: aclk
> > > > > > > +      - const: hclk
> > > > > Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
> > > > > tree clock names you've provided - they're all clocks, so having "clk"
> > > > > in them is just noise :)
> > > > As far as I understand, the names proposed by Dan come directly from the
> > > > IP core documentation.
> > > This is the case, but I do take Conor's point that more descriptive names might be nicer - if I'm
> > > honest I just didn't think about it particularly given "Xclk" is such a common name for them
> > > already, but having been poked into thinking about it I do agree.
> > Isn't the usual practice in DT bindings is to name GPIOs, clocks and reset
> > signals based on the hardware documentation ?
> 
> 
> Ah - I don't know honestly. If that's so then yeah - these are the names the documentation prescribes.

If a direct doc match is what you're going for, then sure, keep it.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Rob Herring]

On Fri, Feb 16, 2024 at 07:07:58PM +0000, Conor Dooley wrote:
> On Fri, Feb 16, 2024 at 02:45:31PM +0000, Dan Scally wrote:
> 
> > > > > > > > +      - description: ISP AXI clock
> > > > > > > > +      - description: ISP AHB-lite clock
> > > > > > > These two other clocks look good to me.
> > > > > > > 
> > > > > > > > +
> > > > > > > > +  clock-names:
> > > > > > > > +    items:
> > > > > > > > +      - const: vclk
> > > > > > > > +      - const: aclk
> > > > > > > > +      - const: hclk
> > > > > > Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
> > > > > > tree clock names you've provided - they're all clocks, so having "clk"
> > > > > > in them is just noise :)
> > > > > As far as I understand, the names proposed by Dan come directly from the
> > > > > IP core documentation.
> > > > This is the case, but I do take Conor's point that more descriptive names might be nicer - if I'm
> > > > honest I just didn't think about it particularly given "Xclk" is such a common name for them
> > > > already, but having been poked into thinking about it I do agree.
> > > Isn't the usual practice in DT bindings is to name GPIOs, clocks and reset
> > > signals based on the hardware documentation ?
> > 
> > 
> > Ah - I don't know honestly. If that's so then yeah - these are the names the documentation prescribes.
> 
> If a direct doc match is what you're going for, then sure, keep it.

pclk, aclk, and hclk are generally the names used for APB, AXI, and AHB 
bus clocks, so I'd stick with them. Though we also have cases of the bus 
names used... 

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] media: Documentation: Add Mali-C55 ISP Documentation

[Kieran Bingham]

Hi Dan,

Quoting Daniel Scally (2024-02-14 14:19:05)
> Add a documentation page for the mali-c55 driver, which gives a brief
> overview of the hardware and explains how to use the driver's capture
> devices and the crop/scaler functions.
> 
> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
>         - none
> 
>  .../admin-guide/media/mali-c55-graph.dot      |  19 ++
>  Documentation/admin-guide/media/mali-c55.rst  | 318 ++++++++++++++++++
>  .../admin-guide/media/v4l-drivers.rst         |   1 +
>  3 files changed, 338 insertions(+)
>  create mode 100644 Documentation/admin-guide/media/mali-c55-graph.dot
>  create mode 100644 Documentation/admin-guide/media/mali-c55.rst
> 
> diff --git a/Documentation/admin-guide/media/mali-c55-graph.dot b/Documentation/admin-guide/media/mali-c55-graph.dot
> new file mode 100644
> index 000000000000..0775ba42bf4c
> --- /dev/null
> +++ b/Documentation/admin-guide/media/mali-c55-graph.dot
> @@ -0,0 +1,19 @@
> +digraph board {
> +        rankdir=TB
> +        n00000001 [label="{{} | mali-c55 tpg\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> +        n00000001:port0 -> n00000003:port0 [style=dashed]
> +        n00000003 [label="{{<port0> 0} | mali-c55 isp\n/dev/v4l-subdev1 | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
> +        n00000003:port1 -> n00000007:port0 [style=bold]
> +        n00000003:port2 -> n00000007:port2 [style=bold]
> +        n00000003:port1 -> n0000000b:port0 [style=bold]
> +        n00000007 [label="{{<port0> 0 | <port2> 2} | mali-c55 resizer fr\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +        n00000007:port1 -> n0000000e [style=bold]
> +        n0000000b [label="{{<port0> 0} | mali-c55 resizer ds\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +        n0000000b:port1 -> n00000012 [style=bold]
> +        n0000000e [label="mali-c55 fr\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> +        n00000012 [label="mali-c55 ds\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> +        n00000022 [label="{{<port0> 0} | csi2-rx\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> +        n00000022:port1 -> n00000003:port0
> +        n00000027 [label="{{} | imx415 1-001a\n/dev/v4l-subdev5 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> +        n00000027:port0 -> n00000022:port0 [style=bold]
> +}
> \ No newline at end of file
> diff --git a/Documentation/admin-guide/media/mali-c55.rst b/Documentation/admin-guide/media/mali-c55.rst
> new file mode 100644
> index 000000000000..83f630c3bd9d
> --- /dev/null
> +++ b/Documentation/admin-guide/media/mali-c55.rst
> @@ -0,0 +1,318 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==========================================
> +ARM Mali-C55 Image Signal Processor driver
> +==========================================
> +
> +Introduction
> +============
> +
> +This file documents the driver for ARM's Mali-C55 Image Signal Processor. The
> +driver is located under drivers/media/platform/arm/mali-c55.
> +
> +The Mali-C55 ISP receives data in either raw Bayer format or RGB/YUV format from
> +sensors through either a parallel interface or a memory bus before processing it
> +and outputting it through an internal DMA engine. Two output pipelines are
> +possible (though one may not be fitted, depending on the implementation). These
> +are referred to as "Full resolution" and "Downscale", but the naming is historic
> +and both pipes are capable of cropping/scaling operations. An integrated test
> +pattern generator can be used to drive the ISP and produce image data in the
> +absence of a connected camera sensor. The driver module is named mali_c55, and
> +is enabled through the CONFIG_VIDEO_MALI_C55 config option.

Can it handle metadata or other datatypes separately? anything else
that's worthy of mention? Or maybe not yet in this version.

> +
> +The driver implements V4L2, Media Controller and V4L2 Subdevice interfaces and
> +expects camera sensors connected to the ISP to have V4L2 subdevice interfaces.
> +
> +Mali-C55 ISP hardware
> +=====================
> +
> +A high level functional view of the Mali-C55 ISP is presented below. The ISP
> +takes input from either a live source or through a DMA engine for memory input,
> +depending on the SoC integration.::
> +
> +  +---------+    +----------+                                     +--------+
> +  | Sensor  |--->| CSI-2 Rx |                "Full Resolution"    |  DMA   |
> +  +---------+    +----------+   |\                 Output    +--->| Writer |
> +                       |        | \                          |    +--------+
> +                       |        |  \    +----------+  +------+---> Streaming I/O
> +  +------------+       +------->|   |   |          |  |
> +  |            |                |   |-->| Mali-C55 |--+
> +  | DMA Reader |--------------->|   |   |    ISP   |  |
> +  |            |                |  /    |          |  |      +---> Streaming I/O
> +  +------------+                | /     +----------+  |      |
> +                                |/                    +------+
> +                                                            |    +--------+
> +                                                             +--->|  DMA   |
> +                                               "Downscaled"       | Writer |
> +                                                 Output          +--------+
> +
> +Media Controller Topology
> +=========================
> +
> +An example of the ISP's topology (as implemented in a system with an IMX415
> +camera sensor and generic CSI-2 receiver) is below:
> +
> +
> +.. kernel-figure:: mali-c55-graph.dot
> +    :alt:   mali-c55-graph.dot
> +    :align: center
> +
> +The driver has 4 V4L2 subdevices:
> +
> +- `mali_c55 isp`: Responsible for configuring input crop and color space
> +                  conversion
> +- `mali_c55 tpg`: The test pattern generator, emulating a camera sensor.
> +- `mali_c55 resizer fr`: The Full-Resolution pipe resizer
> +- `mali_c55 resizer ds`: The Downscale pipe resizer
> +
> +The driver has 2 V4L2 video devices:
> +
> +- `mali-c55 fr`: The full-resolution pipe's capture device
> +- `mali-c55 ds`: The downscale pipe's capture device
> +
> +Idiosyncrasies
> +--------------
> +
> +**mali-c55 isp**
> +The `mali-c55 isp` subdevice has a single sink pad to which all sources of data
> +should be connected. The active source is selected by enabling the appropriate
> +media link and disabling all others.

By that I presume you mean you can only accept a single input link at a
time?

> The ISP has two source pads, reflecting the
> +different paths through which it can internally route data. Tap points within
> +the ISP allow users to divert data to avoid processing by some or all of the
> +hardware's processing steps. The diagram below is intended only to highlight how
> +the bypassing works and is not a true reflection of those processing steps; for
> +a high-level functional block diagram see ARM's developer page for the
> +ISP [3]_::
> +
> +  +--------------------------------------------------------------+
> +  |                Possible Internal ISP Data Routes             |
> +  |          +------------+  +----------+  +------------+        |
> +  +---+      |            |  |          |  |  Colour    |    +---+
> +  | 0 |--+-->| Processing |->| Demosaic |->|   Space    |--->| 1 |
> +  +---+  |   |            |  |          |  | Conversion |    +---+
> +  |      |   +------------+  +----------+  +------------+        |
> +  |      |                                                   +---+
> +  |      +---------------------------------------------------| 2 |
> +  |                                                          +---+
> +  |                                                              |
> +  +--------------------------------------------------------------+
> +
> +
> +.. flat-table::
> +    :header-rows: 1
> +
> +    * - Pad
> +      - Direction
> +      - Purpose
> +
> +    * - 0
> +      - sink
> +      - Data input, connected to the TPG and camera sensors
> +
> +    * - 1
> +      - source
> +      - RGB/YUV data, connected to the FR and DS V4L2 subdevices
> +
> +    * - 2
> +      - source
> +      - RAW bayer data, connected to the FR V4L2 subdevices
> +
> +**mali-c55 resizer fr**
> +The `mali-c55 resizer fr` subdevice has two _sink_ pads to reflect the different
> +insertion points in the hardware (either RAW or demosaiced data):
> +
> +.. flat-table::
> +    :header-rows: 1
> +
> +    * - Pad
> +      - Direction
> +      - Purpose
> +
> +    * - 0
> +      - sink
> +      - Data input connected to the ISP's demosaiced stream.
> +
> +    * - 1
> +      - source
> +      - Data output connected to the capture video device
> +
> +    * - 2
> +      - sink
> +      - Data input connected to the ISP's raw data stream
> +
> +The data source in use is selected through the routing API; two routes each of a
> +single stream are available:
> +
> +.. flat-table::
> +    :header-rows: 1
> +
> +    * - Sink Pad
> +      - Source Pad
> +      - Purpose
> +
> +    * - 0
> +      - 1
> +      - Demosaiced data route
> +
> +    * - 2
> +      - 1
> +      - Raw data route
> +
> +
> +If the demosaiced route is active then the FR pipe is only capable of output
> +in RGB/YUV formats. If the raw route is active then the output reflects the
> +input (which may be either Bayer or RGB/YUV data).
> +
> +Using the driver to capture video
> +=================================
> +
> +Using the media controller APIs we can configure the input source and ISP to
> +capture images in a variety of formats. In the examples below, configuring the
> +media graph is done with the v4l-utils [1]_ package's media-ctl utility.
> +Capturing the images is done with yavta [2]_.
> +
> +Configuring the input source
> +----------------------------
> +
> +The first step is to set the input source that we wish by enabling the correct
> +media link. Using the example topology above, we can select the TPG as follows:
> +
> +.. code-block:: none
> +
> +    media-ctl -l "'lte-csi2-rx':1->'mali-c55 isp':0[0]"
> +    media-ctl -l "'mali-c55 tpg':0->'mali-c55 isp':0[1]"
> +
> +Capturing bayer data from the source and processing to RGB/YUV
> +--------------------------------------------------------------
> +To capture 1920x1080 bayer data from the source and push it through the ISP's
> +full processing pipeline, we configure the data formats appropriately on the
> +source, ISP and resizer subdevices and set the FR resizer's routing to select
> +processed data. The media bus format on the resizer's source pad will be either
> +RGB121212_1X36 or YUV10_1X30, depending on whether you want to capture RGB or
> +YUV. The ISP's debayering block outputs RGB data natively, setting the source
> +pad format to YUV10_1X30 enables the colour space conversion block.
> +
> +In this example we target RGB565 output, so select RGB121212_1X36 as the resizer
> +source pad's format:
> +
> +.. code-block:: none
> +
> +    # Set formats on the TPG and ISP
> +    media-ctl -V "'mali-c55 tpg':0[fmt:SRGGB16_1X16/1920x1080]"
> +    media-ctl -V "'mali-c55 isp':0[fmt:SRGGB16_1X16/1920x1080]"
> +    media-ctl -V "'mali-c55 isp':1[fmt:SRGGB16_1X16/1920x1080]"
> +
> +    # Set routing on the FR resizer
> +    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[1],2/0->1/0[0]]"
> +
> +    # Set format on the resizer, must be done AFTER the routing.
> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/1920x1080]"
> +
> +The downscale output can also be used to stream data at the same time. In this
> +case only processed data can be captured and so no routing need be set:

Is the route automatically reset? or is it just that the default route
is already the correct choice for this instance?

> +
> +.. code-block:: none
> +
> +    # Set format on the resizer
> +    media-ctl -V "'mali-c55 resizer ds':1[fmt:RGB121212_1X36/1920x1080]"
> +
> +Following which images can be captured from both the FR and DS output's video
> +devices (simultaneously, if desired):
> +
> +.. code-block:: none
> +
> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video1
> +

Is there any synchronisation required? Can one pipe run without the
other if they are both enabled? or will stalling on one pipe stall the
other?

> +Cropping the image
> +~~~~~~~~~~~~~~~~~~
> +
> +Both the full resolution and downscale pipes can crop to a minimum resolution of
> +640x480. To crop the image simply configure the resizer's sink pad's crop and
> +compose rectangles and set the format on the video device:
> +
> +.. code-block:: none
> +
> +    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(480,270)/640x480 compose:(0,0)/640x480]"
> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
> +    yavta -f RGB565 -s 640x480 -c10 /dev/video0
> +
> +Downscaling the image
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +Both the full resolution and downscale pipes can downscale the image by up to 8x
> +provided the minimum 640x480 resolution is adhered to. For the best image result

'minimum 640x480 output resolution' I presume.

Maybe the ISP limits/restrictions need to be defined before here?

> +the scaling ratio for each dimension should be the same. To configure scaling we
> +use the compose rectangle on the resizer's sink pad:
> +
> +.. code-block:: none
> +
> +    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(0,0)/1920x1080 compose:(0,0)/640x480]"
> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
> +    yavta -f RGB565 -s 640x480 -c10 /dev/video0
> +
> +Capturing images in YUV formats
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +If we need to output YUV data rather than RGB the color space conversion block
> +needs to be active, which is achieved by setting MEDIA_BUS_FMT_YUV10_1X30 on the

Is 10 bit required here? (As opposed to the 12 bit before in the
pipeline? is that a limitation on the component?) or is this to match
the desired output format?

> +resizer's source pad. We can then configure a capture format like NV12 (here in
> +its multi-planar variant)
> +
> +.. code-block:: none
> +
> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:YUV10_1X30/1920x1080]"
> +    yavta -f NV12M -s 1920x1080 -c10 /dev/video0
> +
> +Capturing RGB data from the source and processing it with the resizers
> +----------------------------------------------------------------------
> +
> +The Mali-C55 ISP can work with sensors capable of outputting RGB data. In this
> +case although none of the image quality blocks would be used it can still
> +crop/scale the data in the usual way.
> +
> +To achieve this, the ISP's sink pad's format is set to
> +MEDIA_BUS_FMT_RGB202020_1X60 - this reflects the format that data must be in to
> +work with the ISP. Converting the camera sensor's output to that format is the
> +responsibility of external hardware.
> +
> +In this example we ask the test pattern generator to give us RGB data instead of
> +bayer.
> +
> +.. code-block:: none
> +
> +    media-ctl -V "'mali-c55 tpg':0[fmt:RGB202020_1X60/1920x1080]"
> +    media-ctl -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080]"
> +
> +Cropping or scaling the data can be done in exactly the same way as outlined
> +earlier.
> +
> +Capturing raw data from the source and outputting it unmodified
> +-----------------------------------------------------------------
> +
> +The ISP can additionally capture raw data from the source and output it on the
> +full resolution pipe only, completely unmodified. In this case the downscale
> +pipe can still process the data normally and be used at the same time.
> +
> +To configure raw bypass the FR resizer's subdevice's routing table needs to be
> +configured, followed by formats in the appropriate places:
> +
> +.. code-block:: none
> +
> +    # We need to configure the routing table for the resizer to use the bypass
> +    # path along with set formats on the resizer's bypass sink pad. Doing this
> +    # necessitates a single media-ctl command, as multiple calls to the program
> +    # reset the routing table.
> +    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[0],2/0->1/0[1]]"\
> +    -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080],"\
> +       "'mali-c55 resizer fr':2[fmt:RGB202020_1X60/1920x1080],"\
> +       "'mali-c55 resizer fr':1[fmt:RGB202020_1X60/1920x1080]"
> +
> +    # Set format on the video device and stream
> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
> +
> +References
> +==========
> +.. [1] https://git.linuxtv.org/v4l-utils.git/
> +.. [2] https://git.ideasonboard.org/yavta.git
> +.. [3] https://developer.arm.com/Processors/Mali-C55
> diff --git a/Documentation/admin-guide/media/v4l-drivers.rst b/Documentation/admin-guide/media/v4l-drivers.rst
> index f4bb2605f07e..af033c892808 100644
> --- a/Documentation/admin-guide/media/v4l-drivers.rst
> +++ b/Documentation/admin-guide/media/v4l-drivers.rst
> @@ -17,6 +17,7 @@ Video4Linux (V4L) driver-specific documentation
>         imx7
>         ipu3
>         ivtv
> +       mali-c55
>         mgb4
>         omap3isp
>         omap4_camera
> -- 
> 2.34.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] media: Documentation: Add Mali-C55 ISP Documentation

[Dan Scally]

Morning Kieran

On 25/02/2024 21:22, Kieran Bingham wrote:
> Hi Dan,
>
> Quoting Daniel Scally (2024-02-14 14:19:05)
>> Add a documentation page for the mali-c55 driver, which gives a brief
>> overview of the hardware and explains how to use the driver's capture
>> devices and the crop/scaler functions.
>>
>> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>> Changes in v2:
>>
>>          - none
>>
>>   .../admin-guide/media/mali-c55-graph.dot      |  19 ++
>>   Documentation/admin-guide/media/mali-c55.rst  | 318 ++++++++++++++++++
>>   .../admin-guide/media/v4l-drivers.rst         |   1 +
>>   3 files changed, 338 insertions(+)
>>   create mode 100644 Documentation/admin-guide/media/mali-c55-graph.dot
>>   create mode 100644 Documentation/admin-guide/media/mali-c55.rst
>>
>> diff --git a/Documentation/admin-guide/media/mali-c55-graph.dot b/Documentation/admin-guide/media/mali-c55-graph.dot
>> new file mode 100644
>> index 000000000000..0775ba42bf4c
>> --- /dev/null
>> +++ b/Documentation/admin-guide/media/mali-c55-graph.dot
>> @@ -0,0 +1,19 @@
>> +digraph board {
>> +        rankdir=TB
>> +        n00000001 [label="{{} | mali-c55 tpg\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
>> +        n00000001:port0 -> n00000003:port0 [style=dashed]
>> +        n00000003 [label="{{<port0> 0} | mali-c55 isp\n/dev/v4l-subdev1 | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
>> +        n00000003:port1 -> n00000007:port0 [style=bold]
>> +        n00000003:port2 -> n00000007:port2 [style=bold]
>> +        n00000003:port1 -> n0000000b:port0 [style=bold]
>> +        n00000007 [label="{{<port0> 0 | <port2> 2} | mali-c55 resizer fr\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>> +        n00000007:port1 -> n0000000e [style=bold]
>> +        n0000000b [label="{{<port0> 0} | mali-c55 resizer ds\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>> +        n0000000b:port1 -> n00000012 [style=bold]
>> +        n0000000e [label="mali-c55 fr\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
>> +        n00000012 [label="mali-c55 ds\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
>> +        n00000022 [label="{{<port0> 0} | csi2-rx\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
>> +        n00000022:port1 -> n00000003:port0
>> +        n00000027 [label="{{} | imx415 1-001a\n/dev/v4l-subdev5 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
>> +        n00000027:port0 -> n00000022:port0 [style=bold]
>> +}
>> \ No newline at end of file
>> diff --git a/Documentation/admin-guide/media/mali-c55.rst b/Documentation/admin-guide/media/mali-c55.rst
>> new file mode 100644
>> index 000000000000..83f630c3bd9d
>> --- /dev/null
>> +++ b/Documentation/admin-guide/media/mali-c55.rst
>> @@ -0,0 +1,318 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +==========================================
>> +ARM Mali-C55 Image Signal Processor driver
>> +==========================================
>> +
>> +Introduction
>> +============
>> +
>> +This file documents the driver for ARM's Mali-C55 Image Signal Processor. The
>> +driver is located under drivers/media/platform/arm/mali-c55.
>> +
>> +The Mali-C55 ISP receives data in either raw Bayer format or RGB/YUV format from
>> +sensors through either a parallel interface or a memory bus before processing it
>> +and outputting it through an internal DMA engine. Two output pipelines are
>> +possible (though one may not be fitted, depending on the implementation). These
>> +are referred to as "Full resolution" and "Downscale", but the naming is historic
>> +and both pipes are capable of cropping/scaling operations. An integrated test
>> +pattern generator can be used to drive the ISP and produce image data in the
>> +absence of a connected camera sensor. The driver module is named mali_c55, and
>> +is enabled through the CONFIG_VIDEO_MALI_C55 config option.
> Can it handle metadata or other datatypes separately? anything else
> that's worthy of mention? Or maybe not yet in this version.


As in, can you stream the parameters/statistics without streaming video data? Not at present but the 
change to add that ability would be very very minor - though none of that is in this version.

>
>> +
>> +The driver implements V4L2, Media Controller and V4L2 Subdevice interfaces and
>> +expects camera sensors connected to the ISP to have V4L2 subdevice interfaces.
>> +
>> +Mali-C55 ISP hardware
>> +=====================
>> +
>> +A high level functional view of the Mali-C55 ISP is presented below. The ISP
>> +takes input from either a live source or through a DMA engine for memory input,
>> +depending on the SoC integration.::
>> +
>> +  +---------+    +----------+                                     +--------+
>> +  | Sensor  |--->| CSI-2 Rx |                "Full Resolution"    |  DMA   |
>> +  +---------+    +----------+   |\                 Output    +--->| Writer |
>> +                       |        | \                          |    +--------+
>> +                       |        |  \    +----------+  +------+---> Streaming I/O
>> +  +------------+       +------->|   |   |          |  |
>> +  |            |                |   |-->| Mali-C55 |--+
>> +  | DMA Reader |--------------->|   |   |    ISP   |  |
>> +  |            |                |  /    |          |  |      +---> Streaming I/O
>> +  +------------+                | /     +----------+  |      |
>> +                                |/                    +------+
>> +                                                            |    +--------+
>> +                                                             +--->|  DMA   |
>> +                                               "Downscaled"       | Writer |
>> +                                                 Output          +--------+
>> +
>> +Media Controller Topology
>> +=========================
>> +
>> +An example of the ISP's topology (as implemented in a system with an IMX415
>> +camera sensor and generic CSI-2 receiver) is below:
>> +
>> +
>> +.. kernel-figure:: mali-c55-graph.dot
>> +    :alt:   mali-c55-graph.dot
>> +    :align: center
>> +
>> +The driver has 4 V4L2 subdevices:
>> +
>> +- `mali_c55 isp`: Responsible for configuring input crop and color space
>> +                  conversion
>> +- `mali_c55 tpg`: The test pattern generator, emulating a camera sensor.
>> +- `mali_c55 resizer fr`: The Full-Resolution pipe resizer
>> +- `mali_c55 resizer ds`: The Downscale pipe resizer
>> +
>> +The driver has 2 V4L2 video devices:
>> +
>> +- `mali-c55 fr`: The full-resolution pipe's capture device
>> +- `mali-c55 ds`: The downscale pipe's capture device
>> +
>> +Idiosyncrasies
>> +--------------
>> +
>> +**mali-c55 isp**
>> +The `mali-c55 isp` subdevice has a single sink pad to which all sources of data
>> +should be connected. The active source is selected by enabling the appropriate
>> +media link and disabling all others.
> By that I presume you mean you can only accept a single input link at a
> time?


Yes

>
>> The ISP has two source pads, reflecting the
>> +different paths through which it can internally route data. Tap points within
>> +the ISP allow users to divert data to avoid processing by some or all of the
>> +hardware's processing steps. The diagram below is intended only to highlight how
>> +the bypassing works and is not a true reflection of those processing steps; for
>> +a high-level functional block diagram see ARM's developer page for the
>> +ISP [3]_::
>> +
>> +  +--------------------------------------------------------------+
>> +  |                Possible Internal ISP Data Routes             |
>> +  |          +------------+  +----------+  +------------+        |
>> +  +---+      |            |  |          |  |  Colour    |    +---+
>> +  | 0 |--+-->| Processing |->| Demosaic |->|   Space    |--->| 1 |
>> +  +---+  |   |            |  |          |  | Conversion |    +---+
>> +  |      |   +------------+  +----------+  +------------+        |
>> +  |      |                                                   +---+
>> +  |      +---------------------------------------------------| 2 |
>> +  |                                                          +---+
>> +  |                                                              |
>> +  +--------------------------------------------------------------+
>> +
>> +
>> +.. flat-table::
>> +    :header-rows: 1
>> +
>> +    * - Pad
>> +      - Direction
>> +      - Purpose
>> +
>> +    * - 0
>> +      - sink
>> +      - Data input, connected to the TPG and camera sensors
>> +
>> +    * - 1
>> +      - source
>> +      - RGB/YUV data, connected to the FR and DS V4L2 subdevices
>> +
>> +    * - 2
>> +      - source
>> +      - RAW bayer data, connected to the FR V4L2 subdevices
>> +
>> +**mali-c55 resizer fr**
>> +The `mali-c55 resizer fr` subdevice has two _sink_ pads to reflect the different
>> +insertion points in the hardware (either RAW or demosaiced data):
>> +
>> +.. flat-table::
>> +    :header-rows: 1
>> +
>> +    * - Pad
>> +      - Direction
>> +      - Purpose
>> +
>> +    * - 0
>> +      - sink
>> +      - Data input connected to the ISP's demosaiced stream.
>> +
>> +    * - 1
>> +      - source
>> +      - Data output connected to the capture video device
>> +
>> +    * - 2
>> +      - sink
>> +      - Data input connected to the ISP's raw data stream
>> +
>> +The data source in use is selected through the routing API; two routes each of a
>> +single stream are available:
>> +
>> +.. flat-table::
>> +    :header-rows: 1
>> +
>> +    * - Sink Pad
>> +      - Source Pad
>> +      - Purpose
>> +
>> +    * - 0
>> +      - 1
>> +      - Demosaiced data route
>> +
>> +    * - 2
>> +      - 1
>> +      - Raw data route
>> +
>> +
>> +If the demosaiced route is active then the FR pipe is only capable of output
>> +in RGB/YUV formats. If the raw route is active then the output reflects the
>> +input (which may be either Bayer or RGB/YUV data).
>> +
>> +Using the driver to capture video
>> +=================================
>> +
>> +Using the media controller APIs we can configure the input source and ISP to
>> +capture images in a variety of formats. In the examples below, configuring the
>> +media graph is done with the v4l-utils [1]_ package's media-ctl utility.
>> +Capturing the images is done with yavta [2]_.
>> +
>> +Configuring the input source
>> +----------------------------
>> +
>> +The first step is to set the input source that we wish by enabling the correct
>> +media link. Using the example topology above, we can select the TPG as follows:
>> +
>> +.. code-block:: none
>> +
>> +    media-ctl -l "'lte-csi2-rx':1->'mali-c55 isp':0[0]"
>> +    media-ctl -l "'mali-c55 tpg':0->'mali-c55 isp':0[1]"
>> +
>> +Capturing bayer data from the source and processing to RGB/YUV
>> +--------------------------------------------------------------
>> +To capture 1920x1080 bayer data from the source and push it through the ISP's
>> +full processing pipeline, we configure the data formats appropriately on the
>> +source, ISP and resizer subdevices and set the FR resizer's routing to select
>> +processed data. The media bus format on the resizer's source pad will be either
>> +RGB121212_1X36 or YUV10_1X30, depending on whether you want to capture RGB or
>> +YUV. The ISP's debayering block outputs RGB data natively, setting the source
>> +pad format to YUV10_1X30 enables the colour space conversion block.
>> +
>> +In this example we target RGB565 output, so select RGB121212_1X36 as the resizer
>> +source pad's format:
>> +
>> +.. code-block:: none
>> +
>> +    # Set formats on the TPG and ISP
>> +    media-ctl -V "'mali-c55 tpg':0[fmt:SRGGB16_1X16/1920x1080]"
>> +    media-ctl -V "'mali-c55 isp':0[fmt:SRGGB16_1X16/1920x1080]"
>> +    media-ctl -V "'mali-c55 isp':1[fmt:SRGGB16_1X16/1920x1080]"
>> +
>> +    # Set routing on the FR resizer
>> +    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[1],2/0->1/0[0]]"
>> +
>> +    # Set format on the resizer, must be done AFTER the routing.
>> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/1920x1080]"
>> +
>> +The downscale output can also be used to stream data at the same time. In this
>> +case only processed data can be captured and so no routing need be set:
> Is the route automatically reset? or is it just that the default route
> is already the correct choice for this instance?


Err, the default route is already the correct choice for this instance but I'm nonetheless setting 
it above...perhaps I'm misunderstanding the question?

>
>> +
>> +.. code-block:: none
>> +
>> +    # Set format on the resizer
>> +    media-ctl -V "'mali-c55 resizer ds':1[fmt:RGB121212_1X36/1920x1080]"
>> +
>> +Following which images can be captured from both the FR and DS output's video
>> +devices (simultaneously, if desired):
>> +
>> +.. code-block:: none
>> +
>> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
>> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video1
>> +
> Is there any synchronisation required? Can one pipe run without the
> other if they are both enabled? or will stalling on one pipe stall the
> other?


The outputs are synchronised already to the extent that if both are enabled they'll receive data 
from the same input frame at the same time. They can be run independently though, if an application 
stopped queuing frames to one but continued queuing to the other then the latter would work fine.

>
>> +Cropping the image
>> +~~~~~~~~~~~~~~~~~~
>> +
>> +Both the full resolution and downscale pipes can crop to a minimum resolution of
>> +640x480. To crop the image simply configure the resizer's sink pad's crop and
>> +compose rectangles and set the format on the video device:
>> +
>> +.. code-block:: none
>> +
>> +    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(480,270)/640x480 compose:(0,0)/640x480]"
>> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
>> +    yavta -f RGB565 -s 640x480 -c10 /dev/video0
>> +
>> +Downscaling the image
>> +~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Both the full resolution and downscale pipes can downscale the image by up to 8x
>> +provided the minimum 640x480 resolution is adhered to. For the best image result
> 'minimum 640x480 output resolution' I presume.


And input, actually.

>
> Maybe the ISP limits/restrictions need to be defined before here?


I'll expand on them yes.

>
>> +the scaling ratio for each dimension should be the same. To configure scaling we
>> +use the compose rectangle on the resizer's sink pad:
>> +
>> +.. code-block:: none
>> +
>> +    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(0,0)/1920x1080 compose:(0,0)/640x480]"
>> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
>> +    yavta -f RGB565 -s 640x480 -c10 /dev/video0
>> +
>> +Capturing images in YUV formats
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +If we need to output YUV data rather than RGB the color space conversion block
>> +needs to be active, which is achieved by setting MEDIA_BUS_FMT_YUV10_1X30 on the
> Is 10 bit required here? (As opposed to the 12 bit before in the
> pipeline? is that a limitation on the component?)


It's a reflection of the output from the colour-space conversion block.

> or is this to match
> the desired output format?


Sort of, it's used to inform the driver about which of the output formats can be generated - without 
conversion to YUV here only RGB formats can be set on the capture device nodes.

>
>> +resizer's source pad. We can then configure a capture format like NV12 (here in
>> +its multi-planar variant)
>> +
>> +.. code-block:: none
>> +
>> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:YUV10_1X30/1920x1080]"
>> +    yavta -f NV12M -s 1920x1080 -c10 /dev/video0
>> +
>> +Capturing RGB data from the source and processing it with the resizers
>> +----------------------------------------------------------------------
>> +
>> +The Mali-C55 ISP can work with sensors capable of outputting RGB data. In this
>> +case although none of the image quality blocks would be used it can still
>> +crop/scale the data in the usual way.
>> +
>> +To achieve this, the ISP's sink pad's format is set to
>> +MEDIA_BUS_FMT_RGB202020_1X60 - this reflects the format that data must be in to
>> +work with the ISP. Converting the camera sensor's output to that format is the
>> +responsibility of external hardware.
>> +
>> +In this example we ask the test pattern generator to give us RGB data instead of
>> +bayer.
>> +
>> +.. code-block:: none
>> +
>> +    media-ctl -V "'mali-c55 tpg':0[fmt:RGB202020_1X60/1920x1080]"
>> +    media-ctl -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080]"
>> +
>> +Cropping or scaling the data can be done in exactly the same way as outlined
>> +earlier.
>> +
>> +Capturing raw data from the source and outputting it unmodified
>> +-----------------------------------------------------------------
>> +
>> +The ISP can additionally capture raw data from the source and output it on the
>> +full resolution pipe only, completely unmodified. In this case the downscale
>> +pipe can still process the data normally and be used at the same time.
>> +
>> +To configure raw bypass the FR resizer's subdevice's routing table needs to be
>> +configured, followed by formats in the appropriate places:
>> +
>> +.. code-block:: none
>> +
>> +    # We need to configure the routing table for the resizer to use the bypass
>> +    # path along with set formats on the resizer's bypass sink pad. Doing this
>> +    # necessitates a single media-ctl command, as multiple calls to the program
>> +    # reset the routing table.
>> +    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[0],2/0->1/0[1]]"\
>> +    -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080],"\
>> +       "'mali-c55 resizer fr':2[fmt:RGB202020_1X60/1920x1080],"\
>> +       "'mali-c55 resizer fr':1[fmt:RGB202020_1X60/1920x1080]"
>> +
>> +    # Set format on the video device and stream
>> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
>> +
>> +References
>> +==========
>> +.. [1] https://git.linuxtv.org/v4l-utils.git/
>> +.. [2] https://git.ideasonboard.org/yavta.git
>> +.. [3] https://developer.arm.com/Processors/Mali-C55
>> diff --git a/Documentation/admin-guide/media/v4l-drivers.rst b/Documentation/admin-guide/media/v4l-drivers.rst
>> index f4bb2605f07e..af033c892808 100644
>> --- a/Documentation/admin-guide/media/v4l-drivers.rst
>> +++ b/Documentation/admin-guide/media/v4l-drivers.rst
>> @@ -17,6 +17,7 @@ Video4Linux (V4L) driver-specific documentation
>>          imx7
>>          ipu3
>>          ivtv
>> +       mali-c55
>>          mgb4
>>          omap3isp
>>          omap4_camera
>> -- 
>> 2.34.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] media: Documentation: Add Mali-C55 ISP Documentation

[Kieran Bingham]

Quoting Dan Scally (2024-02-26 07:48:44)
> Morning Kieran
> 
> On 25/02/2024 21:22, Kieran Bingham wrote:
> > Hi Dan,
> >
> > Quoting Daniel Scally (2024-02-14 14:19:05)
> >> Add a documentation page for the mali-c55 driver, which gives a brief
> >> overview of the hardware and explains how to use the driver's capture
> >> devices and the crop/scaler functions.
> >>
> >> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> >> ---
> >> Changes in v2:
> >>
> >>          - none
> >>
> >>   .../admin-guide/media/mali-c55-graph.dot      |  19 ++
> >>   Documentation/admin-guide/media/mali-c55.rst  | 318 ++++++++++++++++++
> >>   .../admin-guide/media/v4l-drivers.rst         |   1 +
> >>   3 files changed, 338 insertions(+)
> >>   create mode 100644 Documentation/admin-guide/media/mali-c55-graph.dot
> >>   create mode 100644 Documentation/admin-guide/media/mali-c55.rst
> >>
> >> diff --git a/Documentation/admin-guide/media/mali-c55-graph.dot b/Documentation/admin-guide/media/mali-c55-graph.dot
> >> new file mode 100644
> >> index 000000000000..0775ba42bf4c
> >> --- /dev/null
> >> +++ b/Documentation/admin-guide/media/mali-c55-graph.dot
> >> @@ -0,0 +1,19 @@
> >> +digraph board {
> >> +        rankdir=TB
> >> +        n00000001 [label="{{} | mali-c55 tpg\n/dev/v4l-subdev0 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> >> +        n00000001:port0 -> n00000003:port0 [style=dashed]
> >> +        n00000003 [label="{{<port0> 0} | mali-c55 isp\n/dev/v4l-subdev1 | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
> >> +        n00000003:port1 -> n00000007:port0 [style=bold]
> >> +        n00000003:port2 -> n00000007:port2 [style=bold]
> >> +        n00000003:port1 -> n0000000b:port0 [style=bold]
> >> +        n00000007 [label="{{<port0> 0 | <port2> 2} | mali-c55 resizer fr\n/dev/v4l-subdev2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> >> +        n00000007:port1 -> n0000000e [style=bold]
> >> +        n0000000b [label="{{<port0> 0} | mali-c55 resizer ds\n/dev/v4l-subdev3 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> >> +        n0000000b:port1 -> n00000012 [style=bold]
> >> +        n0000000e [label="mali-c55 fr\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> >> +        n00000012 [label="mali-c55 ds\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> >> +        n00000022 [label="{{<port0> 0} | csi2-rx\n/dev/v4l-subdev4 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> >> +        n00000022:port1 -> n00000003:port0
> >> +        n00000027 [label="{{} | imx415 1-001a\n/dev/v4l-subdev5 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
> >> +        n00000027:port0 -> n00000022:port0 [style=bold]
> >> +}
> >> \ No newline at end of file
> >> diff --git a/Documentation/admin-guide/media/mali-c55.rst b/Documentation/admin-guide/media/mali-c55.rst
> >> new file mode 100644
> >> index 000000000000..83f630c3bd9d
> >> --- /dev/null
> >> +++ b/Documentation/admin-guide/media/mali-c55.rst
> >> @@ -0,0 +1,318 @@
> >> +.. SPDX-License-Identifier: GPL-2.0
> >> +
> >> +==========================================
> >> +ARM Mali-C55 Image Signal Processor driver
> >> +==========================================
> >> +
> >> +Introduction
> >> +============
> >> +
> >> +This file documents the driver for ARM's Mali-C55 Image Signal Processor. The
> >> +driver is located under drivers/media/platform/arm/mali-c55.
> >> +
> >> +The Mali-C55 ISP receives data in either raw Bayer format or RGB/YUV format from
> >> +sensors through either a parallel interface or a memory bus before processing it
> >> +and outputting it through an internal DMA engine. Two output pipelines are
> >> +possible (though one may not be fitted, depending on the implementation). These
> >> +are referred to as "Full resolution" and "Downscale", but the naming is historic
> >> +and both pipes are capable of cropping/scaling operations. An integrated test
> >> +pattern generator can be used to drive the ISP and produce image data in the
> >> +absence of a connected camera sensor. The driver module is named mali_c55, and
> >> +is enabled through the CONFIG_VIDEO_MALI_C55 config option.
> > Can it handle metadata or other datatypes separately? anything else
> > that's worthy of mention? Or maybe not yet in this version.
> 
> 
> As in, can you stream the parameters/statistics without streaming video data? Not at present but the 
> change to add that ability would be very very minor - though none of that is in this version.

It was other stream data I was curious about clarifying. I think the
answer is the same though, not yet in this version, but will be added on
top.


> >> +
> >> +The driver implements V4L2, Media Controller and V4L2 Subdevice interfaces and
> >> +expects camera sensors connected to the ISP to have V4L2 subdevice interfaces.
> >> +
> >> +Mali-C55 ISP hardware
> >> +=====================
> >> +
> >> +A high level functional view of the Mali-C55 ISP is presented below. The ISP
> >> +takes input from either a live source or through a DMA engine for memory input,
> >> +depending on the SoC integration.::
> >> +
> >> +  +---------+    +----------+                                     +--------+
> >> +  | Sensor  |--->| CSI-2 Rx |                "Full Resolution"    |  DMA   |
> >> +  +---------+    +----------+   |\                 Output    +--->| Writer |
> >> +                       |        | \                          |    +--------+
> >> +                       |        |  \    +----------+  +------+---> Streaming I/O
> >> +  +------------+       +------->|   |   |          |  |
> >> +  |            |                |   |-->| Mali-C55 |--+
> >> +  | DMA Reader |--------------->|   |   |    ISP   |  |
> >> +  |            |                |  /    |          |  |      +---> Streaming I/O
> >> +  +------------+                | /     +----------+  |      |
> >> +                                |/                    +------+
> >> +                                                            |    +--------+
> >> +                                                             +--->|  DMA   |
> >> +                                               "Downscaled"       | Writer |
> >> +                                                 Output          +--------+
> >> +
> >> +Media Controller Topology
> >> +=========================
> >> +
> >> +An example of the ISP's topology (as implemented in a system with an IMX415
> >> +camera sensor and generic CSI-2 receiver) is below:
> >> +
> >> +
> >> +.. kernel-figure:: mali-c55-graph.dot
> >> +    :alt:   mali-c55-graph.dot
> >> +    :align: center
> >> +
> >> +The driver has 4 V4L2 subdevices:
> >> +
> >> +- `mali_c55 isp`: Responsible for configuring input crop and color space
> >> +                  conversion
> >> +- `mali_c55 tpg`: The test pattern generator, emulating a camera sensor.
> >> +- `mali_c55 resizer fr`: The Full-Resolution pipe resizer
> >> +- `mali_c55 resizer ds`: The Downscale pipe resizer
> >> +
> >> +The driver has 2 V4L2 video devices:
> >> +
> >> +- `mali-c55 fr`: The full-resolution pipe's capture device
> >> +- `mali-c55 ds`: The downscale pipe's capture device
> >> +
> >> +Idiosyncrasies
> >> +--------------
> >> +
> >> +**mali-c55 isp**
> >> +The `mali-c55 isp` subdevice has a single sink pad to which all sources of data
> >> +should be connected. The active source is selected by enabling the appropriate
> >> +media link and disabling all others.
> > By that I presume you mean you can only accept a single input link at a
> > time?
> 
> 
> Yes
> 
> >
> >> The ISP has two source pads, reflecting the
> >> +different paths through which it can internally route data. Tap points within
> >> +the ISP allow users to divert data to avoid processing by some or all of the
> >> +hardware's processing steps. The diagram below is intended only to highlight how
> >> +the bypassing works and is not a true reflection of those processing steps; for
> >> +a high-level functional block diagram see ARM's developer page for the
> >> +ISP [3]_::
> >> +
> >> +  +--------------------------------------------------------------+
> >> +  |                Possible Internal ISP Data Routes             |
> >> +  |          +------------+  +----------+  +------------+        |
> >> +  +---+      |            |  |          |  |  Colour    |    +---+
> >> +  | 0 |--+-->| Processing |->| Demosaic |->|   Space    |--->| 1 |
> >> +  +---+  |   |            |  |          |  | Conversion |    +---+
> >> +  |      |   +------------+  +----------+  +------------+        |
> >> +  |      |                                                   +---+
> >> +  |      +---------------------------------------------------| 2 |
> >> +  |                                                          +---+
> >> +  |                                                              |
> >> +  +--------------------------------------------------------------+
> >> +
> >> +
> >> +.. flat-table::
> >> +    :header-rows: 1
> >> +
> >> +    * - Pad
> >> +      - Direction
> >> +      - Purpose
> >> +
> >> +    * - 0
> >> +      - sink
> >> +      - Data input, connected to the TPG and camera sensors
> >> +
> >> +    * - 1
> >> +      - source
> >> +      - RGB/YUV data, connected to the FR and DS V4L2 subdevices
> >> +
> >> +    * - 2
> >> +      - source
> >> +      - RAW bayer data, connected to the FR V4L2 subdevices
> >> +
> >> +**mali-c55 resizer fr**
> >> +The `mali-c55 resizer fr` subdevice has two _sink_ pads to reflect the different
> >> +insertion points in the hardware (either RAW or demosaiced data):
> >> +
> >> +.. flat-table::
> >> +    :header-rows: 1
> >> +
> >> +    * - Pad
> >> +      - Direction
> >> +      - Purpose
> >> +
> >> +    * - 0
> >> +      - sink
> >> +      - Data input connected to the ISP's demosaiced stream.
> >> +
> >> +    * - 1
> >> +      - source
> >> +      - Data output connected to the capture video device
> >> +
> >> +    * - 2
> >> +      - sink
> >> +      - Data input connected to the ISP's raw data stream
> >> +
> >> +The data source in use is selected through the routing API; two routes each of a
> >> +single stream are available:
> >> +
> >> +.. flat-table::
> >> +    :header-rows: 1
> >> +
> >> +    * - Sink Pad
> >> +      - Source Pad
> >> +      - Purpose
> >> +
> >> +    * - 0
> >> +      - 1
> >> +      - Demosaiced data route
> >> +
> >> +    * - 2
> >> +      - 1
> >> +      - Raw data route
> >> +
> >> +
> >> +If the demosaiced route is active then the FR pipe is only capable of output
> >> +in RGB/YUV formats. If the raw route is active then the output reflects the
> >> +input (which may be either Bayer or RGB/YUV data).
> >> +
> >> +Using the driver to capture video
> >> +=================================
> >> +
> >> +Using the media controller APIs we can configure the input source and ISP to
> >> +capture images in a variety of formats. In the examples below, configuring the
> >> +media graph is done with the v4l-utils [1]_ package's media-ctl utility.
> >> +Capturing the images is done with yavta [2]_.
> >> +
> >> +Configuring the input source
> >> +----------------------------
> >> +
> >> +The first step is to set the input source that we wish by enabling the correct
> >> +media link. Using the example topology above, we can select the TPG as follows:
> >> +
> >> +.. code-block:: none
> >> +
> >> +    media-ctl -l "'lte-csi2-rx':1->'mali-c55 isp':0[0]"
> >> +    media-ctl -l "'mali-c55 tpg':0->'mali-c55 isp':0[1]"
> >> +
> >> +Capturing bayer data from the source and processing to RGB/YUV
> >> +--------------------------------------------------------------
> >> +To capture 1920x1080 bayer data from the source and push it through the ISP's
> >> +full processing pipeline, we configure the data formats appropriately on the
> >> +source, ISP and resizer subdevices and set the FR resizer's routing to select
> >> +processed data. The media bus format on the resizer's source pad will be either
> >> +RGB121212_1X36 or YUV10_1X30, depending on whether you want to capture RGB or
> >> +YUV. The ISP's debayering block outputs RGB data natively, setting the source
> >> +pad format to YUV10_1X30 enables the colour space conversion block.
> >> +
> >> +In this example we target RGB565 output, so select RGB121212_1X36 as the resizer
> >> +source pad's format:
> >> +
> >> +.. code-block:: none
> >> +
> >> +    # Set formats on the TPG and ISP
> >> +    media-ctl -V "'mali-c55 tpg':0[fmt:SRGGB16_1X16/1920x1080]"
> >> +    media-ctl -V "'mali-c55 isp':0[fmt:SRGGB16_1X16/1920x1080]"
> >> +    media-ctl -V "'mali-c55 isp':1[fmt:SRGGB16_1X16/1920x1080]"
> >> +
> >> +    # Set routing on the FR resizer
> >> +    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[1],2/0->1/0[0]]"
> >> +
> >> +    # Set format on the resizer, must be done AFTER the routing.
> >> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/1920x1080]"
> >> +
> >> +The downscale output can also be used to stream data at the same time. In this
> >> +case only processed data can be captured and so no routing need be set:
> > Is the route automatically reset? or is it just that the default route
> > is already the correct choice for this instance?
> 
> 
> Err, the default route is already the correct choice for this instance but I'm nonetheless setting 
> it above...perhaps I'm misunderstanding the question?

My question was on why the text stated "and so no routing need be set:"

> >> +
> >> +.. code-block:: none
> >> +
> >> +    # Set format on the resizer
> >> +    media-ctl -V "'mali-c55 resizer ds':1[fmt:RGB121212_1X36/1920x1080]"
> >> +
> >> +Following which images can be captured from both the FR and DS output's video
> >> +devices (simultaneously, if desired):
> >> +
> >> +.. code-block:: none
> >> +
> >> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
> >> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video1
> >> +
> > Is there any synchronisation required? Can one pipe run without the
> > other if they are both enabled? or will stalling on one pipe stall the
> > other?
> 
> 
> The outputs are synchronised already to the extent that if both are enabled they'll receive data 
> from the same input frame at the same time. They can be run independently though, if an application 
> stopped queuing frames to one but continued queuing to the other then the latter would work fine.
> 
> >
> >> +Cropping the image
> >> +~~~~~~~~~~~~~~~~~~
> >> +
> >> +Both the full resolution and downscale pipes can crop to a minimum resolution of
> >> +640x480. To crop the image simply configure the resizer's sink pad's crop and
> >> +compose rectangles and set the format on the video device:
> >> +
> >> +.. code-block:: none
> >> +
> >> +    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(480,270)/640x480 compose:(0,0)/640x480]"
> >> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
> >> +    yavta -f RGB565 -s 640x480 -c10 /dev/video0
> >> +
> >> +Downscaling the image
> >> +~~~~~~~~~~~~~~~~~~~~~
> >> +
> >> +Both the full resolution and downscale pipes can downscale the image by up to 8x
> >> +provided the minimum 640x480 resolution is adhered to. For the best image result
> > 'minimum 640x480 output resolution' I presume.
> 
> 
> And input, actually.
> 
> >
> > Maybe the ISP limits/restrictions need to be defined before here?
> 
> 
> I'll expand on them yes.
> 
> >
> >> +the scaling ratio for each dimension should be the same. To configure scaling we
> >> +use the compose rectangle on the resizer's sink pad:
> >> +
> >> +.. code-block:: none
> >> +
> >> +    media-ctl -V "'mali-c55 resizer fr':0[fmt:RGB121212_1X36/1920x1080 crop:(0,0)/1920x1080 compose:(0,0)/640x480]"
> >> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:RGB121212_1X36/640x480]"
> >> +    yavta -f RGB565 -s 640x480 -c10 /dev/video0
> >> +
> >> +Capturing images in YUV formats
> >> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> +
> >> +If we need to output YUV data rather than RGB the color space conversion block
> >> +needs to be active, which is achieved by setting MEDIA_BUS_FMT_YUV10_1X30 on the
> > Is 10 bit required here? (As opposed to the 12 bit before in the
> > pipeline? is that a limitation on the component?)
> 
> 
> It's a reflection of the output from the colour-space conversion block.
> 
> > or is this to match
> > the desired output format?
> 
> 
> Sort of, it's used to inform the driver about which of the output formats can be generated - without 
> conversion to YUV here only RGB formats can be set on the capture device nodes.

Hrm, sorry - my question is why we went from 12 bit to 10 bit in the
example. Maybe we need to document the capabilities of each stage if the
bitdepth will be reduced by force somewhere along the pipe?

> >> +resizer's source pad. We can then configure a capture format like NV12 (here in
> >> +its multi-planar variant)
> >> +
> >> +.. code-block:: none
> >> +
> >> +    media-ctl -V "'mali-c55 resizer fr':1[fmt:YUV10_1X30/1920x1080]"
> >> +    yavta -f NV12M -s 1920x1080 -c10 /dev/video0
> >> +
> >> +Capturing RGB data from the source and processing it with the resizers
> >> +----------------------------------------------------------------------
> >> +
> >> +The Mali-C55 ISP can work with sensors capable of outputting RGB data. In this
> >> +case although none of the image quality blocks would be used it can still
> >> +crop/scale the data in the usual way.
> >> +
> >> +To achieve this, the ISP's sink pad's format is set to
> >> +MEDIA_BUS_FMT_RGB202020_1X60 - this reflects the format that data must be in to
> >> +work with the ISP. Converting the camera sensor's output to that format is the
> >> +responsibility of external hardware.
> >> +
> >> +In this example we ask the test pattern generator to give us RGB data instead of
> >> +bayer.
> >> +
> >> +.. code-block:: none
> >> +
> >> +    media-ctl -V "'mali-c55 tpg':0[fmt:RGB202020_1X60/1920x1080]"
> >> +    media-ctl -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080]"
> >> +
> >> +Cropping or scaling the data can be done in exactly the same way as outlined
> >> +earlier.
> >> +
> >> +Capturing raw data from the source and outputting it unmodified
> >> +-----------------------------------------------------------------
> >> +
> >> +The ISP can additionally capture raw data from the source and output it on the
> >> +full resolution pipe only, completely unmodified. In this case the downscale
> >> +pipe can still process the data normally and be used at the same time.
> >> +
> >> +To configure raw bypass the FR resizer's subdevice's routing table needs to be
> >> +configured, followed by formats in the appropriate places:
> >> +
> >> +.. code-block:: none
> >> +
> >> +    # We need to configure the routing table for the resizer to use the bypass
> >> +    # path along with set formats on the resizer's bypass sink pad. Doing this
> >> +    # necessitates a single media-ctl command, as multiple calls to the program
> >> +    # reset the routing table.
> >> +    media-ctl -R "'mali-c55 resizer fr'[0/0->1/0[0],2/0->1/0[1]]"\
> >> +    -V "'mali-c55 isp':0[fmt:RGB202020_1X60/1920x1080],"\
> >> +       "'mali-c55 resizer fr':2[fmt:RGB202020_1X60/1920x1080],"\
> >> +       "'mali-c55 resizer fr':1[fmt:RGB202020_1X60/1920x1080]"
> >> +
> >> +    # Set format on the video device and stream
> >> +    yavta -f RGB565 -s 1920x1080 -c10 /dev/video0
> >> +
> >> +References
> >> +==========
> >> +.. [1] https://git.linuxtv.org/v4l-utils.git/
> >> +.. [2] https://git.ideasonboard.org/yavta.git
> >> +.. [3] https://developer.arm.com/Processors/Mali-C55
> >> diff --git a/Documentation/admin-guide/media/v4l-drivers.rst b/Documentation/admin-guide/media/v4l-drivers.rst
> >> index f4bb2605f07e..af033c892808 100644
> >> --- a/Documentation/admin-guide/media/v4l-drivers.rst
> >> +++ b/Documentation/admin-guide/media/v4l-drivers.rst
> >> @@ -17,6 +17,7 @@ Video4Linux (V4L) driver-specific documentation
> >>          imx7
> >>          ipu3
> >>          ivtv
> >> +       mali-c55
> >>          mgb4
> >>          omap3isp
> >>          omap4_camera
> >> -- 
> >> 2.34.1
> >>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Sakari Ailus]

Hi Dan,

On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> 
> Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
> 	- Added clocks information
> 	- Fixed the warnings raised by Rob
> 
>  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> new file mode 100644
> index 000000000000..30038cfec3a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM Mali-C55 Image Signal Processor
> +
> +maintainers:
> +  - Daniel Scally <dan.scally@ideasonboard.com>
> +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> +
> +properties:
> +  compatible:
> +    const: arm,mali-c55
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: ISP video clock
> +      - description: ISP AXI clock
> +      - description: ISP AHB-lite clock
> +
> +  clock-names:
> +    items:
> +      - const: vclk
> +      - const: aclk
> +      - const: hclk
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/graph.yaml#/properties/endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    mali_c55: isp@400000 {
> +      compatible = "arm,mali-c55";
> +      reg = <0x400000 0x200000>;
> +      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> +      clock-names = "vclk", "aclk", "hclk";
> +      interrupts = <0>;
> +
> +      ports {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        port@0 {
> +          reg = <0>;
> +          isp_in: endpoint {
> +              remote-endpoint = <&mipi_out>;

I suppose this is a CSI-2 interface with D-PHY?

How many lanes do you have and is lane remapping / polarity inversion
supported?

This should be reflected in bindings.

> +          };
> +        };
> +      };
> +    };
> +...

-- 
Regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Laurent Pinchart]

Hi Sakari,

On Mon, Feb 26, 2024 at 11:54:22AM +0000, Sakari Ailus wrote:
> On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> > Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> > 
> > Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> > Changes in v2:
> > 
> > 	- Added clocks information
> > 	- Fixed the warnings raised by Rob
> > 
> >  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > new file mode 100644
> > index 000000000000..30038cfec3a4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM Mali-C55 Image Signal Processor
> > +
> > +maintainers:
> > +  - Daniel Scally <dan.scally@ideasonboard.com>
> > +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: arm,mali-c55
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: ISP video clock
> > +      - description: ISP AXI clock
> > +      - description: ISP AHB-lite clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: vclk
> > +      - const: aclk
> > +      - const: hclk
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    mali_c55: isp@400000 {
> > +      compatible = "arm,mali-c55";
> > +      reg = <0x400000 0x200000>;
> > +      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> > +      clock-names = "vclk", "aclk", "hclk";
> > +      interrupts = <0>;
> > +
> > +      ports {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        port@0 {
> > +          reg = <0>;
> > +          isp_in: endpoint {
> > +              remote-endpoint = <&mipi_out>;
> 
> I suppose this is a CSI-2 interface with D-PHY?

No, that's an internal parallel bus. Depending on the SoC integration,
it can be connected to a CSI-2 receiver, a DMA engine, or a mux to
select between different sources.

> How many lanes do you have and is lane remapping / polarity inversion
> supported?
> 
> This should be reflected in bindings.
> 
> > +          };
> > +        };
> > +      };
> > +    };
> > +...

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Sakari Ailus]

Hi Laurent,

On Mon, Feb 26, 2024 at 02:04:31PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Mon, Feb 26, 2024 at 11:54:22AM +0000, Sakari Ailus wrote:
> > On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> > > Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> > > 
> > > Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > ---
> > > Changes in v2:
> > > 
> > > 	- Added clocks information
> > > 	- Fixed the warnings raised by Rob
> > > 
> > >  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
> > >  1 file changed, 77 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > new file mode 100644
> > > index 000000000000..30038cfec3a4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > @@ -0,0 +1,77 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ARM Mali-C55 Image Signal Processor
> > > +
> > > +maintainers:
> > > +  - Daniel Scally <dan.scally@ideasonboard.com>
> > > +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: arm,mali-c55
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: ISP video clock
> > > +      - description: ISP AXI clock
> > > +      - description: ISP AHB-lite clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: vclk
> > > +      - const: aclk
> > > +      - const: hclk
> > > +
> > > +  ports:
> > > +    $ref: /schemas/graph.yaml#/properties/ports
> > > +
> > > +    properties:
> > > +      port@0:
> > > +        $ref: /schemas/graph.yaml#/properties/port
> > > +
> > > +        properties:
> > > +          endpoint:
> > > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - clock-names
> > > +  - ports
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    mali_c55: isp@400000 {
> > > +      compatible = "arm,mali-c55";
> > > +      reg = <0x400000 0x200000>;
> > > +      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> > > +      clock-names = "vclk", "aclk", "hclk";
> > > +      interrupts = <0>;
> > > +
> > > +      ports {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        port@0 {
> > > +          reg = <0>;
> > > +          isp_in: endpoint {
> > > +              remote-endpoint = <&mipi_out>;
> > 
> > I suppose this is a CSI-2 interface with D-PHY?
> 
> No, that's an internal parallel bus. Depending on the SoC integration,
> it can be connected to a CSI-2 receiver, a DMA engine, or a mux to
> select between different sources.

The name suggests otherwise. Maybe change that to something more
descriptive?

> 
> > How many lanes do you have and is lane remapping / polarity inversion
> > supported?
> > 
> > This should be reflected in bindings.
> > 
> > > +          };
> > > +        };
> > > +      };
> > > +    };
> > > +...
> 

-- 
Regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Laurent Pinchart]

On Mon, Feb 26, 2024 at 12:20:15PM +0000, Sakari Ailus wrote:
> On Mon, Feb 26, 2024 at 02:04:31PM +0200, Laurent Pinchart wrote:
> > On Mon, Feb 26, 2024 at 11:54:22AM +0000, Sakari Ailus wrote:
> > > On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
> > > > Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
> > > > 
> > > > Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > > ---
> > > > Changes in v2:
> > > > 
> > > > 	- Added clocks information
> > > > 	- Fixed the warnings raised by Rob
> > > > 
> > > >  .../bindings/media/arm,mali-c55.yaml          | 77 +++++++++++++++++++
> > > >  1 file changed, 77 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > > new file mode 100644
> > > > index 000000000000..30038cfec3a4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
> > > > @@ -0,0 +1,77 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: ARM Mali-C55 Image Signal Processor
> > > > +
> > > > +maintainers:
> > > > +  - Daniel Scally <dan.scally@ideasonboard.com>
> > > > +  - Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: arm,mali-c55
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: ISP video clock
> > > > +      - description: ISP AXI clock
> > > > +      - description: ISP AHB-lite clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: vclk
> > > > +      - const: aclk
> > > > +      - const: hclk
> > > > +
> > > > +  ports:
> > > > +    $ref: /schemas/graph.yaml#/properties/ports
> > > > +
> > > > +    properties:
> > > > +      port@0:
> > > > +        $ref: /schemas/graph.yaml#/properties/port

Adding

        description: Input parallel video bus

here would be useful.

> > > > +
> > > > +        properties:
> > > > +          endpoint:
> > > > +            $ref: /schemas/graph.yaml#/properties/endpoint
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - interrupts
> > > > +  - clocks
> > > > +  - clock-names
> > > > +  - ports
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    mali_c55: isp@400000 {
> > > > +      compatible = "arm,mali-c55";
> > > > +      reg = <0x400000 0x200000>;
> > > > +      clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> > > > +      clock-names = "vclk", "aclk", "hclk";
> > > > +      interrupts = <0>;
> > > > +
> > > > +      ports {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        port@0 {
> > > > +          reg = <0>;
> > > > +          isp_in: endpoint {
> > > > +              remote-endpoint = <&mipi_out>;
> > > 
> > > I suppose this is a CSI-2 interface with D-PHY?
> > 
> > No, that's an internal parallel bus. Depending on the SoC integration,
> > it can be connected to a CSI-2 receiver, a DMA engine, or a mux to
> > select between different sources.
> 
> The name suggests otherwise. Maybe change that to something more
> descriptive?

We could rename mipi_out to csi2_rx_out, sure.

> > > How many lanes do you have and is lane remapping / polarity inversion
> > > supported?
> > > 
> > > This should be reflected in bindings.
> > > 
> > > > +          };
> > > > +        };
> > > > +      };
> > > > +    };
> > > > +...

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Sakari Ailus]

Hi Laurent,

On Mon, Feb 26, 2024 at 02:58:18PM +0200, Laurent Pinchart wrote:
> > > > > +              remote-endpoint = <&mipi_out>;
> > > > 
> > > > I suppose this is a CSI-2 interface with D-PHY?
> > > 
> > > No, that's an internal parallel bus. Depending on the SoC integration,
> > > it can be connected to a CSI-2 receiver, a DMA engine, or a mux to
> > > select between different sources.
> > 
> > The name suggests otherwise. Maybe change that to something more
> > descriptive?
> 
> We could rename mipi_out to csi2_rx_out, sure.

Sounds good to me.

-- 
Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/5] dt-bindings: media: Add bindings for ARM mali-c55

[Dan Scally]

Hi Sakari - thanks for reviews!

On 26/02/2024 13:37, Sakari Ailus wrote:
> Hi Laurent,
>
> On Mon, Feb 26, 2024 at 02:58:18PM +0200, Laurent Pinchart wrote:
>>>>>> +              remote-endpoint = <&mipi_out>;
>>>>> I suppose this is a CSI-2 interface with D-PHY?
>>>> No, that's an internal parallel bus. Depending on the SoC integration,
>>>> it can be connected to a CSI-2 receiver, a DMA engine, or a mux to
>>>> select between different sources.
>>> The name suggests otherwise. Maybe change that to something more
>>> descriptive?
>> We could rename mipi_out to csi2_rx_out, sure.
> Sounds good to me.
>
OK, will do - and likewise the description you suggested above.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Jacopo Mondi]

Hi Sakari

On Mon, Feb 26, 2024 at 11:03:47AM +0000, Sakari Ailus wrote:
> Hi Daniel,
>
> Thanks for the set.
>
> How do you determine which buffers go together on the video buffer queues?
> Or do you need a buffer on both for every frame if the nodes are set
> streaming?
>
> This should be also documented, in the documentation patch. At least I
> didn't find it there.
>
> On Wed, Feb 14, 2024 at 02:19:04PM +0000, Daniel Scally wrote:
> > Add a driver for Arm's Mali-C55 Image Signal Processor. The driver is
> > V4L2 and Media Controller compliant and creates subdevices to manage
> > the ISP itself, its internal test pattern generator as well as the
> > crop, scaler and output format functionality for each of its two
> > output devices.
> >
> > Acked-by: Nayden Kanchev <nayden.kanchev@arm.com>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> > Changes in v2:
> >
> > 	- Clock handling
> > 	- Fixed the warnings raised by the kernel test robot
> >
> >  drivers/media/platform/Kconfig                |    1 +
> >  drivers/media/platform/Makefile               |    1 +
> >  drivers/media/platform/arm/Kconfig            |    5 +
> >  drivers/media/platform/arm/Makefile           |    2 +
> >  drivers/media/platform/arm/mali-c55/Kconfig   |   18 +
> >  drivers/media/platform/arm/mali-c55/Makefile  |    9 +
> >  .../platform/arm/mali-c55/mali-c55-capture.c  | 1021 +++++++++++++++++
> >  .../platform/arm/mali-c55/mali-c55-common.h   |  271 +++++
> >  .../platform/arm/mali-c55/mali-c55-core.c     |  767 +++++++++++++
> >  .../platform/arm/mali-c55/mali-c55-isp.c      |  682 +++++++++++
> >  .../arm/mali-c55/mali-c55-registers.h         |  180 +++
> >  .../arm/mali-c55/mali-c55-resizer-coefs.h     |  382 ++++++
> >  .../platform/arm/mali-c55/mali-c55-resizer.c  |  678 +++++++++++
> >  .../platform/arm/mali-c55/mali-c55-tpg.c      |  425 +++++++
> >  14 files changed, 4442 insertions(+)
> >  create mode 100644 drivers/media/platform/arm/Kconfig
> >  create mode 100644 drivers/media/platform/arm/Makefile
> >  create mode 100644 drivers/media/platform/arm/mali-c55/Kconfig
> >  create mode 100644 drivers/media/platform/arm/mali-c55/Makefile
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-common.h
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-core.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-isp.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-resizer-coefs.h
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-resizer.c
> >  create mode 100644 drivers/media/platform/arm/mali-c55/mali-c55-tpg.c
> >
> > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > index 91e54215de3a..cd92c024e039 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -65,6 +65,7 @@ config VIDEO_MUX
> >  source "drivers/media/platform/allegro-dvt/Kconfig"
> >  source "drivers/media/platform/amlogic/Kconfig"
> >  source "drivers/media/platform/amphion/Kconfig"
> > +source "drivers/media/platform/arm/Kconfig"
> >  source "drivers/media/platform/aspeed/Kconfig"
> >  source "drivers/media/platform/atmel/Kconfig"
> >  source "drivers/media/platform/cadence/Kconfig"
> > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> > index 3296ec1ebe16..ea6624c62559 100644
> > --- a/drivers/media/platform/Makefile
> > +++ b/drivers/media/platform/Makefile
> > @@ -8,6 +8,7 @@
> >  obj-y += allegro-dvt/
> >  obj-y += amlogic/
> >  obj-y += amphion/
> > +obj-y += arm/
> >  obj-y += aspeed/
> >  obj-y += atmel/
> >  obj-y += cadence/
> > diff --git a/drivers/media/platform/arm/Kconfig b/drivers/media/platform/arm/Kconfig
> > new file mode 100644
> > index 000000000000..4f0764c329c7
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/Kconfig
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +comment "ARM media platform drivers"
> > +
> > +source "drivers/media/platform/arm/mali-c55/Kconfig"
> > diff --git a/drivers/media/platform/arm/Makefile b/drivers/media/platform/arm/Makefile
> > new file mode 100644
> > index 000000000000..8cc4918725ef
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +obj-y += mali-c55/
> > diff --git a/drivers/media/platform/arm/mali-c55/Kconfig b/drivers/media/platform/arm/mali-c55/Kconfig
> > new file mode 100644
> > index 000000000000..602085e28b01
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/mali-c55/Kconfig
> > @@ -0,0 +1,18 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +config VIDEO_MALI_C55
> > +	tristate "ARM Mali-C55 Image Signal Processor driver"
> > +	depends on V4L_PLATFORM_DRIVERS
> > +	depends on VIDEO_DEV && OF
> > +	depends on ARCH_VEXPRESS || COMPILE_TEST
> > +	select MEDIA_CONTROLLER
> > +	select VIDEO_V4L2_SUBDEV_API
> > +	select VIDEOBUF2_DMA_CONTIG
> > +	select VIDEOBUF2_VMALLOC
> > +	select V4L2_FWNODE
> > +	select GENERIC_PHY_MIPI_DPHY
> > +	default n
> > +	help
> > +	  Enable this to support Arm's Mali-C55 Image Signal Processor.
> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called mali-c55.
> > diff --git a/drivers/media/platform/arm/mali-c55/Makefile b/drivers/media/platform/arm/mali-c55/Makefile
> > new file mode 100644
> > index 000000000000..77dcb2fbf0f4
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/mali-c55/Makefile
> > @@ -0,0 +1,9 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +mali-c55-y := mali-c55-capture.o \
> > +	      mali-c55-core.o \
> > +	      mali-c55-isp.o \
> > +	      mali-c55-tpg.o \
> > +	      mali-c55-resizer.o
> > +
> > +obj-$(CONFIG_VIDEO_MALI_C55) += mali-c55.o
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-capture.c b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> > new file mode 100644
> > index 000000000000..98020b7ecb1e
> > --- /dev/null
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> > @@ -0,0 +1,1021 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ARM Mali-C55 ISP Driver - Video capture devices
> > + *
> > + * Copyright (C) 2023 Ideas on Board Oy
>
> 2024
>
> > + */
> > +
> > +#include <linux/minmax.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/string.h>
> > +#include <linux/videodev2.h>
> > +
> > +#include <media/v4l2-dev.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/v4l2-subdev.h>
> > +#include <media/videobuf2-core.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +
> > +#include "mali-c55-common.h"
> > +#include "mali-c55-registers.h"
> > +
> > +/*
> > + * The Mali-C55 ISP has up to two output pipes; known as full resolution and
> > + * down scaled. The register space for these is laid out identically, but offset
> > + * by 372 bytes.
> > + */
> > +#define MALI_C55_CAP_DEV_FR_REG_OFFSET		0x0
> > +#define MALI_C55_CAP_DEV_DS_REG_OFFSET		0x174
> > +
> > +static const struct mali_c55_fmt mali_c55_fmts[] = {
> > +	/*
> > +	 * This table is missing some entries which need further work or
> > +	 * investigation:
> > +	 *
> > +	 * Base mode 1 is a backwards V4L2_PIX_FMT_XRGB32 with no V4L2 equivalent
> > +	 * Base mode 5 is "Generic Data"
> > +	 * Base mode 8 is a backwards V4L2_PIX_FMT_XYUV32 - no V4L2 equivalent
> > +	 * Base mode 9 seems to have no V4L2 equivalent
> > +	 * Base mode 17, 19 and 20 describe formats which seem to have no V4L2
> > +	 * equivalent
> > +	 */
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_ARGB2101010,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_RGB121212_1X36,
> > +			MEDIA_BUS_FMT_RGB202020_1X60,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_A2R10G10B10,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_RGB565,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_RGB121212_1X36,
> > +			MEDIA_BUS_FMT_RGB202020_1X60,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RGB565,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_BGR24,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_RGB121212_1X36,
> > +			MEDIA_BUS_FMT_RGB202020_1X60,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RGB24,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_YUYV,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_YUY2,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_UYVY,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_UYVY,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_Y210,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_Y210,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	/*
> > +	 * This is something of a hack, the ISP thinks it's running NV12M but
> > +	 * by setting uv_plane = 0 we simply discard that planes and only output
> > +	 * the Y-plane.
> > +	 */
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_GREY,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_NV12_21,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_NV12M,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_NV12_21,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT1
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_NV21M,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_YUV10_1X30,
> > +		},
> > +		.enumerate = false,
> > +		.is_raw = false,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_NV12_21,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT2
> > +		}
> > +	},
> > +	/*
> > +	 * RAW uncompressed formats are all packed in 16 bpp.
> > +	 * TODO: Expand this list to encompass all possible RAW formats.
> > +	 */
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SRGGB12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SRGGB12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SBGGR12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SBGGR12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SGBRG12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SGBRG12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +	{
> > +		.fourcc = V4L2_PIX_FMT_SGRBG12,
> > +		.mbus_codes = {
> > +			MEDIA_BUS_FMT_SGRBG12_1X12,
> > +		},
> > +		.enumerate = true,
> > +		.is_raw = true,
> > +		.registers = {
> > +			.base_mode = MALI_C55_OUTPUT_RAW16,
> > +			.uv_plane = MALI_C55_OUTPUT_PLANE_ALT0
> > +		}
> > +	},
> > +};
> > +
> > +static bool mali_c55_mbus_code_can_produce_fmt(const struct mali_c55_fmt *fmt,
> > +					       u32 code)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(fmt->mbus_codes); i++) {
> > +		if (fmt->mbus_codes[i] == code)
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
> > +						 bool allow_raw, bool unique)
> > +{
> > +	if (!fmt)
> > +		fmt = &mali_c55_fmts[0];
> > +	else
> > +		++fmt;
>
> fmt++, please.
>

Can I ask why ? (here and in the next occurrences you have reported)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Sakari Ailus]

Hi Jacopo,

On Wed, Feb 28, 2024 at 01:50:14PM +0100, Jacopo Mondi wrote:
> > > +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
> > > +						 bool allow_raw, bool unique)
> > > +{
> > > +	if (!fmt)
> > > +		fmt = &mali_c55_fmts[0];
> > > +	else
> > > +		++fmt;
> >
> > fmt++, please.
> >
> 
> Can I ask why ? (here and in the next occurrences you have reported)

It's much, much more common and using that form makes the code easier to
read. The rest of the driver primarily uses variable++, too, AFAIR.

So you should use ++variable only when you need it.

-- 
Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Kieran Bingham]

Quoting Sakari Ailus (2024-02-28 13:11:39)
> Hi Jacopo,
> 
> On Wed, Feb 28, 2024 at 01:50:14PM +0100, Jacopo Mondi wrote:
> > > > +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
> > > > +                                          bool allow_raw, bool unique)
> > > > +{
> > > > + if (!fmt)
> > > > +         fmt = &mali_c55_fmts[0];
> > > > + else
> > > > +         ++fmt;
> > >
> > > fmt++, please.
> > >
> > 
> > Can I ask why ? (here and in the next occurrences you have reported)
> 
> It's much, much more common and using that form makes the code easier to
> read. The rest of the driver primarily uses variable++, too, AFAIR.
> 
> So you should use ++variable only when you need it.

I don't think this is a hot path, but I'll never forget my C tutor
telling us how ++i is more efficient than i++ somewhere to do with the
opcode ordering, and not having to make a copy [*1]

Though I bet any clever optimising compiler could spot this anyway.

[*1]. Whatever plausibility there is based on a 20 year old memory and
should be verified elsewhere.

--
Kieran


> 
> -- 
> Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Laurent Pinchart]

On Wed, Feb 28, 2024 at 01:29:36PM +0000, Kieran Bingham wrote:
> Quoting Sakari Ailus (2024-02-28 13:11:39)
> > On Wed, Feb 28, 2024 at 01:50:14PM +0100, Jacopo Mondi wrote:
> > > > > +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
> > > > > +                                          bool allow_raw, bool unique)
> > > > > +{
> > > > > + if (!fmt)
> > > > > +         fmt = &mali_c55_fmts[0];
> > > > > + else
> > > > > +         ++fmt;
> > > >
> > > > fmt++, please.
> > > 
> > > Can I ask why ? (here and in the next occurrences you have reported)
> > 
> > It's much, much more common and using that form makes the code easier to
> > read. The rest of the driver primarily uses variable++, too, AFAIR.
> > 
> > So you should use ++variable only when you need it.
> 
> I don't think this is a hot path, but I'll never forget my C tutor
> telling us how ++i is more efficient than i++ somewhere to do with the
> opcode ordering, and not having to make a copy [*1]
> 
> Though I bet any clever optimising compiler could spot this anyway.
> 
> [*1]. Whatever plausibility there is based on a 20 year old memory and
> should be verified elsewhere.

In C I wouldn't expect any practical difference. C++ is a different
matter, as the prefix and postfix operators can have different
implementations.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Dan Scally]

Hi Sakari

On 26/02/2024 11:03, Sakari Ailus wrote:
> < snip>
>> +const struct mali_c55_fmt *mali_c55_cap_fmt_next(const struct mali_c55_fmt *fmt,
>> +						 bool allow_raw, bool unique)
>> +{
>> +	if (!fmt)
>> +		fmt = &mali_c55_fmts[0];
>> +	else
>> +		++fmt;
> fmt++, please.
>
>> +
>> +	for (; fmt < &mali_c55_fmts[ARRAY_SIZE(mali_c55_fmts)]; ++fmt) {
> Ditto.
>
>> +		if (!allow_raw && fmt->is_raw) {
>> +			fmt++;
> Why?


Sorry, I forgot to reply here...I think neither this nor the enumeration filter below is actually 
used - I'll double check and remove them for the v3.


Thanks

Dan

>
>> +			continue;
>> +		}
>> +
>> +		if (unique && !fmt->enumerate) {
>> +			fmt++;
> Here, too.



>
>> +			continue;
>> +		}
>> +
>> +		return fmt;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel