drm/i915: Wait for completion of pending flips when starved of fences

On older generations (gen2, gen3) the GPU requires fences for many operations, such as blits. The display hardware also requires fences for scanouts and this leads to a situation where an arbitrary number of fences may be pinned by old scanouts following a pageflip but before we have executed the unpin workqueue. This is unpredictable by userspace and leads to random EDEADLK when submitting an otherwise benign execbuffer. However, we can detect when we have an outstanding flip and so cause userspace to wait upon their completion before finally declaring that the system is starved of fences. This is really no worse than forcing the GPU to stall waiting for older execbuffer to retire and release their fences before we can reallocate them for the next execbuffer. v2: move the test for a pending fb unpin to a common routine for later reuse during eviction Reported-and-tested-by: dimon@gmx.net Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73696 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

drm/i915: Wait for completion of pending flips when starved of fences
On older generations (gen2, gen3) the GPU requires fences for many operations, such as blits. The display hardware also requires fences for scanouts and this leads to a situation where an arbitrary number of fences may be pinned by old scanouts following a pageflip but before we have executed the unpin workqueue. This is unpredictable by userspace and leads to random EDEADLK when submitting an otherwise benign execbuffer. However, we can detect when we have an outstanding flip and so cause userspace to wait upon their completion before finally declaring that the system is starved of fences. This is really no worse than forcing the GPU to stall waiting for older execbuffer to retire and release their fences before we can reallocate them for the next execbuffer. v2: move the test for a pending fb unpin to a common routine for later reuse during eviction Reported-and-tested-by: dimon@gmx.net Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73696 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Chris Wilson · Daniel Vetter
1 parent 2e82a72031
Showing 3 changed files with 34 additions and 4 deletions Side-by-side Diff
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_drv.h
@@ -3106,7 +3106,7 @@
 	}
  
 	if (avail == NULL)
-		return NULL;
+		goto deadlock;
  
 	/* None available, try to steal one or wait for a user to finish */
 	list_for_each_entry(reg, &dev_priv->mm.fence_list, lru_list) {
@@ -3116,7 +3116,12 @@
 		return reg;
 	}
  
-	return NULL;
+deadlock:
+	/* Wait for completion of pending flips which consume fences */
+	if (intel_has_pending_fb_unpin(dev))
+		return ERR_PTR(-EAGAIN);
+
+	return ERR_PTR(-EDEADLK);
 }
  
 /**
@@ -3161,8 +3166,8 @@
 		}
 	} else if (enable) {
 		reg = i915_find_fence_reg(dev);
-		if (reg == NULL)
-			return -EDEADLK;
+		if (IS_ERR(reg))
+			return PTR_ERR(reg);
  
 		if (reg->obj) {
 			struct drm_i915_gem_object *old = reg->obj;
@@ -2982,6 +2982,30 @@
 	return pending;
 }
  
+bool intel_has_pending_fb_unpin(struct drm_device *dev)
+{
+	struct intel_crtc *crtc;
+
+	/* Note that we don't need to be called with mode_config.lock here
+	 * as our list of CRTC objects is static for the lifetime of the
+	 * device and so cannot disappear as we iterate. Similarly, we can
+	 * happily treat the predicates as racy, atomic checks as userspace
+	 * cannot claim and pin a new fb without at least acquring the
+	 * struct_mutex and so serialising with us.
+	 */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
+		if (atomic_read(&crtc->unpin_work_count) == 0)
+			continue;
+
+		if (crtc->unpin_work)
+			intel_wait_for_vblank(dev, crtc->pipe);
+
+		return true;
+	}
+
+	return false;
+}
+
 static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -626,6 +626,7 @@
  
 /* intel_display.c */
 const char *intel_output_name(int output);
+bool intel_has_pending_fb_unpin(struct drm_device *dev);
 int intel_pch_rawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
...	...	@@ -3106,7 +3106,7 @@
3106	3106	}
3107	3107
3108	3108	if (avail == NULL)
3109		- return NULL;
	3109	+ goto deadlock;
3110	3110
3111	3111	/* None available, try to steal one or wait for a user to finish */
3112	3112	list_for_each_entry(reg, &dev_priv->mm.fence_list, lru_list) {
...	...	@@ -3116,7 +3116,12 @@
3116	3116	return reg;
3117	3117	}
3118	3118
3119		- return NULL;
	3119	+deadlock:
	3120	+ /* Wait for completion of pending flips which consume fences */
	3121	+ if (intel_has_pending_fb_unpin(dev))
	3122	+ return ERR_PTR(-EAGAIN);
	3123	+
	3124	+ return ERR_PTR(-EDEADLK);
3120	3125	}
3121	3126
3122	3127	/**
...	...	@@ -3161,8 +3166,8 @@
3161	3166	}
3162	3167	} else if (enable) {
3163	3168	reg = i915_find_fence_reg(dev);
3164		- if (reg == NULL)
3165		- return -EDEADLK;
	3169	+ if (IS_ERR(reg))
	3170	+ return PTR_ERR(reg);
3166	3171
3167	3172	if (reg->obj) {
3168	3173	struct drm_i915_gem_object *old = reg->obj;
...	...	@@ -2982,6 +2982,30 @@
2982	2982	return pending;
2983	2983	}
2984	2984
	2985	+bool intel_has_pending_fb_unpin(struct drm_device *dev)
	2986	+{
	2987	+ struct intel_crtc *crtc;
	2988	+
	2989	+ /* Note that we don't need to be called with mode_config.lock here
	2990	+ * as our list of CRTC objects is static for the lifetime of the
	2991	+ * device and so cannot disappear as we iterate. Similarly, we can
	2992	+ * happily treat the predicates as racy, atomic checks as userspace
	2993	+ * cannot claim and pin a new fb without at least acquring the
	2994	+ * struct_mutex and so serialising with us.
	2995	+ */
	2996	+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
	2997	+ if (atomic_read(&crtc->unpin_work_count) == 0)
	2998	+ continue;
	2999	+
	3000	+ if (crtc->unpin_work)
	3001	+ intel_wait_for_vblank(dev, crtc->pipe);
	3002	+
	3003	+ return true;
	3004	+ }
	3005	+
	3006	+ return false;
	3007	+}
	3008	+
2985	3009	static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
2986	3010	{
2987	3011	struct drm_device *dev = crtc->dev;
...	...	@@ -626,6 +626,7 @@
626	626
627	627	/* intel_display.c */
628	628	const char *intel_output_name(int output);
	629	+bool intel_has_pending_fb_unpin(struct drm_device *dev);
629	630	int intel_pch_rawclk(struct drm_device *dev);
630	631	void intel_mark_busy(struct drm_device *dev);
631	632	void intel_mark_fb_busy(struct drm_i915_gem_object *obj,