Re: [PATCH 07/15] drm/i915: Hold a reference on the request for its fence chain

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

On pe, 2016-11-25 at 09:30 +0000, Chris Wilson wrote:
> Currently, we have an active reference for the request until it is
> retired. Though it cannot be retired before it has been executed by
> hardware, the request may be completed before we have finished
> processing the execute fence, i.e. we may continue to process that fence
> as we free the request.
> 
> Fixes: 5590af3e115a ("drm/i915: Drive request submission through fence callbacks")
> Fixes: 23902e49c999 ("drm/i915: Split request submit/execute phase into two")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -551,8 +569,10 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
>  		       req->timeline->fence_context,
>  		       __timeline_get_seqno(req->timeline->common));
>  
> -	i915_sw_fence_init(&req->submit, submit_notify);
> -	i915_sw_fence_init(&req->execute, execute_notify);
> +	/* We bump the ref for the fence chain */
> +	i915_sw_fence_init(&i915_gem_request_get(req)->submit, submit_notify);
> +	i915_sw_fence_init(&i915_gem_request_get(req)->execute, execute_notify);

I think having the gem_request_get in a separate line before fence_init
and remove the comment. Make the code speak for itself instead of
camouflaging it. Now it looks much like a type conversion helper.

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

Regards, Joonas
-- 
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