Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 9 Feb 2021 11:31:58 +0100
From:   Maxime Ripard <maxime@cerno.tech>
To:     Jernej =?utf-8?Q?=C5=A0krabec?= <jernej.skrabec@siol.net>
Cc:     Chen-Yu Tsai <wens@csie.org>,
        Mike Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        linux-clk <linux-clk@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        linux-sunxi <linux-sunxi@googlegroups.com>,
        Andre Heider <a.heider@gmail.com>
Subject: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel
Message-ID: <20210209103158.rnzvlvo44nnzrmk4@gilmour>
References: <20210204184710.1880895-1-jernej.skrabec@siol.net>
 <2156838.FvJGUiYDvf@kista>
 <CAGb2v65cZ7DXzcPzt8ER1ucwhbhTOc0-mCCye3eK-ZgP3a6ivA@mail.gmail.com>
 <3972834.uPFdDeCNB3@kista>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="lvwjttyd23hbxwrd"
Content-Disposition: inline
In-Reply-To: <3972834.uPFdDeCNB3@kista>
Precedence: bulk


--lvwjttyd23hbxwrd
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 05, 2021 at 07:47:17PM +0100, Jernej =C5=A0krabec 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 =C5=A0krabec <jernej.skrabec@sio=
l.net>=20
> 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=20
> <jernej.skrabec@siol.net>
> > > wrote:
> > > > > >
> > > > > > Channel 1 has polarity bits for vsync and hsync signals but dri=
ver=20
> never
> > > > > > sets them. It turns out that with pre-HDMI2 controllers seeming=
ly=20
> there
> > > > > > is no issue if polarity is not set. However, with HDMI2 control=
lers
> > > > > > (H6) there often comes to de-synchronization due to phase shift=
=2E=20
> This
> > > > > > causes flickering screen. It's safe to assume that similar issu=
es=20
> might
> > > > > > happen also with pre-HDMI2 controllers.
> > > > > >
> > > > > > Solve issue with setting vsync and hsync polarity. Note that di=
splay
> > > > > > stacks with tcon top have polarity bits actually in tcon0 polar=
ity
> > > > > > register.
> > > > > >
> > > > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine=20
> 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/d=
rm/
> 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=20
> 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 =3D 0;
> > > > > > +
> > > > > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > > > +                       val |=3D SUN4I_TCON0_IO_POL_HSYNC_POSIT=
IVE;
> > > > > > +
> > > > > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > > > +                       val |=3D SUN4I_TCON0_IO_POL_VSYNC_POSIT=
IVE;
> > > > > > +
> > > > > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG=
,=20
> val);
> > > > > > +       } else {
> > > > > > +               val =3D 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 hsyn=
c and
> > > vsync. It could be either or none of those.
> > >
> > > What should I write in comment? BSP drivers and documentation use onl=
y=20
> generic
> > > names like io2_inv.
> >=20
> > Just say that we don't know exactly what it is, but it is required for=
=20
> 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.
>=20
> Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At lea=
st I=20
> 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

--lvwjttyd23hbxwrd
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYCJkngAKCRDj7w1vZxhR
xedJAQCvyb+V3yoXMwG3DqhG21kI9ivkqbheculNc5PYOa79ZQD/cKZtr++3tXXL
b0t+vjHAf3eohmy56y3F2ZBXFWYzhg8=
=MfCa
-----END PGP SIGNATURE-----

--lvwjttyd23hbxwrd--