Attachment 44552 Details for Bug 34584 – Fix piplelined fence corruption

[patch] Fix piplelined fence corruption

0001-drm-i915-Fix-tiling-corruption-from-pipelined-fencin.patch (text/plain), 9.33 KB, created by Chris Wilson on 2011-03-17 12:04:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2011-03-17 12:04:59 UTC

Size: 9.33 KB

patch

obsolete

>From cc4a65972f8237e2fdcd67622d7c8bd360ae72d4 Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Thu, 17 Mar 2011 15:23:22 +0000
>Subject: [PATCH] drm/i915: Fix tiling corruption from pipelined fencing
>
>... even though it was disabled. A mistake in the handling of fence reuse
>caused us to skip the vital delay of waiting for the object to be finish
>rendering before changing the register.
>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=34584
>Cc: Andy Whitcroft <apw@canonical.com>
>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>Cc: stable@kernel.org # 2.6.38
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> drivers/gpu/drm/i915/i915_drv.h            |    2 -
> drivers/gpu/drm/i915/i915_gem.c            |  111 ++++++++++------------------
> drivers/gpu/drm/i915/i915_gem_execbuffer.c |   12 +--
> 3 files changed, 43 insertions(+), 82 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 4496505..d5abd2c 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -129,7 +129,6 @@ struct drm_i915_master_private {
> struct drm_i915_fence_reg {
> 	struct list_head lru_list;
> 	struct drm_i915_gem_object *obj;
>-	uint32_t setup_seqno;
> };
> 
> struct sdvo_device_mapping {
>@@ -818,7 +817,6 @@ struct drm_i915_gem_object {
> 
> 	/** Breadcrumb of last fenced GPU access to the buffer. */
> 	uint32_t last_fenced_seqno;
>-	struct intel_ring_buffer *last_fenced_ring;
> 
> 	/** Current tiling stride for the object, if it's tiled. */
> 	uint32_t stride;
>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>index d4bf061..ac28fd5 100644
>--- a/drivers/gpu/drm/i915/i915_gem.c
>+++ b/drivers/gpu/drm/i915/i915_gem.c
>@@ -1638,9 +1638,7 @@ i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
> 		struct drm_i915_fence_reg *reg;
> 
> 		BUG_ON(obj->fence_reg == I915_FENCE_REG_NONE);
>-
> 		obj->last_fenced_seqno = seqno;
>-		obj->last_fenced_ring = ring;
> 
> 		reg = &dev_priv->fence_regs[obj->fence_reg];
> 		list_move_tail(&reg->lru_list, &dev_priv->mm.fence_list);
>@@ -1682,10 +1680,11 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
> 	obj->ring = NULL;
> 
> 	i915_gem_object_move_off_active(obj);
>-	obj->fenced_gpu_access = false;
> 
> 	obj->active = 0;
> 	obj->pending_gpu_write = false;
>+	obj->fenced_gpu_access = false;
>+	obj->last_fenced_seqno = 0;
> 	drm_gem_object_unreference(&obj->base);
> 
> 	WARN_ON(i915_verify_lists(dev));
>@@ -1836,6 +1835,8 @@ static void i915_gem_reset_fences(struct drm_device *dev)
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	int i;
> 
>+	mb();
>+
> 	for (i = 0; i < 16; i++) {
> 		struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
> 		struct drm_i915_gem_object *obj = reg->obj;
>@@ -1849,9 +1850,10 @@ static void i915_gem_reset_fences(struct drm_device *dev)
> 		reg->obj->fence_reg = I915_FENCE_REG_NONE;
> 		reg->obj->fenced_gpu_access = false;
> 		reg->obj->last_fenced_seqno = 0;
>-		reg->obj->last_fenced_ring = NULL;
> 		i915_gem_clear_fence_reg(dev, reg);
> 	}
>+
>+	mb();
> }
> 
> void i915_gem_reset(struct drm_device *dev)
>@@ -2450,26 +2452,28 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj,
> 
> 	if (obj->fenced_gpu_access) {
> 		if (obj->base.write_domain & I915_GEM_GPU_DOMAINS) {
>-			ret = i915_gem_flush_ring(obj->last_fenced_ring,
>+			ret = i915_gem_flush_ring(obj->ring,
> 						  0, obj->base.write_domain);
> 			if (ret)
> 				return ret;
> 		}
> 
>+		/* Invalidate the GPU TLBs for any future reads */
>+		obj->base.read_domains &= ~I915_GEM_GPU_DOMAINS;
> 		obj->fenced_gpu_access = false;
> 	}
> 
>-	if (obj->last_fenced_seqno && pipelined != obj->last_fenced_ring) {
>-		if (!ring_passed_seqno(obj->last_fenced_ring,
>-				       obj->last_fenced_seqno)) {
>-			ret = i915_wait_request(obj->last_fenced_ring,
>+	if (obj->last_fenced_seqno) {
>+		if (ring_passed_seqno(obj->ring, obj->last_fenced_seqno)) {
>+			obj->last_fenced_seqno = 0;
>+		} else if (pipelined != obj->ring) {
>+			ret = i915_wait_request(obj->ring,
> 						obj->last_fenced_seqno);
> 			if (ret)
> 				return ret;
>-		}
> 
>-		obj->last_fenced_seqno = 0;
>-		obj->last_fenced_ring = NULL;
>+			obj->last_fenced_seqno = 0;
>+		}
> 	}
> 
> 	/* Ensure that all CPU reads are completed before installing a fence
>@@ -2495,9 +2499,9 @@ i915_gem_object_put_fence(struct drm_i915_gem_object *obj)
> 
> 	if (obj->fence_reg != I915_FENCE_REG_NONE) {
> 		struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
>+
> 		i915_gem_clear_fence_reg(obj->base.dev,
> 					 &dev_priv->fence_regs[obj->fence_reg]);
>-
> 		obj->fence_reg = I915_FENCE_REG_NONE;
> 	}
> 
>@@ -2529,15 +2533,15 @@ i915_find_fence_reg(struct drm_device *dev,
> 	/* None available, try to steal one or wait for a user to finish */
> 	avail = first = NULL;
> 	list_for_each_entry(reg, &dev_priv->mm.fence_list, lru_list) {
>-		if (reg->obj->pin_count)
>+		struct drm_i915_gem_object *obj = reg->obj;
>+
>+		if (obj->pin_count)
> 			continue;
> 
> 		if (first == NULL)
> 			first = reg;
> 
>-		if (!pipelined ||
>-		    !reg->obj->last_fenced_ring ||
>-		    reg->obj->last_fenced_ring == pipelined) {
>+		if (!pipelined || !obj->ring || obj->ring == pipelined) {
> 			avail = reg;
> 			break;
> 		}
>@@ -2581,50 +2585,11 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
> 		reg = &dev_priv->fence_regs[obj->fence_reg];
> 		list_move_tail(&reg->lru_list, &dev_priv->mm.fence_list);
> 
>-		if (!obj->fenced_gpu_access && !obj->last_fenced_seqno)
>-			pipelined = NULL;
>-
>-		if (!pipelined) {
>-			if (reg->setup_seqno) {
>-				if (!ring_passed_seqno(obj->last_fenced_ring,
>-						       reg->setup_seqno)) {
>-					ret = i915_wait_request(obj->last_fenced_ring,
>-								reg->setup_seqno);
>-					if (ret)
>-						return ret;
>-				}
>-
>-				reg->setup_seqno = 0;
>-			}
>-		} else if (obj->last_fenced_ring &&
>-			   obj->last_fenced_ring != pipelined) {
>+		if (obj->tiling_changed) {
> 			ret = i915_gem_object_flush_fence(obj, pipelined);
> 			if (ret)
> 				return ret;
>-		} else if (obj->tiling_changed) {
>-			if (obj->fenced_gpu_access) {
>-				if (obj->base.write_domain & I915_GEM_GPU_DOMAINS) {
>-					ret = i915_gem_flush_ring(obj->ring,
>-								  0, obj->base.write_domain);
>-					if (ret)
>-						return ret;
>-				}
> 
>-				obj->fenced_gpu_access = false;
>-			}
>-		}
>-
>-		if (!obj->fenced_gpu_access && !obj->last_fenced_seqno)
>-			pipelined = NULL;
>-		BUG_ON(!pipelined && reg->setup_seqno);
>-
>-		if (obj->tiling_changed) {
>-			if (pipelined) {
>-				reg->setup_seqno =
>-					i915_gem_next_request_seqno(pipelined);
>-				obj->last_fenced_seqno = reg->setup_seqno;
>-				obj->last_fenced_ring = pipelined;
>-			}
> 			goto update;
> 		}
> 
>@@ -2653,29 +2618,29 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
> 			return ret;
> 		}
> 
>-		if (old->last_fenced_seqno == 0 && obj->last_fenced_seqno == 0)
>-			pipelined = NULL;
>-
> 		old->fence_reg = I915_FENCE_REG_NONE;
>-		old->last_fenced_ring = pipelined;
>-		old->last_fenced_seqno =
>-			pipelined ? i915_gem_next_request_seqno(pipelined) : 0;
>-
>+		if (pipelined && old->last_fenced_seqno) {
>+			old->last_fenced_seqno =
>+				i915_gem_next_request_seqno(pipelined);
>+			i915_gem_object_move_to_active(old, pipelined,
>+						       old->last_fenced_seqno);
>+		}
>+		obj->last_fenced_seqno = old->last_fenced_seqno;
> 		drm_gem_object_unreference(&old->base);
>-	} else if (obj->last_fenced_seqno == 0)
>-		pipelined = NULL;
>+	}
> 
> 	reg->obj = obj;
> 	list_move_tail(&reg->lru_list, &dev_priv->mm.fence_list);
> 	obj->fence_reg = reg - dev_priv->fence_regs;
>-	obj->last_fenced_ring = pipelined;
>-
>-	reg->setup_seqno =
>-		pipelined ? i915_gem_next_request_seqno(pipelined) : 0;
>-	obj->last_fenced_seqno = reg->setup_seqno;
> 
> update:
>+	if (obj->last_fenced_seqno == 0)
>+		pipelined = NULL;
>+
>+	obj->last_fenced_seqno =
>+		pipelined ? i915_gem_next_request_seqno(pipelined) : 0;
> 	obj->tiling_changed = false;
>+
> 	switch (INTEL_INFO(dev)->gen) {
> 	case 6:
> 		ret = sandybridge_write_fence_reg(obj, pipelined);
>@@ -2692,6 +2657,9 @@ update:
> 		break;
> 	}
> 
>+	if (!pipelined)
>+		mb();
>+
> 	return ret;
> }
> 
>@@ -2730,7 +2698,6 @@ i915_gem_clear_fence_reg(struct drm_device *dev,
> 
> 	list_del_init(&reg->lru_list);
> 	reg->obj = NULL;
>-	reg->setup_seqno = 0;
> }
> 
> /**
>diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>index 7ff7f93..82b24d5 100644
>--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>@@ -172,23 +172,19 @@ i915_gem_object_set_to_gpu_domain(struct drm_i915_gem_object *obj,
> 	 * write domain
> 	 */
> 	if (obj->base.write_domain &&
>-	    (((obj->base.write_domain != obj->base.pending_read_domains ||
>-	       obj->ring != ring)) ||
>-	     (obj->fenced_gpu_access && !obj->pending_fenced_gpu_access))) {
>+	    (obj->base.write_domain != obj->base.pending_read_domains ||
>+	     obj->ring != ring))
> 		flush_domains |= obj->base.write_domain;
>-		invalidate_domains |=
>-			obj->base.pending_read_domains & ~obj->base.write_domain;
>-	}
> 	/*
> 	 * Invalidate any read caches which may have
> 	 * stale data. That is, any new read domains.
> 	 */
> 	invalidate_domains |= obj->base.pending_read_domains & ~obj->base.read_domains;
>-	if ((flush_domains | invalidate_domains) & I915_GEM_DOMAIN_CPU)
>+	if (flush_domains & I915_GEM_DOMAIN_CPU)
> 		i915_gem_clflush_object(obj);
> 
> 	/* blow away mappings if mapped through GTT */
>-	if ((flush_domains | invalidate_domains) & I915_GEM_DOMAIN_GTT)
>+	if (flush_domains & I915_GEM_DOMAIN_GTT)
> 		i915_gem_release_mmap(obj);
> 
> 	if (obj->base.pending_write_domain)
>-- 
>1.7.2.3
>

Actions: View | Splinter Review

Attachments on bug 34584: 43684 | 43685 | 43686 | 43687 | 44535 | 44552 | 44553 | 44574

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.