[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 <chris at chris-wilson.co.uk>
>>> ---
>>>   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 = &to_i915(obj->base.dev)->ggtt.base;
>>> +	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
>>> +	struct i915_address_space *vm = &dev_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 <tvrtko.ursulin at intel.com>

Regards,

Tvrtko