mbox series

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

Message ID 20250421111240.789510-1-niklas.soderlund+renesas@ragnatech.se
Headers show
Series rcar-isp: Prepare for ISP core support | expand

Message

Niklas Söderlund April 21, 2025, 11:12 a.m. UTC
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%)

Comments

Laurent Pinchart April 21, 2025, 10:36 p.m. UTC | #1
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>;
Laurent Pinchart April 21, 2025, 10:55 p.m. UTC | #2
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);
>