linux-kernel - Re: [PATCH] drm/sun4i: Workaround TCON TOP conflict between DE0 and DE1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zy4SKCBwce3q0yj5@titan>
Date: Sat, 9 Nov 2024 00:29:12 +1100
From: John Watts <contact@...kia.org>
To: Andre Przywara <andre.przywara@....com>
Cc: Maxime Ripard <mripard@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
	Samuel Holland <samuel@...lland.org>,
	dri-devel@...ts.freedesktop.org,
	linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/sun4i: Workaround TCON TOP conflict between DE0 and
 DE1

On Fri, Nov 08, 2024 at 11:53:57AM +0000, Andre Przywara wrote:
> Hi John,

Hi Andre!

> Can you say *why* this patch is needed? Is there something broken that
> needs fixing? Where does this show and why wasn't this a problem before?

Oops, that's a good point. There is currently a bug where the LCD output will
be tinted. I have full context here which I should have probably linked in the
patch description:

https://lore.kernel.org/linux-sunxi/Zn8GVkpwXwhaUFno@titan/T/#u

> To be honest, given the isolation on this patch, I'd rather wait for this
> full fledged solution, especially if there is no pressing need (see above).

I'd be interested to hear if that's still the wanted solution given the link
above. This currently blocks many people from having working LCD output.

Doing it the proper way might be overkill for now unless someone deliberately
tries to run two DEs to the same output. I haven't seen this use case.

Allwinner kernel fork initially sets them up to values like these then makes
sure both DEs can't be set to the same TCON.

> > -	writel(0, regs + TCON_TOP_PORT_SEL_REG);
> > +	writel(0x20, regs + TCON_TOP_PORT_SEL_REG);
> 
> Sorry, but that looks weird:
> First, please explain the 0x20. Is it bit 5? If yes, what does that bit
> mean? The commit message suggests you know that?
> 
> And second: the comment above clearly states that those two writes just
> *clear* some registers, to have some sane base line. So please adjust this
> comment, and copy in some of the rationale from the commit message.
> Explaining things in the commit message is good (so thanks for that!), but
> having at least some terse technical explanations near the code, in a
> comment, is better.

Bit 5 is value 3 of TCON_TOP_PORT_DE1_MSK. The R40 datasheet explains the
values of both masks as follows:

00: TCON_LCD0
01: TCON_LCD1
10: TCON_TV0
11: TCON_TV1

So this sets DE1's input to DE0.

> 
> Cheers,
> Andre

Thanks,
John Watts