Re: [PATCH 06/12] drm/i915: Name the inner most per-engine intel_context struct

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.

> i.e. lrc->context_desc != ce->lrc_desc and I would prefer it to be
> clarified as to exactly what the GuC expects.

Yes that would be best. Cc-ed Dave for when he is back to comment what 
does GuC really wants in there.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx