[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Troy Kisky]

+/* Workaround to clear the overflow condition */
+static void imx_hdmi_clear_overflow(struct imx_hdmi *hdmi)
+{
+	int count;
+	u8 val;
+
+	/* TMDS software reset */
+	hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, HDMI_MC_SWRSTZ);
+
+	val = hdmi_readb(hdmi, HDMI_FC_INVIDCONF);
+	if (hdmi->dev_type == IMX6DL_HDMI) {
+		hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+		return;
+	}
+
+	for (count = 0; count < 5; count++)
+		hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
+}
+

Hi Fabio,

I get a magenta line down the left side of the screen unless I replace 
the 5 with a 6.
ie.

+	for (count = 0; count < 6; count++)
+		hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);


This is with a imx6q processor. With the 6, the picture looks great!

Troy

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Troy Kisky]

On 10/31/2013 1:23 PM, Troy Kisky wrote:
>
> +/* Workaround to clear the overflow condition */
> +static void imx_hdmi_clear_overflow(struct imx_hdmi *hdmi)
> +{
> +    int count;
> +    u8 val;
> +
> +    /* TMDS software reset */
> +    hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, 
> HDMI_MC_SWRSTZ);
> +
> +    val = hdmi_readb(hdmi, HDMI_FC_INVIDCONF);
> +    if (hdmi->dev_type == IMX6DL_HDMI) {
> +        hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
> +        return;
> +    }
> +
> +    for (count = 0; count < 5; count++)
> +        hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
> +}
> +
>
> Hi Fabio,
>
> I get a magenta line down the left side of the screen unless I replace 
> the 5 with a 6.
> ie.
>
> +    for (count = 0; count < 6; count++)
> +        hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
>
>
> This is with a imx6q processor. With the 6, the picture looks great!
>
> Troy
>

It also works if I change the 5 to a 4! A 3 fails though.
To summarize:
0 works
1 fails
2 works
3 fails
4 works
5 fails
6 works
Very strange.

Troy

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Fabio Estevam]

Hi Troy,

On Thu, Oct 31, 2013 at 6:38 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

>> I get a magenta line down the left side of the screen unless I replace the
>> 5 with a 6.
>> ie.
>>
>> +    for (count = 0; count < 6; count++)
>> +        hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
>>
>>
>> This is with a imx6q processor. With the 6, the picture looks great!
>>
>> Troy
>>
>
> It also works if I change the 5 to a 4! A 3 fails though.
> To summarize:
> 0 works
> 1 fails
> 2 works
> 3 fails
> 4 works
> 5 fails
> 6 works
> Very strange.

Interesting. With the monitor I have tested I am not able to see this
magenta line.

As the erratum says to write 3 or 4 times to the register, then I will
use 4 as per your results.

Regards,

Fabio Estevam

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Russell King - ARM Linux]

On Thu, Oct 31, 2013 at 06:45:41PM -0200, Fabio Estevam wrote:
> Hi Troy,
> 
> On Thu, Oct 31, 2013 at 6:38 PM, Troy Kisky
> <troy.kisky@boundarydevices.com> wrote:
> 
> >> I get a magenta line down the left side of the screen unless I replace the
> >> 5 with a 6.
> >> ie.
> >>
> >> +    for (count = 0; count < 6; count++)
> >> +        hdmi_writeb(hdmi, val, HDMI_FC_INVIDCONF);
> >>
> >>
> >> This is with a imx6q processor. With the 6, the picture looks great!
> >>
> >> Troy
> >>
> >
> > It also works if I change the 5 to a 4! A 3 fails though.
> > To summarize:
> > 0 works
> > 1 fails
> > 2 works
> > 3 fails
> > 4 works
> > 5 fails
> > 6 works
> > Very strange.
> 
> Interesting. With the monitor I have tested I am not able to see this
> magenta line.

Monitor or TV?  Beware of overscans which will hide this effect.

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Fabio Estevam]

On Thu, Oct 31, 2013 at 7:00 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

>> Interesting. With the monitor I have tested I am not able to see this
>> magenta line.
>
> Monitor or TV?  Beware of overscans which will hide this effect.

I used a PC monitor on my tests.

I sent v5 using 4 as the number of loops according to Troy's results.

Regards,

Fabio Estevam

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Matt Sealey]

On Thu, Oct 31, 2013 at 4:08 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Oct 31, 2013 at 7:00 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>
>>> Interesting. With the monitor I have tested I am not able to see this
>>> magenta line.
>>
>> Monitor or TV?  Beware of overscans which will hide this effect.
>
> I used a PC monitor on my tests.
>
> I sent v5 using 4 as the number of loops according to Troy's results.

Stop, stop, stop!

Has nobody noticed this register isn't described in the reference manual?

What does it even do? How can we quantify Troy's results by just
throwing numbers at it and then saying "no magenta line here"?

Growl,
Matt Sealey <neko@bakuhatsu.net>

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Matt Sealey]

On Thu, Oct 31, 2013 at 4:48 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
> On Thu, Oct 31, 2013 at 4:08 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Thu, Oct 31, 2013 at 7:00 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>
>>>> Interesting. With the monitor I have tested I am not able to see this
>>>> magenta line.
>>>
>>> Monitor or TV?  Beware of overscans which will hide this effect.
>>
>> I used a PC monitor on my tests.
>>
>> I sent v5 using 4 as the number of loops according to Troy's results.
>
> Stop, stop, stop!
>
> Has nobody noticed this register isn't described in the reference manual?
>
> What does it even do? How can we quantify Troy's results by just
> throwing numbers at it and then saying "no magenta line here"?

Okay a little worry here is that the last time I kicked an HDMI
transmitter around a huge magenta line was basically a side effect of
doing the following things that weren't necessary to spec; either
enabling audio infoframes when in DVI mode (the transmitter doesn't
care if you're in DVI mode, it will shove audio out there anyway) or
some really weird timing effects, almost a race condition, if you're
generating infoframes at the wrong points (such as SPD or suchlike) -
sometimes the monitor will figure it out. Sometimes not.

The easiest case is enable "HDMI transmitter mode" on a monitor that
does not actually have an HDMI-spec-capable decoder on it. If you have
an HDMI to DVI cable, you have this. If you have an older or cheaper
monitor with no speakers, this can also be entirely possible. You
almost never see this on TVs, but you DO see it on maybe the "3rd"
HDMI port on some TVs (where audio mysteriously doesn't work, but it
works fine on the other two ports). I wrote a little treatise on this
about those Radeon HDMI->DVI dongles to the DRM list, it's the same
deal.

Essentially there's not a lot of point enabling HDMI mode if your
monitor doesn't support audio, and if you do enable it, you HAVE to
make sure your AVI InfoFrames are being sent (but, obviously, don't
send AVI InfoFrames in DVI mode or all you'll see is a magenta line if
it extends the blanking timings, and the monitor doesn't handle it).

I notice the register enums have DVI mode in there, also I saw a weird
case where the CEA modes in the kernel that got introduced with DRM
had a couple backwards polarities, and one or two of them on some
monitors that specified them would give the magenta line (and on
others that had the same VIC in their CEA list) would work fine.. I
guess they had more resilient logic to detect things like the wrong
HSYNC polarity.

What modes are being set here (are they derived from VIC codes or
actual detail timings?) that cause the magenta line? The actual fix
may be to go through the HDMI spec and fix the CEA mode definitions
that correspond to each VIC and be sure they're accurate.

Hammering the register 4 times probably just resolves it by
approaching what seems like a more correct waveform by constantly
re-starting the transmission path. That might explain why some values
work for some and some don't for others.

I'm concerned that the real problem isn't addressed (and I did see a
patch that fiddled a polarity value earlier..) and the actual effects
of these bits is not actually documented in the manuals.

Fabio, Shawn, could you go so far as to bring this up with the i.MX
guys responsible for documentation or find the original transmitter IP
specs, or find the two pages missing from the manual? This is a really
important register if the definition in the source code is to be
believed (do we believe it?) if working HDMI output is to be achieved.

Thanks,
Matt Sealey <neko@bakuhatsu.net>

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Fabio Estevam]

On Mon, Nov 4, 2013 at 9:20 PM, Matt Sealey <neko@bakuhatsu.net> wrote:

> Fabio, Shawn, could you go so far as to bring this up with the i.MX
> guys responsible for documentation or find the original transmitter IP
> specs, or find the two pages missing from the manual? This is a really

The multiple writes comes from the following erratum:
ERR005173 HDMI: Clarification on HDMI programming procedure to avoid
FIFO overflow

Regards,

Fabio Estevam

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Matt Sealey]

On Mon, Nov 4, 2013 at 7:13 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Nov 4, 2013 at 9:20 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
>
>> Fabio, Shawn, could you go so far as to bring this up with the i.MX
>> guys responsible for documentation or find the original transmitter IP
>> specs, or find the two pages missing from the manual? This is a really
>
> The multiple writes comes from the following erratum:
> ERR005173 HDMI: Clarification on HDMI programming procedure to avoid
> FIFO overflow

I know about that one. BTW this isn't in the MX6SDL errata
documentation, but it is in the MX6QD errata. If this actually affects
the Solo etc. as Russell dictates, isn't the errata document missing
some info here?

Also the patch doesn't ACTUALLY do what the erratum states you should
do - nobody is checking audio FIFO status here before retrying.

What I am more curious about is the net effect of every bit in this 8
bit undocumented register which seems to have a major errata around
it.

I noticed a couple which have very abstract definitions which could
mean a couple things regarding packetizing information. What does
R_V_BLANK_IN_OSC mean? It's being set when the VIC selected is 39,
which makes sense in that there is a magic interlace timing for this
VIC if it also exists in the EDID DTDs. However, no check for this is
made, it just sets it outright. In fact, it doesn't even make sense
since in theory it would not be being told to set the VIC timing, it
would be passed the DTD timing, which need to match a very specific
set of values, and if they match you need to, instead, fix up the
timing used based on the fact that VIC 39 was also listed in the CEA
extension block...

If you don't follow the CEA-861-E specs or HDMI 1.3a specs *at least*
when configuring a mode, and packetizing your infoframes, and the
content of those infoframes, then you're going to end up with magenta
lines regardless of esoteric audio FIFO underflows.

What I don't understand here is how a FIFO underflow in the audio
packetizer is causing this magenta line. What you should get is a pop
or crackle in the audio (or silence..) or just zero effect on the
display if the packetizer can't put valid data in there. There aren't
any reasons why a FIFO underflow would somehow change mode timings to
cause a magenta line.

That said, I got the same thing accidentally enabling audio on a DVI
monitor, although this was an SPDIF connection to an external
transmitter (not an internal IP block) - even if you didn't enable
audio and it was in DVI mode, it caused enough "drag" on the logic to
sample audio and be getting nothing over the audio link to put a huge
pink line down the side of at least 2 monitors. There doesn't seem to
be any audio setup going on in the driver, so I would suggest trying
to just force DVI mode and see if the line goes away or gets worse
with those different values for retrying, and then forcing HDMI mode..
same test.


Matt Sealey <neko@bakuhatsu.net>

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Fabio Estevam]

On Tue, Nov 5, 2013 at 9:29 PM, Matt Sealey <neko@bakuhatsu.net> wrote:

> I know about that one. BTW this isn't in the MX6SDL errata
> documentation, but it is in the MX6QD errata. If this actually affects
> the Solo etc. as Russell dictates, isn't the errata document missing
> some info here?

If you read the code you will notice that the multiple writes affects
only mx6quad.

[PATCH v4] imx-drm: Add mx6 hdmi transmitter support

[Matt Sealey]

On Tue, Nov 5, 2013 at 5:35 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Tue, Nov 5, 2013 at 9:29 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
>
>> I know about that one. BTW this isn't in the MX6SDL errata
>> documentation, but it is in the MX6QD errata. If this actually affects
>> the Solo etc. as Russell dictates, isn't the errata document missing
>> some info here?
>
> If you read the code you will notice that the multiple writes affects
> only mx6quad.

I might have missed a patch.. mails are going missing.

Who's tree is this in, as actual code with git commits and not a
stream of mis-threaded emails?

Matt <neko@bakuhatsu.net>