[PATCH 1/2] drm/i915: Trim struct_mutex usage for kms

[PATCH 1/2] drm/i915: Trim struct_mutex usage for kms

[Chris Wilson]

Reduce acquisition of struct_mutex to the critical regions that must
hold it; for KMS, we need struct_mutex currently only for the purpose of
pinning/unpinning the framebuffer's VMA into the global GTT. This allows
us to avoid taking the struct_mutex when disabling the CRTC (i.e. NULL
framebuffer objects) before a reset. (Not yet achieving the full goal of
avoiding the strut_mutex nesting, but good enough to break the first
half of the reset deadlock.)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 85 +++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 25390dd8e08e..121fdd278fcd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12771,14 +12771,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 			flush_workqueue(dev_priv->wq);
 	}
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
-
-	ret = drm_atomic_helper_prepare_planes(dev, state);
-	mutex_unlock(&dev->struct_mutex);
-
-	return ret;
+	return drm_atomic_helper_prepare_planes(dev, state);
 }
 
 u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
@@ -13141,9 +13134,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
 	}
 
-	mutex_lock(&dev->struct_mutex);
 	drm_atomic_helper_cleanup_planes(dev, state);
-	mutex_unlock(&dev->struct_mutex);
 
 	drm_atomic_helper_commit_cleanup_done(state);
 
@@ -13317,32 +13308,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
 	int ret;
 
-	if (obj) {
-		if (plane->type == DRM_PLANE_TYPE_CURSOR &&
-		    INTEL_INFO(dev_priv)->cursor_needs_physical) {
-			const int align = intel_cursor_alignment(dev_priv);
-
-			ret = i915_gem_object_attach_phys(obj, align);
-			if (ret) {
-				DRM_DEBUG_KMS("failed to attach phys object\n");
-				return ret;
-			}
-		} else {
-			struct i915_vma *vma;
-
-			vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
-			if (IS_ERR(vma)) {
-				DRM_DEBUG_KMS("failed to pin object\n");
-				return PTR_ERR(vma);
-			}
-
-			to_intel_plane_state(new_state)->vma = vma;
-		}
-	}
-
-	if (!obj && !old_obj)
-		return 0;
-
 	if (old_obj) {
 		struct drm_crtc_state *crtc_state =
 			drm_atomic_get_existing_crtc_state(new_state->state,
@@ -13381,6 +13346,38 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	if (!obj)
 		return 0;
 
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		return ret;
+
+	ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
+	if (ret) {
+		i915_gem_object_unpin_pages(obj);
+		return ret;
+	}
+
+	i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
+
+	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
+	    INTEL_INFO(dev_priv)->cursor_needs_physical) {
+		const int align = intel_cursor_alignment(dev_priv);
+
+		ret = i915_gem_object_attach_phys(obj, align);
+	} else {
+		struct i915_vma *vma;
+
+		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+		if (!IS_ERR(vma))
+			to_intel_plane_state(new_state)->vma = vma;
+		else
+			ret =  PTR_ERR(vma);
+	}
+
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+	i915_gem_object_unpin_pages(obj);
+	if (ret)
+		return ret;
+
 	if (!new_state->fence) { /* implicit fencing */
 		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
 						      obj->resv, NULL,
@@ -13388,8 +13385,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 						      GFP_KERNEL);
 		if (ret < 0)
 			return ret;
-
-		i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
 	}
 
 	return 0;
@@ -13412,8 +13407,11 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 
 	/* Should only be called after a successful intel_prepare_plane_fb()! */
 	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
-	if (vma)
+	if (vma) {
+		mutex_lock(&plane->dev->struct_mutex);
 		intel_unpin_fb_vma(vma);
+		mutex_unlock(&plane->dev->struct_mutex);
+	}
 }
 
 int
@@ -13583,7 +13581,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *old_fb;
 	struct drm_crtc_state *crtc_state = crtc->state;
-	struct i915_vma *old_vma;
+	struct i915_vma *old_vma, *vma;
 
 	/*
 	 * When crtc is inactive or there is a modeset pending,
@@ -13641,8 +13639,6 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 			goto out_unlock;
 		}
 	} else {
-		struct i915_vma *vma;
-
 		vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
@@ -13665,7 +13661,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	*to_intel_plane_state(old_plane_state) = *to_intel_plane_state(new_plane_state);
 	new_plane_state->fence = NULL;
 	new_plane_state->fb = old_fb;
-	to_intel_plane_state(new_plane_state)->vma = old_vma;
+	to_intel_plane_state(new_plane_state)->vma = NULL;
 
 	if (plane->state->visible) {
 		trace_intel_update_plane(plane, to_intel_crtc(crtc));
@@ -13677,7 +13673,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
 	}
 
-	intel_cleanup_plane_fb(plane, new_plane_state);
+	if (old_vma)
+		intel_unpin_fb_vma(old_vma);
 
 out_unlock:
 	mutex_unlock(&dev_priv->drm.struct_mutex);
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Defer cleanup of active KMS state

[Chris Wilson]

During cleanup we release the VMA of the previous framebuffer. This
requires taking a struct_mutex, and potential recursion when handling a
reset. A simple device here is to move that locking into its own work
and we can avoid blocking on it for the reset by waiting on the flip
completion (and not cleanup completion) instead. Ideally this reduces
the duration of a blocking KMS operation by offloading the cleanup. Once
again we dream of the legendary vblank worker.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 53 +++++++++++++++++++++++++++++-------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 121fdd278fcd..0a1170de96e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12982,7 +12982,23 @@ static void intel_atomic_helper_free_state_worker(struct work_struct *work)
 	intel_atomic_helper_free_state(dev_priv);
 }
 
-static void intel_atomic_commit_tail(struct drm_atomic_state *state)
+static void intel_atomic_commit_cleanup(struct work_struct *work)
+{
+	struct drm_atomic_state *state =
+		container_of(work, struct drm_atomic_state, commit_work);
+	struct drm_device *dev = state->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	drm_atomic_helper_cleanup_planes(dev, state);
+	drm_atomic_helper_commit_cleanup_done(state);
+
+	drm_atomic_state_put(state);
+
+	intel_atomic_helper_free_state(dev_priv);
+}
+
+static void intel_atomic_commit_tail(struct drm_atomic_state *state,
+				     bool nonblock)
 {
 	struct drm_device *dev = state->dev;
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
@@ -13134,13 +13150,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
 	}
 
-	drm_atomic_helper_cleanup_planes(dev, state);
-
-	drm_atomic_helper_commit_cleanup_done(state);
-
-	drm_atomic_state_put(state);
-
-	intel_atomic_helper_free_state(dev_priv);
+	if (!nonblock) {
+		INIT_WORK(&state->commit_work, intel_atomic_commit_cleanup);
+		schedule_work(&state->commit_work);
+	} else {
+		intel_atomic_commit_cleanup(&state->commit_work);
+	}
 }
 
 static void intel_atomic_commit_work(struct work_struct *work)
@@ -13148,7 +13163,7 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	struct drm_atomic_state *state =
 		container_of(work, struct drm_atomic_state, commit_work);
 
-	intel_atomic_commit_tail(state);
+	intel_atomic_commit_tail(state, true);
 }
 
 static int __i915_sw_fence_call
@@ -13190,6 +13205,23 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
 				  to_intel_plane(plane)->frontbuffer_bit);
 }
 
+static void intel_atomic_state_wait_for_flips(struct intel_atomic_state *state)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i;
+
+	for_each_new_crtc_in_state(&state->base, crtc, crtc_state, i) {
+		struct drm_crtc_commit *commit = state->base.crtcs[i].commit;
+		long ret;
+
+		ret = wait_for_completion_timeout(&commit->flip_done, HZ);
+		if (ret == 0)
+			DRM_ERROR("[CRTC:%d:%s] flip_done timed out\n",
+				  crtc->base.id, crtc->name);
+	}
+}
+
 /**
  * intel_atomic_commit - commit validated state object
  * @dev: DRM device
@@ -13265,7 +13297,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 	i915_sw_fence_commit(&intel_state->commit_ready);
 	if (!nonblock) {
 		i915_sw_fence_wait(&intel_state->commit_ready);
-		intel_atomic_commit_tail(state);
+		intel_atomic_commit_tail(state, false);
+		intel_atomic_state_wait_for_flips(intel_state);
 	}
 
 	return 0;
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Trim struct_mutex usage for kms

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Trim struct_mutex usage for kms
URL   : https://patchwork.freedesktop.org/series/25465/
State : success

== Summary ==

Series 25465v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/25465/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-r) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:444s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:433s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:579s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:516s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:485s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:481s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:596s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:438s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:416s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:577s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:578s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:460s
fi-skl-6700hq    total:278  pass:229  dwarn:1   dfail:0   fail:26  skip:22  time:408s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:437s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:534s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:403s
fi-skl-6700k failed to collect. IGT log at Patchwork_4906/fi-skl-6700k/igt.log

24db004734623b21acb25f58084e5b8eadc4f908 drm-tip: 2017y-06m-08d-09h-22m-33s UTC integration manifest
64e3e2b drm/i915: Defer cleanup of active KMS state
e94d6d0 drm/i915: Trim struct_mutex usage for kms

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4906/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Trim struct_mutex usage for kms

[Ville Syrjälä]

On Thu, Jun 08, 2017 at 10:51:34AM +0100, Chris Wilson wrote:
> Reduce acquisition of struct_mutex to the critical regions that must
> hold it; for KMS, we need struct_mutex currently only for the purpose of
> pinning/unpinning the framebuffer's VMA into the global GTT. This allows
> us to avoid taking the struct_mutex when disabling the CRTC (i.e. NULL
> framebuffer objects) before a reset. (Not yet achieving the full goal of
> avoiding the strut_mutex nesting, but good enough to break the first
> half of the reset deadlock.)
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 85 +++++++++++++++++-------------------
>  1 file changed, 41 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 25390dd8e08e..121fdd278fcd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12771,14 +12771,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
>  			flush_workqueue(dev_priv->wq);
>  	}
>  
> -	ret = mutex_lock_interruptible(&dev->struct_mutex);
> -	if (ret)
> -		return ret;
> -
> -	ret = drm_atomic_helper_prepare_planes(dev, state);
> -	mutex_unlock(&dev->struct_mutex);
> -
> -	return ret;
> +	return drm_atomic_helper_prepare_planes(dev, state);
>  }
>  
>  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
> @@ -13141,9 +13134,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>  	}
>  
> -	mutex_lock(&dev->struct_mutex);
>  	drm_atomic_helper_cleanup_planes(dev, state);
> -	mutex_unlock(&dev->struct_mutex);
>  
>  	drm_atomic_helper_commit_cleanup_done(state);
>  
> @@ -13317,32 +13308,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
>  	int ret;
>  
> -	if (obj) {
> -		if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> -		    INTEL_INFO(dev_priv)->cursor_needs_physical) {
> -			const int align = intel_cursor_alignment(dev_priv);
> -
> -			ret = i915_gem_object_attach_phys(obj, align);
> -			if (ret) {
> -				DRM_DEBUG_KMS("failed to attach phys object\n");
> -				return ret;
> -			}
> -		} else {
> -			struct i915_vma *vma;
> -
> -			vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> -			if (IS_ERR(vma)) {
> -				DRM_DEBUG_KMS("failed to pin object\n");
> -				return PTR_ERR(vma);
> -			}
> -
> -			to_intel_plane_state(new_state)->vma = vma;
> -		}
> -	}
> -
> -	if (!obj && !old_obj)
> -		return 0;
> -
>  	if (old_obj) {
>  		struct drm_crtc_state *crtc_state =
>  			drm_atomic_get_existing_crtc_state(new_state->state,
> @@ -13381,6 +13346,38 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	if (!obj)
>  		return 0;
>  
> +	ret = i915_gem_object_pin_pages(obj);
> +	if (ret)
> +		return ret;
> +
> +	ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
> +	if (ret) {
> +		i915_gem_object_unpin_pages(obj);
> +		return ret;
> +	}
> +
> +	i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);

Does this guy need struct mutex? Maybe it could use a lockdep assert if
so.

> +
> +	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> +	    INTEL_INFO(dev_priv)->cursor_needs_physical) {
> +		const int align = intel_cursor_alignment(dev_priv);
> +
> +		ret = i915_gem_object_attach_phys(obj, align);

Isn't this going to fail due to the i915_gem_object_pin_pages() above?

> +	} else {
> +		struct i915_vma *vma;
> +
> +		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> +		if (!IS_ERR(vma))
> +			to_intel_plane_state(new_state)->vma = vma;
> +		else
> +			ret =  PTR_ERR(vma);
> +	}
> +
> +	mutex_unlock(&dev_priv->drm.struct_mutex);
> +	i915_gem_object_unpin_pages(obj);
> +	if (ret)
> +		return ret;
> +
>  	if (!new_state->fence) { /* implicit fencing */
>  		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
>  						      obj->resv, NULL,
> @@ -13388,8 +13385,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  						      GFP_KERNEL);
>  		if (ret < 0)
>  			return ret;
> -
> -		i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
>  	}
>  
>  	return 0;
> @@ -13412,8 +13407,11 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  
>  	/* Should only be called after a successful intel_prepare_plane_fb()! */
>  	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
> -	if (vma)
> +	if (vma) {
> +		mutex_lock(&plane->dev->struct_mutex);
>  		intel_unpin_fb_vma(vma);
> +		mutex_unlock(&plane->dev->struct_mutex);
> +	}
>  }
>  
>  int
> @@ -13583,7 +13581,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_framebuffer *old_fb;
>  	struct drm_crtc_state *crtc_state = crtc->state;
> -	struct i915_vma *old_vma;
> +	struct i915_vma *old_vma, *vma;
>  
>  	/*
>  	 * When crtc is inactive or there is a modeset pending,
> @@ -13641,8 +13639,6 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  			goto out_unlock;
>  		}
>  	} else {
> -		struct i915_vma *vma;
> -
>  		vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation);
>  		if (IS_ERR(vma)) {
>  			DRM_DEBUG_KMS("failed to pin object\n");
> @@ -13665,7 +13661,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  	*to_intel_plane_state(old_plane_state) = *to_intel_plane_state(new_plane_state);
>  	new_plane_state->fence = NULL;
>  	new_plane_state->fb = old_fb;
> -	to_intel_plane_state(new_plane_state)->vma = old_vma;
> +	to_intel_plane_state(new_plane_state)->vma = NULL;
>  
>  	if (plane->state->visible) {
>  		trace_intel_update_plane(plane, to_intel_crtc(crtc));
> @@ -13677,7 +13673,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  		intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
>  	}
>  
> -	intel_cleanup_plane_fb(plane, new_plane_state);
> +	if (old_vma)
> +		intel_unpin_fb_vma(old_vma);
>  
>  out_unlock:
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Trim struct_mutex usage for kms

[Chris Wilson]

Quoting Ville Syrjälä (2017-06-08 14:47:27)
> On Thu, Jun 08, 2017 at 10:51:34AM +0100, Chris Wilson wrote:
> > +     ret = i915_gem_object_pin_pages(obj);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
> > +     if (ret) {
> > +             i915_gem_object_unpin_pages(obj);
> > +             return ret;
> > +     }
> > +
> > +     i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
> 
> Does this guy need struct mutex? Maybe it could use a lockdep assert if
> so.

It does today. One day it could be reduced to a obj->resv->lock and a
global spinlock.
> 
> > +
> > +     if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> > +         INTEL_INFO(dev_priv)->cursor_needs_physical) {
> > +             const int align = intel_cursor_alignment(dev_priv);
> > +
> > +             ret = i915_gem_object_attach_phys(obj, align);
> 
> Isn't this going to fail due to the i915_gem_object_pin_pages() above?

Yes, so I rewrote attach_phys.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx