diff mbox

[v2] drm/i915: Drop i915_gem_obj_is_pinned() from set-cache-level

Message ID 1444396287-23268-1-git-send-email-chris@chris-wilson.co.uk
State New, archived
Headers show

Commit Message

Chris Wilson Oct. 9, 2015, 1:11 p.m. UTC
Since the remove of the pin-ioctl, we only care about not changing the
cache level on buffers pinned to the hardware as indicated by
obj->pin_display. By knowing that only objects pinned to the hardware
will have an elevated vma->pin_count, so we can coallesce many of the
linear walks over the obj->vma_list.

v2: Try and retrospectively add comments explaining the steps in
rebinding the active VMA.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 99 ++++++++++++++++++++++++++++++++---------
 1 file changed, 78 insertions(+), 21 deletions(-)

Comments

kernel test robot Oct. 9, 2015, 1:39 p.m. UTC | #1
Hi Chris,

[auto build test WARNING on next-20151009 -- if it's inappropriate base, please ignore]

reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/drm/drm_crtc.h:1162: warning: No description found for parameter 'dirty_info_property'
   include/drm/drm_crtc.h:1162: warning: No description found for parameter 'suggested_x_property'
   include/drm/drm_crtc.h:1162: warning: No description found for parameter 'suggested_y_property'
   include/drm/drm_crtc.h:1162: warning: No description found for parameter 'allow_fb_modifiers'
   include/drm/drm_fb_helper.h:148: warning: No description found for parameter 'connector_info'
   include/drm/drm_dp_helper.h:713: warning: No description found for parameter 'i2c_nack_count'
   include/drm/drm_dp_helper.h:713: warning: No description found for parameter 'i2c_defer_count'
   drivers/gpu/drm/drm_dp_mst_topology.c:2226: warning: No description found for parameter 'connector'
   include/drm/drm_dp_mst_helper.h:97: warning: No description found for parameter 'cached_edid'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'max_dpcd_transaction_bytes'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'sink_count'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'total_slots'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'avail_slots'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'total_pbn'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'qlock'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_msg_downq'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_msg_upq'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_down_in_progress'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_up_in_progress'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'payload_lock'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'proposed_vcpis'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'payloads'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'payload_mask'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'vcpi_mask'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_waitq'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'work'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'tx_work'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'destroy_connector_list'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'destroy_connector_lock'
   include/drm/drm_dp_mst_helper.h:471: warning: No description found for parameter 'destroy_connector_work'
   drivers/gpu/drm/drm_dp_mst_topology.c:2226: warning: No description found for parameter 'connector'
   drivers/gpu/drm/drm_irq.c:173: warning: No description found for parameter 'flags'
   include/drm/drmP.h:164: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:180: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:198: warning: No description found for parameter 'fmt'
   include/drm/drmP.h:238: warning: No description found for parameter 'dev'
   include/drm/drmP.h:238: warning: No description found for parameter 'data'
   include/drm/drmP.h:238: warning: No description found for parameter 'file_priv'
   include/drm/drmP.h:271: warning: No description found for parameter 'ioctl'
   include/drm/drmP.h:271: warning: No description found for parameter '_func'
   include/drm/drmP.h:271: warning: No description found for parameter '_flags'
   include/drm/drmP.h:344: warning: cannot understand function prototype: 'struct drm_lock_data '
   include/drm/drmP.h:397: warning: cannot understand function prototype: 'struct drm_driver '
   include/drm/drmP.h:647: warning: cannot understand function prototype: 'struct drm_info_list '
   include/drm/drmP.h:657: warning: cannot understand function prototype: 'struct drm_info_node '
   include/drm/drmP.h:667: warning: cannot understand function prototype: 'struct drm_minor '
   include/drm/drmP.h:715: warning: cannot understand function prototype: 'struct drm_device '
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'wedged'
   drivers/gpu/drm/i915/i915_irq.c:2582: warning: No description found for parameter 'fmt'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:421: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:686: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'args'
   drivers/gpu/drm/i915/i915_gem.c:767: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1029: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1194: warning: No description found for parameter 'rps'
   drivers/gpu/drm/i915/i915_gem.c:1400: warning: No description found for parameter 'req'
   drivers/gpu/drm/i915/i915_gem.c:1435: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:1435: warning: No description found for parameter 'readonly'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1558: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1621: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:1666: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'size'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'tiling_mode'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: No description found for parameter 'fenced'
   drivers/gpu/drm/i915/i915_gem.c:1954: warning: Excess function parameter 'obj' description in 'i915_gem_get_gtt_alignment'
   drivers/gpu/drm/i915/i915_gem.c:2816: warning: No description found for parameter 'ring'
   drivers/gpu/drm/i915/i915_gem.c:2945: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:2995: warning: No description found for parameter 'dev'
   drivers/gpu/drm/i915/i915_gem.c:2995: warning: No description found for parameter 'data'
   drivers/gpu/drm/i915/i915_gem.c:2995: warning: No description found for parameter 'file'
   drivers/gpu/drm/i915/i915_gem.c:2995: warning: Excess function parameter 'DRM_IOCTL_ARGS' description in 'i915_gem_wait_ioctl'
   drivers/gpu/drm/i915/i915_gem.c:3364: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3364: warning: No description found for parameter 'vm'
   drivers/gpu/drm/i915/i915_gem.c:3364: warning: No description found for parameter 'ggtt_view'
   drivers/gpu/drm/i915/i915_gem.c:3364: warning: No description found for parameter 'alignment'
   drivers/gpu/drm/i915/i915_gem.c:3364: warning: No description found for parameter 'flags'
   drivers/gpu/drm/i915/i915_gem.c:3599: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3599: warning: No description found for parameter 'write'
   drivers/gpu/drm/i915/i915_gem.c:3674: warning: No description found for parameter 'obj'
>> drivers/gpu/drm/i915/i915_gem.c:3674: warning: No description found for parameter 'cache_level'
   drivers/gpu/drm/i915/i915_gem.c:3948: warning: No description found for parameter 'obj'
   drivers/gpu/drm/i915/i915_gem.c:3948: warning: No description found for parameter 'write'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: No description found for parameter 'params'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'dev' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'file' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'ring' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'ctx' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'batch_obj' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'exec_start' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'dispatch_flags' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: No description found for parameter 'params'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'dev' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'file' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'ring' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'ctx' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'batch_obj' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'exec_start' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_lrc.c:871: warning: Excess function parameter 'dispatch_flags' description in 'intel_execlists_submission'
   drivers/gpu/drm/i915/intel_guc_loader.c:1: warning: no structured comments found
       Was looking for 'GuC-specific firmware loader'.
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: No description found for parameter 'rq'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ctx' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ring' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:741: warning: No description found for parameter 'ctx'
   drivers/gpu/drm/i915/i915_guc_submission.c:1: warning: no structured comments found
       Was looking for 'GuC-based command submissison'.
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: No description found for parameter 'rq'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ctx' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:598: warning: Excess function parameter 'ring' description in 'i915_guc_submit'
   drivers/gpu/drm/i915/i915_guc_submission.c:741: warning: No description found for parameter 'ctx'
   Warning: didn't use docs for i915_hotplug_interrupt_update
   Warning: didn't use docs for ilk_update_display_irq
   Warning: didn't use docs for ilk_update_gt_irq
   Warning: didn't use docs for snb_update_pm_irq
   Warning: didn't use docs for bdw_update_port_irq
   Warning: didn't use docs for ibx_display_interrupt_update
   Warning: didn't use docs for i915_enable_asle_pipestat
   Warning: didn't use docs for ivybridge_parity_work
   Warning: didn't use docs for i915_reset_and_wakeup
   Warning: didn't use docs for i915_handle_error
   Warning: didn't use docs for intel_irq_install
   Warning: didn't use docs for intel_irq_uninstall

vim +/cache_level +3674 drivers/gpu/drm/i915/i915_gem.c

e47c68e9 Eric Anholt   2008-11-14  3658  
fc05080c Chris Wilson  2015-10-09  3659  /**
fc05080c Chris Wilson  2015-10-09  3660   * Changes the cache-level of an object across all VMA.
fc05080c Chris Wilson  2015-10-09  3661   *
fc05080c Chris Wilson  2015-10-09  3662   * After this function returns, the object will be in the new cache-level
fc05080c Chris Wilson  2015-10-09  3663   * across all GTT and the contents of the backing storage will be coherent,
fc05080c Chris Wilson  2015-10-09  3664   * with respect to the new cache-level. In order to keep the backing storage
fc05080c Chris Wilson  2015-10-09  3665   * coherent for all users, we only allow a single cache level to be set
fc05080c Chris Wilson  2015-10-09  3666   * globally on the object and prevent it from being changed whilst the
fc05080c Chris Wilson  2015-10-09  3667   * hardware is reading from the object. That is if the object is currently
fc05080c Chris Wilson  2015-10-09  3668   * on the scanout it will be set to uncached (or equivalent display
fc05080c Chris Wilson  2015-10-09  3669   * cache coherency) and all non-MOCS GPU access will also be uncached so
fc05080c Chris Wilson  2015-10-09  3670   * that all direct access to the scanout remains coherent.
fc05080c Chris Wilson  2015-10-09  3671   */
e4ffd173 Chris Wilson  2011-04-04  3672  int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
e4ffd173 Chris Wilson  2011-04-04  3673  				    enum i915_cache_level cache_level)
e4ffd173 Chris Wilson  2011-04-04 @3674  {
7bddb01f Daniel Vetter 2012-02-09  3675  	struct drm_device *dev = obj->base.dev;
df6f783a Chris Wilson  2014-03-21  3676  	struct i915_vma *vma, *next;
fc05080c Chris Wilson  2015-10-09  3677  	bool bound = false;
ed75a55b Ville Syrjälä 2015-08-11  3678  	int ret = 0;
e4ffd173 Chris Wilson  2011-04-04  3679  
e4ffd173 Chris Wilson  2011-04-04  3680  	if (obj->cache_level == cache_level)
ed75a55b Ville Syrjälä 2015-08-11  3681  		goto out;
e4ffd173 Chris Wilson  2011-04-04  3682  

:::::: The code at line 3674 was first introduced by commit
:::::: e4ffd173a1c2f96b43127c2537dd99d89e759bba drm/i915: Add an interface to dynamically change the cache level

:::::: TO: Chris Wilson <chris@chris-wilson.co.uk>
:::::: CC: Keith Packard <keithp@keithp.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Tvrtko Ursulin Oct. 9, 2015, 1:43 p.m. UTC | #2
On 09/10/15 14:11, Chris Wilson wrote:
> Since the remove of the pin-ioctl, we only care about not changing the
> cache level on buffers pinned to the hardware as indicated by
> obj->pin_display. By knowing that only objects pinned to the hardware
> will have an elevated vma->pin_count, so we can coallesce many of the
> linear walks over the obj->vma_list.
>
> v2: Try and retrospectively add comments explaining the steps in
> rebinding the active VMA.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>

Very nice!

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
Daniel Vetter Oct. 13, 2015, 1:52 p.m. UTC | #3
On Fri, Oct 09, 2015 at 02:43:21PM +0100, Tvrtko Ursulin wrote:
> 
> On 09/10/15 14:11, Chris Wilson wrote:
> >Since the remove of the pin-ioctl, we only care about not changing the
> >cache level on buffers pinned to the hardware as indicated by
> >obj->pin_display. By knowing that only objects pinned to the hardware
> >will have an elevated vma->pin_count, so we can coallesce many of the
> >linear walks over the obj->vma_list.
> >
> >v2: Try and retrospectively add comments explaining the steps in
> >rebinding the active VMA.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> 
> Very nice!
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Queued for -next, thanks for the patch.
-Daniel
Chris Wilson Oct. 14, 2015, 10:56 a.m. UTC | #4
On Tue, Oct 13, 2015 at 03:52:57PM +0200, Daniel Vetter wrote:
> On Fri, Oct 09, 2015 at 02:43:21PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 09/10/15 14:11, Chris Wilson wrote:
> > >Since the remove of the pin-ioctl, we only care about not changing the
> > >cache level on buffers pinned to the hardware as indicated by
> > >obj->pin_display. By knowing that only objects pinned to the hardware
> > >will have an elevated vma->pin_count, so we can coallesce many of the
> > >linear walks over the obj->vma_list.
> > >
> > >v2: Try and retrospectively add comments explaining the steps in
> > >rebinding the active VMA.
> > >
> > >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > 
> > Very nice!
> > 
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Queued for -next, thanks for the patch.

Note you also want

drm/i915: Move the mb() following release-mmap into release-mmap
http://patchwork.freedesktop.org/patch/61128/

to answer the question you posed in review.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d4a3bdf0c5b6..fb4cf0ab8c18 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3624,53 +3624,106 @@  i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 	return 0;
 }
 
+/**
+ * Changes the cache-level of an object across all VMA.
+ *
+ * After this function returns, the object will be in the new cache-level
+ * across all GTT and the contents of the backing storage will be coherent,
+ * with respect to the new cache-level. In order to keep the backing storage
+ * coherent for all users, we only allow a single cache level to be set
+ * globally on the object and prevent it from being changed whilst the
+ * hardware is reading from the object. That is if the object is currently
+ * on the scanout it will be set to uncached (or equivalent display
+ * cache coherency) and all non-MOCS GPU access will also be uncached so
+ * that all direct access to the scanout remains coherent.
+ */
 int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				    enum i915_cache_level cache_level)
 {
 	struct drm_device *dev = obj->base.dev;
 	struct i915_vma *vma, *next;
+	bool bound = false;
 	int ret = 0;
 
 	if (obj->cache_level == cache_level)
 		goto out;
 
-	if (i915_gem_obj_is_pinned(obj)) {
-		DRM_DEBUG("can not change the cache level of pinned objects\n");
-		return -EBUSY;
-	}
-
+	/* Inspect the list of currently bound VMA and unbind any that would
+	 * be invalid given the new cache-level. This is principally to
+	 * catch the issue of the CS prefetch crossing page boundaries and
+	 * reading an invalid PTE on older architectures.
+	 */
 	list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link) {
+		if (!drm_mm_node_allocated(&vma->node))
+			continue;
+
+		if (vma->pin_count) {
+			DRM_DEBUG("can not change the cache level of pinned objects\n");
+			return -EBUSY;
+		}
+
 		if (!i915_gem_valid_gtt_space(vma, cache_level)) {
 			ret = i915_vma_unbind(vma);
 			if (ret)
 				return ret;
-		}
+		} else
+			bound = true;
 	}
 
-	if (i915_gem_obj_bound_any(obj)) {
+	/* We can reuse the existing drm_mm nodes but need to change the
+	 * cache-level on the PTE. We could simply unbind them all and
+	 * rebind with the correct cache-level on next use. However since
+	 * we already have a valid slot, dma mapping, pages etc, we may as
+	 * rewrite the PTE in the belief that doing so tramples upon less
+	 * state and so involves less work.
+	 */
+	if (bound) {
+		/* Before we change the PTE, the GPU must not be accessing it.
+		 * If we wait upon the object, we know that all the bound
+		 * VMA are no longer active.
+		 */
 		ret = i915_gem_object_wait_rendering(obj, false);
 		if (ret)
 			return ret;
 
-		i915_gem_object_finish_gtt(obj);
-
-		/* Before SandyBridge, you could not use tiling or fence
-		 * registers with snooped memory, so relinquish any fences
-		 * currently pointing to our region in the aperture.
-		 */
-		if (INTEL_INFO(dev)->gen < 6) {
+		if (!HAS_LLC(dev) && cache_level != I915_CACHE_NONE) {
+			/* Access to snoopable pages through the GTT is
+			 * incoherent and on some machines causes a hard
+			 * lockup. Relinquish the CPU mmaping to force
+			 * userspace to refault in the pages and we can
+			 * then double check if the GTT mapping is still
+			 * valid for that pointer access.
+			 */
+			i915_gem_release_mmap(obj);
+
+			/* As we no longer need a fence for GTT access,
+			 * we can relinquish it now (and so prevent having
+			 * to steal a fence from someone else on the next
+			 * fence request). Note GPU activity would have
+			 * dropped the fence as all snoopable access is
+			 * supposed to be linear.
+			 */
 			ret = i915_gem_object_put_fence(obj);
 			if (ret)
 				return ret;
+		} else {
+			/* We either have incoherent backing store and
+			 * so no GTT access or the architecture is fully
+			 * coherent. In such cases, existing GTT mmaps
+			 * ignore the cache bit in the PTE and we can
+			 * rewrite it without confusing the GPU or having
+			 * to force userspace to fault back in its mmaps.
+			 */
 		}
 
-		list_for_each_entry(vma, &obj->vma_list, vma_link)
-			if (drm_mm_node_allocated(&vma->node)) {
-				ret = i915_vma_bind(vma, cache_level,
-						    PIN_UPDATE);
-				if (ret)
-					return ret;
-			}
+		list_for_each_entry(vma, &obj->vma_list, vma_link) {
+			if (!drm_mm_node_allocated(&vma->node))
+				continue;
+
+			ret = i915_vma_bind(vma, cache_level, PIN_UPDATE);
+			if (ret)
+				return ret;
+		}
 	}
 
 	list_for_each_entry(vma, &obj->vma_list, vma_link)
@@ -3678,6 +3731,10 @@  int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 	obj->cache_level = cache_level;
 
 out:
+	/* Flush the dirty CPU caches to the backing storage so that the
+	 * object is now coherent at its new cache level (with respect
+	 * to the access domain).
+	 */
 	if (obj->cache_dirty &&
 	    obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
 	    cpu_write_needs_clflush(obj)) {