Re: [PATCH] drm/i915: Remove request->reset_counter

[Arun Siluvery <arun.siluvery@linux.intel.com>]

On 29/06/2016 15:51, Chris Wilson wrote:
> Since commit 2ed53a94d8cb ("drm/i915: On GPU reset, set the HWS
> breadcrumb to the last seqno") once a hang is completed, the seqno is
> advanced past all current requests. With this we know that if we wake up
> on waiting for a request, if a hang has occurred and reset completed,
> our request will be considered complete (i.e.
> i915_gem_request_completed() returns true). Therefore we only need to
> worry about the situation where a hang has occurred, but not yet reset,
> where we may need to release our struct_mutex. Since we don't need to
> detect the competed reset using the global gpu_error->reset_counter

s/competed/completed

> anymore, we do not need to track the reset_counter epoch inside the
> request.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Arun Siluvery <arun.siluvery@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  1 -
>   drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++--------
>   2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index def011811421..485ab1148181 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2368,7 +2368,6 @@ struct drm_i915_gem_request {
>   	/** On Which ring this request was generated */
>   	struct drm_i915_private *i915;
>   	struct intel_engine_cs *engine;
> -	unsigned reset_counter;
>
>   	 /** GEM sequence number associated with the previous request,
>   	  * when the HWS breadcrumb is equal to this the GPU is processing
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 51191b879747..1d9878258103 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1506,12 +1506,13 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>
>   		/* We need to check whether any gpu reset happened in between
>   		 * the request being submitted and now. If a reset has occurred,
> -		 * the request is effectively complete (we either are in the
> -		 * process of or have discarded the rendering and completely
> -		 * reset the GPU. The results of the request are lost and we
> -		 * are free to continue on with the original operation.
> +		 * the seqno will have been advance past ours and our request
> +		 * is complete. If we are in the process of handling a reset,
> +		 * the request is effectively complete as the rendering will
> +		 * be discarded, but we need to return in order to drop the
> +		 * struct_mutex.
>   		 */
> -		if (req->reset_counter != i915_reset_counter(&dev_priv->gpu_error)) {
> +		if (i915_reset_in_progress(&dev_priv->gpu_error)) {
>   			ret = 0;
>   			break;
>   		}
> @@ -1685,7 +1686,7 @@ i915_wait_request(struct drm_i915_gem_request *req)
>   		return ret;
>
>   	/* If the GPU hung, we want to keep the requests to find the guilty. */
> -	if (req->reset_counter == i915_reset_counter(&dev_priv->gpu_error))
> +	if (!i915_reset_in_progress(&dev_priv->gpu_error))
>   		__i915_gem_request_retire__upto(req);
>
>   	return 0;
> @@ -1746,7 +1747,7 @@ i915_gem_object_retire_request(struct drm_i915_gem_object *obj,
>   	else if (obj->last_write_req == req)
>   		i915_gem_object_retire__write(obj);
>
> -	if (req->reset_counter == i915_reset_counter(&req->i915->gpu_error))
> +	if (!i915_reset_in_progress(&req->i915->gpu_error))
>   		__i915_gem_request_retire__upto(req);
>   }
>
> @@ -3021,7 +3022,6 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
>   	kref_init(&req->ref);
>   	req->i915 = dev_priv;
>   	req->engine = engine;
> -	req->reset_counter = reset_counter;
>   	req->ctx  = ctx;
>   	i915_gem_context_reference(req->ctx);
>
>

this looks good to me,
Reviewed-by: Arun Siluvery <arun.siluvery@linux.intel.com>

regards
Arun

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