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

2015-04-27 Thread Chris Wilson
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.lahti...@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=bug90137id=40e8a9cddb4a1e39a2fdcff4878b61c666e1a51e
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-27 Thread Joonas Lahtinen
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.lahti...@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


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

2015-04-27 Thread Joonas Lahtinen
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.lahti...@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=bug90137id=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
 


___
Intel-gfx 

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

2015-04-27 Thread Chris Wilson
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.lahti...@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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-27 Thread Chris Wilson
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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-27 Thread Joonas Lahtinen
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.lahti...@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 

[Intel-gfx] [PATCH 5/5] drm/i915: Use partial view in mmap fault handler

2015-04-24 Thread Joonas Lahtinen
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.lahti...@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);
+   }
+
+   /* 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) {

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

2015-04-24 Thread Chris Wilson
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.lahti...@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

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-24 Thread shuang . he
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@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  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx