[Intel-xe] [RFC] drm/i915: add kconfig option to enable/disable legacy platform support

[Tvrtko Ursulin]

On 14/03/2023 17:22, Lucas De Marchi wrote:
> On Tue, Mar 14, 2023 at 06:42:56PM +0200, Jani Nikula wrote:
>> On Tue, 14 Mar 2023, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> 
>> wrote:
>>> On 14/03/2023 11:43, Jani Nikula wrote:
>>>> On Thu, 09 Mar 2023, Jani Nikula <jani.nikula at intel.com> wrote:
>>>>> Add config option DRM_I915_LEGACY to enable/disable legacy platform
>>>>> support. This is primarily for the benefit of the drm/xe driver, and
>>>>> legacy is defined in terms of the platforms drm/xe does not support,
>>>>> i.e. anything before Tigerlake.
>>>>>
>>>>> While the kconfig option will be CONFIG_DRM_I915_LEGACY, the intention
>>>>> is that it's not used in code. Instead, we'll pass -DI915_LEGACY=1 in
>>>>> the i915 Makefile for CONFIG_DRM_I915_LEGACY=y, while the xe Makefile
>>>>> does no such thing, regardless of the kconfig value.
>>>>>
>>>>> Initially, the knob does the bare minimum: drops the legacy platforms
>>>>> from module PCI ID table (and the compiler in turn automagically drops
>>>>> all the unreferenced device infos).
>>>>>
>>>>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>>>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>>>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>>
>>>> The discussion stalled a bit.
>>>>
>>>> Do we have consensus to start adding this to upstream i915?
>>>
>>> I always liked the idea of compiling out platform support so I could be
>>> convinced. I view that as a "power user" use case - compiles their own
>>> kernel for a targeted machine. It also translates to building smaller
>>> images in production settings although that is kind of not interesting
>>> with the storage amounts these days. So overall feels could be
>>> justified. There is some benefit and could be done with minimal
>>> maintenance cost.
>>>
>>> But to add a Kconfig calling something "legacy", by the definition of
>>> what Xe will support feels maybe a bit premature. Sure it will become
>>> super useful once Xe is in the tree, to allow exactly the same class
>>> use-case as above, but until then it feels questionable under your own
>>> criteria too.
>>
>> I don't disagree. Partially the idea with "legacy" was to be a bit
>> vague, so we could tweak what it really means later.
>>
>>> If you could add a set of more generic options, which Xe could later tie
>>> into that would work for me. For instance we have some more natural
>>> cross-over points than Tigerlake. So if not per individual platform,
>>> maybe for like ring buffer -> execlists -> GuC transitions. And naming
>>> them without saying legacy for now, but use some descriptive names, and
>>> listing platform code names in help text. "Select this to support
>>> Broadwell, Skylake, etc..", "Select this to support Sandybridge..". Out
>>> of the tree Xe build can then just not use the corresponding defines in
>>> its own build and it would achieve what you need?
>>
>> I kind of wanted to avoid adding a lot of config options, because I
>> think they'll be difficult to maintain and get all the combos right. I
>> don't particularly want all the build bot reports about various kconfig
>> combos failing.

Hm these should not be tricky to that extent to cause any combinatorial 
issues. Famous last words? :)

>> One other problem is that I can't think of a way to do this by using the
>> kconfig CONFIG_FOO macros directly; you have to add separate variables
>> because the same files are built for two drivers. You can't force the
>> CONFIG_FOO macros to different settings for different drivers. So we'd
>> get a lot of proxy macros too.
>>
>> I wonder if there's a name we could use instead of legacy to reasonably
>> match what Xe needs to avoid adding tons of configs at once?
> 
> considering TGL itself is also "xe" arch (not to be confused with xe,
> the kernel module), maybe CONFIG_DRM_I915_PRE_XE?

Makes sense as a name I think. But why is the tricky question? :) Why do 
Xe+ users deserve the privilege of being able to compile out the old 
cruft but users of old machines cannot compile out Xe+? In other words 
how to make the thing attractive for not just Xe (the driver).

Mind you I am not really opposed, as long as it will be made in a 
flexible way. Just would like to explore the option of making it more 
widely useful.

Regards,

Tvrtko