On 26/03/2019 09:53, Chris Wilson wrote:
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.
To check if I follow.. we are talking about potentially abolishing
device info in favour of constructing something at probe time, which
could potentially have fewer and overall smaller static data portion?
Because I don't see how we can eliminate the pciidlist itself, or even
shrink it's size? It has to have one entry per device id, just the
question for what we use driver_data for?
Static vs runtime I think shouldn't have effect on the per platform
builds. As long as all feature tests are done via macros, or small
static inlines, code can still be compiled out.
I do have a small nagging feeling about this series as well, but I have
managed to convince myself it is better than the device id listed in
i915_drv.h. So don't know.. we can always drop it and just expand
platform mask to u64 to solve the immediate need and leave the rest for
later.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
