Re: [PATCH v5 5/7] MIPS: DTS: jz4780: Account for Synopsys HDMI driver and LCD controllers

From: Paul Boddie <paul@boddie.org.uk>
To: Paul Cercueil <paul@crapouillou.net>
Cc: "H. Nikolaus Schaller" <hns@goldelico.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Kees Cook <keescook@chromium.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Robert Foss <robert.foss@linaro.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	Harry Wentland <harry.wentland@amd.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Maxime Ripard <maxime@cerno.tech>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	devicetree@vger.kernel.org, linux-mips@vger.kernel.org,
	linux-kernel@vger.kernel.org, letux-kernel@openphoenux.org,
	Jon as Karlman <jonas@kwiboo.se>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v5 5/7] MIPS: DTS: jz4780: Account for Synopsys HDMI driver and LCD controllers
Date: Tue, 05 Oct 2021 23:44:12 +0200	[thread overview]
Message-ID: <3514743.EH6qe8WxYI@jason> (raw)
In-Reply-To: <O7VI0R.CRIG8R7O0OOI3@crapouillou.net>

On Tuesday, 5 October 2021 22:50:12 CEST Paul Cercueil wrote:
> Hi Nikolaus & Paul,
> 
> Le mar., oct. 5 2021 at 14:29:17 +0200, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> > 
> > diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > index 9e34f433b9b5..c3c18a59c377 100644
> > --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> > @@ -424,6 +424,51 @@ i2c4: i2c@10054000 {
> > 
> >  		status = "disabled";
> >  	
> >  	};
> > 
> > +	hdmi: hdmi@10180000 {
> > +		compatible = "ingenic,jz4780-dw-hdmi";
> > +		reg = <0x10180000 0x8000>;
> > +		reg-io-width = <4>;
> > +
> > +		clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>;
> > +		clock-names = "iahb", "isfr";
> > +
> > +		assigned-clocks = <&cgu JZ4780_CLK_HDMI>;
> > +		assigned-clock-rates = <27000000>;
> 
> Any reason why this is set to 27 MHz? Is it even required? Because with
> the current ci20.dts, it won't be clocked at anything but 48 MHz.

EXCLK will be 48MHz, but the aim is to set the HDMI peripheral clock to 27MHz, 
which is supposedly required. I vaguely recall a conversation about whether we 
were doing this right, but I don't recall any conclusion.

> > +
> > +		interrupt-parent = <&intc>;
> > +		interrupts = <3>;
> > +
> > +		/* ddc-i2c-bus = <&i2c4>; */
> > +
> > +		status = "disabled";
> > +	};
> > +
> > +	lcdc0: lcdc0@13050000 {
> > +		compatible = "ingenic,jz4780-lcd";
> > +		reg = <0x13050000 0x1800>;
> > +
> > +		clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>;
> > +		clock-names = "lcd", "lcd_pclk";
> > +
> > +		interrupt-parent = <&intc>;
> > +		interrupts = <31>;
> > +
> > +		status = "disabled";
> 
> I think you can keep lcdc0 enabled by default (not lcdc1 though), since
> it is highly likely that you'd want that.

As far as I know, the clock gating for the LCD controllers acts like a series 
circuit, meaning that they both need to be enabled. Some testing seemed to 
confirm this. Indeed, I seem to remember only enabling one clock and not 
getting any output until I figured this weird arrangement out.

Paul