[PATCH v2 0/4] media: raspberrypi: Support RPi5's CFE

[PATCH v2 0/4] media: raspberrypi: Support RPi5's CFE

[Tomi Valkeinen]

This series adds support to the CFE hardware block on RaspberryPi 5. The
CFE (Camera Front End) contains a CSI-2 receiver and Front End, a small
ISP.

This series is based on linux-media-stage with Jacopo's "[PATCH v9 0/8]
media: raspberrypi: Add support for PiSP Back End".

To run this, you need the basic RPi5 kernel support plus relevant dts
changes to enable the cfe and camera. My work branch with everything
needed to run CFE can be found from:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git rp1-cfe

A few notes about the patches:

- The original work was done by RaspberryPi, mostly by Naushir Patuck.
- The second video node only sets V4L2_CAP_META_CAPTURE instead of both
  V4L2_CAP_META_CAPTURE and V4L2_CAP_META_CAPTURE like the other nodes.
  This is a temporary workaround for userspace (libcamera), and
  hopefully can be removed soon.

I have tested this with:
- A single IMX219 sensor connected to the RPi5's CSI-2 port
- Arducam's UB960 FPD-Link board with four imx219 sensors connected

 Tomi

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Changes in v2:
- Change the compatible string back to raspberrypi,rp1-cfe from raspberrypi,rpi5-rp1-cfe
- Drop the references to rp1 headers in the DT binding example. This
  allows compiling the example without the rp1 support.
- Fix missing remap lines for mono formats
- Fix csi2_pad_set_fmt() so that the format can be changed back to the
  sink's format from 16-bit or compressed format.
- Link to v1: https://lore.kernel.org/r/20240318-rp1-cfe-v1-0-ac6d960ff22d@ideasonboard.com

---
Tomi Valkeinen (4):
      media: uapi: Add meta formats for PiSP FE config and stats
      dt-bindings: media: Add bindings for raspberrypi,rp1-cfe
      media: raspberrypi: Add support for RP1-CFE
      media: admin-guide: Document the Raspberry Pi CFE (rp1-cfe)

 .../admin-guide/media/raspberrypi-rp1-cfe.dot      |   27 +
 .../admin-guide/media/raspberrypi-rp1-cfe.rst      |   78 +
 Documentation/admin-guide/media/v4l-drivers.rst    |    1 +
 .../bindings/media/raspberrypi,rp1-cfe.yaml        |   98 +
 .../userspace-api/media/v4l/meta-formats.rst       |    1 +
 .../userspace-api/media/v4l/metafmt-pisp-fe.rst    |   39 +
 MAINTAINERS                                        |    8 +
 drivers/media/platform/raspberrypi/Kconfig         |    1 +
 drivers/media/platform/raspberrypi/Makefile        |    1 +
 drivers/media/platform/raspberrypi/rp1-cfe/Kconfig |   14 +
 .../media/platform/raspberrypi/rp1-cfe/Makefile    |    6 +
 .../media/platform/raspberrypi/rp1-cfe/cfe-fmts.h  |  332 +++
 .../media/platform/raspberrypi/rp1-cfe/cfe-trace.h |  196 ++
 drivers/media/platform/raspberrypi/rp1-cfe/cfe.c   | 2526 ++++++++++++++++++++
 drivers/media/platform/raspberrypi/rp1-cfe/cfe.h   |   43 +
 drivers/media/platform/raspberrypi/rp1-cfe/csi2.c  |  583 +++++
 drivers/media/platform/raspberrypi/rp1-cfe/csi2.h  |   89 +
 drivers/media/platform/raspberrypi/rp1-cfe/dphy.c  |  175 ++
 drivers/media/platform/raspberrypi/rp1-cfe/dphy.h  |   27 +
 .../media/platform/raspberrypi/rp1-cfe/pisp-fe.c   |  581 +++++
 .../media/platform/raspberrypi/rp1-cfe/pisp-fe.h   |   53 +
 drivers/media/v4l2-core/v4l2-ioctl.c               |    2 +
 .../uapi/linux/media/raspberrypi/pisp_fe_config.h  |  273 +++
 .../linux/media/raspberrypi/pisp_fe_statistics.h   |   64 +
 include/uapi/linux/videodev2.h                     |    2 +
 25 files changed, 5220 insertions(+)
---
base-commit: 954ee38d40c98001cfdc570c4d5e90bcbc456ce1
change-id: 20240314-rp1-cfe-142b628b7214
prerequisite-message-id: <20240531080707.34568-1-jacopo.mondi@ideasonboard.com>
prerequisite-patch-id: 42749883104fffad151e25d8127eb84800d7e9ca
prerequisite-patch-id: 433ce52b6b1b72f2c29e4474d72cd5106b73c075
prerequisite-patch-id: b9b3006cd03bb4e41e03f735d97e19081a32be02
prerequisite-patch-id: abeb8c40020f0ca6140afc141e535abc86a4773b
prerequisite-patch-id: 4ddf121ee47eeadeb370a5fbabc82ca6f6caa23b
prerequisite-patch-id: ee469d3c346f55483733a04d89e87d02126501cc
prerequisite-patch-id: 9467d5cd306aa29842128294a5f76f184b72eab7
prerequisite-patch-id: e6dbee90dfbd984bd55cbe3db56d9d8de4dc86ae

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH v2 1/4] media: uapi: Add meta formats for PiSP FE config and stats

[Tomi Valkeinen]

Add two meta formats for PiSP FE: V4L2_META_FMT_RPI_FE_CFG and
V4L2_META_FMT_RPI_FE_STATS. The former is used to provide configuration
for the FE and the latter is used to read the statistics from the FE.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 .../userspace-api/media/v4l/meta-formats.rst       |  1 +
 .../userspace-api/media/v4l/metafmt-pisp-fe.rst    | 39 ++++++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c               |  2 ++
 include/uapi/linux/videodev2.h                     |  2 ++
 4 files changed, 44 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
index c6e56b5888bc..86ffb3bc8ade 100644
--- a/Documentation/userspace-api/media/v4l/meta-formats.rst
+++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
@@ -16,6 +16,7 @@ These formats are used for the :ref:`metadata` interface only.
     metafmt-generic
     metafmt-intel-ipu3
     metafmt-pisp-be
+    metafmt-pisp-fe
     metafmt-rkisp1
     metafmt-uvc
     metafmt-vivid
diff --git a/Documentation/userspace-api/media/v4l/metafmt-pisp-fe.rst b/Documentation/userspace-api/media/v4l/metafmt-pisp-fe.rst
new file mode 100644
index 000000000000..fddeada83e4a
--- /dev/null
+++ b/Documentation/userspace-api/media/v4l/metafmt-pisp-fe.rst
@@ -0,0 +1,39 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _v4l2-meta-fmt-rpi-fe-cfg:
+
+************************
+V4L2_META_FMT_RPI_FE_CFG
+************************
+
+Raspberry Pi PiSP Front End configuration format
+================================================
+
+The Raspberry Pi PiSP Front End image signal processor is configured by
+userspace by providing a buffer of configuration parameters to the
+`rp1-cfe-fe-config` output video device node using the
+:c:type:`v4l2_meta_format` interface.
+
+The `Raspberry Pi PiSP technical specification
+<https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf>`_
+provide detailed description of the Front End configuration and programming
+model.
+
+.. _v4l2-meta-fmt-rpi-fe-stats:
+
+**************************
+V4L2_META_FMT_RPI_FE_STATS
+**************************
+
+Raspberry Pi PiSP Front End statistics format
+=============================================
+
+The Raspberry Pi PiSP Front End image signal processor provides statistics data
+by writing to a buffer provided via the `rp1-cfe-fe-stats` capture video device
+node using the
+:c:type:`v4l2_meta_format` interface.
+
+The `Raspberry Pi PiSP technical specification
+<https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf>`_
+provide detailed description of the Front End configuration and programming
+model.
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 5eb4d797d259..95c2c4a97966 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1466,6 +1466,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_PIX_FMT_Y212:		descr = "12-bit YUYV Packed"; break;
 	case V4L2_PIX_FMT_Y216:		descr = "16-bit YUYV Packed"; break;
 	case V4L2_META_FMT_RPI_BE_CFG:	descr = "RPi PiSP BE Config format"; break;
+	case V4L2_META_FMT_RPI_FE_CFG:  descr = "RPi PiSP FE Config format"; break;
+	case V4L2_META_FMT_RPI_FE_STATS: descr = "RPi PiSP FE Statistics format"; break;
 	case V4L2_META_FMT_GENERIC_8:	descr = "8-bit Generic Metadata"; break;
 	case V4L2_META_FMT_GENERIC_CSI2_10:	descr = "8-bit Generic Meta, 10b CSI-2"; break;
 	case V4L2_META_FMT_GENERIC_CSI2_12:	descr = "8-bit Generic Meta, 12b CSI-2"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 4e91362da6da..9f68cd55248f 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -857,6 +857,8 @@ struct v4l2_pix_format {
 
 /* Vendor specific - used for RaspberryPi PiSP */
 #define V4L2_META_FMT_RPI_BE_CFG	v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */
+#define V4L2_META_FMT_RPI_FE_CFG	v4l2_fourcc('R', 'P', 'F', 'C') /* PiSP FE configuration */
+#define V4L2_META_FMT_RPI_FE_STATS	v4l2_fourcc('R', 'P', 'F', 'S') /* PiSP FE stats */
 
 #ifdef __KERNEL__
 /*

-- 
2.34.1

[PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Tomi Valkeinen]

Add DT bindings for raspberrypi,rp1-cfe.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 .../bindings/media/raspberrypi,rp1-cfe.yaml        | 98 ++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
new file mode 100644
index 000000000000..851533de2305
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raspberry Pi PiSP Camera Front End
+
+maintainers:
+  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
+  - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
+
+description: |
+  The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O
+  controller, that contains:
+  - MIPI D-PHY
+  - MIPI CSI-2 receiver
+  - Simple image processor (called PiSP Front End, or FE)
+
+  The FE documentation is available at:
+  https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf
+
+  The PHY and CSI-2 receiver part have no public documentation.
+
+properties:
+  compatible:
+    items:
+      - const: raspberrypi,rp1-cfe
+
+  reg:
+    items:
+      - description: CSI-2 registers
+      - description: D-PHY registers
+      - description: MIPI CFG (a simple top-level mux) registers
+      - description: FE registers
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+    description: CSI-2 RX Port
+
+    properties:
+      endpoint:
+        $ref: video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            minItems: 1
+            maxItems: 4
+
+          clock-lanes:
+            maxItems: 1
+
+        required:
+          - clock-lanes
+          - data-lanes
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    rp1 {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      csi@110000 {
+        compatible = "raspberrypi,rp1-cfe";
+        reg = <0xc0 0x40110000 0x0 0x100>,
+              <0xc0 0x40114000 0x0 0x100>,
+              <0xc0 0x40120000 0x0 0x100>,
+              <0xc0 0x40124000 0x0 0x1000>;
+
+        interrupts = <42>;
+
+        clocks = <&rp1_clocks>;
+
+        port {
+          csi_ep: endpoint {
+            remote-endpoint = <&cam_endpoint>;
+            clock-lanes = <0>;
+            data-lanes = <1 2>;
+          };
+        };
+      };
+    };

-- 
2.34.1

[PATCH v2 4/4] media: admin-guide: Document the Raspberry Pi CFE (rp1-cfe)

[Tomi Valkeinen]

Add documentation for rp1-cfe driver.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 .../admin-guide/media/raspberrypi-rp1-cfe.dot      | 27 ++++++++
 .../admin-guide/media/raspberrypi-rp1-cfe.rst      | 78 ++++++++++++++++++++++
 Documentation/admin-guide/media/v4l-drivers.rst    |  1 +
 3 files changed, 106 insertions(+)

diff --git a/Documentation/admin-guide/media/raspberrypi-rp1-cfe.dot b/Documentation/admin-guide/media/raspberrypi-rp1-cfe.dot
new file mode 100644
index 000000000000..7717f2291049
--- /dev/null
+++ b/Documentation/admin-guide/media/raspberrypi-rp1-cfe.dot
@@ -0,0 +1,27 @@
+digraph board {
+	rankdir=TB
+	n00000001 [label="{{<port0> 0} | csi2\n/dev/v4l-subdev0 | {<port1> 1 | <port2> 2 | <port3> 3 | <port4> 4}}", shape=Mrecord, style=filled, fillcolor=green]
+	n00000001:port1 -> n00000011 [style=dashed]
+	n00000001:port1 -> n00000007:port0
+	n00000001:port2 -> n00000015
+	n00000001:port2 -> n00000007:port0 [style=dashed]
+	n00000001:port3 -> n00000019 [style=dashed]
+	n00000001:port3 -> n00000007:port0 [style=dashed]
+	n00000001:port4 -> n0000001d [style=dashed]
+	n00000001:port4 -> n00000007:port0 [style=dashed]
+	n00000007 [label="{{<port0> 0 | <port1> 1} | pisp-fe\n/dev/v4l-subdev1 | {<port2> 2 | <port3> 3 | <port4> 4}}", shape=Mrecord, style=filled, fillcolor=green]
+	n00000007:port2 -> n00000021
+	n00000007:port3 -> n00000025 [style=dashed]
+	n00000007:port4 -> n00000029
+	n0000000d [label="{imx219 6-0010\n/dev/v4l-subdev2 | {<port0> 0}}", shape=Mrecord, style=filled, fillcolor=green]
+	n0000000d:port0 -> n00000001:port0 [style=bold]
+	n00000011 [label="rp1-cfe-csi2-ch0\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
+	n00000015 [label="rp1-cfe-csi2-ch1\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
+	n00000019 [label="rp1-cfe-csi2-ch2\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
+	n0000001d [label="rp1-cfe-csi2-ch3\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
+	n00000021 [label="rp1-cfe-fe-image0\n/dev/video4", shape=box, style=filled, fillcolor=yellow]
+	n00000025 [label="rp1-cfe-fe-image1\n/dev/video5", shape=box, style=filled, fillcolor=yellow]
+	n00000029 [label="rp1-cfe-fe-stats\n/dev/video6", shape=box, style=filled, fillcolor=yellow]
+	n0000002d [label="rp1-cfe-fe-config\n/dev/video7", shape=box, style=filled, fillcolor=yellow]
+	n0000002d -> n00000007:port1
+}
diff --git a/Documentation/admin-guide/media/raspberrypi-rp1-cfe.rst b/Documentation/admin-guide/media/raspberrypi-rp1-cfe.rst
new file mode 100644
index 000000000000..668d978a9875
--- /dev/null
+++ b/Documentation/admin-guide/media/raspberrypi-rp1-cfe.rst
@@ -0,0 +1,78 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================================
+Raspberry Pi PiSP Camera Front End (rp1-cfe)
+============================================
+
+The PiSP Camera Front End
+=========================
+
+The PiSP Camera Front End (CFE) is a module which combines a CSI-2 receiver with
+a simple ISP, called the Front End (FE).
+
+The CFE has four DMA engines and can write frames from four separate streams
+received from the CSI-2 to the memory. One of those streams can also be routed
+directly to the FE, which can do minimal image processing, write two versions
+(e.g. non-scaled and downscaled versions) of the received frames to memory and
+provide statistics of the received frames.
+
+The FE registers are documented in the `Raspberry Pi Image Signal Processor
+(ISP) Specification document
+<https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf>`_,
+and example code for FE can be found in `libpisp
+<https://github.com/raspberrypi/libpisp>`_.
+
+The rp1-cfe driver
+==================
+
+The Raspberry Pi PiSP Camera Front End (rp1-cfe) driver is located under
+drivers/media/platform/raspberrypi/rp1-cfe. It uses the `V4L2 API` to register
+a number of video capture and output devices, the `V4L2 subdev API` to register
+subdevices for the CSI-2 received and the FE that connects the video devices in
+a single media graph realized using the `Media Controller (MC) API`.
+
+The media topology registered by the `rp1-cfe` driver, in this particular
+example connected to an imx219 sensor, is the following one:
+
+.. _rp1-cfe-topology:
+
+.. kernel-figure:: raspberrypi-rp1-cfe.dot
+    :alt:   Diagram of an example media pipeline topology
+    :align: center
+
+The media graph contains the following video device nodes:
+
+- rp1-cfe-csi2-ch0: capture device for the first CSI-2 stream
+- rp1-cfe-csi2-ch1: capture device for the second CSI-2 stream
+- rp1-cfe-csi2-ch2: capture device for the third CSI-2 stream
+- rp1-cfe-csi2-ch3: capture device for the fourth CSI-2 stream
+- rp1-cfe-fe-image0: capture device for the first FE output
+- rp1-cfe-fe-image1: capture device for the second FE output
+- rp1-cfe-fe-stats: capture device for the FE statistics
+- rp1-cfe-fe-config: output device for FE configuration
+
+rp1-cfe-csi2-chX
+----------------
+
+The rp1-cfe-csi2-chX capture devices are normal V4L2 capture devices which
+can be used to capture video frames or metadata received from the CSI-2.
+
+rp1-cfe-fe-image0, rp1-cfe-fe-image1
+------------------------------------
+
+The rp1-cfe-fe-image0 and rp1-cfe-fe-image1 capture devices are used to write
+the processed frames to memory.
+
+rp1-cfe-fe-stats
+----------------
+
+The format of the FE statistics buffer is defined by
+:c:type:`pisp_statistics` C structure and the meaning of each parameter is
+described in the `PiSP specification` document.
+
+rp1-cfe-fe-config
+-----------------
+
+The format of the FE configuration buffer is defined by
+:c:type:`pisp_fe_config` C structure and the meaning of each parameter is
+described in the `PiSP specification` document.
diff --git a/Documentation/admin-guide/media/v4l-drivers.rst b/Documentation/admin-guide/media/v4l-drivers.rst
index b6af448b9fe9..61da154e079a 100644
--- a/Documentation/admin-guide/media/v4l-drivers.rst
+++ b/Documentation/admin-guide/media/v4l-drivers.rst
@@ -26,6 +26,7 @@ Video4Linux (V4L) driver-specific documentation
 	raspberrypi-pisp-be
 	rcar-fdp1
 	rkisp1
+	raspberrypi-rp1-cfe
 	saa7134
 	si470x
 	si4713

-- 
2.34.1

Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Krzysztof Kozlowski]

On 20/06/2024 13:07, Tomi Valkeinen wrote:
> Add DT bindings for raspberrypi,rp1-cfe.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v2 3/4] media: raspberrypi: Add support for RP1-CFE

[Laurent Pinchart]

On Thu, Jun 20, 2024 at 02:07:52PM +0300, Tomi Valkeinen wrote:
> Add support for Raspberry Pi CFE. The CFE is a hardware block that
> contains:
> 
> - MIPI D-PHY
> - MIPI CSI-2 receiver
> - Front End ISP (FE)
> 
> The driver has been upported from the Raspberry Pi kernel commit
> 88a681df9623 ("ARM: dts: bcm2712-rpi: Add i2c<n>_pins labels").
> 
> Co-developed-by: Naushir Patuck <naush@raspberrypi.com>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  MAINTAINERS                                        |    8 +
>  drivers/media/platform/raspberrypi/Kconfig         |    1 +
>  drivers/media/platform/raspberrypi/Makefile        |    1 +
>  drivers/media/platform/raspberrypi/rp1-cfe/Kconfig |   14 +
>  .../media/platform/raspberrypi/rp1-cfe/Makefile    |    6 +
>  .../media/platform/raspberrypi/rp1-cfe/cfe-fmts.h  |  332 +++
>  .../media/platform/raspberrypi/rp1-cfe/cfe-trace.h |  196 ++
>  drivers/media/platform/raspberrypi/rp1-cfe/cfe.c   | 2526 ++++++++++++++++++++
>  drivers/media/platform/raspberrypi/rp1-cfe/cfe.h   |   43 +
>  drivers/media/platform/raspberrypi/rp1-cfe/csi2.c  |  583 +++++
>  drivers/media/platform/raspberrypi/rp1-cfe/csi2.h  |   89 +
>  drivers/media/platform/raspberrypi/rp1-cfe/dphy.c  |  175 ++
>  drivers/media/platform/raspberrypi/rp1-cfe/dphy.h  |   27 +
>  .../media/platform/raspberrypi/rp1-cfe/pisp-fe.c   |  581 +++++
>  .../media/platform/raspberrypi/rp1-cfe/pisp-fe.h   |   53 +
>  .../uapi/linux/media/raspberrypi/pisp_fe_config.h  |  273 +++
>  .../linux/media/raspberrypi/pisp_fe_statistics.h   |   64 +
>  17 files changed, 4972 insertions(+)

[snip]

> diff --git a/drivers/media/platform/raspberrypi/rp1-cfe/dphy.c b/drivers/media/platform/raspberrypi/rp1-cfe/dphy.c
> new file mode 100644
> index 000000000000..c9e4a94fb80c
> --- /dev/null
> +++ b/drivers/media/platform/raspberrypi/rp1-cfe/dphy.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * RP1 CSI-2 Driver
> + *
> + * Copyright (c) 2021-2024 Raspberry Pi Ltd.
> + * Copyright (c) 2023-2024 Ideas on Board Oy
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "dphy.h"
> +
> +#define dphy_dbg(fmt, arg...) dev_dbg(dphy->dev, fmt, ##arg)
> +#define dphy_err(fmt, arg...) dev_err(dphy->dev, fmt, ##arg)
> +
> +/* DW dphy Host registers */
> +#define DPHY_VERSION		0x000
> +#define DPHY_N_LANES		0x004
> +#define DPHY_RESETN		0x008
> +#define DPHY_PHY_SHUTDOWNZ	0x040
> +#define DPHY_PHY_RSTZ		0x044
> +#define DPHY_PHY_RX		0x048
> +#define	DPHY_PHY_STOPSTATE	0x04c
> +#define DPHY_PHY_TST_CTRL0	0x050
> +#define DPHY_PHY_TST_CTRL1	0x054
> +#define DPHY_PHY2_TST_CTRL0	0x058
> +#define DPHY_PHY2_TST_CTRL1	0x05c

Quoting drivers/media/platform/renesas/rcar-csi2.c:

/* V4H BASE registers */
#define V4H_N_LANES_REG                                 0x0004
#define V4H_CSI2_RESETN_REG                             0x0008
#define V4H_PHY_MODE_REG                                0x001c
#define V4H_PHY_SHUTDOWNZ_REG                           0x0040
#define V4H_DPHY_RSTZ_REG                               0x0044

Time for a common PHY driver ?

> +
> +/* DW dphy Host Transactions */
> +#define DPHY_HS_RX_CTRL_LANE0_OFFSET	0x44
> +#define DPHY_PLL_INPUT_DIV_OFFSET	0x17
> +#define DPHY_PLL_LOOP_DIV_OFFSET	0x18
> +#define DPHY_PLL_DIV_CTRL_OFFSET	0x19
> +
> +static u32 dw_csi2_host_read(struct dphy_data *dphy, u32 offset)
> +{
> +	return readl(dphy->base + offset);
> +}
> +
> +static void dw_csi2_host_write(struct dphy_data *dphy, u32 offset, u32 data)
> +{
> +	writel(data, dphy->base + offset);
> +}
> +
> +static void set_tstclr(struct dphy_data *dphy, u32 val)
> +{
> +	u32 ctrl0 = dw_csi2_host_read(dphy, DPHY_PHY_TST_CTRL0);
> +
> +	dw_csi2_host_write(dphy, DPHY_PHY_TST_CTRL0, (ctrl0 & ~1) | val);
> +}
> +
> +static void set_tstclk(struct dphy_data *dphy, u32 val)
> +{
> +	u32 ctrl0 = dw_csi2_host_read(dphy, DPHY_PHY_TST_CTRL0);
> +
> +	dw_csi2_host_write(dphy, DPHY_PHY_TST_CTRL0, (ctrl0 & ~2) | (val << 1));
> +}
> +
> +static uint8_t get_tstdout(struct dphy_data *dphy)
> +{
> +	u32 ctrl1 = dw_csi2_host_read(dphy, DPHY_PHY_TST_CTRL1);
> +
> +	return ((ctrl1 >> 8) & 0xff);
> +}
> +
> +static void set_testen(struct dphy_data *dphy, u32 val)
> +{
> +	u32 ctrl1 = dw_csi2_host_read(dphy, DPHY_PHY_TST_CTRL1);
> +
> +	dw_csi2_host_write(dphy, DPHY_PHY_TST_CTRL1,
> +			   (ctrl1 & ~(1 << 16)) | (val << 16));
> +}
> +
> +static void set_testdin(struct dphy_data *dphy, u32 val)
> +{
> +	u32 ctrl1 = dw_csi2_host_read(dphy, DPHY_PHY_TST_CTRL1);
> +
> +	dw_csi2_host_write(dphy, DPHY_PHY_TST_CTRL1, (ctrl1 & ~0xff) | val);
> +}
> +
> +static uint8_t dphy_transaction(struct dphy_data *dphy, u8 test_code,
> +				uint8_t test_data)
> +{
> +	/* See page 101 of the MIPI DPHY databook. */
> +	set_tstclk(dphy, 1);
> +	set_testen(dphy, 0);
> +	set_testdin(dphy, test_code);
> +	set_testen(dphy, 1);
> +	set_tstclk(dphy, 0);
> +	set_testen(dphy, 0);
> +	set_testdin(dphy, test_data);
> +	set_tstclk(dphy, 1);
> +	return get_tstdout(dphy);
> +}
> +
> +static void dphy_set_hsfreqrange(struct dphy_data *dphy, uint32_t mbps)
> +{
> +	/* See Table 5-1 on page 65 of dphy databook */
> +	static const u16 hsfreqrange_table[][2] = {
> +		{ 89, 0b000000 },   { 99, 0b010000 },	{ 109, 0b100000 },
> +		{ 129, 0b000001 },  { 139, 0b010001 },	{ 149, 0b100001 },
> +		{ 169, 0b000010 },  { 179, 0b010010 },	{ 199, 0b100010 },
> +		{ 219, 0b000011 },  { 239, 0b010011 },	{ 249, 0b100011 },
> +		{ 269, 0b000100 },  { 299, 0b010100 },	{ 329, 0b000101 },
> +		{ 359, 0b010101 },  { 399, 0b100101 },	{ 449, 0b000110 },
> +		{ 499, 0b010110 },  { 549, 0b000111 },	{ 599, 0b010111 },
> +		{ 649, 0b001000 },  { 699, 0b011000 },	{ 749, 0b001001 },
> +		{ 799, 0b011001 },  { 849, 0b101001 },	{ 899, 0b111001 },
> +		{ 949, 0b001010 },  { 999, 0b011010 },	{ 1049, 0b101010 },
> +		{ 1099, 0b111010 }, { 1149, 0b001011 }, { 1199, 0b011011 },
> +		{ 1249, 0b101011 }, { 1299, 0b111011 }, { 1349, 0b001100 },
> +		{ 1399, 0b011100 }, { 1449, 0b101100 }, { 1500, 0b111100 },
> +	};
> +	unsigned int i;
> +
> +	if (mbps < 80 || mbps > 1500)
> +		dphy_err("DPHY: Datarate %u Mbps out of range\n", mbps);
> +
> +	for (i = 0; i < ARRAY_SIZE(hsfreqrange_table) - 1; i++) {
> +		if (mbps <= hsfreqrange_table[i][0])
> +			break;
> +	}
> +
> +	dphy_transaction(dphy, DPHY_HS_RX_CTRL_LANE0_OFFSET,
> +			 hsfreqrange_table[i][1] << 1);
> +}
> +
> +static void dphy_init(struct dphy_data *dphy)
> +{
> +	dw_csi2_host_write(dphy, DPHY_PHY_RSTZ, 0);
> +	dw_csi2_host_write(dphy, DPHY_PHY_SHUTDOWNZ, 0);
> +	set_tstclk(dphy, 1);
> +	set_testen(dphy, 0);
> +	set_tstclr(dphy, 1);
> +	usleep_range(15, 20);
> +	set_tstclr(dphy, 0);
> +	usleep_range(15, 20);
> +
> +	dphy_set_hsfreqrange(dphy, dphy->dphy_rate);
> +
> +	usleep_range(5, 10);
> +	dw_csi2_host_write(dphy, DPHY_PHY_SHUTDOWNZ, 1);
> +	usleep_range(5, 10);
> +	dw_csi2_host_write(dphy, DPHY_PHY_RSTZ, 1);
> +}
> +
> +void dphy_start(struct dphy_data *dphy)
> +{
> +	dw_csi2_host_write(dphy, DPHY_N_LANES, (dphy->active_lanes - 1));
> +	dphy_init(dphy);
> +	dw_csi2_host_write(dphy, DPHY_RESETN, 0xffffffff);
> +	usleep_range(10, 50);
> +}
> +
> +void dphy_stop(struct dphy_data *dphy)
> +{
> +	/* Set only one lane (lane 0) as active (ON) */
> +	dw_csi2_host_write(dphy, DPHY_N_LANES, 0);
> +	dw_csi2_host_write(dphy, DPHY_RESETN, 0);
> +}
> +
> +void dphy_probe(struct dphy_data *dphy)
> +{
> +	u32 host_ver;
> +	u8 host_ver_major, host_ver_minor;
> +
> +	host_ver = dw_csi2_host_read(dphy, DPHY_VERSION);
> +	host_ver_major = (u8)((host_ver >> 24) - '0');
> +	host_ver_minor = (u8)((host_ver >> 16) - '0');
> +	host_ver_minor = host_ver_minor * 10;
> +	host_ver_minor += (u8)((host_ver >> 8) - '0');
> +
> +	dphy_dbg("DW dphy Host HW v%u.%u\n", host_ver_major, host_ver_minor);
> +}

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Sakari Ailus]

Moi,

Thanks for the set!

On Thu, Jun 20, 2024 at 02:07:51PM +0300, Tomi Valkeinen wrote:
> Add DT bindings for raspberrypi,rp1-cfe.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  .../bindings/media/raspberrypi,rp1-cfe.yaml        | 98 ++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> new file mode 100644
> index 000000000000..851533de2305
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Raspberry Pi PiSP Camera Front End
> +
> +maintainers:
> +  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> +  - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> +
> +description: |
> +  The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O
> +  controller, that contains:
> +  - MIPI D-PHY
> +  - MIPI CSI-2 receiver
> +  - Simple image processor (called PiSP Front End, or FE)
> +
> +  The FE documentation is available at:
> +  https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf
> +
> +  The PHY and CSI-2 receiver part have no public documentation.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: raspberrypi,rp1-cfe
> +
> +  reg:
> +    items:
> +      - description: CSI-2 registers
> +      - description: D-PHY registers
> +      - description: MIPI CFG (a simple top-level mux) registers
> +      - description: FE registers
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +    description: CSI-2 RX Port
> +
> +    properties:
> +      endpoint:
> +        $ref: video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +          clock-lanes:
> +            maxItems: 1

minItems needs to be 1 as well.

Or... is this actually configurable in hardware?

> +
> +        required:
> +          - clock-lanes
> +          - data-lanes
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    rp1 {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      csi@110000 {
> +        compatible = "raspberrypi,rp1-cfe";
> +        reg = <0xc0 0x40110000 0x0 0x100>,
> +              <0xc0 0x40114000 0x0 0x100>,
> +              <0xc0 0x40120000 0x0 0x100>,
> +              <0xc0 0x40124000 0x0 0x1000>;
> +
> +        interrupts = <42>;
> +
> +        clocks = <&rp1_clocks>;
> +
> +        port {
> +          csi_ep: endpoint {
> +            remote-endpoint = <&cam_endpoint>;
> +            clock-lanes = <0>;
> +            data-lanes = <1 2>;
> +          };
> +        };
> +      };
> +    };
> 

-- 
Terveisin,

Sakari Ailus

Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Laurent Pinchart]

On Wed, Aug 28, 2024 at 11:06:23AM +0000, Sakari Ailus wrote:
> On Thu, Jun 20, 2024 at 02:07:51PM +0300, Tomi Valkeinen wrote:
> > Add DT bindings for raspberrypi,rp1-cfe.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > ---
> >  .../bindings/media/raspberrypi,rp1-cfe.yaml        | 98 ++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> > new file mode 100644
> > index 000000000000..851533de2305
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Raspberry Pi PiSP Camera Front End
> > +
> > +maintainers:
> > +  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > +  - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> > +
> > +description: |
> > +  The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O
> > +  controller, that contains:
> > +  - MIPI D-PHY
> > +  - MIPI CSI-2 receiver
> > +  - Simple image processor (called PiSP Front End, or FE)
> > +
> > +  The FE documentation is available at:
> > +  https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf
> > +
> > +  The PHY and CSI-2 receiver part have no public documentation.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: raspberrypi,rp1-cfe
> > +
> > +  reg:
> > +    items:
> > +      - description: CSI-2 registers
> > +      - description: D-PHY registers
> > +      - description: MIPI CFG (a simple top-level mux) registers
> > +      - description: FE registers
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > +    additionalProperties: false
> > +    description: CSI-2 RX Port
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          data-lanes:
> > +            minItems: 1
> > +            maxItems: 4
> > +
> > +          clock-lanes:
> > +            maxItems: 1
> 
> minItems needs to be 1 as well.
> 
> Or... is this actually configurable in hardware?

Looking at the driver, lane reordering is not supported, so we could
drop this property. If the hardware is found to support it later, it can
easily be added back without any backward compatibility issue.

> > +
> > +        required:
> > +          - clock-lanes
> > +          - data-lanes
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    rp1 {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      csi@110000 {
> > +        compatible = "raspberrypi,rp1-cfe";
> > +        reg = <0xc0 0x40110000 0x0 0x100>,
> > +              <0xc0 0x40114000 0x0 0x100>,
> > +              <0xc0 0x40120000 0x0 0x100>,
> > +              <0xc0 0x40124000 0x0 0x1000>;
> > +
> > +        interrupts = <42>;
> > +
> > +        clocks = <&rp1_clocks>;
> > +
> > +        port {
> > +          csi_ep: endpoint {
> > +            remote-endpoint = <&cam_endpoint>;
> > +            clock-lanes = <0>;
> > +            data-lanes = <1 2>;
> > +          };
> > +        };
> > +      };
> > +    };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Tomi Valkeinen]

Hi,

On 28/08/2024 14:12, Laurent Pinchart wrote:
> On Wed, Aug 28, 2024 at 11:06:23AM +0000, Sakari Ailus wrote:
>> On Thu, Jun 20, 2024 at 02:07:51PM +0300, Tomi Valkeinen wrote:
>>> Add DT bindings for raspberrypi,rp1-cfe.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>>   .../bindings/media/raspberrypi,rp1-cfe.yaml        | 98 ++++++++++++++++++++++
>>>   1 file changed, 98 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
>>> new file mode 100644
>>> index 000000000000..851533de2305
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
>>> @@ -0,0 +1,98 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Raspberry Pi PiSP Camera Front End
>>> +
>>> +maintainers:
>>> +  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> +  - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
>>> +
>>> +description: |
>>> +  The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O
>>> +  controller, that contains:
>>> +  - MIPI D-PHY
>>> +  - MIPI CSI-2 receiver
>>> +  - Simple image processor (called PiSP Front End, or FE)
>>> +
>>> +  The FE documentation is available at:
>>> +  https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf
>>> +
>>> +  The PHY and CSI-2 receiver part have no public documentation.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - const: raspberrypi,rp1-cfe
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: CSI-2 registers
>>> +      - description: D-PHY registers
>>> +      - description: MIPI CFG (a simple top-level mux) registers
>>> +      - description: FE registers
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  port:
>>> +    $ref: /schemas/graph.yaml#/$defs/port-base
>>> +    additionalProperties: false
>>> +    description: CSI-2 RX Port
>>> +
>>> +    properties:
>>> +      endpoint:
>>> +        $ref: video-interfaces.yaml#
>>> +        unevaluatedProperties: false
>>> +
>>> +        properties:
>>> +          data-lanes:
>>> +            minItems: 1
>>> +            maxItems: 4
>>> +
>>> +          clock-lanes:
>>> +            maxItems: 1
>>
>> minItems needs to be 1 as well.

Hmm, I see a lot of

clock-lanes:
   maxItems: 1

in the device tree bindings. And 
https://docs.kernel.org/devicetree/bindings/writing-schema.html says 
"Cases that have only a single entry just need to express that with 
maxItems".

>>
>> Or... is this actually configurable in hardware?
> 
> Looking at the driver, lane reordering is not supported, so we could
> drop this property. If the hardware is found to support it later, it can
> easily be added back without any backward compatibility issue.

Re-ordering is not supported. I guess clock-lanes can be dropped, 
although I feel that if we have the clock lane in the hardware, and the 
numbering of data-lanes must take that into account, then:

clock-lanes = <0>;
data-lanes = <1 2>;

looks better than:

data-lanes = <1 2>; /* and implicit clk lane 0 */

But I can't think of any practical benefit it brings...

  Tomi

Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Sakari Ailus]

Heippa,

On Wed, Aug 28, 2024 at 03:14:47PM +0300, Tomi Valkeinen wrote:
> Hi,
> 
> On 28/08/2024 14:12, Laurent Pinchart wrote:
> > On Wed, Aug 28, 2024 at 11:06:23AM +0000, Sakari Ailus wrote:
> > > On Thu, Jun 20, 2024 at 02:07:51PM +0300, Tomi Valkeinen wrote:
> > > > Add DT bindings for raspberrypi,rp1-cfe.
> > > > 
> > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > > > ---
> > > >   .../bindings/media/raspberrypi,rp1-cfe.yaml        | 98 ++++++++++++++++++++++
> > > >   1 file changed, 98 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> > > > new file mode 100644
> > > > index 000000000000..851533de2305
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> > > > @@ -0,0 +1,98 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Raspberry Pi PiSP Camera Front End
> > > > +
> > > > +maintainers:
> > > > +  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > > > +  - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> > > > +
> > > > +description: |
> > > > +  The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O
> > > > +  controller, that contains:
> > > > +  - MIPI D-PHY
> > > > +  - MIPI CSI-2 receiver
> > > > +  - Simple image processor (called PiSP Front End, or FE)
> > > > +
> > > > +  The FE documentation is available at:
> > > > +  https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf
> > > > +
> > > > +  The PHY and CSI-2 receiver part have no public documentation.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - const: raspberrypi,rp1-cfe
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: CSI-2 registers
> > > > +      - description: D-PHY registers
> > > > +      - description: MIPI CFG (a simple top-level mux) registers
> > > > +      - description: FE registers
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    maxItems: 1
> > > > +
> > > > +  port:
> > > > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > > > +    additionalProperties: false
> > > > +    description: CSI-2 RX Port
> > > > +
> > > > +    properties:
> > > > +      endpoint:
> > > > +        $ref: video-interfaces.yaml#
> > > > +        unevaluatedProperties: false
> > > > +
> > > > +        properties:
> > > > +          data-lanes:
> > > > +            minItems: 1
> > > > +            maxItems: 4
> > > > +
> > > > +          clock-lanes:
> > > > +            maxItems: 1
> > > 
> > > minItems needs to be 1 as well.
> 
> Hmm, I see a lot of
> 
> clock-lanes:
>   maxItems: 1
> 
> in the device tree bindings. And
> https://docs.kernel.org/devicetree/bindings/writing-schema.html says "Cases
> that have only a single entry just need to express that with maxItems".

Fair enough.

> 
> > > 
> > > Or... is this actually configurable in hardware?
> > 
> > Looking at the driver, lane reordering is not supported, so we could
> > drop this property. If the hardware is found to support it later, it can
> > easily be added back without any backward compatibility issue.
> 
> Re-ordering is not supported. I guess clock-lanes can be dropped, although I
> feel that if we have the clock lane in the hardware, and the numbering of
> data-lanes must take that into account, then:
> 
> clock-lanes = <0>;
> data-lanes = <1 2>;
> 
> looks better than:
> 
> data-lanes = <1 2>; /* and implicit clk lane 0 */
> 
> But I can't think of any practical benefit it brings...

The clock-lanes property is optional, it only provides any information if
the clock lane is configurable. Please drop it.

-- 
Terveisin,

Sakari Ailus

Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for raspberrypi,rp1-cfe

[Laurent Pinchart]

On Wed, Aug 28, 2024 at 03:14:47PM +0300, Tomi Valkeinen wrote:
> On 28/08/2024 14:12, Laurent Pinchart wrote:
> > On Wed, Aug 28, 2024 at 11:06:23AM +0000, Sakari Ailus wrote:
> >> On Thu, Jun 20, 2024 at 02:07:51PM +0300, Tomi Valkeinen wrote:
> >>> Add DT bindings for raspberrypi,rp1-cfe.
> >>>
> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >>> ---
> >>>   .../bindings/media/raspberrypi,rp1-cfe.yaml        | 98 ++++++++++++++++++++++
> >>>   1 file changed, 98 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> >>> new file mode 100644
> >>> index 000000000000..851533de2305
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml
> >>> @@ -0,0 +1,98 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Raspberry Pi PiSP Camera Front End
> >>> +
> >>> +maintainers:
> >>> +  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >>> +  - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> >>> +
> >>> +description: |
> >>> +  The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O
> >>> +  controller, that contains:
> >>> +  - MIPI D-PHY
> >>> +  - MIPI CSI-2 receiver
> >>> +  - Simple image processor (called PiSP Front End, or FE)
> >>> +
> >>> +  The FE documentation is available at:
> >>> +  https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf
> >>> +
> >>> +  The PHY and CSI-2 receiver part have no public documentation.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    items:
> >>> +      - const: raspberrypi,rp1-cfe
> >>> +
> >>> +  reg:
> >>> +    items:
> >>> +      - description: CSI-2 registers
> >>> +      - description: D-PHY registers
> >>> +      - description: MIPI CFG (a simple top-level mux) registers
> >>> +      - description: FE registers
> >>> +
> >>> +  interrupts:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  port:
> >>> +    $ref: /schemas/graph.yaml#/$defs/port-base
> >>> +    additionalProperties: false
> >>> +    description: CSI-2 RX Port
> >>> +
> >>> +    properties:
> >>> +      endpoint:
> >>> +        $ref: video-interfaces.yaml#
> >>> +        unevaluatedProperties: false
> >>> +
> >>> +        properties:
> >>> +          data-lanes:
> >>> +            minItems: 1
> >>> +            maxItems: 4
> >>> +
> >>> +          clock-lanes:
> >>> +            maxItems: 1
> >>
> >> minItems needs to be 1 as well.
> 
> Hmm, I see a lot of
> 
> clock-lanes:
>    maxItems: 1
> 
> in the device tree bindings. And 
> https://docs.kernel.org/devicetree/bindings/writing-schema.html says 
> "Cases that have only a single entry just need to express that with 
> maxItems".

The rules may have changed recently, I'm not entirely sure. I've asked
in https://lore.kernel.org/dri-devel/20240818173003.122025-1-krzysztof.kozlowski@linaro.org/T/#m7669ca56543567e36733af745798713ae0293654

> >> Or... is this actually configurable in hardware?
> > 
> > Looking at the driver, lane reordering is not supported, so we could
> > drop this property. If the hardware is found to support it later, it can
> > easily be added back without any backward compatibility issue.
> 
> Re-ordering is not supported. I guess clock-lanes can be dropped, 
> although I feel that if we have the clock lane in the hardware, and the 
> numbering of data-lanes must take that into account, then:
> 
> clock-lanes = <0>;
> data-lanes = <1 2>;
> 
> looks better than:
> 
> data-lanes = <1 2>; /* and implicit clk lane 0 */
> 
> But I can't think of any practical benefit it brings...

-- 
Regards,

Laurent Pinchart