[0/5] sunxi: fix H6 HDMI related issues

Message

Jernej Škrabec Feb. 4, 2021, 6:47 p.m. UTC

Over the year I got plenty of reports of troubles with H6 HDMI signal.
Sometimes monitor flickers, sometimes there was no image at all and
sometimes it didn't play well with AVR.

It turns out there are multiple issues. Patch 1 fixes clock issue,
which didn't adjust parent rate, even if it is allowed to do so. Patch 2
adds polarity config in tcon1. This is seemingly not needed for pre-HDMI2
controllers, although BSP drivers set it accordingly every time. It
turns out that HDMI2 controllers often don't work with monitors if
polarity is not set correctly. Patch 3 always set clock rate for HDMI
controller. Patch 4 fixes cpce PHY setting for 594 MHz. Patch 5 fixes
comment and clock rate limit (wrong reasoning).

Please take a look.

Best regards,
Jernej

Jernej Skrabec (5):
  clk: sunxi-ng: mp: fix parent rate change flag check
  drm/sun4i: tcon: set sync polarity for tcon1 channel
  drm/sun4i: dw-hdmi: always set clock rate
  drm/sun4i: Fix H6 HDMI PHY configuration
  drm/sun4i: dw-hdmi: Fix max. frequency for H6

 drivers/clk/sunxi-ng/ccu_mp.c          |  2 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c     | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.h     |  5 +++++
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c  | 10 +++-------
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h  |  1 -
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c |  2 +-
 6 files changed, 34 insertions(+), 10 deletions(-)

Comments

Jernej Škrabec Feb. 5, 2021, 6:47 p.m. UTC | #1

Dne petek, 05. februar 2021 ob 17:28:23 CET je Chen-Yu Tsai napisal(a):
> On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <[e-mail]> 
wrote:
> >
> > Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> > > On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > > > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec 
<[e-mail]>
> > wrote:
> > > > >
> > > > > Channel 1 has polarity bits for vsync and hsync signals but driver 
never
> > > > > sets them. It turns out that with pre-HDMI2 controllers seemingly 
there
> > > > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > > > (H6) there often comes to de-synchronization due to phase shift. 
This
> > > > > causes flickering screen. It's safe to assume that similar issues 
might
> > > > > happen also with pre-HDMI2 controllers.
> > > > >
> > > > > Solve issue with setting vsync and hsync polarity. Note that display
> > > > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > > > register.
> > > > >
> > > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine 
support")
> > > > > Tested-by: Andre Heider <[e-mail]>
> > > > > Signed-off-by: Jernej Skrabec <[e-mail]>
> > > > > ---
> > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> > > > >  2 files changed, 29 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/
sun4i/
> > sun4i_tcon.c
> > > > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct 
sun4i_tcon
> > *tcon,
> > > > >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > > >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > > > >
> > > > > +       /* Setup the polarity of sync signals */
> > > > > +       if (tcon->quirks->polarity_in_ch0) {
> > > > > +               val = 0;
> > > > > +
> > > > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > > > +
> > > > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > > +
> > > > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, 
val);
> > > > > +       } else {
> > > > > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > > >
> > > > I think a comment for the origin of this is warranted.
> > >
> > > If it's anything like TCON0, it's the pixel clock polarity
> >
> > Hard to say, DW HDMI controller has "data enable" polarity along hsync and
> > vsync. It could be either or none of those.
> >
> > What should I write in comment? BSP drivers and documentation use only 
generic
> > names like io2_inv.
> 
> Just say that we don't know exactly what it is, but it is required for 
things
> to work properly? Would be interesting to know what happens if you don't set
> this bit, but do set VSYNC/HSYNC polarity properly.

Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At least I 
didn't notice anything.

Best regards,
Jernej

Maxime Ripard Feb. 9, 2021, 10:31 a.m. UTC | #2

On Fri, Feb 05, 2021 at 07:47:17PM +0100, Jernej Škrabec wrote:
> Dne petek, 05. februar 2021 ob 17:28:23 CET je Chen-Yu Tsai napisal(a):
> > On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <[e-mail]> 
> wrote:
> > >
> > > Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> > > > On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > > > > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec 
> <[e-mail]>
> > > wrote:
> > > > > >
> > > > > > Channel 1 has polarity bits for vsync and hsync signals but driver 
> never
> > > > > > sets them. It turns out that with pre-HDMI2 controllers seemingly 
> there
> > > > > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > > > > (H6) there often comes to de-synchronization due to phase shift. 
> This
> > > > > > causes flickering screen. It's safe to assume that similar issues 
> might
> > > > > > happen also with pre-HDMI2 controllers.
> > > > > >
> > > > > > Solve issue with setting vsync and hsync polarity. Note that display
> > > > > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > > > > register.
> > > > > >
> > > > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine 
> support")
> > > > > > Tested-by: Andre Heider <[e-mail]>
> > > > > > Signed-off-by: Jernej Skrabec <[e-mail]>
> > > > > > ---
> > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> > > > > >  2 files changed, 29 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/
> sun4i/
> > > sun4i_tcon.c
> > > > > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct 
> sun4i_tcon
> > > *tcon,
> > > > > >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > > > >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > > > > >
> > > > > > +       /* Setup the polarity of sync signals */
> > > > > > +       if (tcon->quirks->polarity_in_ch0) {
> > > > > > +               val = 0;
> > > > > > +
> > > > > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > > > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > > > > +
> > > > > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > > > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > > > +
> > > > > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, 
> val);
> > > > > > +       } else {
> > > > > > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > > > >
> > > > > I think a comment for the origin of this is warranted.
> > > >
> > > > If it's anything like TCON0, it's the pixel clock polarity
> > >
> > > Hard to say, DW HDMI controller has "data enable" polarity along hsync and
> > > vsync. It could be either or none of those.
> > >
> > > What should I write in comment? BSP drivers and documentation use only 
> generic
> > > names like io2_inv.
> > 
> > Just say that we don't know exactly what it is, but it is required for 
> things
> > to work properly? Would be interesting to know what happens if you don't set
> > this bit, but do set VSYNC/HSYNC polarity properly.
> 
> Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At least I 
> didn't notice anything.

That's pretty normal, an inverted pixel clock would at worst give you
some weird artifacts and / or pixels being of the wrong color. Data
enable on the other hand would very likely stall the HDMI controller
since you would have only the blanking periods that would be considered
valid.

Maxime