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

[Mauro Carvalho Chehab]

On Tue, 14 Mar 2023 18:27:33 +0000
Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:

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

Agreed. This is a way better than LEGACY, but, this needs to be translated into
a Kconfig option like "enable support for pre-Tiger Lake integrated graphics",
e. g. translating into something understandable for the customers.

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

Yeah, for me, it makes a lot more sense to disable i915 support for
the platforms that Xe driver also support, e. g. we would need also
a CONFIG_DRM_I915_XE to allow disabling i915 support for GPUs 
supported by Xe. When enabled, this can be used to let Xe-supported
PCI IDs to be probed by Xe driver, instead of i915.


Yet, it sounds a little bit premature to have this, at least while
not all GPU Xe models features supported by i915 driver for TGL+ are
implemented. So, maybe we need to describe this as experimental,
having a default to "N".

Regards,
Mauro