[linux-sunxi] Re: HDMI/DVI spurious failure

[Priit Laes]

On Mon, Jan 21, 2019 at 02:25:17PM +0100, Maxime Ripard wrote:
> On Fri, Jan 18, 2019 at 02:51:26PM +0000, Priit Laes wrote:
> > On Fri, Jan 18, 2019 at 03:04:18PM +0100, Maxime Ripard wrote:
> > > On Fri, Jan 18, 2019 at 10:10:53AM +0000, Priit Laes wrote:
> > > > > > > > > It doesn't look related to the clock rate itself, since it doesn't
> > > > > > > > > change between the two cases. However, in one case the DDC clock is
> > > > > > > > > enabled and in the other it's disabled.
> > > > > > > > > 
> > > > > > > > > Was it taken at the same time? Maybe you can try with that patch?
> > > > > > > > > http://code.bulix.org/z7jmkm-555344?raw
> > > > > > > > 
> > > > > > > > Thanks, after doing ~50+ boots I haven't seen a single failure.
> > > > > > > > 
> > > > > > > > Previously I had following failure cases which are now both fixed:
> > > > > > > > 
> > > > > > > > a) Linux without u-boot HDMI, where one in every 6-7 boots failed.
> > > > > > > > b) u--boot with hdmi enabled switching to simplefb and then switching
> > > > > > > > to kms, where previously all boots ended up with garbled screen.
> > > > > > > 
> > > > > > > So it's not really a fix, but it really looks like the clock is not
> > > > > > > enabled when it should.
> > > > > > > 
> > > > > > > Can you describe your test scenario a bit more? What are you doing
> > > > > > > exactly, just booting? When do you start using the display? When did
> > > > > > > you capture the debugfs output that you pasted?
> > > > > > 
> > > > > > Display is already connected via HDMI to the board. I don't really
> > > > > > remove it, I just boot the device and let it start Xorg.
> > > > > > Meanwhile I just ssh into the device and capture debugfs output.
> > > > > > See my 3 testing scenarios below.
> > > > > > 
> > > > > > Kernel also includes one extra patch to fall back to DDC, in case HPD
> > > > > > fails. Mostly the same I already submitted last November [1].
> > > > > 
> > > > > Do you have the same issue without that patch?
> > > > 
> > > > Can't really test this display without this patch and I do not have other
> > > > HDMI/DVI screens. And this issue does not happen with other HDMI displays
> > > > that I have here.
> > > 
> > > Can't you just force the monitor to be reported as present? It's not
> > > great and we don't want to merge it, but that would allow you to test
> > > that setup without too many interferences.
> > 
> > Baseline is clean u-boot / linux. U-boot does not detect/enable display.
> > 
> > 1) Booting Linux with drm.debug=0xe
> > 
> > * Linux does not detect/enable display
> > 
> > 2) Booting with drm.debug=0xe video=HDMI-A-1:640x480 at 60e
> > 
> > * Linux detects display, but display is garbled, and proper edid data is detected:
> > 
> > [snip]
> > pll-video1                     0        0        0   327000000          0     0  50000
> >    pll-video1-2x               0        0        0   654000000          0     0  50000
> >       hdmi-tmds                0        0        0    25153846          0     0  50000
> >          hdmi-ddc              0        0        0       89835          0     0  50000
> > [/snip]
> > 
> > 3) Booting with drm.debug=0xe video=HDMI-A-1:640x480 at 60e
> > And also one extra patch for Linux where HDMI DDC clock marked as critical
> > 
> > Linux detects and initializes display properly:
> > [snip]
> > pll-video1                     1        1        0   327000000          0     0  50000
> >    pll-video1-2x               1        1        0   654000000          0     0  50000
> >       hdmi-tmds                1        1        0    25153846          0     0  50000
> >          hdmi-ddc              1        1        0       89835          0     0  50000
> > [/snip]
> 
> I guess you'll need to track down when the hdmi-tmds and hdmi-ddc are
> enabled and disabled, and if it makes sense :/

OK, figured out the cause.

Apparently, for each ddc poll we enable ddc clock which is a child of TMDS
clock. After transfer is done, we disable the clock and this also tears down
the parent because its only user has gone missing.. :(


So basically, patch below also works, but I guess we should override
the sun4i_tmds_ops.disable to properly account for tmds clock use.

---
@@ -225,7 +235,7 @@ int sun4i_tmds_create(struct sun4i_hdmi *hdmi)
        init.ops = &sun4i_tmds_ops;
        init.parent_names = parents;
        init.num_parents = 2;
-       init.flags = CLK_SET_RATE_PARENT;
+       init.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL;
 
        tmds->hdmi = hdmi;
        tmds->hw.init = &init;


> Maxime
> 
> -- 
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com