On 08/10/15 10:46, Chris Wilson wrote:
On Thu, Oct 08, 2015 at 10:32:39AM +0100, Tvrtko Ursulin wrote:
On 07/10/15 17:19, Chris Wilson wrote:
On Wed, Oct 07, 2015 at 04:57:25PM +0100, Tvrtko Ursulin wrote:
Hi,
On 06/10/15 11:39, 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. So we can safely replace i915_gem_object_is_pinned()
[snip]
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d4a3bdf0c5b6..2b8ed7a2faab 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3629,31 +3629,34 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
{
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;
- }
-
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;
+ }
+
But this is the same as i915_gem_obj_is_pinned, where is the
obj->pin_display change commit message talks about?
Right here. The difference is that we are only iterating the vma list
once rather than 3x.
Thats true, but the commit says it is going to use obj->pin_display
for something and then doesn't use it at all. Riddles in patches are
not that hot. :)
I was trying to explain what the actual rules pertaining to the
rebinding the vma was. We can rebind anything that isn't pinned and the
only thing pinned here can be obj->pin_display.
Okay but the commit message says the is going to use obj->pin_display.
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)) {
+ if (bound) {
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.
@@ -3664,13 +3667,18 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
return ret;
}
- 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?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
