diff mbox series

[RFC,28/42] drm/i915: Split out GTT fault handler to make it generic

Message ID 20190214145740.14521-29-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series Introduce memory region concept (including device local memory) | expand

Commit Message

Matthew Auld Feb. 14, 2019, 2:57 p.m. UTC
From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>

In preparation for using multiple page-fault handlers depending
on the object's backing storage.

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 112 +++++++++++++++++++-------------
 1 file changed, 66 insertions(+), 46 deletions(-)

Comments

Chris Wilson Feb. 14, 2019, 4 p.m. UTC | #1
Quoting Matthew Auld (2019-02-14 14:57:26)
> From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> 
> In preparation for using multiple page-fault handlers depending
> on the object's backing storage.
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 112 +++++++++++++++++++-------------
>  1 file changed, 66 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e59f38e00f0d..95e31529a738 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1782,11 +1782,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
>  }
>  
>  /**
> - * i915_gem_fault - fault a page into the GTT
> - * @vmf: fault info
> - *
> - * The fault handler is set up by drm_gem_mmap() when a object is GTT mapped
> - * from userspace.  The fault handler takes care of binding the object to
> + * The GTT fill pages handler takes care of binding the object to
>   * the GTT (if needed), allocating and programming a fence register (again,
>   * only if needed based on whether the old reg is still valid or the object
>   * is tiled) and inserting a new PTE into the faulting process.
> @@ -1799,57 +1795,20 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
>   * The current feature set supported by i915_gem_fault() and thus GTT mmaps
>   * is exposed via I915_PARAM_MMAP_GTT_VERSION (see i915_gem_mmap_gtt_version).
>   */
> -vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> +static int __vmf_fill_pages_gtt(struct drm_i915_gem_object *obj,
> +                               struct vm_fault *vmf,
> +                               pgoff_t page_offset)
>  {
>  #define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
>         struct vm_area_struct *area = vmf->vma;
> -       struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
>         struct drm_device *dev = obj->base.dev;
>         struct drm_i915_private *dev_priv = to_i915(dev);
>         struct i915_ggtt *ggtt = &dev_priv->ggtt;
>         bool write = area->vm_flags & VM_WRITE;
> -       intel_wakeref_t wakeref;
>         struct i915_vma *vma;
> -       pgoff_t page_offset;
>         int srcu;
>         int ret;
>  
> -       /* Sanity check that we allow writing into this object */
> -       if (i915_gem_object_is_readonly(obj) && write)
> -               return VM_FAULT_SIGBUS;
> -
> -       /* We don't use vmf->pgoff since that has the fake offset */
> -       page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
> -
> -       trace_i915_gem_object_fault(obj, page_offset, true, write);
> -
> -       /* Try to flush the object off the GPU first without holding the lock.
> -        * Upon acquiring the lock, we will perform our sanity checks and then
> -        * repeat the flush holding the lock in the normal manner to catch cases
> -        * where we are gazumped.
> -        */
> -       ret = i915_gem_object_wait(obj,
> -                                  I915_WAIT_INTERRUPTIBLE,
> -                                  MAX_SCHEDULE_TIMEOUT);
> -       if (ret)
> -               goto err;
> -
> -       ret = i915_gem_object_pin_pages(obj);
> -       if (ret)
> -               goto err;
> -
> -       wakeref = intel_runtime_pm_get(dev_priv);
> -
> -       ret = i915_mutex_lock_interruptible(dev);
> -       if (ret)
> -               goto err_rpm;
> -
> -       /* Access to snoopable pages through the GTT is incoherent. */
> -       if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
> -               ret = -EFAULT;
> -               goto err_unlock;
> -       }
> -
>         /* Now pin it into the GTT as needed */
>         vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
>                                        PIN_MAPPABLE |
> @@ -1880,7 +1839,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>         }
>         if (IS_ERR(vma)) {
>                 ret = PTR_ERR(vma);
> -               goto err_unlock;
> +               return ret;
>         }
>  
>         ret = i915_gem_object_set_to_gtt_domain(obj, write);
> @@ -1920,6 +1879,67 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
>         i915_vma_unpin_fence(vma);
>  err_unpin:
>         __i915_vma_unpin(vma);
> +
> +       return ret;
> +}
> +
> +/**
> + * i915_gem_fault - fault a page into the memory
> + * @vmf: fault info
> + *
> + * The fault handler is set up by drm_gem_mmap() when mmap_offset is called on
> + * an object from userspace. The missing pages are setup by an object's
> + * vmf_fill_pages pages handler, depending on it's backing storage.
> + */
> +vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> +{
> +       struct vm_area_struct *area = vmf->vma;
> +       struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
> +       struct drm_device *dev = obj->base.dev;
> +       struct drm_i915_private *dev_priv = to_i915(dev);
> +       intel_wakeref_t wakeref;
> +       bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
> +       pgoff_t page_offset;
> +       int ret;
> +
> +       /* Sanity check that we allow writing into this object */
> +       if (i915_gem_object_is_readonly(obj) && write)
> +               return VM_FAULT_SIGBUS;
> +
> +       /* We don't use vmf->pgoff since that has the fake offset */
> +       page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
> +
> +       trace_i915_gem_object_fault(obj, page_offset, true, write);
> +
> +       /* Try to flush the object off the GPU first without holding the lock.
> +        * Upon acquiring the lock, we will perform our sanity checks and then
> +        * repeat the flush holding the lock in the normal manner to catch cases
> +        * where we are gazumped.
> +        */
> +       ret = i915_gem_object_wait(obj,
> +                                  I915_WAIT_INTERRUPTIBLE,
> +                                  MAX_SCHEDULE_TIMEOUT);
> +       if (ret)
> +               goto err;
> +
> +       ret = i915_gem_object_pin_pages(obj);
> +       if (ret)
> +               goto err;
> +
> +       wakeref = intel_runtime_pm_get(dev_priv);
> +
> +       ret = i915_mutex_lock_interruptible(dev);
> +       if (ret)
> +               goto err_rpm;
> +
> +       /* Access to snoopable pages through the GTT is incoherent. */
> +       if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
> +               ret = -EFAULT;
> +               goto err_unlock;
> +       }

That's a GGTT limitation, you'll probably have different variants for
other hw.

> +
> +       ret = __vmf_fill_pages_gtt(obj, vmf, page_offset);

Not yet seeing much point here instead of using different vm_ops for
different HW. 

> +
>  err_unlock:
>         mutex_unlock(&dev->struct_mutex);

Especially as any new HW is surely not falling for this trap.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e59f38e00f0d..95e31529a738 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1782,11 +1782,7 @@  compute_partial_view(const struct drm_i915_gem_object *obj,
 }
 
 /**
- * i915_gem_fault - fault a page into the GTT
- * @vmf: fault info
- *
- * The fault handler is set up by drm_gem_mmap() when a object is GTT mapped
- * from userspace.  The fault handler takes care of binding the object to
+ * The GTT fill pages handler takes care of binding the object to
  * the GTT (if needed), allocating and programming a fence register (again,
  * only if needed based on whether the old reg is still valid or the object
  * is tiled) and inserting a new PTE into the faulting process.
@@ -1799,57 +1795,20 @@  compute_partial_view(const struct drm_i915_gem_object *obj,
  * The current feature set supported by i915_gem_fault() and thus GTT mmaps
  * is exposed via I915_PARAM_MMAP_GTT_VERSION (see i915_gem_mmap_gtt_version).
  */
-vm_fault_t i915_gem_fault(struct vm_fault *vmf)
+static int __vmf_fill_pages_gtt(struct drm_i915_gem_object *obj,
+				struct vm_fault *vmf,
+				pgoff_t page_offset)
 {
 #define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
 	struct vm_area_struct *area = vmf->vma;
-	struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	bool write = area->vm_flags & VM_WRITE;
-	intel_wakeref_t wakeref;
 	struct i915_vma *vma;
-	pgoff_t page_offset;
 	int srcu;
 	int ret;
 
-	/* Sanity check that we allow writing into this object */
-	if (i915_gem_object_is_readonly(obj) && write)
-		return VM_FAULT_SIGBUS;
-
-	/* We don't use vmf->pgoff since that has the fake offset */
-	page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
-
-	trace_i915_gem_object_fault(obj, page_offset, true, write);
-
-	/* Try to flush the object off the GPU first without holding the lock.
-	 * Upon acquiring the lock, we will perform our sanity checks and then
-	 * repeat the flush holding the lock in the normal manner to catch cases
-	 * where we are gazumped.
-	 */
-	ret = i915_gem_object_wait(obj,
-				   I915_WAIT_INTERRUPTIBLE,
-				   MAX_SCHEDULE_TIMEOUT);
-	if (ret)
-		goto err;
-
-	ret = i915_gem_object_pin_pages(obj);
-	if (ret)
-		goto err;
-
-	wakeref = intel_runtime_pm_get(dev_priv);
-
-	ret = i915_mutex_lock_interruptible(dev);
-	if (ret)
-		goto err_rpm;
-
-	/* Access to snoopable pages through the GTT is incoherent. */
-	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
-		ret = -EFAULT;
-		goto err_unlock;
-	}
-
 	/* Now pin it into the GTT as needed */
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
 				       PIN_MAPPABLE |
@@ -1880,7 +1839,7 @@  vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	}
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
-		goto err_unlock;
+		return ret;
 	}
 
 	ret = i915_gem_object_set_to_gtt_domain(obj, write);
@@ -1920,6 +1879,67 @@  vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	i915_vma_unpin_fence(vma);
 err_unpin:
 	__i915_vma_unpin(vma);
+
+	return ret;
+}
+
+/**
+ * i915_gem_fault - fault a page into the memory
+ * @vmf: fault info
+ *
+ * The fault handler is set up by drm_gem_mmap() when mmap_offset is called on
+ * an object from userspace. The missing pages are setup by an object's
+ * vmf_fill_pages pages handler, depending on it's backing storage.
+ */
+vm_fault_t i915_gem_fault(struct vm_fault *vmf)
+{
+	struct vm_area_struct *area = vmf->vma;
+	struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	intel_wakeref_t wakeref;
+	bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
+	pgoff_t page_offset;
+	int ret;
+
+	/* Sanity check that we allow writing into this object */
+	if (i915_gem_object_is_readonly(obj) && write)
+		return VM_FAULT_SIGBUS;
+
+	/* We don't use vmf->pgoff since that has the fake offset */
+	page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
+
+	trace_i915_gem_object_fault(obj, page_offset, true, write);
+
+	/* Try to flush the object off the GPU first without holding the lock.
+	 * Upon acquiring the lock, we will perform our sanity checks and then
+	 * repeat the flush holding the lock in the normal manner to catch cases
+	 * where we are gazumped.
+	 */
+	ret = i915_gem_object_wait(obj,
+				   I915_WAIT_INTERRUPTIBLE,
+				   MAX_SCHEDULE_TIMEOUT);
+	if (ret)
+		goto err;
+
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		goto err;
+
+	wakeref = intel_runtime_pm_get(dev_priv);
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		goto err_rpm;
+
+	/* Access to snoopable pages through the GTT is incoherent. */
+	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev_priv)) {
+		ret = -EFAULT;
+		goto err_unlock;
+	}
+
+	ret = __vmf_fill_pages_gtt(obj, vmf, page_offset);
+
 err_unlock:
 	mutex_unlock(&dev->struct_mutex);
 err_rpm: