Re: [PATCH v5 2/7] drm/ingenic: Add support for JZ4780 and HDMI output

From: "H. Nikolaus Schaller" <hns@goldelico.com>
To: Paul Cercueil <paul@crapouillou.net>
Cc: 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>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Robert Foss <robert.foss@linaro.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jernej Skrabec <jernej.skrabec@gmail.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>, Paul Boddie <paul@boddie.org.uk>,
	OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS 
	<devicetree@vger.kernel.org>,
	linux-mips <linux-mips@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Discussions about the Letux Kernel 
	<letux-kernel@openphoenux.org>, Jonas Karlman <jonas@kwiboo.se>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v5 2/7] drm/ingenic: Add support for JZ4780 and HDMI output
Date: Mon, 8 Nov 2021 19:33:48 +0100	[thread overview]
Message-ID: <ACEFD0BB-1FCF-4EEB-A40F-1F2543A05BF4@goldelico.com> (raw)
In-Reply-To: <RIL92R.MLAZ6CTO865E1@crapouillou.net>

Hi Paul,

> Am 08.11.2021 um 18:49 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
>>> Variant 4: the variant #2 without the changes to the DTSI files.
>> Hm. If there is no cache and we can safely remove tight boundary checking (by JZ_REG_LCD_SIZE1) for jz4725/40/70 (by not fixing DTSI) why do we still need the max_register calculation from DTSI specifically for jz4780 and at all?
> 
> It's better to have the .max_register actually set to the proper value. Then reading the registers from debugfs (/sys/kernel/debug/regmap/) will print the actual list of registers without bogus values. If .max_register is set too high, it will end up reading outside the registers area.

Ok, that is a good reason to convince me.

> On Ingenic SoCs such reads just return 0, but on some other SoCs it can lock up the system.

Yes, I know some of these...

> So the best way forward is to have .max_register computed from the register area's size, and fix the DTSI with the proper sizes. Since your JZ4780 code needs to update .max_register anyway it's a good moment to add this patch, and the DTSI files can be fixed later (by me or whoever is up to the task).

Well, it would already be part of my Variant #2 (untested). So I could simply split it up further and you can test the pure dtsi changes and apply them later or modify if that makes problems. Saves you a little work. BTW: the jz4740 seems to have even less registers (last register seems to be LCDCMD1 @ 0x1305005C).

> 
> Fixing the DTS is not a problem in any way, btw. We just need to ensure that the drivers still work with old DTB files, which will be the case here.

Yes, that is right since the new values are smaller than the originals.

Ok, then let's do it that way.

BR and thanks,
Nikolaus