Re: [PATCH 2/2] drm/i915: Drop i915_gem_obj_is_pinned() from set-cache-level

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/10/15 11:34, Chris Wilson wrote:
On Fri, Oct 09, 2015 at 11:17:19AM +0100, Tvrtko Ursulin wrote:
-		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;
-			}
+		/* Access to snoopable pages through the GTT is incoherent. */
+		if (cache_level != I915_CACHE_NONE && !HAS_LLC(dev))
+			i915_gem_release_mmap(obj);

Don't fully understand this one - but my question is this.
Previously userspace would lose mappings on cache level changes any
time, after this only on !LLC when turning on caching mode. So this
means userspace needs to know about this change and modify it's
behavior? Or what exactly would happen in practice?

No. Userspace has no knowledge of the kernel handling the PTEs, its
mapping is persistent (i.e. the obj->mmap_offset inside the dev->mappping).
Otoh, we are improving the situation so even if userspace tries to avoid
set-cache-level nothing is lost.

Hm so if a VMA is re-bound in this process and it could have gotten
a new GGTT address, why it is not necessary to always release mmaps
and so to update CPU PTEs?

The VMA are not moved by this function, only the PTEs are rewritten. The
GTT ignores the bits we are changing on llc architectures. On !llc using
the GTT to access snoopable PTE is verboten and does cause machine hangs.

How come they are not moved when they can be unbound and then bound again?

The only relevant vma here are rebound with PIN_UPDATE. If we have to
unbind any due to subsequent placement errors, the behaviour doesn't
change in this patch. So I'm not understanding your concern and can't
address it adequately. :(

I started to understand how this works after a chat on IRC. Before I had a completely wrong assumptions.

(This also demonstrates this code should really have a good high level comment.)

Unless I missed something it really looks the behaviour is unchanged, just a trip to the fault handler is avoided if not needed.

But I still think you need to improve the commit message to be clearer on pin_display (un)usage and SandyBridge referencing.

Regards,

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux