On ti, 2015-05-05 at 10:07 +0100, Tvrtko Ursulin wrote:
> On 05/04/2015 12:51 PM, Joonas Lahtinen wrote:
> > On to, 2015-04-30 at 15:54 +0100, Tvrtko Ursulin wrote:
> >> On 04/30/2015 12:21 PM, Joonas Lahtinen wrote:
> >>>
> >>> Use partial view for huge BOs (bigger than the mappable aperture)
> >>> in fault handler so that they can be accessed without trying to make
> >>> room for them by evicting other objects.
> >>>
> >>> v2:
> >>> - Only use partial views in the case where early rejection was
> >>> previously done.
> >>> - Account variable type changes from previous reroll.
> >>>
> >>> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> >>> Signed-off-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/i915_gem.c | 69
> >>> +++++++++++++++++++++++++++--------------
> >>> 1 file changed, 46 insertions(+), 23 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> >>> b/drivers/gpu/drm/i915/i915_gem.c
> >>> index a020836..2f3fa0b 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >>> @@ -1635,6 +1635,7 @@ int i915_gem_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;
> >>> + struct i915_ggtt_view view = i915_ggtt_view_normal;
> >>> pgoff_t page_offset;
> >>> unsigned long pfn;
> >>> int ret = 0;
> >>> @@ -1667,8 +1668,21 @@ int i915_gem_fault(struct vm_area_struct *vma,
> >>> struct vm_fault *vmf)
> >>> goto unlock;
> >>> }
> >>>
> >>> - /* Now bind it into the GTT if needed */
> >>> - ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> >>> + /* Use a partial view if the object is bigger than the aperture. */
> >>> + if (obj->base.size >= dev_priv->gtt.mappable_end) {
> >>> + static const unsigned int chunk_size = 256; // 1 MiB
> >>> + memset(&view, 0, sizeof(view));
> >>> + view.type = I915_GGTT_VIEW_PARTIAL;
> >>> + view.params.partial.offset = rounddown(page_offset, chunk_size);
> >>> + view.params.partial.size =
> >>> + min_t(unsigned int,
> >>> + chunk_size,
> >>> + (vma->vm_end - vma->vm_start)/PAGE_SIZE -
> >>> + view.params.partial.offset);
> >>> + }
> >>> +
> >>> + /* Now pin it into the GTT if needed */
> >>> + ret = i915_gem_object_ggtt_pin(obj, &view, 0, PIN_MAPPABLE);
> >>> if (ret)
> >>> goto unlock;
> >>>
> >>> @@ -1681,30 +1695,44 @@ int i915_gem_fault(struct vm_area_struct *vma,
> >>> struct vm_fault *vmf)
> >>> goto unpin;
> >>>
> >>> /* Finally, remap it using the new GTT offset */
> >>> - pfn = dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj);
> >>> + pfn = dev_priv->gtt.mappable_base +
> >>> + i915_gem_obj_ggtt_offset_view(obj, &view);
> >>> pfn >>= PAGE_SHIFT;
> >>>
> >>> - if (!obj->fault_mappable) {
> >>> - unsigned long size = min_t(unsigned long,
> >>> - vma->vm_end - vma->vm_start,
> >>> - obj->base.size);
> >>> - int i;
> >>> + if (unlikely(view.type == I915_GGTT_VIEW_PARTIAL)) {
> >>> + unsigned long base = vma->vm_start +
> >>> + (view.params.partial.offset << PAGE_SHIFT);
> >>> + unsigned int i;
> >>>
> >>> - for (i = 0; i < size >> PAGE_SHIFT; i++) {
> >>> - ret = vm_insert_pfn(vma,
> >>> - (unsigned long)vma->vm_start + i *
> >>> PAGE_SIZE,
> >>> - pfn + i);
> >>> + for (i = 0; i < view.params.partial.size; i++) {
> >>> + ret = vm_insert_pfn(vma, base + i * PAGE_SIZE, pfn + i);
> >>> if (ret)
> >>> break;
> >>> }
> >>> -
> >>> obj->fault_mappable = true;
> >>> - } else
> >>> - ret = vm_insert_pfn(vma,
> >>> - (unsigned long)vmf->virtual_address,
> >>> - pfn + page_offset);
> >>
> >> If I read the diff correctly you don't have equivalent handling (as the
> >> normal view) for when the case when the pre-fault fails somewhere in the
> >> middle?
> >>
> >
> > True so, the flag fault_mappable is used for the normal view to track
> > whether all pages were inserted and it makes sense to just insert the
> > faulted one. I just didn't want to add another flag to track the same
> > for each vma.
>
> But it is safe to do it multiple times?
>
Put a comment in there.
> Either way I would put a comment in explaining the difference between
> code paths.
>
> Regards,
>
> Tvrtko
>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
