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.gor...@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

Reply via email to