Re: [PATCH 3/3] drm/i915: Use vblank evade mechanism in mmio_flip

From: Daniel Vetter <daniel@ffwll.ch>
To: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: intel-gfx@lists.freedesktop.org, ville.syrjala@intel.com,
	shuang.he@linux.intel.com, paulo.zanoni@intel.com
Subject: Re: [PATCH 3/3] drm/i915: Use vblank evade mechanism in mmio_flip
Date: Tue, 4 Nov 2014 12:04:03 +0100	[thread overview]
Message-ID: <20141104110403.GJ26941@phenom.ffwll.local> (raw)
In-Reply-To: <1414501814-1465-3-git-send-email-ander.conselvan.de.oliveira@intel.com>

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?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx