[Intel-gfx] [PATCH] drm/i915: Push vblank enable/disable past encoder->enable/disable

[Chris Wilson]

On Mon, Feb 23, 2015 at 05:05:16PM +0100, Daniel Vetter wrote:
> On Wed, Jan 07, 2015 at 09:40:50PM +0000, Chris Wilson wrote:
> > On Wed, Jan 07, 2015 at 02:38:46PM +0100, Daniel Vetter wrote:
> > > It is platform/output depenedent when exactly the pipe will start
> > > running. Sometimes we just need the (cpu) pipe enabled, in other cases
> > > the pch transcoder is enough and in yet other cases the (DP) port is
> > > sending the frame start signal.
> > > 
> > > In a perfect world we'd put the drm_crtc_vblank_on call exactly where
> > > the pipe starts running, but due to cloning and similar things this
> > > will get messy. And the current approach of picking the most
> > > conservative place for all combinations also doesn't work since that
> > > results in legit vblank waits (in encoder->enable hooks, e.g. the 2
> > > vblank waits for sdvo) failing.
> > > 
> > > Completely going back to the old world before
> > > 
> > > commit 51e31d49c89055299e34b8f44d13f70e19aaaad1
> > > Author: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > Date:   Mon Sep 15 12:36:02 2014 +0200
> > > 
> > >     drm/i915: Use generic vblank wait# Please enter the commit message for your changes. Lines starting
> > > 
> > > isn't great either since screaming when the vblank wait work because
> > > the pipe is off is kinda nice.
> > > 
> > > Pick a compromise and move the drm_crtc_vblank_on right before the
> > > encoder->enable call. This is a lie on some outputs/platforms, but
> > > after the ->enable callback the pipe is guaranteed to run everywhere.
> > > So not that bad really. Suggested by Ville.
> > > 
> > > v2: Same treatment for drm_crtc_vblank_off and encoder->disable: I've
> > > missed the ibx pipe B select w/a, which also has a vblank wait in the
> > > disable function (while the pipe is obviously still running).
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Acked-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > 
> > Rather than decreasing the number of WARNs on my pnv during boot, this
> > doubled them.
> > 
> > The original was:
> > 
> > [   34.136161] WARNING: CPU: 3 PID: 206 at drivers/gpu/drm/drm_irq.c:1130 drm_wait_one_vblank+0x15a/0x160()
> > [   34.136166] vblank wait timed out on crtc 1
> > [   34.136402]  [<c13f664a>] drm_wait_one_vblank+0x15a/0x160
> > [   34.136415]  [<c107d7d0>] ? prepare_to_wait_event+0xd0/0xd0
> > [   34.136433]  [<c146a459>] i9xx_crtc_disable+0x59/0x400
> > 
> > and the interloper:
> > 
> > [   47.012212] WARNING: CPU: 2 PID: 1423 at drivers/gpu/drm/drm_irq.c:1130 drm_wait_one_vblank+0x15a/0x160()
> > [   47.012217] vblank wait timed out on crtc 1
> > [   47.012400]  [<c13f664a>] drm_wait_one_vblank+0x15a/0x160
> > [   47.012409]  [<c107d7d0>] ? prepare_to_wait_event+0xd0/0xd0
> > [   47.012420]  [<c146762e>] intel_pipe_set_base+0x11e/0x1f0
> 
> Are you sure? The patch strictly increase the coverage for when vblanks
> works, so more sounds really funky ...

Coincidental maybe? But it only has appeared with that patch...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre