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

[PATCH 1/4] 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.)

v2: Keep pages pinning inside struct_mutex for the moment.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f9bf0d508053..b254094c689c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12789,14 +12789,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)
@@ -13159,9 +13152,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);
 
@@ -13335,32 +13326,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,
@@ -13399,6 +13364,33 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	if (!obj)
 		return 0;
 
+	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);
+	if (ret)
+		return ret;
+
 	if (!new_state->fence) { /* implicit fencing */
 		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
 						      obj->resv, NULL,
@@ -13406,8 +13398,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;
@@ -13430,8 +13420,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
@@ -13601,7 +13594,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,
@@ -13659,8 +13652,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");
@@ -13683,7 +13674,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));
@@ -13695,7 +13686,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/4] 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>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 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 b254094c689c..8897ad369a85 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13000,7 +13000,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);
@@ -13152,13 +13168,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)
@@ -13166,7 +13181,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
@@ -13208,6 +13223,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
@@ -13283,7 +13315,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

[PATCH 3/4] drm/i915: Make i915_gem_object_phys_attach() use obj->mm.lock more appropriately

[Chris Wilson]

Actually transferring from shmemfs to the physically contiguous set of
pages should be wholly guarded by its obj->mm.lock!

v2: Remember to free the old pages.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 44 +++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 31cbe78171a9..7f3be5deeb5e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -574,7 +574,8 @@ int
 i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
 			    int align)
 {
-	int ret;
+	struct sg_table *pages;
+	int err;
 
 	if (align > obj->base.size)
 		return -EINVAL;
@@ -582,32 +583,45 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
 	if (obj->ops == &i915_gem_phys_ops)
 		return 0;
 
-	if (obj->mm.madv != I915_MADV_WILLNEED)
-		return -EFAULT;
-
-	if (obj->base.filp == NULL)
+	if (obj->ops != &i915_gem_object_ops)
 		return -EINVAL;
 
-	ret = i915_gem_object_unbind(obj);
-	if (ret)
-		return ret;
+	err = i915_gem_object_unbind(obj);
+	if (err)
+		return err;
+
+	mutex_lock(&obj->mm.lock);
+
+	if (obj->mm.quirked) {
+		err = -EFAULT;
+		goto err_unlock;
+	}
 
-	__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
-	if (obj->mm.pages)
-		return -EBUSY;
+	if (obj->mm.mapping) {
+		err = -EBUSY;
+		goto err_unlock;
+	}
 
-	GEM_BUG_ON(obj->ops != &i915_gem_object_ops);
+	pages = obj->mm.pages;
 	obj->ops = &i915_gem_phys_ops;
 
-	ret = i915_gem_object_pin_pages(obj);
-	if (ret)
+	err = __i915_gem_object_get_pages(obj);
+	if (err)
 		goto err_xfer;
 
+	/* Perma-pin (until release) the physical set of pages */
+	__i915_gem_object_pin_pages(obj);
+
+	i915_gem_object_ops.put_pages(obj, pages);
+	mutex_unlock(&obj->mm.lock);
 	return 0;
 
 err_xfer:
 	obj->ops = &i915_gem_object_ops;
-	return ret;
+	obj->mm.pages = pages;
+err_unlock:
+	mutex_unlock(&obj->mm.lock);
+	return err;
 }
 
 static int
-- 
2.11.0

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

[PATCH 4/4] drm/i915: Pin the pages before acquiring struct_mutex for display

[Chris Wilson]

Since we don't need the struct_mutex to acquire the object's pages, call
i915_gem_object_pin_pages() before we bind the object into the GGTT.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8897ad369a85..8c2a0b04ad4a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13397,6 +13397,10 @@ 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);
@@ -13421,6 +13425,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	}
 
 	mutex_unlock(&dev_priv->drm.struct_mutex);
+	i915_gem_object_unpin_pages(obj);
 	if (ret)
 		return ret;
 
-- 
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/4] drm/i915: Trim struct_mutex usage for kms

[Patchwork]

== Series Details ==

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

== Summary ==

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

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test kms_busy:
        Subgroup basic-flip-default-b:
                dmesg-warn -> FAIL       (fi-skl-6700hq) fdo#101144
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#100461

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144
fdo#100461 https://bugs.freedesktop.org/show_bug.cgi?id=100461

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:440s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:582s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:510s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:493s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:487s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:586s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:431s
fi-hsw-4770r     total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:278  pass:227  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:18  time:492s
fi-ivb-3770      total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:18  time:470s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:465s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:572s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:579s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:456s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:400s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:463s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:434s
fi-snb-2520m     total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:0   skip:29  time:405s

cafd1e4df1e6e039268c4e4b1a55c88915d21f2e drm-tip: 2017y-06m-14d-19h-56m-24s UTC integration manifest
d1bb9b8 drm/i915: Pin the pages before acquiring struct_mutex for display
fbed565 drm/i915: Make i915_gem_object_phys_attach() use obj->mm.lock more appropriately
c754a4b drm/i915: Defer cleanup of active KMS state
89958cf drm/i915: Trim struct_mutex usage for kms

== Logs ==

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

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

[Maarten Lankhorst]

Op 15-06-17 om 10:25 schreef 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>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 53 +++++++++++++++++++++++++++++-------
>  1 file changed, 43 insertions(+), 10 deletions(-)
Will this still pass the kms_cursor_legacy tests?

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

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

[Maarten Lankhorst]

Op 15-06-17 om 10:25 schreef 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.)
>
> v2: Keep pages pinning inside struct_mutex for the moment.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++--------------------
>  1 file changed, 36 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f9bf0d508053..b254094c689c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> <snip>
> -	if (!obj && !old_obj)
> -		return 0;
^This needs to be a bugfix commit on its own, since it harmonizes fence waiting with drm_atomic_helper_wait_for_fences.


>  	if (old_obj) {
>  		struct drm_crtc_state *crtc_state =
>  			drm_atomic_get_existing_crtc_state(new_state->state,
> @@ -13399,6 +13364,33 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	if (!obj)
>  		return 0;
>  
> +	ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
> +	if (ret) {
> +		i915_gem_object_unpin_pages(obj);
Should be in patch 4?

Otherwise patches look ok. I can't comment on patch 3, but with comments fixed and assuming kms_cursor_legacy still works.. for patch 1, 2, 4:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

Re: [PATCH 3/4] drm/i915: Make i915_gem_object_phys_attach() use obj->mm.lock more appropriately

[Ville Syrjälä]

On Thu, Jun 15, 2017 at 09:25:08AM +0100, Chris Wilson wrote:
> Actually transferring from shmemfs to the physically contiguous set of
> pages should be wholly guarded by its obj->mm.lock!
> 
> v2: Remember to free the old pages.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 44 +++++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 31cbe78171a9..7f3be5deeb5e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -574,7 +574,8 @@ int
>  i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>  			    int align)
>  {
> -	int ret;
> +	struct sg_table *pages;
> +	int err;
>  
>  	if (align > obj->base.size)
>  		return -EINVAL;
> @@ -582,32 +583,45 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>  	if (obj->ops == &i915_gem_phys_ops)
>  		return 0;
>  
> -	if (obj->mm.madv != I915_MADV_WILLNEED)
> -		return -EFAULT;
> -
> -	if (obj->base.filp == NULL)
> +	if (obj->ops != &i915_gem_object_ops)
>  		return -EINVAL;
>  
> -	ret = i915_gem_object_unbind(obj);
> -	if (ret)
> -		return ret;
> +	err = i915_gem_object_unbind(obj);
> +	if (err)
> +		return err;
> +
> +	mutex_lock(&obj->mm.lock);
> +
> +	if (obj->mm.quirked) {
> +		err = -EFAULT;
> +		goto err_unlock;
> +	}
>  
> -	__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
> -	if (obj->mm.pages)
> -		return -EBUSY;
> +	if (obj->mm.mapping) {
> +		err = -EBUSY;
> +		goto err_unlock;
> +	}
>  
> -	GEM_BUG_ON(obj->ops != &i915_gem_object_ops);
> +	pages = obj->mm.pages;
>  	obj->ops = &i915_gem_phys_ops;
>  
> -	ret = i915_gem_object_pin_pages(obj);
> -	if (ret)
> +	err = __i915_gem_object_get_pages(obj);
> +	if (err)
>  		goto err_xfer;
>  
> +	/* Perma-pin (until release) the physical set of pages */
> +	__i915_gem_object_pin_pages(obj);
> +
> +	i915_gem_object_ops.put_pages(obj, pages);

Leak sorted, so this lgtm.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	mutex_unlock(&obj->mm.lock);
>  	return 0;
>  
>  err_xfer:
>  	obj->ops = &i915_gem_object_ops;
> -	return ret;
> +	obj->mm.pages = pages;
> +err_unlock:
> +	mutex_unlock(&obj->mm.lock);
> +	return err;
>  }
>  
>  static int
> -- 
> 2.11.0

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