Re: Re: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel
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
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
> > 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
> > > > > Signed-off-by: Jernej Skrabec
> > > > > ---
> > > > > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel
On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec 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
> 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
> > > > Signed-off-by: Jernej Skrabec
> > > > ---
> > > > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel
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
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
> > > Signed-off-by: Jernej Skrabec
> > > ---
> > > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
