All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Martyn Welch <martyn.welch@collabora.co.uk>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [CRTC:24] vblank wait timed out
Date: Tue, 28 Mar 2017 18:44:50 +0200	[thread overview]
Message-ID: <1490719490.6017.24.camel@pengutronix.de> (raw)
In-Reply-To: <20170328104913.GJ25206@hermes.home>

Hi Martyn,

On Tue, 2017-03-28 at 11:49 +0100, Martyn Welch wrote:
> On Mon, Mar 27, 2017 at 12:11:12PM +0100, Martyn Welch wrote:
> > On Fri, Mar 24, 2017 at 11:42:53AM +0100, Philipp Zabel wrote:
> > > On Fri, 2017-03-24 at 10:24 +0000, Martyn Welch wrote:
> > > [...]
> > > > > Could you move to v4.9 or v4.10 and check if the four patches in
> > > > > https://git.pengutronix.de/cgit/pza/linux/tag/?id=v4.9-ipu-dp-plane-fix
> > > > > or
> > > > > https://git.pengutronix.de/cgit/pza/linux/tag/?id=v4.10-ipu-dp-plane-fix-2
> > > > > help?
> > > > > 
> > > > 
> > > > I've updated to v4.10, the patches from v4.10-ipu-dp-plane-fix-2 resolve
> > > > the error, though we are unfortunately still experiencing the loss of
> > > > output on LVDS display. Time to look elsewhere for the cause of that I
> > > > guess. :-)
> > > 
> > > Is the LVDS serial clock derived from the video PLL on that board?
> > > (What is the output of /sys/kernel/debug/clk/clk_summary?)
> > > 
> > 
> > I beleive so. I've included a few dumps from clk_summary below.
> > 
> 
> After looking at the dumps in more detail, I think I know what's
> happening:
> 
> Problem appears to be a clocking issue.

I concur.

>  When we have a display connected
> to the HDMI port pre-boot, it uses the following chain to drive IPU2
> (which is used for this display):
> 
> PLL5(756000000) -> PLL5_POST_DIV(756000000/2) ->
>     PLL5_VIDEO_DIV(378000000/1) -> LDB_DI1_IPU_DIV(378000000/3.5) ->
>     LDB_DI1(108000000) -> IPU2_DI0(108000000)

That's not what the "HDMI attached pre boot, no LVDS" summary from your
last mail says:

             pll2_pfd2_396m               3            3   432000000          0 0  
                ipu1_di0_pre_sel           1            1   432000000          0 0  
                   ipu1_di0_pre           1            1   108000000          0 0  
                      ipu1_di0_sel           1            1   108000000          0 0  
                         ipu1_di0           1            1   108000000          0 0  

This looks like IPU1 DI0 drives the HDMI display, pll5 and IPU2 are
disabled.

> If the display connected via the LVDS interface is connected either
> before or after boot, we end up with IPU1 being drive thus:
> 
> PLL2_PFD2(432000000) -> IPU2_DI1_PRE_CLK(432000000) ->
>     IPU2DI1_PODF(432000000/4) -> IPU1_DI0(108000000)

I see this in the "HDMI attached pre boot, LVDS attached after boot" and
"HDMI attached pre boot, LVDS attached pre boot" summaries:

             pll2_pfd2_396m               3            3   432000000          0 0  
                ipu1_di0_pre_sel           1            1   432000000          0 0  
                   ipu1_di0_pre           1            1   108000000          0 0  
                      ipu1_di0_sel           1            1   108000000          0 0  
                         ipu1_di0           1            1   108000000          0 0  

Same as above. Did you already set the ipu1_di0_pre_sel parent to
pll2_pfd2_396m?

> On the other hand, if only the LVDS is connected at boot, instead it is
> sets up IPU1 as follows:
> 
> PLL5(756000000) -> PLL5_POST_DIV(756000000/2) ->
>     PLL5_VIDEO_DIV(378000000/1) -> LDB_DI1_IPU_DIV(378000000/3.5) ->
>     LDB_DI1(108000000) -> IPU1_DI0(108000000)

There's the difference. Now IPU1 DI0 is used to drive LVDS, but both
ldb_di0/1_sel (now the clock source for IPU1) and ipu2_di0_pre_sel are
parented to pll5_video_div.

> Which interestingly matches how IPU2 is configured previously.
> 
> When the HDMI interface is then connected it does this to configure IPU2:
> 
> PLL5(864000000) -> PLL5_POST_DIV(756000000/4) ->
>     PLL5_VIDEO_DIV(216000000/2) -> IPU2DI0_PODF(108000000/1) ->
>     IPU2_DI0(108000000)

In this scenario only IPU2 should be used for LVDS, as IPU1 is clocked
from PLL2.

> But as the path "PLL5 -> PLL5_POST_DIV -> PLL5_VIDEO_DIV" is shared with
> IPU1 the change in frequency and divider values results in:
> 
> PLL5(864000000) -> PLL5_POST_DIV(756000000/4) ->
>     PLL5_VIDEO_DIV(216000000/2) -> LDB_DI1_IPU_DIV(108000000/3.5) ->
>     LDB_DI1(30857142) -> IPU1_DI0(30857142)
> 
> 
> So the LVDS Bridge and IPU1 are now underclocked by 3.5 times.

Exactly.

> I suspect that (theoretically) as IPU1_DI0 is already configured at the
> required frequency via LDB_DI1, it might have been sufficient for the
> IPU2_DI0_SEL MUX to select LDB_DI1 as its source?

It is the ipu1_di0_pre_sel and ipu2_di0_pre_sel setup that is important
here (and you have to pin LVDS and HDMI to separate IPUs). The LDB
driver will switch the ipu1_di0_sel from ipu1_di0_pre to ldb_di1 if IPU1
DI0 drives LVDS (imx_ldb_set_clock), so in that case it would be
ipu2_di0_pre_sel that had to be switched away from pll5_video_div, to
pll2_pfd2_396m.
In short, the IPU that drives HDMI must have its pre_sel set to
pll2_pfd_396m in your case, to avoid stepping on the LVDS output's toes,
as the PLL can't be clocked to the pixel clock and to the LVDS serial
clock (3.5*pixel clock) at the same time. The pre_sel setup for the LVDS
IPU shouldn't matter as that will be switched to the ldb_di clocks. So
just switching both ipu1/2_di0_pre_sel to pll2_pfd2_396m could do the
trick?

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2017-03-28 16:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21  9:50 [CRTC:24] vblank wait timed out Martyn Welch
2017-03-21 17:18 ` Philipp Zabel
2017-03-24 10:24   ` Martyn Welch
2017-03-24 10:42     ` Philipp Zabel
2017-03-27 11:11       ` Martyn Welch
2017-03-28 10:49         ` Martyn Welch
2017-03-28 16:44           ` Philipp Zabel [this message]
2017-03-28 17:52             ` Martyn Welch
2017-03-29  8:07               ` Philipp Zabel
2017-03-29  8:21                 ` Martyn Welch

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=1490719490.6017.24.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=martyn.welch@collabora.co.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.