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

[Chen-Yu Tsai]

On Tue, Jan 22, 2019 at 1:18 AM Jernej Škrabec <jernej.skrabec at gmail.com> wrote:
>
> Dne ponedeljek, 21. januar 2019 ob 16:07:28 CET je Priit Laes napisal(a):
> > 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.
> >
>
> As far as I can see, nobody called clk_prepare_enable() on tmds clock. I had
> similar issue with TMDS clock for H3, since I based code on sun4i tmds clk
> driver. Although clock doesn't have enable bit, it still has to be done to
> have proper reference count and to prevent issues like this.

IIRC I had the same gut feeling at the time, but other stuff happened. :(

Priit, could you send a proper patch enabling and disabling the TMDS clock?
It should be done as part of sun4i_hdmi_enable and sun4i_hdmi_disable.

ChenYu