| Message ID | Zn8GVkpwXwhaUFno@titan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | T113 TCON Top tinting troubleshooting | expand |
On Sat, Jun 29, 2024 at 01:49:53AM +0200, Jakob L wrote: > Hi John, > > good find! This seems to fix it on my DSI implementation. Every of the > recent boots resulted in a pink tint (usually it was green for me, or blue) > Booted 10 times - no tint. > > So this patch is good, but probably has to be implemented as a quirk? > > Jakob Hi Jakob, I'm not sure if this needs to be a quirk: There's not really a reason to set both DEs to the same output. My thinking right now is to error in this situation. However I think we have to somehow track which DEs have been set so we can pretend they start unset. For example: DE0: (Unset, LCD0) DE1: (Unset, TVE0) Setting DE0 to LCD0 should work, then setting DE1 to LCD0 should fail. Setting DE0 to LCD0 should work, then setting DE0 to TVE0 should work. Setting DE0 to TVE0 should work, then setting DE1 to TVE0 should fail. The mechanism here for setting a DE I think would go like this: 1. Check the opposite DE's value 2. If it is duplicate and set, error 3. If it is duplicate and unset, change the opposite DE's value 4. Set the DE 5. Mark it as set Step three requires finding an unused DE value, this could be done by inverting the bits of the new DE. Alternatively we could do what Allwinner does and start with both DEs at different values and the move instead the current DE value to the opposite DE to avoid conflicts. 'Set' here may make more sense as 'used'. I'm good at mentally over-engineering things, so if you can think of a better solution please tell me. John.
Hi, On Sat, Jun 29, 2024 at 12:14:39PM +1000, Kirby Nankivell wrote: > Tested this on a MangoPI-MQ-R with a RGB lcd and can confirm that this fix > removes the green tint I experienced. > > Disabling the mixer1 in DTS entirely also works. > > I suspect on the basis that the Mixer/TCON-TOP topology is identical to the > R40 but does not seem to experience this issue, that is a > silicon errata specific to the T113/D1s as far as we currently know. > > However, there is no functional reason from a drm perspective to require > more than one mixer tied to one output via tcon-top - unless someone has a > good argument here? > > in my opinion: > 1) implement this as a quirk on the t113 and fix mixer0 > rgb/dsi and > mixer1 > tv0 > 2) make this a global default for the sun8i tcon top driver where only one > mixer is initialised against an output (which would also impact the R40) The only reason I caught this issue was because I implemented LCD output in U-Boot and saw it looked a little better- I thought the tint was the backlight. Had you not contacted me and had it not affected the person I'm doing work for I would not have bothered looking in to this. It's entirely possible that the other supported devices have this issue, or that future ones will and nobody will notice. Going with a preventative approach here by fixing it for all potential devices and relying on the behaviour the sunxi code assumes rather than the datasheet says seems like the best approach to me. John.
--- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c @@ -179,7 +179,7 @@ static int sun8i_tcon_top_bind(struct device *dev, struct device *master, * At least on H6, some registers have some bits set by default * which may cause issues. Clear them here. */ - writel(0, regs + TCON_TOP_PORT_SEL_REG); + writel(0x20, regs + TCON_TOP_PORT_SEL_REG); writel(0, regs + TCON_TOP_GATE_SRC_REG); /*