Re: [PATCH 5/5] drm/i915: Use partial view in mmap fault handler

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On pe, 2015-04-24 at 13:33 +0100, Chris Wilson wrote:
> On Fri, Apr 24, 2015 at 03:10:20PM +0300, Joonas Lahtinen wrote:
> > Use partial view for huge BOs (bigger than half the mappable aperture)
> > in fault handler so that they can be accessed withough trying to make
> > room for them by evicting other objects.
> > 
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c |   67 ++++++++++++++++++++++++++-------------
> >  1 file changed, 45 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index c2c1819..eb30cee 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 half the aperture. */
> > +	if (obj->base.size > dev_priv->gtt.mappable_end/2) {
> > +		static const size_t 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(size_t,
> > +			      chunk_size,
> > +			      (vma->vm_end - vma->vm_start)/PAGE_SIZE -
> > +			      view.params.partial.offset);
> 
> This isn't what I was imagining.
> 
> I was expecting to see error handling inside the fault path if we could
> not pin the object. This way we could handle fragmentation or display
> objects pinned outside the aperture.

After discussion with Daniel, the idea was dropped due to high amount of
trashing which would occur if each object would be attempted to fit to
the mappable aperture for each fault to that object.

An alternate path could be added to mark an object partially_mappable if
mapping it normally fails, then further attempts could be skipped to
avoid the trashing. But it is still questionable if an object close to
the whole aperture size should be attempted to map normally, which will
cause huge trashing if there are plenty of objects in the mappable
aperture (that problem existed before, if the object was just short of
the aperture size, it was attempted to be mapped).

Regards, Joonas

PS. Sorry for late reply, it seems my original reply didn't get
delivered.

> -Chris
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx