| Message ID | 1429877420.19506.11.camel@jlahtine-mobl1 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -Chris
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6260
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 318/318 318/318
IVB 341/341 341/341
BYT 287/287 287/287
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
Note: You need to pay more attention to line start with '*'
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 >
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@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. 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? I would suggest try first with a PIN_MAPPABLE | PIN_NOEVICT then. You can limit thrashing by a vma->release callback to only zap the PTE mapping into that vma. Also remember to destroy all of the partial vma when we zap the mapping for an 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). Just treat the aperture as first come first served, with perhaps a reservation of the first 256k for transient partial vma. Fix thrashing when we a usecase that falls foul of the page-fault-of-doom, but first try to write one. I think with a mru list of individually faulted vma thrashing will be limited (i.e. no worse than current mru eviction and possibly better albeit at a higher pagefault cost). Also I found a usecase for temporarily mapping invidual pages: http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=bug90137&id=40e8a9cddb4a1e39a2fdcff4878b61c666e1a51e -Chris
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@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. > > 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). I will extend the series with fence calculation for tield partial view, so I would consider this in the rework. Does that sound good? > I would suggest try first with a PIN_MAPPABLE | PIN_NOEVICT then. You > can limit thrashing by a vma->release callback to only zap the PTE > mapping into that vma. Also remember to destroy all of the partial vma > when we zap the mapping for an 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). > > Just treat the aperture as first come first served, with perhaps a > reservation of the first 256k for transient partial vma. Fix thrashing > when we a usecase that falls foul of the page-fault-of-doom, but first > try to write one. I think with a mru list of individually faulted vma > thrashing will be limited (i.e. no worse than current mru eviction and > possibly better albeit at a higher pagefault cost). > Ack. > Also I found a usecase for temporarily mapping invidual pages: > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=bug90137&id=40e8a9cddb4a1e39a2fdcff4878b61c666e1a51e I have not peeked into execbuffers much, but looks quite straightforward. Could use the partial views code if desired. Not sure how effective it is for single pages. Regards, Joonas > -Chris >
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@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. > > > > 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
On ma, 2015-04-27 at 13:25 +0100, Chris Wilson wrote: > 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@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. > > > > > > 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... > Which change is that? If we're talking about the back-up path for failed normal mapping, I can of course add the code for it, it will not cause any worse trashing than it has before for objects smaller than aperture size. Fancier logic to avoid trashing can be added later. > I don't see this as a useful stepping point. The current users of large > objects do not map through the GTT. In XenGT, when the mappable aperture size is decreased due to slicing of the aperture for different guests, it's not about large objects but small aperture. And that is the reason why the feature was initially implemented, and they've already been waiting for the feature for quite a while. All the series was intended to was to provide this support, so they can proceed with testing with small aperture sizes. Everything else is considered extra, and it was in the initial description of the task (by either Jesse or Daniel) that half aperture size is most likely when the current code will start to fail in real use cases. I do not see why all possible features need to be implemented first to get a useful feature for the XenGT team in. After all, before adding the partial views, anything bigger than the mappable aperture was simply rejected in mmap. So I do not follow the logic why the code path for this case can not be added. If some other change was not merged due to waiting for the partial views support with more features than the code path for objects bigger than aperture size, not merging the current code only causes two features to keep waiting instead of just one. And as the XenGT team has been waiting for the feature for months before the feature was even started on, I do think it is not very reasonable. The next revision will be started on immediately after this first one has been merged, and intention is to keep this first one simple. Let me know whatever feature is awaiting for the next revision, and I can take it into consideration to have the support for that use case ready as first priority. Regards, Joonas > -Chris >
On Mon, Apr 27, 2015 at 04:46:20PM +0300, Joonas Lahtinen wrote: > In XenGT, when the mappable aperture size is decreased due to slicing of > the aperture for different guests, it's not about large objects but > small aperture. And that is the reason why the feature was initially > implemented, and they've already been waiting for the feature for quite > a while. Sorry, but the quickest fix for that would have been to add the aperture information to GET_APERTURE, like I have previously said. -Chris
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); + } + + /* 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); + if (unlikely(view.type == I915_GGTT_VIEW_PARTIAL)) { + unsigned long base = vma->vm_start + + (view.params.partial.offset << PAGE_SHIFT); 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); + } else { + if (!obj->fault_mappable) { + unsigned long size = min_t(unsigned long, + vma->vm_end - vma->vm_start, + obj->base.size); + 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); + if (ret) + break; + } + + obj->fault_mappable = true; + } else + ret = vm_insert_pfn(vma, + (unsigned long)vmf->virtual_address, + pfn + page_offset); + } unpin: - i915_gem_object_ggtt_unpin(obj); + i915_gem_object_ggtt_unpin_view(obj, &view); unlock: mutex_unlock(&dev->struct_mutex); out: @@ -1897,11 +1925,6 @@ i915_gem_mmap_gtt(struct drm_file *file, goto unlock; } - if (obj->base.size > dev_priv->gtt.mappable_end) { - ret = -E2BIG; - goto out; - } - if (obj->madv != I915_MADV_WILLNEED) { DRM_DEBUG("Attempting to mmap a purgeable buffer\n"); ret = -EFAULT;
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(-)