Re: [PATCH 2/4] drm/i915: mark the global default (intel_)context as such

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, Dec 16, 2015 at 06:36:49PM +0000, Dave Gordon wrote:
> Some of the LRC-specific context-destruction code has to special-case
> the global default context, because the HWSP is part of that context. At
> present it deduces it indirectly by checking for the backpointer from
> the engine to the context, but that's an unsafe assumption if the setup
> and teardown code is reorganised. (It could also test !ctx->file_priv,
> but again that's a detail that might be subject to change).
> 
> So here we explicitly flag the default context at the point of creation,
> and then reorganise the code in intel_lr_context_free() not to rely on
> the ring->default_pointer (still) being set up; to iterate over engines
> in reverse (as this is teardown code); and to reduce the nesting level
> so it's easier to read.
> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>

#define intel_context_is_global(ctx) ((ctx)->file_priv == NULL)

> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 3aa6147..23f90b2 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2367,22 +2367,21 @@ void intel_lr_context_free(struct intel_context *ctx)
>  {
>  	int i;
>  
> -	for (i = 0; i < I915_NUM_RINGS; i++) {
> +	for (i = I915_NUM_RINGS; --i >= 0; ) {
> +		struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
>  		struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
>  
> -		if (ctx_obj) {
> -			struct intel_ringbuffer *ringbuf =
> -					ctx->engine[i].ringbuf;
> -			struct intel_engine_cs *ring = ringbuf->ring;
> +		if (!ctx_obj)
> +			continue;
>  
> -			if (ctx == ring->default_context) {
> -				intel_unpin_ringbuffer_obj(ringbuf);
> -				i915_gem_object_ggtt_unpin(ctx_obj);
> -			}
> -			WARN_ON(ctx->engine[ring->id].pin_count);
> -			intel_ringbuffer_free(ringbuf);
> -			drm_gem_object_unreference(&ctx_obj->base);
> +		if (ctx->is_global_default) {
> +			intel_unpin_ringbuffer_obj(ringbuf);
> +			i915_gem_object_ggtt_unpin(ctx_obj);
>  		}
> +
> +		WARN_ON(ctx->engine[i].pin_count);
> +		intel_ringbuffer_free(ringbuf);
> +		drm_gem_object_unreference(&ctx_obj->base);

That looks much neater, indeed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx