[Intel-gfx] [RFC 6/7] drm/i915: Introduce subplatform concept

[Tvrtko Ursulin]

On 13/11/2018 11:40, Jani Nikula wrote:
> On Mon, 12 Nov 2018, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Introduce subplatform mask to eliminate throughout the code devid checking
>> sprinkle, mostly courtesy of IS_*_UL[TX] macros.
>>
>> Subplatform mask initialization is done at runtime device info init.
> 
> I kind of like the concept, and I like the centralization of devid
> checks in one function, but I've always wanted to take this to one step
> further: only specify device ids in i915_pciids.h, and *nowhere* else.
> 
> It's perhaps too much duplication to create a device info for all these
> variants, but I think it would be possible to make the subplatform info
> table driven using macros defined in i915_pciids.h.

It would be much nicer, but how would you do it? Perhaps my imagination 
is just strong enough today.

Simply by splitting the id's into subplatform parts, for instance where 
we have today:

#define INTEL_BDW_GT1_IDS(info)  \
         INTEL_VGA_DEVICE(0x1602, info), /* GT1 ULT */ \
         INTEL_VGA_DEVICE(0x1606, info), /* GT1 ULT */ \
         INTEL_VGA_DEVICE(0x160B, info), /* GT1 Iris */ \
         INTEL_VGA_DEVICE(0x160E, info), /* GT1 ULX */ \
         INTEL_VGA_DEVICE(0x160A, info), /* GT1 Server */ \
         INTEL_VGA_DEVICE(0x160D, info)  /* GT1 Workstation */

We'd split to:

#define INTEL_BDW_GT1_ULT_IDS(info)  \
         INTEL_VGA_DEVICE(0x1602, info), /* GT1 ULT */ \
         INTEL_VGA_DEVICE(0x1606, info), /* GT1 ULT */ \

#define INTEL_BDW_GT1_ULX_IDS(info)  \
         INTEL_VGA_DEVICE(0x160E, info), /* GT1 ULX */ \

#define INTEL_BDW_GT1_IDS(info)  \
         INTEL_VGA_DEVICE(0x160B, info), /* GT1 Iris */ \
         INTEL_VGA_DEVICE(0x160A, info), /* GT1 Server */ \
         INTEL_VGA_DEVICE(0x160D, info)  /* GT1 Workstation */

Then in i915_pci.c, instead of:

	...
	INTEL_BDW_GT1_IDS(&intel_broadwell_gt1_info),
	...

We'd have:

	...
	INTEL_BDW_GT1_ULT_IDS(&intel_broadwell_gt1_info),
	INTEL_BDW_GT1_ULX_IDS(&intel_broadwell_gt1_info),
	INTEL_BDW_GT1_IDS(&intel_broadwell_gt1_info),
	...

And a separate table to map the id's to subplatform values.

Hmm, but we would probably need to extrac the id's from the 
INTEL_BDW1_GT_IDS like macros so they can be used in this second site 
without the info parameter. Something like the trick for device info 
flags, but can it be made to generate a macro? I think not..

Regards,

Tvrtko