On 26/01/16 15:10, Chris Wilson wrote:
On Tue, Jan 26, 2016 at 02:53:31PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <[email protected]>
Signed-off-by: Tvrtko Ursulin <[email protected]>
---
drivers/gpu/drm/i915/i915_gem.c | 96 ++++++++++++++++++++++++++++++++++++++---
include/uapi/drm/i915_drm.h | 3 ++
2 files changed, 93 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dacf6a0013c5..039d55a49fc6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1954,6 +1954,60 @@ out:
return i915_gem_ret_to_vm_ret(dev_priv, ret);
}
+static int
+i915_gem_cpu_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+ struct drm_i915_gem_object *obj = to_intel_bo(vma->vm_private_data);
+ struct drm_device *dev = obj->base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
+ pgoff_t page_offset;
+ struct page *page;
+ int ret;
+
+ /* We don't use vmf->pgoff since that has the fake offset */
+ page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
+ PAGE_SHIFT;
+
+ trace_i915_gem_object_fault(obj, page_offset, true, write);
+
+ intel_runtime_pm_get(dev_priv);
+
+ ret = i915_mutex_lock_interruptible(dev);
+ if (ret)
+ goto out;
+
+ ret = i915_gem_object_set_to_cpu_domain(obj, write);
+ if (ret)
+ goto out_unlock;
That was a mistake in the GTT gem_fault(). If you do this, we also want
the nonblocking wait for obvious reasons.
You suggest leaving it for userspace?
And how would a non-blocking wait work?
+ ret = i915_gem_object_get_pages(obj);
+ if (ret)
+ goto out_unlock;
+
+ page = i915_gem_object_get_page(obj, page_offset);
+ if (!page) {
+ ret = -ERANGE;
+ goto out_unlock;
+ }
+
+ mutex_unlock(&dev->struct_mutex);
+
+ ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address,
+ page_to_pfn(page));
We don't have a page ref at this point, so this obj+page could be
freed (via the shrinker at least) before we insert it.
Oh yeah, need to pin the pages..
I would also be more interested in having a version that faulted the
entire object at once - though maybe we will see more random access in
future.
Yeah I did not want to concern myself with more code since this was a
proof of concept anyway.
+ intel_runtime_pm_put(dev_priv);
+
+ return i915_gem_ret_to_vm_ret(dev_priv, ret);
+
+out_unlock:
+ mutex_unlock(&dev->struct_mutex);
+out:
+ intel_runtime_pm_put(dev_priv);
+
+ return i915_gem_ret_to_vm_ret(dev_priv, ret);
+}
+
/**
* i915_gem_release_mmap - remove physical page mappings
* @obj: obj in question
@@ -2078,11 +2132,18 @@ static void i915_gem_object_free_mmap_offset(struct
drm_i915_gem_object *obj)
drm_gem_free_mmap_offset(&obj->base);
}
-int
-i915_gem_mmap_gtt(struct drm_file *file,
- struct drm_device *dev,
- uint32_t handle,
- uint64_t *offset)
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+ .fault = i915_gem_cpu_fault,
+ .open = drm_gem_vm_open,
+ .close = drm_gem_vm_close,
+};
+
+static int
+i915_gem_mmap(struct drm_file *file,
+ struct drm_device *dev,
+ uint32_t handle,
+ uint32_t flags,
+ uint64_t *offset)
{
struct drm_i915_gem_object *obj;
int ret;
@@ -2103,10 +2164,23 @@ i915_gem_mmap_gtt(struct drm_file *file,
goto out;
}
+ if (!obj->base.filp && (flags & I915_MMAP2_CPU)) {
+ DRM_DEBUG("Attempting to mmap non-shm based object via CPU!\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
ret = i915_gem_object_create_mmap_offset(obj);
if (ret)
goto out;
+ if (flags & I915_MMAP2_CPU) {
+ ret = drm_vma_node_set_vm_ops(&obj->base.vma_node,
+ &i915_gem_cpu_vm_ops);
+ if (ret)
+ goto out;
+ }
We would also need a WC equivalent.
It looks fairly sane. I wanted this just a short while ago, but figured
out a way of using regular mmap() to give me the inheritance instead.
So would it be useful to cleanup and finish this work or not?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
