From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: thesven73@gmail.com
Cc: svendev@arcx.com, p.zabel@pengutronix.de, denis@eukrea.com,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [RFC v1 1/1] imx-drm: match ipu_di_signal_cfg's clk_pol with its observed behaviour.
Date: Thu, 25 Oct 2018 17:56:27 +0100 [thread overview]
Message-ID: <20181025165626.GB30658@n2100.armlinux.org.uk> (raw)
In-Reply-To: <20181025161711.13727-2-TheSven73@googlemail.com>
On Thu, Oct 25, 2018 at 12:17:11PM -0400, thesven73@gmail.com wrote:
> From: Sven Van Asbroeck <svendev@arcx.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
prev parent reply other threads:[~2018-10-25 16:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-25 16:17 [RFC v1 0/1] imx-drm: match ipu_di_signal_cfg's clk_pol with its observed behaviour thesven73
2018-10-25 16:17 ` [RFC v1 1/1] " thesven73
2018-10-25 16:56 ` Russell King - ARM Linux [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181025165626.GB30658@n2100.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=denis@eukrea.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=svendev@arcx.com \
--cc=thesven73@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).