On Mon, Apr 27, 2015 at 03:12:01PM +0300, Joonas Lahtinen wrote:
> On ma, 2015-04-27 at 12:21 +0100, Chris Wilson wrote:
> > On Mon, Apr 27, 2015 at 02:01:59PM +0300, Joonas Lahtinen wrote:
> > > 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@xxxxxxxxxxxxxxx>
> > > > > ---
> > > > > 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.
> >
> > The point is that we fail to install a partial view for pinned objects
> > outside of the aperture. Or did I miss how you handle them?
> >
>
> That is true.
>
> By changing the comparison to be against full aperture size, then the
> patch will only bring new functionality to handle the cases when the
> object was actually impossible to map previously (and was early
> rejected).
>
> I'd prefer to have this version in first (with change of removing the /2
> from aperture size comparison), to get some feedback from the XenGT team
> about the usability of it (speed-wise).
No. Daniel rejected a change just because we didn't have this series,
only to find this series doesn't even provide the support Daniel
wanted...
I don't see this as a useful stepping point. The current users of large
objects do not map through the GTT.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
