On 11/8/24 6:59 PM, John Watts wrote: > 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. 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. Thanks, Parthiban > >> >> Cheers, >> Andre > > Thanks, > John Watts >
