diff mbox series

[2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

Message ID 20210204184710.1880895-3-jernej.skrabec@siol.net (mailing list archive)
State New, archived
Headers show
Series sunxi: fix H6 HDMI related issues | expand

Commit Message

Jernej Škrabec Feb. 4, 2021, 6:47 p.m. UTC
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 <a.heider@gmail.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
 2 files changed, 29 insertions(+)

Comments

Chen-Yu Tsai Feb. 5, 2021, 3:21 a.m. UTC | #1
On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> 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 <a.heider@gmail.com>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  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.

Otherwise,

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

> +
> +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +                       val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
> +
> +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +                       val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
> +
> +               regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
> +       }
> +
>         /* Map output pins to channel 1 */
>         regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
>                            SUN4I_TCON_GCTL_IOMAP_MASK,
> @@ -1500,6 +1523,7 @@ static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
>
>  static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
>         .has_channel_1          = true,
> +       .polarity_in_ch0        = true,
>         .set_mux                = sun8i_r40_tcon_tv_set_mux,
>  };
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index c5ac1b02482c..b504fb2d3de5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -154,6 +154,10 @@
>  #define SUN4I_TCON1_BASIC5_V_SYNC(height)              (((height) - 1) & 0x3ff)
>
>  #define SUN4I_TCON1_IO_POL_REG                 0xf0
> +#define SUN4I_TCON1_IO_POL_UNKNOWN                     BIT(26)
> +#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE              BIT(25)
> +#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE              BIT(24)
> +
>  #define SUN4I_TCON1_IO_TRI_REG                 0xf4
>
>  #define SUN4I_TCON_ECC_FIFO_REG                        0xf8
> @@ -236,6 +240,7 @@ struct sun4i_tcon_quirks {
>         bool    needs_de_be_mux; /* sun6i needs mux to select backend */
>         bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
>         bool    supports_lvds;   /* Does the TCON support an LVDS output? */
> +       bool    polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
>         u8      dclk_min_div;   /* minimum divider for TCON0 DCLK */
>
>         /* callback to handle tcon muxing options */
> --
> 2.30.0
>
Maxime Ripard Feb. 5, 2021, 4:01 p.m. UTC | #2
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 <jernej.skrabec@siol.net> 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 <a.heider@gmail.com>
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> >  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

Maxime
Jernej Škrabec Feb. 5, 2021, 4:21 p.m. UTC | #3
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 <jernej.skrabec@siol.net> 
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 <a.heider@gmail.com>
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >  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.

Best regards,
Jernej
Chen-Yu Tsai Feb. 5, 2021, 4:28 p.m. UTC | #4
On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net>
> 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 <a.heider@gmail.com>
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > ---
> > > >  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.

ChenYu
Jernej Škrabec Feb. 5, 2021, 6:47 p.m. UTC | #5
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 <jernej.skrabec@siol.net> 
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 
<jernej.skrabec@siol.net>
> > 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 <a.heider@gmail.com>
> > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > > ---
> > > > >  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 | #6
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 <jernej.skrabec@siol.net> 
> 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 
> <jernej.skrabec@siol.net>
> > > 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 <a.heider@gmail.com>
> > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > > > ---
> > > > > >  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
diff mbox series

Patch

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;
+
+		if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+			val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
+
+		if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+			val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
+
+		regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
+	}
+
 	/* Map output pins to channel 1 */
 	regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
 			   SUN4I_TCON_GCTL_IOMAP_MASK,
@@ -1500,6 +1523,7 @@  static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
 
 static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
 	.has_channel_1		= true,
+	.polarity_in_ch0	= true,
 	.set_mux		= sun8i_r40_tcon_tv_set_mux,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index c5ac1b02482c..b504fb2d3de5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -154,6 +154,10 @@ 
 #define SUN4I_TCON1_BASIC5_V_SYNC(height)		(((height) - 1) & 0x3ff)
 
 #define SUN4I_TCON1_IO_POL_REG			0xf0
+#define SUN4I_TCON1_IO_POL_UNKNOWN			BIT(26)
+#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE		BIT(25)
+#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE		BIT(24)
+
 #define SUN4I_TCON1_IO_TRI_REG			0xf4
 
 #define SUN4I_TCON_ECC_FIFO_REG			0xf8
@@ -236,6 +240,7 @@  struct sun4i_tcon_quirks {
 	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
 	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
 	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
+	bool	polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
 	u8	dclk_min_div;	/* minimum divider for TCON0 DCLK */
 
 	/* callback to handle tcon muxing options */