| Message ID | 1302640318-23165-22-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 12 Apr 2011 21:31:49 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > ... or if we will need to perform a cache-flush on the object anyway. > Unless, of course, we need to use a fence register to perform tiling > operations during the transfer. Here's the case I see: I've GTT-map-written a BO (so it hit backing pages), then that object becomes the framebuffer (PTEs changed to uncached), then we try to GTT-map-write it some more. The fake GTT map skips that. Also, looks like unrelated change to madvise? > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem.c | 34 ++++++++++++++++++++++++++++++++-- > 1 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 9d87258..2961f37 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1211,12 +1211,40 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) > > trace_i915_gem_object_fault(obj, page_offset, true, write); > > - /* Now bind it into the GTT if needed */ > if (!obj->map_and_fenceable) { > ret = i915_gem_object_unbind(obj); > if (ret) > goto unlock; > } > + > + /* If it is unbound or we are currently writing through the CPU > + * domain, continue to do so. > + */ > + if (obj->tiling_mode == I915_TILING_NONE && > + (obj->cache_level != I915_CACHE_NONE || > + obj->base.write_domain == I915_GEM_DOMAIN_CPU)) { > + struct page *page; > + > + ret = i915_gem_object_set_to_cpu_domain(obj, write); > + if (ret) > + goto unlock; > + > + obj->dirty = 1; > + obj->fault_mappable = true; > + mutex_unlock(&dev->struct_mutex); > + > + page = read_cache_page_gfp(obj->base.filp->f_path.dentry->d_inode->i_mapping, > + page_offset, > + GFP_HIGHUSER | __GFP_RECLAIMABLE); > + if (IS_ERR(page)) { > + ret = PTR_ERR(page); > + goto out; > + } > + > + vmf->page = page; > + return VM_FAULT_LOCKED; > + } > + > if (!obj->gtt_space) { > ret = i915_gem_object_bind_to_gtt(obj, 0, true); > if (ret) > @@ -3597,8 +3625,10 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, > > /* if the object is no longer bound, discard its backing storage */ > if (i915_gem_object_is_purgeable(obj) && > - obj->gtt_space == NULL) > + obj->gtt_space == NULL) { > + i915_gem_release_mmap(obj); > i915_gem_object_truncate(obj); > + } > > args->retained = obj->madv != __I915_MADV_PURGED; > > -- > 1.7.4.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 13 Apr 2011 08:57:01 -0700, Eric Anholt <eric@anholt.net> wrote: > On Tue, 12 Apr 2011 21:31:49 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > ... or if we will need to perform a cache-flush on the object anyway. > > Unless, of course, we need to use a fence register to perform tiling > > operations during the transfer. > > Here's the case I see: I've GTT-map-written a BO (so it hit backing > pages), then that object becomes the framebuffer (PTEs changed to > uncached), then we try to GTT-map-write it some more. The fake GTT map > skips that. Hmm, we missed a i915_gem_release_mmap in set_cache_level(). But otherwise if we attempt to read an I915_CACHE_NONE object we do so through the GTT. So: set_cache_level(bo, CACHE_LLC); ptr = mmap_gtt(bo); *ptr --> pages are left in the CPU domain and read via the normal page. set_cache_level(bo, CACHE_NONE); --> i915_gem_release_mmap(bo); *ptr --> the pagefault handler is called again and now we return a UC page > Also, looks like unrelated change to madvise? No, it is related since the vma is populated outside of being bound by the GTT now and so needs to be cleared along with truncate. Deserves a comment for being non-obvious. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 9d87258..2961f37 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1211,12 +1211,40 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) trace_i915_gem_object_fault(obj, page_offset, true, write); - /* Now bind it into the GTT if needed */ if (!obj->map_and_fenceable) { ret = i915_gem_object_unbind(obj); if (ret) goto unlock; } + + /* If it is unbound or we are currently writing through the CPU + * domain, continue to do so. + */ + if (obj->tiling_mode == I915_TILING_NONE && + (obj->cache_level != I915_CACHE_NONE || + obj->base.write_domain == I915_GEM_DOMAIN_CPU)) { + struct page *page; + + ret = i915_gem_object_set_to_cpu_domain(obj, write); + if (ret) + goto unlock; + + obj->dirty = 1; + obj->fault_mappable = true; + mutex_unlock(&dev->struct_mutex); + + page = read_cache_page_gfp(obj->base.filp->f_path.dentry->d_inode->i_mapping, + page_offset, + GFP_HIGHUSER | __GFP_RECLAIMABLE); + if (IS_ERR(page)) { + ret = PTR_ERR(page); + goto out; + } + + vmf->page = page; + return VM_FAULT_LOCKED; + } + if (!obj->gtt_space) { ret = i915_gem_object_bind_to_gtt(obj, 0, true); if (ret) @@ -3597,8 +3625,10 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, /* if the object is no longer bound, discard its backing storage */ if (i915_gem_object_is_purgeable(obj) && - obj->gtt_space == NULL) + obj->gtt_space == NULL) { + i915_gem_release_mmap(obj); i915_gem_object_truncate(obj); + } args->retained = obj->madv != __I915_MADV_PURGED;
... or if we will need to perform a cache-flush on the object anyway. Unless, of course, we need to use a fence register to perform tiling operations during the transfer. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.c | 34 ++++++++++++++++++++++++++++++++-- 1 files changed, 32 insertions(+), 2 deletions(-)