Re: [Intel-gfx] [PATCH] drm/i915: Skip unbinding large unmappable global buffers

[Chris Wilson]

On Thu, Oct 13, 2016 at 03:13:51PM +0100, Tvrtko Ursulin wrote:
> 
> On 13/10/2016 13:59, Chris Wilson wrote:
> >On Thu, Oct 13, 2016 at 12:29:44PM +0100, Tvrtko Ursulin wrote:
> >>On 13/10/2016 09:55, Chris Wilson wrote:
> >>>If the user requests a mappable binding to the global GTT, we will first
> >>>unbind an existing mapping if it doesn't match. We will unbind even if
> >>>there is no possibility that the object can fit in the mappable
> >>>aperture. This may lead to a ping-pong migration of the object, for
> >>>example igt/gem_exec_big.
> >>>
> >>>Testcase: igt/gem_exec_big
> >>>Signed-off-by: Chris Wilson 
> >>>---
> >>>  drivers/gpu/drm/i915/i915_gem.c | 19 ++-
> >>>  1 file changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> >>>b/drivers/gpu/drm/i915/i915_gem.c
> >>>index 2f939f90984b..f33aa9bb95c0 100644
> >>>--- a/drivers/gpu/drm/i915/i915_gem.c
> >>>+++ b/drivers/gpu/drm/i915/i915_gem.c
> >>>@@ -3144,6 +3144,9 @@ search_free:
> >>>   goto err_unpin;
> >>>   }
> >>>+
> >>>+  GEM_BUG_ON(vma->node.start < start);
> >>>+  GEM_BUG_ON(vma->node.start + vma->node.size > end);
> >>>   }
> >>>   GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
> >>>@@ -3843,7 +3846,8 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object 
> >>>*obj,
> >>>u64 alignment,
> >>>u64 flags)
> >>>  {
> >>>-  struct i915_address_space *vm = _i915(obj->base.dev)->ggtt.base;
> >>>+  struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> >>>+  struct i915_address_space *vm = _priv->ggtt.base;
> >>>   struct i915_vma *vma;
> >>>   int ret;
> >>>@@ -3858,6 +3862,19 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object 
> >>>*obj,
> >>>   (i915_vma_is_pinned(vma) || i915_vma_is_active(vma)))
> >>>   return ERR_PTR(-ENOSPC);
> >>>+  if (flags & PIN_MAPPABLE) {
> >>>+  u32 fence_size;
> >>>+
> >>>+  fence_size = i915_gem_get_ggtt_size(dev_priv, vma->size,
> >>>+  
> >>>i915_gem_object_get_tiling(obj));
> >>>+  if (fence_size > dev_priv->ggtt.mappable_end)
> >>>+  return ERR_PTR(-E2BIG);
> >>>+
> >>>+  if (flags & PIN_NONBLOCK &&
> >>>+  fence_size > dev_priv->ggtt.mappable_end / 2)
> >>>+  return ERR_PTR(-ENOSPC);
> >>One half of aperture is a magic number or something else?
> >Magic number. NONBLOCK means try, we have a fallback planned.
> >
> >It more or less comes from the old habit of having to assume that all
> >fences take up twice the space we expect due to alignment. And given
> >NONBLOCK means that we should have a better fallback than evicting the
> >majority of the aperture on a whim.
> 
> Ok I couldn't not think of any potential bad interactions.
> 
> If you add a comment explaining the reasoning:

Felt in the mood to write a few sentences. Hopefully they make sense in
the morning. Thanks,
-Chris

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

Re: [Intel-gfx] [PATCH] drm/i915: Skip unbinding large unmappable global buffers

[Tvrtko Ursulin]

On 13/10/2016 13:59, Chris Wilson wrote:

On Thu, Oct 13, 2016 at 12:29:44PM +0100, Tvrtko Ursulin wrote:

On 13/10/2016 09:55, Chris Wilson wrote:

If the user requests a mappable binding to the global GTT, we will first
unbind an existing mapping if it doesn't match. We will unbind even if
there is no possibility that the object can fit in the mappable
aperture. This may lead to a ping-pong migration of the object, for
example igt/gem_exec_big.

Testcase: igt/gem_exec_big
Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c | 19 ++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2f939f90984b..f33aa9bb95c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3144,6 +3144,9 @@ search_free:
goto err_unpin;
}
+
+   GEM_BUG_ON(vma->node.start < start);
+   GEM_BUG_ON(vma->node.start + vma->node.size > end);
}
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
@@ -3843,7 +3846,8 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 u64 alignment,
 u64 flags)
  {
-   struct i915_address_space *vm = _i915(obj->base.dev)->ggtt.base;
+   struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+   struct i915_address_space *vm = _priv->ggtt.base;
struct i915_vma *vma;
int ret;
@@ -3858,6 +3862,19 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
(i915_vma_is_pinned(vma) || i915_vma_is_active(vma)))
return ERR_PTR(-ENOSPC);
+   if (flags & PIN_MAPPABLE) {
+   u32 fence_size;
+
+   fence_size = i915_gem_get_ggtt_size(dev_priv, vma->size,
+   
i915_gem_object_get_tiling(obj));
+   if (fence_size > dev_priv->ggtt.mappable_end)
+   return ERR_PTR(-E2BIG);
+
+   if (flags & PIN_NONBLOCK &&
+   fence_size > dev_priv->ggtt.mappable_end / 2)
+   return ERR_PTR(-ENOSPC);

One half of aperture is a magic number or something else?

Magic number. NONBLOCK means try, we have a fallback planned.

It more or less comes from the old habit of having to assume that all
fences take up twice the space we expect due to alignment. And given
NONBLOCK means that we should have a better fallback than evicting the
majority of the aperture on a whim.


Ok I couldn't not think of any potential bad interactions.

If you add a comment explaining the reasoning:

Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Skip unbinding large unmappable global buffers

[Chris Wilson]

On Thu, Oct 13, 2016 at 12:29:44PM +0100, Tvrtko Ursulin wrote:
> 
> On 13/10/2016 09:55, Chris Wilson wrote:
> >If the user requests a mappable binding to the global GTT, we will first
> >unbind an existing mapping if it doesn't match. We will unbind even if
> >there is no possibility that the object can fit in the mappable
> >aperture. This may lead to a ping-pong migration of the object, for
> >example igt/gem_exec_big.
> >
> >Testcase: igt/gem_exec_big
> >Signed-off-by: Chris Wilson 
> >---
> >  drivers/gpu/drm/i915/i915_gem.c | 19 ++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> >b/drivers/gpu/drm/i915/i915_gem.c
> >index 2f939f90984b..f33aa9bb95c0 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -3144,6 +3144,9 @@ search_free:
> > goto err_unpin;
> > }
> >+
> >+GEM_BUG_ON(vma->node.start < start);
> >+GEM_BUG_ON(vma->node.start + vma->node.size > end);
> > }
> > GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
> >@@ -3843,7 +3846,8 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object 
> >*obj,
> >  u64 alignment,
> >  u64 flags)
> >  {
> >-struct i915_address_space *vm = _i915(obj->base.dev)->ggtt.base;
> >+struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> >+struct i915_address_space *vm = _priv->ggtt.base;
> > struct i915_vma *vma;
> > int ret;
> >@@ -3858,6 +3862,19 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object 
> >*obj,
> > (i915_vma_is_pinned(vma) || i915_vma_is_active(vma)))
> > return ERR_PTR(-ENOSPC);
> >+if (flags & PIN_MAPPABLE) {
> >+u32 fence_size;
> >+
> >+fence_size = i915_gem_get_ggtt_size(dev_priv, vma->size,
> >+
> >i915_gem_object_get_tiling(obj));
> >+if (fence_size > dev_priv->ggtt.mappable_end)
> >+return ERR_PTR(-E2BIG);
> >+
> >+if (flags & PIN_NONBLOCK &&
> >+fence_size > dev_priv->ggtt.mappable_end / 2)
> >+return ERR_PTR(-ENOSPC);
> 
> One half of aperture is a magic number or something else?

Magic number. NONBLOCK means try, we have a fallback planned.

It more or less comes from the old habit of having to assume that all
fences take up twice the space we expect due to alignment. And given
NONBLOCK means that we should have a better fallback than evicting the
majority of the aperture on a whim.
-Chris

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

Re: [Intel-gfx] [PATCH] drm/i915: Skip unbinding large unmappable global buffers

[Tvrtko Ursulin]

On 13/10/2016 09:55, Chris Wilson wrote:

If the user requests a mappable binding to the global GTT, we will first
unbind an existing mapping if it doesn't match. We will unbind even if
there is no possibility that the object can fit in the mappable
aperture. This may lead to a ping-pong migration of the object, for
example igt/gem_exec_big.

Testcase: igt/gem_exec_big
Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_gem.c | 19 ++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2f939f90984b..f33aa9bb95c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3144,6 +3144,9 @@ search_free:
  
  			goto err_unpin;

}
+
+   GEM_BUG_ON(vma->node.start < start);
+   GEM_BUG_ON(vma->node.start + vma->node.size > end);
}
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
  
@@ -3843,7 +3846,8 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,

 u64 alignment,
 u64 flags)
  {
-   struct i915_address_space *vm = _i915(obj->base.dev)->ggtt.base;
+   struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+   struct i915_address_space *vm = _priv->ggtt.base;
struct i915_vma *vma;
int ret;
  
@@ -3858,6 +3862,19 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,

(i915_vma_is_pinned(vma) || i915_vma_is_active(vma)))
return ERR_PTR(-ENOSPC);
  
+		if (flags & PIN_MAPPABLE) {

+   u32 fence_size;
+
+   fence_size = i915_gem_get_ggtt_size(dev_priv, vma->size,
+   
i915_gem_object_get_tiling(obj));
+   if (fence_size > dev_priv->ggtt.mappable_end)
+   return ERR_PTR(-E2BIG);
+
+   if (flags & PIN_NONBLOCK &&
+   fence_size > dev_priv->ggtt.mappable_end / 2)
+   return ERR_PTR(-ENOSPC);


One half of aperture is a magic number or something else?

Regards,

Tvrtko


+   }
+
WARN(i915_vma_is_pinned(vma),
 "bo is already pinned in ggtt with incorrect alignment:"
 " offset=%08x, req.alignment=%llx,"


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx