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

[Lucas De Marchi]

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.
>
>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?


Lucas De Marchi