linux-kernel - Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210111172052.7v522xam74xkq6se@gilmour>
Date:   Mon, 11 Jan 2021 18:20:52 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Giulio Benetti <giulio.benetti@...ettiengineering.com>
Cc:     Marjan Pascolo <marjan.pascolo@...xom.it>, wens@...e.org,
        daniel@...ll.ch, airlied@...ux.ie, treding@...dia.com,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Giulio Benetti <giulio.benetti@...ronovasrl.com>
Subject: Re: [PATCH v2 2/2] drm/sun4i: tcon: improve DCLK polarity handling

On Fri, Jan 08, 2021 at 03:34:52PM +0100, Giulio Benetti wrote:
> Hi,
> 
> On 1/8/21 10:23 AM, Maxime Ripard wrote:
> > Hi,
> > 
> > Thanks for those patches
> > 
> > On Thu, Jan 07, 2021 at 03:30:32AM +0100, Giulio Benetti wrote:
> > > From: Giulio Benetti <giulio.benetti@...ronovasrl.com>
> > > 
> > > It turned out(Maxime suggestion) that bit 26 of SUN4I_TCON0_IO_POL_REG is
> > > dedicated to invert DCLK polarity and this makes thing really easier than
> > > before. So let's handle DCLK polarity by adding
> > > SUN4I_TCON0_IO_POL_DCLK_POSITIVE as bit 26 and activating according to
> > > bus_flags the same way is done for all the other signals.
> > > 
> > > Cc: Maxime Ripard <maxime@...no.tech>
> > 
> > Suggested-by would be nice here :)
> 
> Ok, didn't know about this tag
> 
> > > Signed-off-by: Giulio Benetti <giulio.benetti@...ronovasrl.com>
> > > ---
> > >   drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +-------------------
> > >   drivers/gpu/drm/sun4i/sun4i_tcon.h |  1 +
> > >   2 files changed, 2 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > index 52598bb0fb0b..30171ccd87e5 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -569,26 +569,8 @@ static void sun4i_tcon0_mode_set_rgb(struct sun4i_tcon *tcon,
> > >   	if (info->bus_flags & DRM_BUS_FLAG_DE_LOW)
> > >   		val |= SUN4I_TCON0_IO_POL_DE_NEGATIVE;
> > > -	/*
> > > -	 * On A20 and similar SoCs, the only way to achieve Positive Edge
> > > -	 * (Rising Edge), is setting dclk clock phase to 2/3(240°).
> > > -	 * By default TCON works in Negative Edge(Falling Edge),
> > > -	 * this is why phase is set to 0 in that case.
> > > -	 * Unfortunately there's no way to logically invert dclk through
> > > -	 * IO_POL register.
> > > -	 * The only acceptable way to work, triple checked with scope,
> > > -	 * is using clock phase set to 0° for Negative Edge and set to 240°
> > > -	 * for Positive Edge.
> > > -	 * On A33 and similar SoCs there would be a 90° phase option,
> > > -	 * but it divides also dclk by 2.
> > > -	 * Following code is a way to avoid quirks all around TCON
> > > -	 * and DOTCLOCK drivers.
> > > -	 */
> > >   	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> > > -		clk_set_phase(tcon->dclk, 0);
> > > -
> > > -	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE)
> > > -		clk_set_phase(tcon->dclk, 240);
> > > +		val |= SUN4I_TCON0_IO_POL_DCLK_POSITIVE;
> > 
> > I'm not really sure why we need the first patch of this series here?
> 
> The idea was to have 2 for testing, 1st one is already applicable, while the
> other must be tested, but I can send only one with no problem.
> 
> > That patch only seem to undo what you did in patch 1
> 
> No, it doesn't, the 2nd one change the way it achieve the same thing,
> because the 1st swap DCLK phase, while the 2nd uses the IO_POL bit to set IO
> polarity according to bus_flags.

It makes sense for testing, but I'm not sure we want to carry it into
the history. Can you squash them both into the same patch?

Thanks!
Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)