[Intel-gfx] [PATCH 4/4] drm/i915: Introduce concept of a sub-platform

[Chris Wilson]

Quoting Jani Nikula (2019-03-26 09:34:45)
> Not to block this series, but looking further outside the box...
> 
> I've still got the constant vs. runtime device info split
> unfinished. We've got so many things that are mostly constant, but
> occasionally need changes. And we've got so many things that could be
> device info flags, but would lead to proliferation of plenty of almost
> identical device info structures. Like this ULX/ULT and GT number.
> 
> So I guess I'm wondering if we're doing the right thing by assigning
> device info pointers to the struct pci_device_id driver_data member in
> pciidlist[] table.
> 
> For one thing, that's a whole lot of bits that could be used directly
> for assigning platform and subplatform, or features.
> 
> Of course, we'd then need another table besides pciidlist[] to map to
> the device info, but we're sort of doing some of that with the ULX/ULT
> parts.
> 
> I just overall feel that there must be a better way to do all this, and
> we just haven't figured it out yet, and we're partially putting
> ourselves into a box we can't break out of.
> 
> Thoughts?

I think intel_device_info is still fundamentally useful. The
disadvantage of having the feature discovery separate from use is
outweighed by having consistent stanzas for those features - it makes
comparing platforms, finding feature sets much easier. (The cost being
that with the setting of the feature flag far away from the code using
it, people updating the cost are more likely to forget the flag.)

One end goal of this madness, is that we can recompile the kernel module
to only support a single sku and dce the rest. But what are the
diminishing returns here? Without measurement, I'd say a single
platform.

So that dictates what can be in the static intel_device_info, features
that are constant across a whole platform. And as you point out, we
don't need a pointer to the device_info itself, just a platform field
which is an index into the device_info block, with plenty of room for
subplatform flags.

While that says how we hook up device_info, I don't think that reflects
on the use of feature flags themselves, or our ability to statically
determine a reduced feature set.

So not a box, just a mere wet paper bag.
-Chris