Re: [PATCH v2 13/15] drm/i915: Refactor direct GPU reset from request waiters

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 22/02/2017 11:46, Chris Wilson wrote:
> Combine the common code for the pair of waiters into a single function.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 42250eaf56ec..66d292384e60 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1024,6 +1024,16 @@ bool __i915_spin_request(const struct drm_i915_gem_request *req,
>  	return false;
>  }
>
> +static bool __i915_reset_request(struct drm_i915_gem_request *request)
> +{
> +	if (likely(!i915_reset_in_progress(&request->i915->gpu_error)))
> +		return false;
> +
> +	__set_current_state(TASK_RUNNING);
> +	i915_reset(request->i915);
> +	return true;
> +}
> +
>  /**
>   * i915_wait_request - wait until execution of request has finished
>   * @req: the request to wait upon
> @@ -1087,11 +1097,8 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  				break;
>
>  			if (flags & I915_WAIT_LOCKED &&
> -			    i915_reset_in_progress(&req->i915->gpu_error)) {
> -				__set_current_state(TASK_RUNNING);
> -				i915_reset(req->i915);
> +			    __i915_reset_request(req))
>  				continue;
> -			}
>
>  			if (signal_pending_state(state, current)) {
>  				timeout = -ERESTARTSYS;
> @@ -1160,12 +1167,8 @@ long i915_wait_request(struct drm_i915_gem_request *req,
>  		 * to come along and update the breadcrumb (either directly
>  		 * itself, or indirectly by recovering the GPU).
>  		 */
> -		if (flags & I915_WAIT_LOCKED &&
> -		    i915_reset_in_progress(&req->i915->gpu_error)) {
> -			__set_current_state(TASK_RUNNING);
> -			i915_reset(req->i915);
> +		if (flags & I915_WAIT_LOCKED && __i915_reset_request(req))
>  			continue;
> -		}
>
>  		/* Only spin if we know the GPU is processing this request */
>  		if (i915_spin_request(req, state, 2))
>

Okay in principle just unclear from the code now what it does. I suggest 
__i915_check_and_reset_request or __i915_request_check_and_reset so it 
is obvious the statement not only checks but also does stuff.

Regards,

Tvrtko

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