On Mon, May 23, 2016 at 11:55:16AM +0100, Tvrtko Ursulin wrote: > > On 23/05/16 11:17, Chris Wilson wrote: > >On Mon, May 23, 2016 at 10:26:39AM +0100, Tvrtko Ursulin wrote: > >>>@@ -385,20 +384,18 @@ static void guc_init_ctx_desc(struct intel_guc *guc, > >>> * for now who owns a GuC client. But for future owner of GuC > >>> * client, need to make sure lrc is pinned prior to enter here. > >>> */ > >>>- obj = ctx->engine[id].state; > >>>- if (!obj) > >>>+ if (!ce->state) > >>> break; /* XXX: continue? */ > >>> > >>>- ctx_desc = intel_lr_context_descriptor(ctx, engine); > >>>- lrc->context_desc = (u32)ctx_desc; > >>>+ lrc->context_desc = lower_32_bits(ce->lrc_desc); > >> > >>Could have kept use of intel_lr_context_descriptor for better separation. > > > >I was leaning the other way, since the code doesn't want > >intel_lr_context_descriptor() just happens to want to reuse some of the > >bits e.g. engine->ctx_desc_template | lrca > > Thats true, but it was at least using an exported function with > documented content, rather than directly fishing out stuff from > essentially private data elsewhere.
It's still fishing out essentially private data though :) If engine->ctx_desc_template considers GuC for its set of flags, it is purely by happenstance. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
