[Intel-gfx] [PATCH 1/5] drm/i915: unify first-stage engine struct setup

[Dave Gordon]

On 13/07/16 14:16, Tvrtko Ursulin wrote:
>
> On 13/07/16 13:23, Daniel Vetter wrote:
>> On Fri, Jul 01, 2016 at 05:47:11PM +0100, Tvrtko Ursulin wrote:
>>> From: Dave Gordon <david.s.gordon at intel.com>
>>>
[snip]
>>>   {
>>> -    const struct logical_ring_info *info = &logical_rings[id];
>>> +    const struct engine_info *info = &intel_engines[id];
>>>       struct intel_engine_cs *engine = &dev_priv->engine[id];
>>> -    enum forcewake_domains fw_domains;
>>>
>>>       engine->id = id;
>>> +    engine->i915 = dev_priv;
>>>       engine->name = info->name;
>>>       engine->exec_id = info->exec_id;
>>> -    engine->guc_id = info->guc_id;
>>> +    engine->hw_id = engine->guc_id = info->guc_id;
>>
>> Optional bikeshed: s/info->guc_id/info->hw_id/ makes sense imo in the new
>> context. Or nuking engine->guc_id.
>
> Someone said we cannot be sure they will be the same in the future. So
> maybe just rename to hw_id for now.
>
> Regards,
> Tvrtko

The GuC firmware *could* use a completely different enumeration of the 
engines, but why would it? Since it's closely tied to the hardware, it 
*ought* to use the same naming scheme as the h/w. So I have no objection 
to getting rid of guc_id entirely, and changing existing uses to use 
hw_id instead.

OTOH it seems little benefit and would certainly involve more work to 
reverse. The firmware team might, after all, decide that they too want 
to decouple the logical-engine-numbers used for the KMD interface from 
whatever the hardware team decide is the best way to number engines -- 
which might after all change between generations or even different SKUs 
of the same device!

SO, I think the "best" version of that line is probably:

+	engine->hw_id = info->hw_id;
+
+	/* Current GuC f/w uses hw_id not driver id */
+	engine->guc_id = info->hw_id;

and we'll add "info->guc_id" back again if it ever becomes necessary.

.Dave.