Message-ID: <f0d5b314-cfcc-4856-8d6e-09e437c075ec@linumiz.com>
Date: Tue, 12 Nov 2024 22:43:44 +0530
From: Parthiban <parthiban@...umiz.com>
To: John Watts <contact@...kia.org>
Cc: parthiban@...umiz.com, Andre Przywara <andre.przywara@....com>,
 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 11/12/24 5:27 PM, John Watts wrote:
> Hey everyone,
> 
> I'm not sure exactly where to add this but I discussed some of this with
> Parthiban on #linux-sunxi a few days ago, so I want to write it down
> before I work on the next version of the patch.
> 
> I had assumed for some reason in my mind that DE0 and DE1 here referred
> to mixers, but they actually refer to chips that have multiple DEs. It
> looks like at least with the A133 it has two DEs instead of two mixers.
> 
> This can be found by looking at the Allwinner BSP: SUN50IW10 requires
> CONFIG_INDEPENDENT_DE and has a device tree with an extra reg and clock:
> 
> <0x0 0x06800000 0x0 0x3fffff>,/*de1*/
> <&clk_dpss_top1>

Independent DE is unique to A133/A100 AFAIK.

> 
> However the tcon-top code seems to conflate mixers and DE in the
> mainline code and the Allwinner code. So ... It seems like 'DE0' and
> 'DE1' really do mean mixers in this case. It's probably best to note

#define TCON_TOP_PORT_DE0_MSK                   GENMASK(1, 0)
#define TCON_TOP_PORT_DE1_MSK                   GENMASK(5, 4)

references towards DE0 and DE1 is for DE itself, not the mixers in the
current implementation.

Handling for mixer0 <-> lcd1 and mixer1 <-> lcd0 also needs to set
DE2TCON_MUX in de clock, which is missing now.

> that down.
> 
> I thought a bit more about how to solve this properly- setting two
> mixers to the same output is something people probably won't do in
> practice, so the only way you could really arrive at this bugged state
> is by setting it as the default state. This patch may be the correct
> solution after all.

sun8i_tcon_top_set_hdmi_src for R40 already sets these values via quirks.
i.e controlling the port muxing. Also D1 quirks is same as R40. So the
original changes to make the DE1 point to TVx can also done in this quirk
without hardcoded value?

Thanks,
Parthiban

> 
> John Watts
> 
> On Sat, Nov 09, 2024 at 01:15:16AM +1100, John Watts wrote:
>> On Fri, Nov 08, 2024 at 07:36:16PM +0530, Parthiban wrote:
>>> To add, 0x20 will be DE0 <--> LCD0 and DE1 <--> TV0. Below note (copied from
>>> R40) states the priority of the DE selection, which fails to work? Not sure,
>>> may be disabling CORE1_SCLK_GATE and CORE1_HCLK_GATE in de2-clk helps.
>>>
>>> With A133 following the same as T113 with single mixer without TV, still
>>> sets 0x20 in vendor kernel.
>>>
>>> copied from R40:
>>> Note: The priority of DE0 is higher than DE1.
>>> If TCON_LCD0 selects DE0 and DE1 as source at the same time, then
>>> DE0 will be used for the source of TCON_LCD0.
>>
>> Hi there,
>>
>> Yes that was a pretty bad typo, I meant to say DE1 to TV0
>> The prioritization seems broken in the T113 at least, it's racy from
>> what I see in testing. I should note this in the patch too.
>>
>> I looked at the datasheets and kernel code briefly: I can't seem to
>> figure out what SCLK/HCLK gating does and I don't think the kernel
>> touches these registers which are gated by default.
>>
>>> Thanks,
>>> Parthiban
>>
>> John Watts

Powered by blists - more mailing lists