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

On Tue, Mar 28, 2017 at 06:44:50PM +0200, Philipp Zabel wrote:
> 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?
> 

As you can probably tell by how much I got in a muddle with my explanation
above, this isn't territory I'm familiar with.

It sounds good, not sure I 100% understand how to do this yet. :-)
I'll look into this more.

Thanks for your help, I'll give this a try.

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

  reply	other threads:[~2017-03-28 17:52 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
2017-03-28 17:52             ` Martyn Welch [this message]
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=20170328175247.GS25206@hermes.home \
    --to=martyn.welch@collabora.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=p.zabel@pengutronix.de \
    /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).