Devicetree
 help / color / mirror / Atom feed
From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
To: Rob Herring <robh@kernel.org>
Cc: tomm.merciai@gmail.com, geert@linux-m68k.org,
	laurent.pinchart@ideasonboard.com,
	linux-renesas-soc@vger.kernel.org, biju.das.jz@bp.renesas.com,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 02/13] dt-bindings: display: renesas,rzg2l-du: Add support for RZ/G3E SoC
Date: Thu, 11 Jun 2026 14:42:49 +0200	[thread overview]
Message-ID: <aiqtSWTTa4ZIThrp@tom-desktop> (raw)
In-Reply-To: <20260513222725.GA2069022-robh@kernel.org>

Hi Rob,
Thanks for your review.

On Wed, May 13, 2026 at 05:27:25PM -0500, Rob Herring wrote:
> On Thu, May 07, 2026 at 11:21:30AM +0200, Tommaso Merciai wrote:
> > The RZ/G3E SoC integrates two LCD controllers (LCDC0 and LCDC1), each
> > containing a FCPVD, VSPD, and Display Unit (DU).
> > 
> >  - LCDC0 supports DSI and LVDS (single or dual-channel) outputs.
> >  - LCDC1 supports DSI, LVDS (single-channel), and RGB outputs.
> > 
> > Add compatible string 'renesas,r9a09g047-du' and extend the binding to
> > support two DU instances: add reg-names ('du0'/'du1'), extend reg,
> > interrupts, and resets to maxItems: 2, and extend clocks/clock-names to
> > six entries (aclk/pclk/vclk per instance, minItems: 3).
> 
> Don't write what the diff has. I can read the diff for that.

Ouch, thanks.

> 
> > 
> > Drop the "Each port shall have a single endpoint." constraint since
> > RZ/G3E ports expose multiple endpoints.
> > 
> > Add a RZ/G3E-specific allOf rule mapping two DU instances to two ports:
> > 
> >  - port@0 (DU0): endpoint@0 DSI, endpoint@2 LVDS ch0, endpoint@3 LVDS ch1
> >  - port@1 (DU1): endpoint@0 DSI, endpoint@1 RGB (DPAD), endpoint@3 LVDS ch1
> > 
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> > v6->v7:
> >  - Rebased on top of [1]
> >    [1] https://lore.kernel.org/all/20260429170012.366537-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> >  - Use single DRM device aggregating both DU instances (1 DU dt node),
> >    modelling single port for each DU0, DU1 and multiple endpoints for
> >    outputs.
> 
> That seems like the completely wrong thing to do and you've given no 
> reason why you think it is the right choice.


We had a discussion with Laurent at [1] about this topic.
In particular:

DSI ip can select at runtime input data path or DU0 or DU1
via DSI_LINK_GPO0R VICH register. This can be done by managing the
2 DUs as single DRM device aggregating both DU instances that will spawn
2 crtcs. In this way at runtime we can select the output for DSI ip
via the following commands:

modetest -M rzg2l-du -s 58@55:800x600-56.25@XR24 (DU0 -> DSI)
modetest -M rzg2l-du -s 58@56:800x600-56.25@XR24 (DU1 -> DSI)

This can be done using option [B] (single drm device that spawn 2 crtc).

Using option [A] we will have 2 drm devices 1 for DU0 and 1 for DU1
that each will spawn a single CRTC and the above feature will be not
achievable.

In the end we need a way to have single DRM device that spawn 2 CRTCs.

A) Two device tree nodes rapresenting DU0 and DU1 design [v6]:

du0: display@16460000 {
	compatible = "renesas,r9a09g047-du";
	reg = <0 0x16460000 0 0x10000>;
	interrupts = <GIC_SPI 882 IRQ_TYPE_LEVEL_HIGH>;
	clocks = <&cpg CPG_MOD 0xed>,
			<&cpg CPG_MOD 0xee>,
			<&cpg CPG_MOD 0xef>;
	clock-names = "aclk", "pclk", "vclk";
	power-domains = <&cpg>;
	resets = <&cpg 0xdc>;
	renesas,vsps = <&vspd0 0>;
	status = "disabled";

	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		port@0 {
			reg = <0>;
			du0_out_dsi: endpoint {
			};
		};

		port@2 {
			reg = <2>;
			du0_out_lvds0: endpoint {
			};
		};

		port@3 {
			reg = <3>;
			du0_out_lvds1: endpoint {
			};
		};
	};
};

du1: display@16490000 {
	compatible = "renesas,r9a09g047-du";
	reg = <0 0x16490000 0 0x10000>;
	interrupts = <GIC_SPI 922 IRQ_TYPE_LEVEL_HIGH>;
	clocks = <&cpg CPG_MOD 0x1a8>,
			<&cpg CPG_MOD 0x1a9>,
			<&cpg CPG_MOD 0x1aa>;
	clock-names = "aclk", "pclk", "vclk";
	power-domains = <&cpg>;
	resets = <&cpg 0x11e>;
	renesas,vsps = <&vspd1 0>;
	status = "disabled";

	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		port@0 {
			reg = <0>;
			du1_out_dsi: endpoint {
			};
		};

		port@1 {
			reg = <1>;
			du1_out_rgb: endpoint {
			};
		};

		port@3 {
			reg = <3>;
			du1_out_lvds1: endpoint {
			};
		};
	};
};

---

B) Single device tree node design aggregating both DU instances [v7]:

du: display@16460000 {
	compatible = "renesas,r9a09g047-du";
	reg = <0 0x16460000 0 0x10000>,
		<0 0x16490000 0 0x10000>;
	reg-names = "du0", "du1";
	interrupts = <GIC_SPI 882 IRQ_TYPE_LEVEL_HIGH>,
			<GIC_SPI 922 IRQ_TYPE_LEVEL_HIGH>;
	clocks = <&cpg CPG_MOD 0xed>,
			<&cpg CPG_MOD 0xee>,
			<&cpg CPG_MOD 0xef>,
			<&cpg CPG_MOD 0x1a8>,
			<&cpg CPG_MOD 0x1a9>,
			<&cpg CPG_MOD 0x1aa>;
	clock-names = "aclk", "pclk", "vclk",
			"aclk1", "pclk1", "vclk1";
	power-domains = <&cpg>;
	resets = <&cpg 0xdc>, <&cpg 0x11e>;
	reset-names = "resetn", "resetn1";
	renesas,vsps = <&vspd0 0>, <&vspd1 0>;
	status = "disabled";

	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		port@0 {
			#address-cells = <1>;
			#size-cells = <0>;

			reg = <0>;

			du0_out_dsi: endpoint@0 {
				reg = <0>;
			};

			du0_out_lvds0: endpoint@2 {
				reg = <2>;
			};

			du0_out_lvds1: endpoint@3 {
				reg = <3>;
			};
		};

		port@1 {
			#address-cells = <1>;
			#size-cells = <0>;

			reg = <1>;

			du1_out_dsi: endpoint@0 {
				reg = <0>;
			};

			du1_out_rgb: endpoint@1 {
				reg = <1>;
			};

			du1_out_lvds1: endpoint@3 {
				reg = <3>;
			};
		};
	};
};

---

Please Biju, Laurent correct me if I'm missing something.

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/8f814f22ff62dcde6153260e2c8c29a5415c9a89.1775636898.git.tommaso.merciai.xr@bp.renesas.com/
[v6] https://patchwork.kernel.org/project/linux-renesas-soc/patch/8f814f22ff62dcde6153260e2c8c29a5415c9a89.1775636898.git.tommaso.merciai.xr@bp.renesas.com/
[v7] https://patchwork.kernel.org/project/linux-renesas-soc/patch/ff8e401a0667970a42a55420dcb071e34730a923.1778141145.git.tommaso.merciai.xr@bp.renesas.com/

Kind Regards,
Tommaso

> 
> Rob

  reply	other threads:[~2026-06-11 12:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-07  9:21 [PATCH v7 00/13] drm: renesas: rz-du: Add RZ/G3E support Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 01/13] dt-bindings: display: renesas,rzg2l-du: Refuse port@1 for RZ/G2UL Tommaso Merciai
2026-05-13 22:23   ` Rob Herring (Arm)
2026-05-07  9:21 ` [PATCH v7 02/13] dt-bindings: display: renesas,rzg2l-du: Add support for RZ/G3E SoC Tommaso Merciai
2026-05-13 22:27   ` Rob Herring
2026-06-11 12:42     ` Tommaso Merciai [this message]
2026-05-07  9:21 ` [PATCH v7 03/13] drm: renesas: rz-du: Move mmio from rzg2l_du_device to rzg2l_du_crtc Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 04/13] drm: renesas: rz-du: crtc: Introduce rzg2l_du_crtc_clk_get() Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 05/13] reset: Add devm_reset_control_get_optional_shared_by_index() Tommaso Merciai
2026-05-07  9:58   ` Philipp Zabel
2026-05-07  9:21 ` [PATCH v7 06/13] drm: renesas: rz-du: crtc: Parameterize rzg2l_du_crtc_create() with hw/sw index Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 07/13] drm: renesas: rz-du: kms: Enable multi CRTC creation Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 08/13] drm: renesas: rz-du: Add endpoint-based output routing Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 09/13] drm: renesas: rz-du: Narrow active channels to DT-connected outputs Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 10/13] drm: renesas: rz-du: Skip disabled endpoint nodes in encoder init Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 11/13] drm: renesas: rz-du: Add multi-VSP instance support in plane selection Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 12/13] drm: renesas: rz-du: Add RZ/G3E support Tommaso Merciai
2026-05-07  9:21 ` [PATCH v7 13/13] drm: renesas: rz-du: Update output routing comment to reflect all supported outputs Tommaso Merciai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aiqtSWTTa4ZIThrp@tom-desktop \
    --to=tommaso.merciai.xr@bp.renesas.com \
    --cc=airlied@gmail.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert+renesas@glider.be \
    --cc=geert@linux-m68k.org \
    --cc=krzk+dt@kernel.org \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=magnus.damm@gmail.com \
    --cc=mripard@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tomm.merciai@gmail.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox