[PATCH v2 0/7] rcar-isp: Prepare for ISP core support

[PATCH v2 0/7] rcar-isp: Prepare for ISP core support

[Niklas Söderlund]

Hello,

This series prepares for adding support for the ISP core functionality
found on some R-Car ISP instances. No core support is however added in
this series, the focus is to get the easy changes out of the way to
avoid conflicts of fixes and new features being added in parallel on top
of this.

Patch 1/7 extends the dt-bindings to allow describing both the CSISP and
ISPCORE blocks. Patch 2/7, 3/7 and 4/7 updates the existing bindings to
match the new style. While the change breaks the dt-bindings the driver
is compatible with both styles.

Patch 5/7 prepares for the addition of the ISP core functions that will
span multiple files by moving the driver implementation from a single C
file to a directory where it can grow. The intent is to get this out of
the way without bikeshedding the real ISP core work so fixes and such
can be based on the new file structure as early as possible.

Patch 6/7 and 7/7 prepares the driver for dealing with two regions for
when the ISP core work is integrated.

There is no functional gain in this series apart from correctly
describing the hardware in dt.

See individual patches for changelog.

Niklas Söderlund (7):
  dt-bindings: media: renesas,isp: Add ISP core function block
  arm64: dts: renesas: r8a779a0: Add ISP core function block
  arm64: dts: renesas: r8a779g0: Add ISP core function block
  arm64: dts: renesas: r8a779h0: Add ISP core function block
  media: rcar-isp: Move driver to own directory
  media: rcar-isp: Rename base register variable
  media: rcar-isp: Parse named cs memory region

 .../bindings/media/renesas,isp.yaml           | 63 ++++++++++++++++---
 MAINTAINERS                                   |  2 +-
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi     | 60 +++++++++++++-----
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi     | 30 ++++++---
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi     | 21 +++++--
 drivers/media/platform/renesas/Kconfig        | 18 +-----
 drivers/media/platform/renesas/Makefile       |  2 +-
 .../media/platform/renesas/rcar-isp/Kconfig   | 17 +++++
 .../media/platform/renesas/rcar-isp/Makefile  |  4 ++
 .../renesas/{rcar-isp.c => rcar-isp/csisp.c}  | 56 ++++++++++-------
 10 files changed, 194 insertions(+), 79 deletions(-)
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Kconfig
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Makefile
 rename drivers/media/platform/renesas/{rcar-isp.c => rcar-isp/csisp.c} (89%)

-- 
2.49.0

[PATCH v2 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

[Niklas Söderlund]

Some R-Car ISP instances have in addition to the channel selector (CS)
an ISP core (CORE )to perform operations on an image stream. The core
function is mapped to a different memory region and have a separate
interrupt then CS, extend the bindings to allow describing this.

On the same SoC different instances of the ISP IP may have, or not have,
the CORE functionality. The CS function on all instances on the SoC are
the same and the documentation describes the full ISP (CS + CORE) as a
single IP block. Where instances not having the CORE function simple
lacking the functionality to modify the image data. There dependencies
on the CS functionality while operating the CORE functionality.

In order for the ISP core to function in memory-to-memory mode it needs
to be feed input data from a Streaming Bridge interface. This interface
is provided thru the VSP-X device. Add an optional new property
"renesas,vspx" to provide a phandle to describe this relationship.

While adding mandatory reg-names and interrupt-names breaks existing
bindings the driver itself remains backward compatible and provides CS
functionality if a single unnamed reg and interrupt property is present.
Furthermore all existing users of the bindings are updated in following
work to add these new mandatory properties.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v1
- Extend the commit message to make it explicit that different ISP
  instances on the same SoC (same compatible value) can have, or not
  have, a CORE function block attached.
- Update documentation for renesas,vspx property.
- Update example to cover all new properties.
---
 .../bindings/media/renesas,isp.yaml           | 63 ++++++++++++++++---
 1 file changed, 55 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
index c4de4555b753..927be02347e5 100644
--- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
+++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
@@ -25,19 +25,55 @@ properties:
           - renesas,r8a779h0-isp # V4M
       - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: cs
+      - const: core
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:
+    minItems: 1
+    items:
+      - const: cs
+      - const: core
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: cs
+      - const: core
 
   power-domains:
     maxItems: 1
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  reset-names:
+    minItems: 1
+    items:
+      - const: cs
+      - const: core
+
+  renesas,vspx:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      A phandle to the companion VSPX responsible for the Streaming Bridge
+      functionality. The Streaming Bridge is responsible for feeding image
+      and configuration data to the ISP when operating in memory-to-memory
+      mode.
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -103,10 +139,14 @@ properties:
 required:
   - compatible
   - reg
+  - reg-names
   - interrupts
+  - interrupt-names
   - clocks
+  - clock-names
   - power-domains
   - resets
+  - reset-names
   - ports
 
 additionalProperties: false
@@ -119,11 +159,18 @@ examples:
 
     isp1: isp@fed20000 {
             compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
-            reg = <0xfed20000 0x10000>;
-            interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
-            clocks = <&cpg CPG_MOD 613>;
+            reg = <0xfed20000 0x10000>, <0xfee00000 0x10000>;
+            reg-names = "cs", "core";
+            interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "cs", "core";
+            clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
+            clock-names = "cs", "core";
             power-domains = <&sysc R8A779A0_PD_A3ISP01>;
-            resets = <&cpg 613>;
+            resets = <&cpg 613>, <&cpg 17>;
+            reset-names = "cs", "core";
+
+            renesas,vspx = <&vspx1>;
 
             ports {
                     #address-cells = <1>;
-- 
2.49.0

[PATCH v2 2/7] arm64: dts: renesas: r8a779a0: Add ISP core function block

[Niklas Söderlund]

All ISP instances on V3U have both a channel select and core function
block, describe the core region in addition to the existing cs region.

The interrupt number already described intended to reflect the cs
function but did incorrectly describe the core block. This was not
noticed until now as the driver do not make use of the interrupt for the
cs block.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a779a0.dtsi | 60 +++++++++++++++++------
 1 file changed, 44 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
index f1613bfd1632..95ff69339991 100644
--- a/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779a0.dtsi
@@ -2588,13 +2588,20 @@ du_out_dsi1: endpoint {
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed00000 0 0x10000>;
-			interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 612>;
+			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP01>;
-			resets = <&cpg 612>;
+			resets = <&cpg 612>, <&cpg 16>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx0>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2672,13 +2679,20 @@ isp0vin07: endpoint {
 		isp1: isp@fed20000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed20000 0 0x10000>;
-			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 613>;
+			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP01>;
-			resets = <&cpg 613>;
+			resets = <&cpg 613>, <&cpg 17>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2756,13 +2770,20 @@ isp1vin15: endpoint {
 		isp2: isp@fed30000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed30000 0 0x10000>;
-			interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 614>;
+			reg = <0 0xfed30000 0 0x10000>, <0 0xfef00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 614>, <&cpg CPG_MOD 18>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
-			resets = <&cpg 614>;
+			resets = <&cpg 614>, <&cpg 18>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx2>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2840,13 +2861,20 @@ isp2vin23: endpoint {
 		isp3: isp@fed40000 {
 			compatible = "renesas,r8a779a0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed40000 0 0x10000>;
-			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&cpg CPG_MOD 615>;
+			reg = <0 0xfed40000 0 0x10000>, <0 0xfe400000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 615>, <&cpg CPG_MOD 19>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779A0_PD_A3ISP23>;
-			resets = <&cpg 615>;
+			resets = <&cpg 615>, <&cpg 19>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx3>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.49.0

[PATCH v2 3/7] arm64: dts: renesas: r8a779g0: Add ISP core function block

[Niklas Söderlund]

All ISP instances on V4H have both a channel select and core function
block, describe the core region in addition to the existing cs region.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 30 +++++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 1760720b7128..6dbf05a55935 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -2277,13 +2277,20 @@ du_out_dsi1: endpoint {
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779g0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed00000 0 0x10000>;
-			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_LOW>;
-			clocks = <&cpg CPG_MOD 612>;
+			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779G0_PD_A3ISP0>;
-			resets = <&cpg 612>;
+			resets = <&cpg 612>, <&cpg 16>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx0>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2361,13 +2368,20 @@ isp0vin07: endpoint {
 		isp1: isp@fed20000 {
 			compatible = "renesas,r8a779g0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed20000 0 0x10000>;
-			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
-			clocks = <&cpg CPG_MOD 613>;
+			reg = <0 0xfed20000 0 0x10000>, <0 0xfee00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 476 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779G0_PD_A3ISP1>;
-			resets = <&cpg 613>;
+			resets = <&cpg 613>, <&cpg 17>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.49.0

[PATCH v2 4/7] arm64: dts: renesas: r8a779h0: Add ISP core function block

[Niklas Söderlund]

The first ISP instance on V4M has both a channel select and core
function block, describe the core region in addition to the existing cs
region. While at it update the second ISP to match the new bindings and
add the reg-names and interrupt-names property.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a779h0.dtsi | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
index 8524a1e7205e..ed1eefa3515d 100644
--- a/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779h0.dtsi
@@ -1968,13 +1968,20 @@ du_out_dsi0: endpoint {
 		isp0: isp@fed00000 {
 			compatible = "renesas,r8a779h0-isp",
 				     "renesas,rcar-gen4-isp";
-			reg = <0 0xfed00000 0 0x10000>;
-			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_LOW>;
-			clocks = <&cpg CPG_MOD 612>;
+			reg = <0 0xfed00000 0 0x10000>, <0 0xfec00000 0 0x100000>;
+			reg-names = "cs", "core";
+			interrupts = <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs", "core";
+			clocks = <&cpg CPG_MOD 612>, <&cpg CPG_MOD 16>;
+			clock-names = "cs", "core";
 			power-domains = <&sysc R8A779H0_PD_A3ISP0>;
-			resets = <&cpg 612>;
+			resets = <&cpg 612>, <&cpg 16>;
+			reset-names = "cs", "core";
 			status = "disabled";
 
+			renesas,vspx = <&vspx0>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
@@ -2053,10 +2060,14 @@ isp1: isp@fed20000 {
 			compatible = "renesas,r8a779h0-isp",
 				     "renesas,rcar-gen4-isp";
 			reg = <0 0xfed20000 0 0x10000>;
-			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_LOW>;
+			reg-names = "cs";
+			interrupts = <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "cs";
 			clocks = <&cpg CPG_MOD 613>;
+			clock-names = "cs";
 			power-domains = <&sysc R8A779H0_PD_A3ISP0>;
 			resets = <&cpg 613>;
+			reset-names = "cs";
 			status = "disabled";
 
 			ports {
-- 
2.49.0

[PATCH v2 5/7] media: rcar-isp: Move driver to own directory

[Niklas Söderlund]

Before extending the driver with functions from the R-Car ISP core that
will span multiple files move the existing driver to a separate
directory. While at it rename the single source file to allow future
files to be grouped by functions.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 MAINTAINERS                                    |  2 +-
 drivers/media/platform/renesas/Kconfig         | 18 +-----------------
 drivers/media/platform/renesas/Makefile        |  2 +-
 .../media/platform/renesas/rcar-isp/Kconfig    | 17 +++++++++++++++++
 .../media/platform/renesas/rcar-isp/Makefile   |  4 ++++
 .../renesas/{rcar-isp.c => rcar-isp/csisp.c}   |  0
 6 files changed, 24 insertions(+), 19 deletions(-)
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Kconfig
 create mode 100644 drivers/media/platform/renesas/rcar-isp/Makefile
 rename drivers/media/platform/renesas/{rcar-isp.c => rcar-isp/csisp.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 465569a7b264..4904d0896773 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14917,7 +14917,7 @@ F:	Documentation/devicetree/bindings/media/renesas,csi2.yaml
 F:	Documentation/devicetree/bindings/media/renesas,isp.yaml
 F:	Documentation/devicetree/bindings/media/renesas,vin.yaml
 F:	drivers/media/platform/renesas/rcar-csi2.c
-F:	drivers/media/platform/renesas/rcar-isp.c
+F:	drivers/media/platform/renesas/rcar-isp/
 F:	drivers/media/platform/renesas/rcar-vin/
 
 MEDIA DRIVERS FOR RENESAS - VSP1
diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
index c7fc718a30a5..27a54fa79083 100644
--- a/drivers/media/platform/renesas/Kconfig
+++ b/drivers/media/platform/renesas/Kconfig
@@ -30,23 +30,6 @@ config VIDEO_RCAR_CSI2
 	  To compile this driver as a module, choose M here: the
 	  module will be called rcar-csi2.
 
-config VIDEO_RCAR_ISP
-	tristate "R-Car Image Signal Processor (ISP)"
-	depends on V4L_PLATFORM_DRIVERS
-	depends on VIDEO_DEV && OF
-	depends on ARCH_RENESAS || COMPILE_TEST
-	select MEDIA_CONTROLLER
-	select VIDEO_V4L2_SUBDEV_API
-	select RESET_CONTROLLER
-	select V4L2_FWNODE
-	help
-	  Support for Renesas R-Car Image Signal Processor (ISP).
-	  Enable this to support the Renesas R-Car Image Signal
-	  Processor (ISP).
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called rcar-isp.
-
 config VIDEO_SH_VOU
 	tristate "SuperH VOU video output driver"
 	depends on V4L_PLATFORM_DRIVERS
@@ -56,6 +39,7 @@ config VIDEO_SH_VOU
 	help
 	  Support for the Video Output Unit (VOU) on SuperH SoCs.
 
+source "drivers/media/platform/renesas/rcar-isp/Kconfig"
 source "drivers/media/platform/renesas/rcar-vin/Kconfig"
 source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
 
diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
index 50774a20330c..1127259c09d6 100644
--- a/drivers/media/platform/renesas/Makefile
+++ b/drivers/media/platform/renesas/Makefile
@@ -3,13 +3,13 @@
 # Makefile for the Renesas capture/playback device drivers.
 #
 
+obj-y += rcar-isp/
 obj-y += rcar-vin/
 obj-y += rzg2l-cru/
 obj-y += vsp1/
 
 obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
 obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
-obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
 obj-$(CONFIG_VIDEO_RENESAS_CEU) += renesas-ceu.o
 obj-$(CONFIG_VIDEO_RENESAS_FCP) += rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
diff --git a/drivers/media/platform/renesas/rcar-isp/Kconfig b/drivers/media/platform/renesas/rcar-isp/Kconfig
new file mode 100644
index 000000000000..59e0d91862d1
--- /dev/null
+++ b/drivers/media/platform/renesas/rcar-isp/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+config VIDEO_RCAR_ISP
+	tristate "R-Car Image Signal Processor (ISP)"
+	depends on V4L_PLATFORM_DRIVERS
+	depends on VIDEO_DEV && OF
+	depends on ARCH_RENESAS || COMPILE_TEST
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select RESET_CONTROLLER
+	select V4L2_FWNODE
+	help
+	  Support for Renesas R-Car Image Signal Processor (ISP).
+	  Enable this to support the Renesas R-Car Image Signal
+	  Processor (ISP).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rcar-isp.
diff --git a/drivers/media/platform/renesas/rcar-isp/Makefile b/drivers/media/platform/renesas/rcar-isp/Makefile
new file mode 100644
index 000000000000..b542118c831e
--- /dev/null
+++ b/drivers/media/platform/renesas/rcar-isp/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+rcar-isp-objs = csisp.o
+
+obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
similarity index 100%
rename from drivers/media/platform/renesas/rcar-isp.c
rename to drivers/media/platform/renesas/rcar-isp/csisp.c
-- 
2.49.0

[PATCH v2 6/7] media: rcar-isp: Rename base register variable

[Niklas Söderlund]

Prepare for extending the driver to in addition to support the channel
selector (CS) also support the core ISP. The two different functions
have different base addresses so the driver needs to distinguish between
them.

Prepare for this by marking existing base address variable and
read/write functions to make it clear it operates on the CS portion of
the driver. There is no functional change.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../media/platform/renesas/rcar-isp/csisp.c   | 46 +++++++++----------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index 4bc89d4757fa..f36d43c2e0a2 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -159,7 +159,7 @@ enum rcar_isp_pads {
 
 struct rcar_isp {
 	struct device *dev;
-	void __iomem *base;
+	void __iomem *csbase;
 	struct reset_control *rstc;
 
 	enum rcar_isp_input csi_input;
@@ -184,14 +184,14 @@ static inline struct rcar_isp *notifier_to_isp(struct v4l2_async_notifier *n)
 	return container_of(n, struct rcar_isp, notifier);
 }
 
-static void risp_write(struct rcar_isp *isp, u32 offset, u32 value)
+static void risp_write_cs(struct rcar_isp *isp, u32 offset, u32 value)
 {
-	iowrite32(value, isp->base + offset);
+	iowrite32(value, isp->csbase + offset);
 }
 
-static u32 risp_read(struct rcar_isp *isp, u32 offset)
+static u32 risp_read_cs(struct rcar_isp *isp, u32 offset)
 {
-	return ioread32(isp->base + offset);
+	return ioread32(isp->csbase + offset);
 }
 
 static int risp_power_on(struct rcar_isp *isp)
@@ -245,31 +245,31 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
 	if (isp->csi_input == RISP_CSI_INPUT1)
 		sel_csi = ISPINPUTSEL0_SEL_CSI0;
 
-	risp_write(isp, ISPINPUTSEL0_REG,
-		   risp_read(isp, ISPINPUTSEL0_REG) | sel_csi);
+	risp_write_cs(isp, ISPINPUTSEL0_REG,
+		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
 
 	/* Configure Channel Selector. */
 	for (vc = 0; vc < 4; vc++) {
 		u8 ch = vc + 4;
 		u8 dt = format->datatype;
 
-		risp_write(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
-		risp_write(isp, ISPCS_DT_CODE03_CH_REG(ch),
-			   ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
-			   ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
-			   ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
-			   ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
+		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
+		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
+			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
+			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
+			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
+			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
 	}
 
 	/* Setup processing method. */
-	risp_write(isp, ISPPROCMODE_DT_REG(format->datatype),
-		   ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
-		   ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
-		   ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
-		   ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
+	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
+		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
+		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
 
 	/* Start ISP. */
-	risp_write(isp, ISPSTART_REG, ISPSTART_START);
+	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
 
 	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
 					 BIT_ULL(0));
@@ -284,7 +284,7 @@ static void risp_stop(struct rcar_isp *isp)
 	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
 
 	/* Stop ISP. */
-	risp_write(isp, ISPSTART_REG, ISPSTART_STOP);
+	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
 
 	risp_power_off(isp);
 }
@@ -465,9 +465,9 @@ static const struct media_entity_operations risp_entity_ops = {
 static int risp_probe_resources(struct rcar_isp *isp,
 				struct platform_device *pdev)
 {
-	isp->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
-	if (IS_ERR(isp->base))
-		return PTR_ERR(isp->base);
+	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	if (IS_ERR(isp->csbase))
+		return PTR_ERR(isp->csbase);
 
 	isp->rstc = devm_reset_control_get(&pdev->dev, NULL);
 
-- 
2.49.0

[PATCH v2 7/7] media: rcar-isp: Parse named cs memory region

[Niklas Söderlund]

Extend the device tree parsing to optionally parse the cs memory region
by name. The change is backward compatible with the device tree model
where a single unnamed region describing only the ISP channel select
function.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/platform/renesas/rcar-isp/csisp.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
index f36d43c2e0a2..0b6fa62467e4 100644
--- a/drivers/media/platform/renesas/rcar-isp/csisp.c
+++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
@@ -465,7 +465,17 @@ static const struct media_entity_operations risp_entity_ops = {
 static int risp_probe_resources(struct rcar_isp *isp,
 				struct platform_device *pdev)
 {
-	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	struct resource *res;
+
+	/* For backward compatibility allow cs base to be the only reg if no
+	 * reg-names are set in DT.
+	 */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs");
+	if (!res)
+		isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	else
+		isp->csbase = devm_ioremap_resource(&pdev->dev, res);
+
 	if (IS_ERR(isp->csbase))
 		return PTR_ERR(isp->csbase);
 
-- 
2.49.0

Re: [PATCH v2 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Apr 21, 2025 at 01:12:34PM +0200, Niklas Söderlund wrote:
> Some R-Car ISP instances have in addition to the channel selector (CS)
> an ISP core (CORE )to perform operations on an image stream. The core

s/ )/) /

> function is mapped to a different memory region and have a separate

s/have/has/

> interrupt then CS, extend the bindings to allow describing this.

s/then/than/

> 
> On the same SoC different instances of the ISP IP may have, or not have,
> the CORE functionality. The CS function on all instances on the SoC are
> the same and the documentation describes the full ISP (CS + CORE) as a
> single IP block. Where instances not having the CORE function simple
> lacking the functionality to modify the image data. There dependencies

s/simple lacking/simply lack/

s/There/There are/ ? Or did you mean something else ?

> on the CS functionality while operating the CORE functionality.
> 
> In order for the ISP core to function in memory-to-memory mode it needs
> to be feed input data from a Streaming Bridge interface. This interface
> is provided thru the VSP-X device. Add an optional new property
> "renesas,vspx" to provide a phandle to describe this relationship.
> 
> While adding mandatory reg-names and interrupt-names breaks existing
> bindings the driver itself remains backward compatible and provides CS
> functionality if a single unnamed reg and interrupt property is present.
> Furthermore all existing users of the bindings are updated in following
> work to add these new mandatory properties.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v1
> - Extend the commit message to make it explicit that different ISP
>   instances on the same SoC (same compatible value) can have, or not
>   have, a CORE function block attached.
> - Update documentation for renesas,vspx property.
> - Update example to cover all new properties.
> ---
>  .../bindings/media/renesas,isp.yaml           | 63 ++++++++++++++++---
>  1 file changed, 55 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,isp.yaml b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> index c4de4555b753..927be02347e5 100644
> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> @@ -25,19 +25,55 @@ properties:
>            - renesas,r8a779h0-isp # V4M
>        - const: renesas,rcar-gen4-isp # Generic R-Car Gen4
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  reg-names:
> +    minItems: 1
> +    items:
> +      - const: cs
> +      - const: core
>  
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-names:
> +    minItems: 1
> +    items:
> +      - const: cs
> +      - const: core
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: cs
> +      - const: core
>  
>    power-domains:
>      maxItems: 1
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  reset-names:
> +    minItems: 1
> +    items:
> +      - const: cs
> +      - const: core
> +
> +  renesas,vspx:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle to the companion VSPX responsible for the Streaming Bridge
> +      functionality. The Streaming Bridge is responsible for feeding image
> +      and configuration data to the ISP when operating in memory-to-memory
> +      mode.
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -103,10 +139,14 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - reg-names
>    - interrupts
> +  - interrupt-names
>    - clocks
> +  - clock-names
>    - power-domains
>    - resets
> +  - reset-names
>    - ports
>  
>  additionalProperties: false
> @@ -119,11 +159,18 @@ examples:
>  
>      isp1: isp@fed20000 {
>              compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
> -            reg = <0xfed20000 0x10000>;
> -            interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> -            clocks = <&cpg CPG_MOD 613>;
> +            reg = <0xfed20000 0x10000>, <0xfee00000 0x10000>;
> +            reg-names = "cs", "core";
> +            interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "cs", "core";
> +            clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
> +            clock-names = "cs", "core";
>              power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> -            resets = <&cpg 613>;
> +            resets = <&cpg 613>, <&cpg 17>;
> +            reset-names = "cs", "core";
> +
> +            renesas,vspx = <&vspx1>;
>  
>              ports {
>                      #address-cells = <1>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 5/7] media: rcar-isp: Move driver to own directory

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Apr 21, 2025 at 01:12:38PM +0200, Niklas Söderlund wrote:
> Before extending the driver with functions from the R-Car ISP core that
> will span multiple files move the existing driver to a separate
> directory. While at it rename the single source file to allow future
> files to be grouped by functions.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  MAINTAINERS                                    |  2 +-
>  drivers/media/platform/renesas/Kconfig         | 18 +-----------------
>  drivers/media/platform/renesas/Makefile        |  2 +-
>  .../media/platform/renesas/rcar-isp/Kconfig    | 17 +++++++++++++++++
>  .../media/platform/renesas/rcar-isp/Makefile   |  4 ++++
>  .../renesas/{rcar-isp.c => rcar-isp/csisp.c}   |  0
>  6 files changed, 24 insertions(+), 19 deletions(-)
>  create mode 100644 drivers/media/platform/renesas/rcar-isp/Kconfig
>  create mode 100644 drivers/media/platform/renesas/rcar-isp/Makefile
>  rename drivers/media/platform/renesas/{rcar-isp.c => rcar-isp/csisp.c} (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 465569a7b264..4904d0896773 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14917,7 +14917,7 @@ F:	Documentation/devicetree/bindings/media/renesas,csi2.yaml
>  F:	Documentation/devicetree/bindings/media/renesas,isp.yaml
>  F:	Documentation/devicetree/bindings/media/renesas,vin.yaml
>  F:	drivers/media/platform/renesas/rcar-csi2.c
> -F:	drivers/media/platform/renesas/rcar-isp.c
> +F:	drivers/media/platform/renesas/rcar-isp/
>  F:	drivers/media/platform/renesas/rcar-vin/
>  
>  MEDIA DRIVERS FOR RENESAS - VSP1
> diff --git a/drivers/media/platform/renesas/Kconfig b/drivers/media/platform/renesas/Kconfig
> index c7fc718a30a5..27a54fa79083 100644
> --- a/drivers/media/platform/renesas/Kconfig
> +++ b/drivers/media/platform/renesas/Kconfig
> @@ -30,23 +30,6 @@ config VIDEO_RCAR_CSI2
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called rcar-csi2.
>  
> -config VIDEO_RCAR_ISP
> -	tristate "R-Car Image Signal Processor (ISP)"
> -	depends on V4L_PLATFORM_DRIVERS
> -	depends on VIDEO_DEV && OF
> -	depends on ARCH_RENESAS || COMPILE_TEST
> -	select MEDIA_CONTROLLER
> -	select VIDEO_V4L2_SUBDEV_API
> -	select RESET_CONTROLLER
> -	select V4L2_FWNODE
> -	help
> -	  Support for Renesas R-Car Image Signal Processor (ISP).
> -	  Enable this to support the Renesas R-Car Image Signal
> -	  Processor (ISP).
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called rcar-isp.
> -
>  config VIDEO_SH_VOU
>  	tristate "SuperH VOU video output driver"
>  	depends on V4L_PLATFORM_DRIVERS
> @@ -56,6 +39,7 @@ config VIDEO_SH_VOU
>  	help
>  	  Support for the Video Output Unit (VOU) on SuperH SoCs.
>  
> +source "drivers/media/platform/renesas/rcar-isp/Kconfig"
>  source "drivers/media/platform/renesas/rcar-vin/Kconfig"
>  source "drivers/media/platform/renesas/rzg2l-cru/Kconfig"
>  
> diff --git a/drivers/media/platform/renesas/Makefile b/drivers/media/platform/renesas/Makefile
> index 50774a20330c..1127259c09d6 100644
> --- a/drivers/media/platform/renesas/Makefile
> +++ b/drivers/media/platform/renesas/Makefile
> @@ -3,13 +3,13 @@
>  # Makefile for the Renesas capture/playback device drivers.
>  #
>  
> +obj-y += rcar-isp/
>  obj-y += rcar-vin/
>  obj-y += rzg2l-cru/
>  obj-y += vsp1/
>  
>  obj-$(CONFIG_VIDEO_RCAR_CSI2) += rcar-csi2.o
>  obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o
> -obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
>  obj-$(CONFIG_VIDEO_RENESAS_CEU) += renesas-ceu.o
>  obj-$(CONFIG_VIDEO_RENESAS_FCP) += rcar-fcp.o
>  obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o
> diff --git a/drivers/media/platform/renesas/rcar-isp/Kconfig b/drivers/media/platform/renesas/rcar-isp/Kconfig
> new file mode 100644
> index 000000000000..59e0d91862d1
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rcar-isp/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0

Add a blank line here.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +config VIDEO_RCAR_ISP
> +	tristate "R-Car Image Signal Processor (ISP)"
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV && OF
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	select MEDIA_CONTROLLER
> +	select VIDEO_V4L2_SUBDEV_API
> +	select RESET_CONTROLLER
> +	select V4L2_FWNODE
> +	help
> +	  Support for Renesas R-Car Image Signal Processor (ISP).
> +	  Enable this to support the Renesas R-Car Image Signal
> +	  Processor (ISP).
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rcar-isp.
> diff --git a/drivers/media/platform/renesas/rcar-isp/Makefile b/drivers/media/platform/renesas/rcar-isp/Makefile
> new file mode 100644
> index 000000000000..b542118c831e
> --- /dev/null
> +++ b/drivers/media/platform/renesas/rcar-isp/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +rcar-isp-objs = csisp.o
> +
> +obj-$(CONFIG_VIDEO_RCAR_ISP) += rcar-isp.o
> diff --git a/drivers/media/platform/renesas/rcar-isp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> similarity index 100%
> rename from drivers/media/platform/renesas/rcar-isp.c
> rename to drivers/media/platform/renesas/rcar-isp/csisp.c

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/7] media: rcar-isp: Rename base register variable

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Apr 21, 2025 at 01:12:39PM +0200, Niklas Söderlund wrote:
> Prepare for extending the driver to in addition to support the channel

s/support/supporting/

> selector (CS) also support the core ISP. The two different functions
> have different base addresses so the driver needs to distinguish between
> them.
> 
> Prepare for this by marking existing base address variable and
> read/write functions to make it clear it operates on the CS portion of

s/it operates/they operate/

> the driver. There is no functional change.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  .../media/platform/renesas/rcar-isp/csisp.c   | 46 +++++++++----------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index 4bc89d4757fa..f36d43c2e0a2 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -159,7 +159,7 @@ enum rcar_isp_pads {
>  
>  struct rcar_isp {
>  	struct device *dev;
> -	void __iomem *base;
> +	void __iomem *csbase;
>  	struct reset_control *rstc;
>  
>  	enum rcar_isp_input csi_input;
> @@ -184,14 +184,14 @@ static inline struct rcar_isp *notifier_to_isp(struct v4l2_async_notifier *n)
>  	return container_of(n, struct rcar_isp, notifier);
>  }
>  
> -static void risp_write(struct rcar_isp *isp, u32 offset, u32 value)
> +static void risp_write_cs(struct rcar_isp *isp, u32 offset, u32 value)

Not sure what the other write function will be called, but I would have
called this risp_cs_write(). Up to you.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  {
> -	iowrite32(value, isp->base + offset);
> +	iowrite32(value, isp->csbase + offset);
>  }
>  
> -static u32 risp_read(struct rcar_isp *isp, u32 offset)
> +static u32 risp_read_cs(struct rcar_isp *isp, u32 offset)
>  {
> -	return ioread32(isp->base + offset);
> +	return ioread32(isp->csbase + offset);
>  }
>  
>  static int risp_power_on(struct rcar_isp *isp)
> @@ -245,31 +245,31 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
>  	if (isp->csi_input == RISP_CSI_INPUT1)
>  		sel_csi = ISPINPUTSEL0_SEL_CSI0;
>  
> -	risp_write(isp, ISPINPUTSEL0_REG,
> -		   risp_read(isp, ISPINPUTSEL0_REG) | sel_csi);
> +	risp_write_cs(isp, ISPINPUTSEL0_REG,
> +		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
>  
>  	/* Configure Channel Selector. */
>  	for (vc = 0; vc < 4; vc++) {
>  		u8 ch = vc + 4;
>  		u8 dt = format->datatype;
>  
> -		risp_write(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> -		risp_write(isp, ISPCS_DT_CODE03_CH_REG(ch),
> -			   ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> -			   ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> -			   ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> -			   ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> +			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> +			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> +			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> +			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
>  	}
>  
>  	/* Setup processing method. */
> -	risp_write(isp, ISPPROCMODE_DT_REG(format->datatype),
> -		   ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> -		   ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> -		   ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> -		   ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> +	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> +		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> +		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
>  
>  	/* Start ISP. */
> -	risp_write(isp, ISPSTART_REG, ISPSTART_START);
> +	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
>  
>  	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
>  					 BIT_ULL(0));
> @@ -284,7 +284,7 @@ static void risp_stop(struct rcar_isp *isp)
>  	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
>  
>  	/* Stop ISP. */
> -	risp_write(isp, ISPSTART_REG, ISPSTART_STOP);
> +	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
>  
>  	risp_power_off(isp);
>  }
> @@ -465,9 +465,9 @@ static const struct media_entity_operations risp_entity_ops = {
>  static int risp_probe_resources(struct rcar_isp *isp,
>  				struct platform_device *pdev)
>  {
> -	isp->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> -	if (IS_ERR(isp->base))
> -		return PTR_ERR(isp->base);
> +	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +	if (IS_ERR(isp->csbase))
> +		return PTR_ERR(isp->csbase);
>  
>  	isp->rstc = devm_reset_control_get(&pdev->dev, NULL);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 6/7] media: rcar-isp: Rename base register variable

[Laurent Pinchart]

On Tue, Apr 22, 2025 at 01:47:35AM +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Mon, Apr 21, 2025 at 01:12:39PM +0200, Niklas Söderlund wrote:
> > Prepare for extending the driver to in addition to support the channel
> 
> s/support/supporting/
> 
> > selector (CS) also support the core ISP. The two different functions
> > have different base addresses so the driver needs to distinguish between
> > them.
> > 
> > Prepare for this by marking existing base address variable and
> > read/write functions to make it clear it operates on the CS portion of
> 
> s/it operates/they operate/
> 
> > the driver. There is no functional change.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  .../media/platform/renesas/rcar-isp/csisp.c   | 46 +++++++++----------
> >  1 file changed, 23 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> > index 4bc89d4757fa..f36d43c2e0a2 100644
> > --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> > +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> > @@ -159,7 +159,7 @@ enum rcar_isp_pads {
> >  
> >  struct rcar_isp {
> >  	struct device *dev;
> > -	void __iomem *base;
> > +	void __iomem *csbase;
> >  	struct reset_control *rstc;
> >  
> >  	enum rcar_isp_input csi_input;
> > @@ -184,14 +184,14 @@ static inline struct rcar_isp *notifier_to_isp(struct v4l2_async_notifier *n)
> >  	return container_of(n, struct rcar_isp, notifier);
> >  }
> >  
> > -static void risp_write(struct rcar_isp *isp, u32 offset, u32 value)
> > +static void risp_write_cs(struct rcar_isp *isp, u32 offset, u32 value)
> 
> Not sure what the other write function will be called, but I would have
> called this risp_cs_write(). Up to you.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I meant

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> >  {
> > -	iowrite32(value, isp->base + offset);
> > +	iowrite32(value, isp->csbase + offset);
> >  }
> >  
> > -static u32 risp_read(struct rcar_isp *isp, u32 offset)
> > +static u32 risp_read_cs(struct rcar_isp *isp, u32 offset)
> >  {
> > -	return ioread32(isp->base + offset);
> > +	return ioread32(isp->csbase + offset);
> >  }
> >  
> >  static int risp_power_on(struct rcar_isp *isp)
> > @@ -245,31 +245,31 @@ static int risp_start(struct rcar_isp *isp, struct v4l2_subdev_state *state)
> >  	if (isp->csi_input == RISP_CSI_INPUT1)
> >  		sel_csi = ISPINPUTSEL0_SEL_CSI0;
> >  
> > -	risp_write(isp, ISPINPUTSEL0_REG,
> > -		   risp_read(isp, ISPINPUTSEL0_REG) | sel_csi);
> > +	risp_write_cs(isp, ISPINPUTSEL0_REG,
> > +		      risp_read_cs(isp, ISPINPUTSEL0_REG) | sel_csi);
> >  
> >  	/* Configure Channel Selector. */
> >  	for (vc = 0; vc < 4; vc++) {
> >  		u8 ch = vc + 4;
> >  		u8 dt = format->datatype;
> >  
> > -		risp_write(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> > -		risp_write(isp, ISPCS_DT_CODE03_CH_REG(ch),
> > -			   ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> > -			   ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> > -			   ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> > -			   ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> > +		risp_write_cs(isp, ISPCS_FILTER_ID_CH_REG(ch), BIT(vc));
> > +		risp_write_cs(isp, ISPCS_DT_CODE03_CH_REG(ch),
> > +			      ISPCS_DT_CODE03_EN3 | ISPCS_DT_CODE03_DT3(dt) |
> > +			      ISPCS_DT_CODE03_EN2 | ISPCS_DT_CODE03_DT2(dt) |
> > +			      ISPCS_DT_CODE03_EN1 | ISPCS_DT_CODE03_DT1(dt) |
> > +			      ISPCS_DT_CODE03_EN0 | ISPCS_DT_CODE03_DT0(dt));
> >  	}
> >  
> >  	/* Setup processing method. */
> > -	risp_write(isp, ISPPROCMODE_DT_REG(format->datatype),
> > -		   ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> > -		   ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> > -		   ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> > -		   ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> > +	risp_write_cs(isp, ISPPROCMODE_DT_REG(format->datatype),
> > +		      ISPPROCMODE_DT_PROC_MODE_VC3(format->procmode) |
> > +		      ISPPROCMODE_DT_PROC_MODE_VC2(format->procmode) |
> > +		      ISPPROCMODE_DT_PROC_MODE_VC1(format->procmode) |
> > +		      ISPPROCMODE_DT_PROC_MODE_VC0(format->procmode));
> >  
> >  	/* Start ISP. */
> > -	risp_write(isp, ISPSTART_REG, ISPSTART_START);
> > +	risp_write_cs(isp, ISPSTART_REG, ISPSTART_START);
> >  
> >  	ret = v4l2_subdev_enable_streams(isp->remote, isp->remote_pad,
> >  					 BIT_ULL(0));
> > @@ -284,7 +284,7 @@ static void risp_stop(struct rcar_isp *isp)
> >  	v4l2_subdev_disable_streams(isp->remote, isp->remote_pad, BIT_ULL(0));
> >  
> >  	/* Stop ISP. */
> > -	risp_write(isp, ISPSTART_REG, ISPSTART_STOP);
> > +	risp_write_cs(isp, ISPSTART_REG, ISPSTART_STOP);
> >  
> >  	risp_power_off(isp);
> >  }
> > @@ -465,9 +465,9 @@ static const struct media_entity_operations risp_entity_ops = {
> >  static int risp_probe_resources(struct rcar_isp *isp,
> >  				struct platform_device *pdev)
> >  {
> > -	isp->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> > -	if (IS_ERR(isp->base))
> > -		return PTR_ERR(isp->base);
> > +	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> > +	if (IS_ERR(isp->csbase))
> > +		return PTR_ERR(isp->csbase);
> >  
> >  	isp->rstc = devm_reset_control_get(&pdev->dev, NULL);
> >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 7/7] media: rcar-isp: Parse named cs memory region

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Apr 21, 2025 at 01:12:40PM +0200, Niklas Söderlund wrote:
> Extend the device tree parsing to optionally parse the cs memory region
> by name. The change is backward compatible with the device tree model
> where a single unnamed region describing only the ISP channel select

s/decribing/describes/

> function.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/rcar-isp/csisp.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-isp/csisp.c b/drivers/media/platform/renesas/rcar-isp/csisp.c
> index f36d43c2e0a2..0b6fa62467e4 100644
> --- a/drivers/media/platform/renesas/rcar-isp/csisp.c
> +++ b/drivers/media/platform/renesas/rcar-isp/csisp.c
> @@ -465,7 +465,17 @@ static const struct media_entity_operations risp_entity_ops = {
>  static int risp_probe_resources(struct rcar_isp *isp,
>  				struct platform_device *pdev)
>  {
> -	isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +	struct resource *res;
> +
> +	/* For backward compatibility allow cs base to be the only reg if no

	/*
	 * For backward compatibility allow cs base to be the only reg if no

> +	 * reg-names are set in DT.
> +	 */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs");
> +	if (!res)
> +		isp->csbase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);

You can call devm_platform_ioremap_resource().

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +	else
> +		isp->csbase = devm_ioremap_resource(&pdev->dev, res);
> +
>  	if (IS_ERR(isp->csbase))
>  		return PTR_ERR(isp->csbase);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

[Geert Uytterhoeven]

Hi Niklas,

On Mon, 21 Apr 2025 at 13:12, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Some R-Car ISP instances have in addition to the channel selector (CS)
> an ISP core (CORE )to perform operations on an image stream. The core
> function is mapped to a different memory region and have a separate
> interrupt then CS, extend the bindings to allow describing this.
>
> On the same SoC different instances of the ISP IP may have, or not have,
> the CORE functionality. The CS function on all instances on the SoC are
> the same and the documentation describes the full ISP (CS + CORE) as a
> single IP block. Where instances not having the CORE function simple
> lacking the functionality to modify the image data. There dependencies
> on the CS functionality while operating the CORE functionality.
>
> In order for the ISP core to function in memory-to-memory mode it needs
> to be feed input data from a Streaming Bridge interface. This interface
> is provided thru the VSP-X device. Add an optional new property
> "renesas,vspx" to provide a phandle to describe this relationship.
>
> While adding mandatory reg-names and interrupt-names breaks existing
> bindings the driver itself remains backward compatible and provides CS
> functionality if a single unnamed reg and interrupt property is present.
> Furthermore all existing users of the bindings are updated in following
> work to add these new mandatory properties.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - Extend the commit message to make it explicit that different ISP
>   instances on the same SoC (same compatible value) can have, or not
>   have, a CORE function block attached.
> - Update documentation for renesas,vspx property.
> - Update example to cover all new properties.

Thanks for the update!

> --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml

> @@ -119,11 +159,18 @@ examples:
>
>      isp1: isp@fed20000 {
>              compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
> -            reg = <0xfed20000 0x10000>;
> -            interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> -            clocks = <&cpg CPG_MOD 613>;
> +            reg = <0xfed20000 0x10000>, <0xfee00000 0x10000>;

IThe second size should be 0x100000.

> +            reg-names = "cs", "core";
> +            interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "cs", "core";
> +            clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
> +            clock-names = "cs", "core";
>              power-domains = <&sysc R8A779A0_PD_A3ISP01>;

With the above and the wording issues pointed out by Laurent fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

[Laurent Pinchart]

On Wed, Apr 23, 2025 at 01:24:25PM +0200, Geert Uytterhoeven wrote:
> On Mon, 21 Apr 2025 at 13:12, Niklas Söderlund wrote:
> > Some R-Car ISP instances have in addition to the channel selector (CS)
> > an ISP core (CORE )to perform operations on an image stream. The core
> > function is mapped to a different memory region and have a separate
> > interrupt then CS, extend the bindings to allow describing this.
> >
> > On the same SoC different instances of the ISP IP may have, or not have,
> > the CORE functionality. The CS function on all instances on the SoC are
> > the same and the documentation describes the full ISP (CS + CORE) as a
> > single IP block. Where instances not having the CORE function simple
> > lacking the functionality to modify the image data. There dependencies
> > on the CS functionality while operating the CORE functionality.
> >
> > In order for the ISP core to function in memory-to-memory mode it needs
> > to be feed input data from a Streaming Bridge interface. This interface
> > is provided thru the VSP-X device. Add an optional new property
> > "renesas,vspx" to provide a phandle to describe this relationship.
> >
> > While adding mandatory reg-names and interrupt-names breaks existing
> > bindings the driver itself remains backward compatible and provides CS
> > functionality if a single unnamed reg and interrupt property is present.
> > Furthermore all existing users of the bindings are updated in following
> > work to add these new mandatory properties.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > * Changes since v1
> > - Extend the commit message to make it explicit that different ISP
> >   instances on the same SoC (same compatible value) can have, or not
> >   have, a CORE function block attached.
> > - Update documentation for renesas,vspx property.
> > - Update example to cover all new properties.
> 
> Thanks for the update!
> 
> > --- a/Documentation/devicetree/bindings/media/renesas,isp.yaml
> > +++ b/Documentation/devicetree/bindings/media/renesas,isp.yaml
> 
> > @@ -119,11 +159,18 @@ examples:
> >
> >      isp1: isp@fed20000 {
> >              compatible = "renesas,r8a779a0-isp", "renesas,rcar-gen4-isp";
> > -            reg = <0xfed20000 0x10000>;
> > -            interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> > -            clocks = <&cpg CPG_MOD 613>;
> > +            reg = <0xfed20000 0x10000>, <0xfee00000 0x10000>;
> 
> IThe second size should be 0x100000.
> 
> > +            reg-names = "cs", "core";
> > +            interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "cs", "core";
> > +            clocks = <&cpg CPG_MOD 613>, <&cpg CPG_MOD 17>;
> > +            clock-names = "cs", "core";
> >              power-domains = <&sysc R8A779A0_PD_A3ISP01>;
> 
> With the above and the wording issues pointed out by Laurent fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Once we get a review from the DT bindings maintainers for this patch,
I'll take the next version addressing the small issues in my tree for
1/7 and 5/7 to 7/7. I'll let Geert merge 2/7 to 4/7.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/7] dt-bindings: media: renesas,isp: Add ISP core function block

[Rob Herring (Arm)]

On Mon, 21 Apr 2025 13:12:34 +0200, Niklas Söderlund wrote:
> Some R-Car ISP instances have in addition to the channel selector (CS)
> an ISP core (CORE )to perform operations on an image stream. The core
> function is mapped to a different memory region and have a separate
> interrupt then CS, extend the bindings to allow describing this.
> 
> On the same SoC different instances of the ISP IP may have, or not have,
> the CORE functionality. The CS function on all instances on the SoC are
> the same and the documentation describes the full ISP (CS + CORE) as a
> single IP block. Where instances not having the CORE function simple
> lacking the functionality to modify the image data. There dependencies
> on the CS functionality while operating the CORE functionality.
> 
> In order for the ISP core to function in memory-to-memory mode it needs
> to be feed input data from a Streaming Bridge interface. This interface
> is provided thru the VSP-X device. Add an optional new property
> "renesas,vspx" to provide a phandle to describe this relationship.
> 
> While adding mandatory reg-names and interrupt-names breaks existing
> bindings the driver itself remains backward compatible and provides CS
> functionality if a single unnamed reg and interrupt property is present.
> Furthermore all existing users of the bindings are updated in following
> work to add these new mandatory properties.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v1
> - Extend the commit message to make it explicit that different ISP
>   instances on the same SoC (same compatible value) can have, or not
>   have, a CORE function block attached.
> - Update documentation for renesas,vspx property.
> - Update example to cover all new properties.
> ---
>  .../bindings/media/renesas,isp.yaml           | 63 ++++++++++++++++---
>  1 file changed, 55 insertions(+), 8 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>