linux-kernel - Re: [RFC v1 1/1] imx-drm: match ipu_di_signal_cfg's clk

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] [day] [month] [year] [list]

Message-ID: <20181025165626.GB30658@n2100.armlinux.org.uk>
Date:   Thu, 25 Oct 2018 17:56:27 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     thesven73@...il.com
Cc:     svendev@...x.com, p.zabel@...gutronix.de, denis@...rea.com,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [RFC v1 1/1] imx-drm: match ipu_di_signal_cfg's clk_pol with its
 observed behaviour.

On Thu, Oct 25, 2018 at 12:17:11PM -0400, thesven73@...il.com wrote:
> From: Sven Van Asbroeck <svendev@...x.com>
> 
> We used an oscilloscope to observe the actual polarity of the
> DI's pixel clock, and saw the following:
> 
> DI_GENERAL bit 17 is SET:
> 	pixel data is stable on the pixel clock's FALLING edge
> DI_GENERAL bit 17 is CLEAR:
> 	pixel data is stable on the pixel clock's RISING  edge
> 
> However, the current driver configures the exact opposite of the
> behaviour documented in video/imx-ipu-v3.h:
>   unsigned clk_pol:1;     /* true = rising edge */

The definition in the manual is:

       17           DI0 Output Clock's polarity
  di0_polarity_     This bits define the polarity of the DI0's clock.
    disp_clk        1    The output clock is active high
                    0    The output clock is active low

but what does that mean...

There's the hint in IMX6SDLCEC that when the clock is active high,
it's expected that the panel will latch data on the _falling_ edge:

  IPP_DISP_CLK latches data into the panel on its negative edge (when
  positive polarity is selected). In active mode, IPP_DISP_CLK runs
  continuously.

That seems to imply that when DI_GEN_POLARITY_DISP_CLK is set, it is
expected that the panel latches data on the _falling_ edge, not on
the positive edge.

What this actually means as far as the output signal is not defined
very well beyond what I've quoted above in any of the IMX6 manuals
that I've checked so far.

Now, the interpretation of the comment "/* true = rising edge */"
is ambiguous, because it doesn't state what "rising edge" refers to -
is that referring to the edge that the data changes, or the edge that
the panel is supposed to latch the data.

Given what's in the documentation, I'd opt for it describing the
edge that the output data changes, not the latch edge.  With that
interpretation, the existing code is correct.

In any case, I suspect if we were to change this as per your patch,
we'd end up breaking stuff that works today, thereby causing
regressions.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up