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@xxxxxxxxx>
#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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
