diff mbox series

[v2,15/29] drm/bridge/synopsys: dw-hdmi: Enable workaround for v2.12a

Message ID 20181007093905.11253-16-jernej.skrabec@siol.net (mailing list archive)
State Not Applicable, archived
Headers show
Series Allwinner H6 DE3 and HDMI support | expand

Commit Message

Jernej Škrabec Oct. 7, 2018, 9:38 a.m. UTC
It turns out that even new DW HDMI controllers exhibits same magenta
line issues as older versions.

Enable workaround for v2.12a.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Laurent Pinchart Oct. 9, 2018, 5:40 p.m. UTC | #1
Hi Jernej,

Thank you for the patch.

On Sunday, 7 October 2018 12:38:51 EEST Jernej Skrabec wrote:
> It turns out that even new DW HDMI controllers exhibits same magenta
> line issues as older versions.
> 
> Enable workaround for v2.12a.

This doesn't affect the platforms I maintain, so I can't really test this, but 
I'm wondering whether there could be other platforms using a v2.12a DW HDMI 
that wouldn't need the workaround.

My platforms use a previous version, namely v2.01a. The workaround for that 
version has been enabled by

commit 9c305eb442f3b371fc722ade827bbf673514123e
Author: Neil Armstrong <narmstrong@baylibre.com>
Date:   Fri Feb 23 12:44:37 2018 +0100

    drm: bridge: dw-hdmi: Fix overflow workaround for Amlogic Meson GX SoCs

I haven't paid too much attention to the patch back then, and have now double-
checked the HDMI output on R-Car Gen3. Enabling the workaround doesn't cause 
any regression, and reverting the commit doesn't cause any issue either. I 
thus wonder whether we shouldn't enable the workaround with count = 1 in the 
default case instead of adding new IP core versions to the list. It would be 
nice if someone from Synopsys could comment on this.

> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> 5971976284bf..df1c7a2d6961 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1664,6 +1664,7 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi
> *hdmi) case 0x131a:
>  	case 0x132a:
>  	case 0x201a:
> +	case 0x212a:
>  		count = 1;
>  		break;
>  	default:
Ilia Mirkin Oct. 9, 2018, 5:56 p.m. UTC | #2
On Tue, Oct 9, 2018 at 1:40 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jernej,
>
> Thank you for the patch.
>
> On Sunday, 7 October 2018 12:38:51 EEST Jernej Skrabec wrote:
> > It turns out that even new DW HDMI controllers exhibits same magenta
> > line issues as older versions.
> >
> > Enable workaround for v2.12a.
>
> This doesn't affect the platforms I maintain, so I can't really test this, but
> I'm wondering whether there could be other platforms using a v2.12a DW HDMI
> that wouldn't need the workaround.
>
> My platforms use a previous version, namely v2.01a. The workaround for that
> version has been enabled by
>
> commit 9c305eb442f3b371fc722ade827bbf673514123e
> Author: Neil Armstrong <narmstrong@baylibre.com>
> Date:   Fri Feb 23 12:44:37 2018 +0100
>
>     drm: bridge: dw-hdmi: Fix overflow workaround for Amlogic Meson GX SoCs
>
> I haven't paid too much attention to the patch back then, and have now double-
> checked the HDMI output on R-Car Gen3. Enabling the workaround doesn't cause
> any regression, and reverting the commit doesn't cause any issue either. I
> thus wonder whether we shouldn't enable the workaround with count = 1 in the
> default case instead of adding new IP core versions to the list. It would be
> nice if someone from Synopsys could comment on this.

I hope this comment isn't *incredibly* off-topic, but we encountered a
similar issue with NVIDIA (and I believe AMD) hardware a while back,
related to HDMI. This was due to infoframes not being sent, but
(perhaps) HDMI Audio being enabled.

This was a single vertical(!) line. It was described as "purple", but
not sure that's distinguishable from "magenta" by most people. [ Fixed
by a522946174 on nouveau, sample bug report
https://bugs.freedesktop.org/show_bug.cgi?id=79912 ]

Cheers,

  -ilia
Russell King (Oracle) Oct. 9, 2018, 9:23 p.m. UTC | #3
On Tue, Oct 09, 2018 at 01:56:04PM -0400, Ilia Mirkin wrote:
> On Tue, Oct 9, 2018 at 1:40 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > commit 9c305eb442f3b371fc722ade827bbf673514123e
> > Author: Neil Armstrong <narmstrong@baylibre.com>
> > Date:   Fri Feb 23 12:44:37 2018 +0100
> >
> >     drm: bridge: dw-hdmi: Fix overflow workaround for Amlogic Meson GX SoCs
> >
> > I haven't paid too much attention to the patch back then, and have now double-
> > checked the HDMI output on R-Car Gen3. Enabling the workaround doesn't cause
> > any regression, and reverting the commit doesn't cause any issue either. I
> > thus wonder whether we shouldn't enable the workaround with count = 1 in the
> > default case instead of adding new IP core versions to the list. It would be
> > nice if someone from Synopsys could comment on this.
> 
> I hope this comment isn't *incredibly* off-topic, but we encountered a
> similar issue with NVIDIA (and I believe AMD) hardware a while back,
> related to HDMI. This was due to infoframes not being sent, but
> (perhaps) HDMI Audio being enabled.

You're probably right about the infoframes.  The errata documentation
for this workaround on iMX6 states:

 Each time one writes to some FC registers, and depending on the clock relation of sfr clk and tmds
 clk, some of these train of pulses (when these registers are configured in sequence), may not be
 caught by the arithmetic unit while it is busy processing/updating the first ones, so, it gets wrong
 video timing values, although the registers FC_* hold correct values. Even a soft reset will not
 make the arithmetic unit update correctly. Video will still pass correctly to the HDMI, but packets
 would not because the frame composer is holding internally incorrect video timing and this will
 quickly build up and overflow the packet FIFOs.

So, the workaround is about kicking the frame composer so that the
packets (iow, non-video data) are passed through correctly.
Jernej Škrabec Oct. 15, 2018, 5:43 p.m. UTC | #4
Hi,

Dne torek, 09. oktober 2018 ob 19:40:44 CEST je Laurent Pinchart napisal(a):
> Hi Jernej,
> 
> Thank you for the patch.
> 
> On Sunday, 7 October 2018 12:38:51 EEST Jernej Skrabec wrote:
> > It turns out that even new DW HDMI controllers exhibits same magenta
> > line issues as older versions.
> > 
> > Enable workaround for v2.12a.
> 
> This doesn't affect the platforms I maintain, so I can't really test this,
> but I'm wondering whether there could be other platforms using a v2.12a DW
> HDMI that wouldn't need the workaround.
> 
> My platforms use a previous version, namely v2.01a. The workaround for that
> version has been enabled by
> 
> commit 9c305eb442f3b371fc722ade827bbf673514123e
> Author: Neil Armstrong <narmstrong@baylibre.com>
> Date:   Fri Feb 23 12:44:37 2018 +0100
> 
>     drm: bridge: dw-hdmi: Fix overflow workaround for Amlogic Meson GX SoCs
> 
> I haven't paid too much attention to the patch back then, and have now
> double- checked the HDMI output on R-Car Gen3. Enabling the workaround
> doesn't cause any regression, and reverting the commit doesn't cause any
> issue either. I thus wonder whether we shouldn't enable the workaround with
> count = 1 in the default case instead of adding new IP core versions to the
> list. It would be nice if someone from Synopsys could comment on this.

I was thinking about that too, or even having parameter in dw_hdmi_plat_data 
which would tell how many times to repeat workaround procedure for a specific 
platform. But this might be an overkill.

Best regards,
Jernej

> 
> > Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index
> > 5971976284bf..df1c7a2d6961 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1664,6 +1664,7 @@ static void dw_hdmi_clear_overflow(struct dw_hdmi
> > 
> > *hdmi) case 0x131a:
> >  	case 0x132a:
> > 
> >  	case 0x201a:
> > +	case 0x212a:
> >  		count = 1;
> >  		break;
> >  	
> >  	default:
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 5971976284bf..df1c7a2d6961 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1664,6 +1664,7 @@  static void dw_hdmi_clear_overflow(struct dw_hdmi *hdmi)
 	case 0x131a:
 	case 0x132a:
 	case 0x201a:
+	case 0x212a:
 		count = 1;
 		break;
 	default: