Attachment 138556 Details for Bug 105720 – Reorder vblank/flip event handler

[patch] Reorder vblank/flip event handler

mode-wakeup.diff (text/plain), 6.48 KB, created by Chris Wilson on 2018-04-03 18:32:17 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2018-04-03 18:32:17 UTC

Size: 6.48 KB

patch

obsolete

>diff --git a/src/sna/sna.h b/src/sna/sna.h
>index 3ccc74c8..6fe1f0d2 100644
>--- a/src/sna/sna.h
>+++ b/src/sna/sna.h
>@@ -310,7 +310,6 @@ struct sna {
> 		unsigned flip_active;
> 		unsigned hidden;
> 		bool shadow_enabled;
>-		bool shadow_wait;
> 		bool dirty;
> 
> 		struct drm_event_vblank *shadow_events;
>diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
>index becc6061..a611b46d 100644
>--- a/src/sna/sna_display.c
>+++ b/src/sna/sna_display.c
>@@ -1703,8 +1703,8 @@ static bool wait_for_shadow(struct sna *sna,
> 	int flip_active;
> 	bool ret = true;
> 
>-	DBG(("%s: enabled? %d waiting? %d, flags=%x, flips=%d, pixmap=%ld [front?=%d], handle=%d, shadow=%d\n",
>-	     __FUNCTION__, sna->mode.shadow_enabled, sna->mode.shadow_wait,
>+	DBG(("%s: enabled? %d flags=%x, flips=%d, pixmap=%ld [front?=%d], handle=%d, shadow=%d\n",
>+	     __FUNCTION__, sna->mode.shadow_enabled,
> 	     flags, sna->mode.flip_active,
> 	     pixmap->drawable.serialNumber, pixmap == sna->front,
> 	     priv->gpu_bo->handle, sna->mode.shadow->handle));
>@@ -1716,7 +1716,6 @@ static bool wait_for_shadow(struct sna *sna,
> 		goto done;
> 
> 	assert(sna->mode.shadow_damage);
>-	assert(!sna->mode.shadow_wait);
> 
> 	if ((flags & MOVE_WRITE) == 0) {
> 		if ((flags & __MOVE_SCANOUT) == 0) {
>@@ -1758,7 +1757,6 @@ static bool wait_for_shadow(struct sna *sna,
> 	}
> 
> 	assert(sna->mode.shadow_active);
>-	sna->mode.shadow_wait = true;
> 
> 	flip_active = sna->mode.flip_active;
> 	if (flip_active) {
>@@ -1768,21 +1766,6 @@ static bool wait_for_shadow(struct sna *sna,
> 		DBG(("%s: %d flips still pending, shadow flip_active=%d\n",
> 		     __FUNCTION__, sna->mode.flip_active, flip_active));
> 	}
>-	if (flip_active) {
>-		/* raw cmd to avoid setting wedged in the middle of an op */
>-		drmIoctl(sna->kgem.fd, DRM_IOCTL_I915_GEM_THROTTLE, 0);
>-		sna->kgem.need_throttle = false;
>-
>-		while (flip_active && sna_mode_wakeup(sna)) {
>-			struct sna_crtc *crtc;
>-
>-			flip_active = sna->mode.flip_active;
>-			list_for_each_entry(crtc, &sna->mode.shadow_crtc, shadow_link)
>-				flip_active -= crtc->flip_pending;
>-		}
>-		DBG(("%s: after waiting %d flips outstanding, flip_active=%d\n",
>-		     __FUNCTION__, sna->mode.flip_active, flip_active));
>-	}
> 
> 	bo = sna->mode.shadow;
> 	if (flip_active) {
>@@ -1792,26 +1775,19 @@ static bool wait_for_shadow(struct sna *sna,
> 				    pixmap->drawable.bitsPerPixel,
> 				    priv->gpu_bo->tiling,
> 				    CREATE_EXACT | CREATE_SCANOUT);
>-		if (bo != NULL) {
>-			DBG(("%s: replacing still-attached GPU bo handle=%d, flips=%d\n",
>-			     __FUNCTION__, priv->gpu_bo->tiling, sna->mode.flip_active));
>+		if (bo == NULL)
>+			return false;
> 
>-			RegionUninit(&sna->mode.shadow_region);
>-			sna->mode.shadow_region.extents.x1 = 0;
>-			sna->mode.shadow_region.extents.y1 = 0;
>-			sna->mode.shadow_region.extents.x2 = pixmap->drawable.width;
>-			sna->mode.shadow_region.extents.y2 = pixmap->drawable.height;
>-			sna->mode.shadow_region.data = NULL;
>-		} else {
>-			while (sna->mode.flip_active &&
>-			       sna_mode_wait_for_event(sna))
>-				sna_mode_wakeup(sna);
>+		DBG(("%s: replacing still-attached GPU bo handle=%d, flips=%d\n",
>+		     __FUNCTION__, priv->gpu_bo->tiling, sna->mode.flip_active));
> 
>-			bo = sna->mode.shadow;
>-		}
>+		RegionUninit(&sna->mode.shadow_region);
>+		sna->mode.shadow_region.extents.x1 = 0;
>+		sna->mode.shadow_region.extents.y1 = 0;
>+		sna->mode.shadow_region.extents.x2 = pixmap->drawable.width;
>+		sna->mode.shadow_region.extents.y2 = pixmap->drawable.height;
>+		sna->mode.shadow_region.data = NULL;
> 	}
>-	assert(sna->mode.shadow_wait);
>-	sna->mode.shadow_wait = false;
> 
> 	if (bo->refcnt > 1) {
> 		bo = kgem_create_2d(&sna->kgem,
>@@ -1919,9 +1895,6 @@ done:
> 	priv->move_to_gpu_data = NULL;
> 	priv->move_to_gpu = NULL;
> 
>-	assert(!sna->mode.shadow_wait);
>-	flush_events(sna);
>-
> 	assert(sna->mode.shadow->active_scanout);
> 	return ret;
> }
>@@ -9050,6 +9023,12 @@ void sna_mode_redisplay(struct sna *sna)
> 	if (sna->mode.dirty)
> 		return;
> 
>+	if (sna->mode.flip_active) {
>+		DBG(("%s: %d outstanding flips\n",
>+		     __FUNCTION__, sna->mode.flip_active));
>+		return;
>+	}
>+
> 	region = DamageRegion(sna->mode.shadow_damage);
> 	if (RegionNil(region))
> 		return;
>@@ -9059,23 +9038,6 @@ void sna_mode_redisplay(struct sna *sna)
> 	     region->extents.x1, region->extents.y1,
> 	     region->extents.x2, region->extents.y2));
> 
>-	if (sna->mode.flip_active) {
>-		DBG(("%s: checking for %d outstanding flip completions\n",
>-		     __FUNCTION__, sna->mode.flip_active));
>-
>-		sna->mode.dirty = true;
>-		while (sna->mode.flip_active && sna_mode_wakeup(sna))
>-			;
>-		sna->mode.dirty = false;
>-
>-		DBG(("%s: now %d outstanding flip completions (enabled? %d)\n",
>-		     __FUNCTION__,
>-		     sna->mode.flip_active,
>-		     sna->mode.shadow_enabled));
>-		if (sna->mode.flip_active || !sna->mode.shadow_enabled)
>-			return;
>-	}
>-
> 	if (!move_crtc_to_gpu(sna)) {
> 		DBG(("%s: forcing scanout update using the CPU\n", __FUNCTION__));
> 		if (!sna_pixmap_move_to_cpu(sna->front, MOVE_READ))
>@@ -9463,6 +9425,7 @@ fixup_flip:
> 
> int sna_mode_wakeup(struct sna *sna)
> {
>+	bool defer_vblanks = sna->mode.flip_active && sna->mode.shadow_enabled;
> 	char buffer[1024];
> 	int len, i;
> 	int ret = 0;
>@@ -9473,14 +9436,14 @@ again:
> 	 * event from drm.
> 	 */
> 	if (!event_pending(sna->kgem.fd))
>-		return ret;
>+		goto done;
> 
> 	/* The DRM read semantics guarantees that we always get only
> 	 * complete events.
> 	 */
> 	len = read(sna->kgem.fd, buffer, sizeof (buffer));
> 	if (len < (int)sizeof(struct drm_event))
>-		return ret;
>+		goto done;
> 
> 	/* Note that we cannot rely on the passed in struct sna matching
> 	 * the struct sna used for the vblank event (in case it was submitted
>@@ -9495,7 +9458,7 @@ again:
> 		struct drm_event *e = (struct drm_event *)&buffer[i];
> 		switch (e->type) {
> 		case DRM_EVENT_VBLANK:
>-			if (sna->mode.shadow_wait)
>+			if (defer_vblanks)
> 				defer_event(sna, e);
> 			else if (((uintptr_t)((struct drm_event_vblank *)e)->user_data) & 2)
> 				sna_present_vblank_handler((struct drm_event_vblank *)e);
>@@ -9566,4 +9529,8 @@ again:
> 	}
> 
> 	goto again;
>+
>+done:
>+	flush_events(sna);
>+	return ret;
> }
>diff --git a/src/sna/sna_present.c b/src/sna/sna_present.c
>index a2ce808f..523261cd 100644
>--- a/src/sna/sna_present.c
>+++ b/src/sna/sna_present.c
>@@ -460,12 +460,6 @@ sna_present_vblank_handler(struct drm_event_vblank *event)
> 
> 	msc = sna_crtc_record_event(info->crtc, event);
> 
>-	if (info->sna->mode.shadow_wait) {
>-		DBG(("%s: recursed from TearFree\n", __FUNCTION__));
>-		if (TimerSet(NULL, 0, 1, sna_fake_vblank_handler, info))
>-			return;
>-	}
>-
> 	vblank_complete(info, ust64(event->tv_sec, event->tv_usec), msc);
> }
>

Actions: View | Splinter Review

Attachments on bug 105720: 138318 | 138319 | 138320 | 138358 | 138363 | 138397 | 138556

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.