On Mon, Feb 23, 2015 at 04:54:32PM +0000, Chris Wilson wrote: > 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@xxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx> > > > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > > Acked-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > > > > > 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... intel_pipe_set_base() isn't a thing anymore, so I take it this is some slightly older kernel? Looking at the code, the wait in intel_pipe_set_base() was protected by crtc->active, and intel_pipe_set_base() wasn't actually called for the full modeset path. So, if we're getting a timeout outside the full modeset path we've either messed up in the irq code, or we somehow end up think a pipe is active when it's not. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
