Re: [Freedreno] [PATCH RFC v2 0/6] ARM: qcom: initial Nexus 5 display support

[Jeffrey Hugo]

On 5/29/2019 1:30 PM, Brian Masney wrote:
> On Wed, May 29, 2019 at 08:41:31AM -0600, Jeffrey Hugo wrote:
> > On Wed, May 29, 2019 at 4:28 AM Brian Masney <masneyb@xxxxxxxxxxxxx> wrote:
> > > On Tue, May 28, 2019 at 08:53:49PM -0600, Jeffrey Hugo wrote:
> > > > On Tue, May 28, 2019 at 8:46 PM Brian Masney <masneyb@xxxxxxxxxxxxx> wrote:
> > > > > On Tue, May 28, 2019 at 07:42:19PM -0600, Jeffrey Hugo wrote:
> > > > > > > > Do you know if the nexus 5 has a video or command mode panel?  There
> > > > > > > > is some glitchyness with vblanks and command mode panels.
> > > > > > >
> > > > > > > Its in command mode. I know this because I see two 'pp done time out'
> > > > > > > messages, even on 4.17. Based on my understanding, the ping pong code is
> > > > > > > only applicable for command mode panels.
> > > > > >
> > > > > > Actually, the ping pong element exists in both modes, but 'pp done
> > > > > > time out' is a good indicator that it is command mode.
> > > > > >
> > > > > > Are you also seeing vblank timeouts?
> > > > >
> > > > > Yes, here's a snippet of the first one.
> > > > >
> > > > > [    2.556014] WARNING: CPU: 0 PID: 5 at drivers/gpu/drm/drm_atomic_helper.c:1429 drm_atomic_helper_wait_for_vblanks.part.1+0x288/0x290
> > > > > [    2.556020] [CRTC:49:crtc-0] vblank wait timed out
> > > > > [    2.556023] Modules linked in:
> > > > > [    2.556034] CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.2.0-rc1-00178-g72c3c1fd5f86-dirty #426
> > > > > [    2.556038] Hardware name: Generic DT based system
> > > > > [    2.556056] Workqueue: events deferred_probe_work_func
> > > > > ...
> > > > >
> > > > > > Do you have busybox?
> > > > > >
> > > > > > Can you run -
> > > > > > sudo busybox devmem 0xFD900614
> > > > > > sudo busybox devmem 0xFD900714
> > > > > > sudo busybox devmem 0xFD900814
> > > > > > sudo busybox devmem 0xFD900914
> > > > > > sudo busybox devmem 0xFD900A14
> > > > >
> > > > > # busybox devmem 0xFD900614
> > > > > 0x00020020
> > > >
> > > > Ok, so CTL_0 path, command mode, ping pong 0, with the output going to DSI 1.
> > > >
> > > > Next one please:
> > > >
> > > > busybox devmem 0xFD912D30
> > >
> > > It's 0x00000000 on mainline and 4.17. I used the following script to
> > > dump the entire mdp5 memory region and attached the dump from 4.17 and
> > > 5.2rc1.
> >
> > ok, 0 means autorefresh is not on.  Which is fine.  My next guess
> > would be the vblank code checking the hardware vblank counter, which
> > doesn't exist.
> > In video mode, there is a frame counter which increments, which can be
> > used as the vblank counter.  Unfortunately, that hardware isn't active
> > in command mode, and there isn't an equivalent.
> >
> > So, the vblank code is going to read the register, and look for an
> > update, which will never happen, thus it will timeout.  There is a
> > backup path which uses timestamps (no hardware), which you can
> > activate with a quick hack - make max_vblank_count = 0 at the
> > following line
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c#L753
>
> That fixed the issue!

Awesome.  I'm glad it was something simple.

> I previously observed that mdp5_get_vblank_counter, specifically
> mdp5_encoder_get_framecount, would always return 0.
>
> What's the best way to fix this in mainline? Set that to zero if any
> of the interface modes is MDP5_INTF_DSI_MODE_COMMAND?

Short version, yes.  Long version:

I still have that hack in my tree and haven't come back to formulating
a proper fix yet.  Feel free to run with it.

Thinking about it briefly, we could do two things.  We could fake a
hardware counter by just increment an int every time the vblank irq is
processed, but that seems clunky.  Otherwise, we could force a
fallback onto the timestamp solution, which seems less invasive.

In theory, we could service multiple displays, with different
properties (ie a combination of command and video mode).  The hack
then, is not good, because it would break video mode (at-least we
wouldn't be using the register when we could).  It would be great if
the use of the hardware register could be done per display.

Luckily, it looks like someone just made that possible -
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/drm_vblank.c?h=v5.2-rc2&id=ed20151a7699bb2c77eba3610199789a126940c4

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.