[Intel-gfx] [PATCH] drm/i915: Flush the existing fence before GGTT read/write

[Matthew Auld]

On 23/08/2019 16:03, Chris Wilson wrote:
> Our fence management is lazy, very lazy. If the user marks an object as
> untiled, we do not immediately flush the fence but merely mark it as
> dirty. On the use we have to remember to check and remove the fence, by
> which time we hope it is idle and we do not have to wait.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111468
> Fixes: 1f7fd484fff1 ("drm/i915: Replace i915_vma_put_fence()")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld at intel.com>

Well that sucks, should we also try to be more solid on the reloc path?

> ---
>   drivers/gpu/drm/i915/i915_gem.c | 30 ++++++++++++++++++++++++++++--
>   1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index eb31b69a316a..41b28f6d8620 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -324,6 +324,26 @@ gtt_user_read(struct io_mapping *mapping,
>   	return unwritten;
>   }
>   
> +static int linear_ggtt_offset(struct i915_vma *vma, u64 *offset)
> +{
> +	int ret;
> +
> +	GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
> +
> +	/* Open-coded i915_vma_pin_iomap() */
> +
> +	if (vma->fence) {
> +		mutex_lock(&vma->vm->mutex);
> +		ret = i915_vma_revoke_fence(vma);
> +		mutex_unlock(&vma->vm->mutex);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	*offset = i915_ggtt_offset(vma);
> +	return 0;
> +}
> +
>   static int
>   i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   		   const struct drm_i915_gem_pread *args)
> @@ -350,7 +370,10 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   					       PIN_NONBLOCK /* NOWARN */ |
>   					       PIN_NOEVICT);
>   	if (!IS_ERR(vma)) {
> -		node.start = i915_ggtt_offset(vma);
> +		ret = linear_ggtt_offset(vma, &node.start);
> +		if (ret)
> +			goto out_unpin;
> +
>   		node.allocated = false;

node.allocated = false should go before the jump.

>   	} else {
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
> @@ -560,7 +583,10 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   					       PIN_NONBLOCK /* NOWARN */ |
>   					       PIN_NOEVICT);
>   	if (!IS_ERR(vma)) {
> -		node.start = i915_ggtt_offset(vma);
> +		ret = linear_ggtt_offset(vma, &node.start);
> +		if (ret)
> +			goto out_unpin;
> +
>   		node.allocated = false;
>   	} else {
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>