Re: [PATCH 42/55] drm/i915: Remove obsolete i915_gem_object_flush_active()

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> Since we track requests, and requests are always added to the GPU fully
> formed, we never have to flush the incomplete request and know that the
> given request will eventually complete without any further action on our
> part.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

This series will need plenty of CI regression and benchmark testing...

Regards, Joonas

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 59 +++--------------------------------------
>  1 file changed, 3 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e5be4eb0a397..3bff6b560f97 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2746,35 +2746,6 @@ out_rearm:
>  }
>  
>  /**
> - * Ensures that an object will eventually get non-busy by flushing any required
> - * write domains, emitting any outstanding lazy request and retiring and
> - * completed requests.
> - * @obj: object to flush
> - */
> -static int
> -i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
> -{
> -	int i;
> -
> -	if (!obj->active)
> -		return 0;
> -
> -	for (i = 0; i < I915_NUM_ENGINES; i++) {
> -		struct drm_i915_gem_request *req;
> -
> -		req = i915_gem_active_peek(&obj->last_read[i],
> -					   &obj->base.dev->struct_mutex);
> -		if (req == NULL)
> -			continue;
> -
> -		if (i915_gem_request_completed(req))
> -			i915_gem_object_retire__read(obj, i);
> -	}
> -
> -	return 0;
> -}
> -
> -/**
>   * i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT
>   * @dev: drm device pointer
>   * @data: ioctl data blob
> @@ -2820,24 +2791,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  		return -ENOENT;
>  	}
>  
> -	/* Need to make sure the object gets inactive eventually. */
> -	ret = i915_gem_object_flush_active(obj);
> -	if (ret)
> -		goto out;
> -
>  	if (!obj->active)
>  		goto out;
>  
> -	/* Do this after OLR check to make sure we make forward progress polling
> -	 * on this IOCTL with a timeout == 0 (like busy ioctl)
> -	 */
> -	if (args->timeout_ns == 0) {
> -		ret = -ETIME;
> -		goto out;
> -	}
> -
> -	i915_gem_object_put(obj);
> -
>  	for (i = 0; i < I915_NUM_ENGINES; i++) {
>  		struct drm_i915_gem_request *req;
>  
> @@ -2847,6 +2803,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  			requests[n++] = req;
>  	}
>  
> +out:
> +	i915_gem_object_put(obj);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	for (i = 0; i < n; i++) {
> @@ -2857,11 +2815,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  		i915_gem_request_put(requests[i]);
>  	}
>  	return ret;
> -
> -out:
> -	i915_gem_object_put(obj);
> -	mutex_unlock(&dev->struct_mutex);
> -	return ret;
>  }
>  
>  static int
> @@ -4036,13 +3989,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  
>  	/* Count all active objects as busy, even if they are currently not used
>  	 * by the gpu. Users of this interface expect objects to eventually
> -	 * become non-busy without any further actions, therefore emit any
> -	 * necessary flushes here.
> +	 * become non-busy without any further actions.
>  	 */
> -	ret = i915_gem_object_flush_active(obj);
> -	if (ret)
> -		goto unref;
> -
>  	args->busy = 0;
>  	if (obj->active) {
>  		struct drm_i915_gem_request *req;
> @@ -4060,7 +4008,6 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  			args->busy |= req->engine->exec_id;
>  	}
>  
> -unref:
>  	i915_gem_object_put(obj);
>  unlock:
>  	mutex_unlock(&dev->struct_mutex);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx