[Intel-gfx] [PATCH 3/3] drm/i915: Use vblank evade mechanism in mmio_flip

Chris Wilson chris at chris-wilson.co.uk
Tue Nov 4 12:10:18 CET 2014


On Tue, Nov 04, 2014 at 12:04:03PM +0100, Daniel Vetter wrote:
> On Tue, Oct 28, 2014 at 03:10:14PM +0200, Ander Conselvan de Oliveira wrote:
> > @@ -9400,7 +9419,7 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	int ret;
> >  
> > -	if (WARN_ON(intel_crtc->mmio_flip.seqno))
> > +	if (WARN_ON(intel_crtc->mmio_flip.status != INTEL_MMIO_FLIP_IDLE))
> >  		return -EBUSY;
> 
> Ok I've merged the patch, but I think this check is wrong. do_mmio_flip
> only sets this to idle _after_ the hw register writing is done. Which
> means the flip could complete and userspace could receive the event all
> before we set status to IDLE. Which means you can WARN and return -EBUSY
> here when the flip definitely completed.
> 
> So either clear IDLE at the head of do_mmio_flip before doing anything, or
> drop this state again. After all the previous seqno check only looked for
> waiting or not. tbh I'm not even sure whether this buys us anything at all
> - we already check that the flip completed, which should ensure ordering.
> I think we could just rip out mmio_flip.status completely.
> 
> Can you pls do a fixup patch?

A better strategy would be to use the generic seqno mechanism for the
flip from a worker which then wouldn't need this extra vblank state
machinery on top of the vblank evade...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre



More information about the Intel-gfx mailing list