Re: [RFC] drm/{i915,xe}/display: pass hooks to display probe

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Oct 06, 2025 at 06:11:41PM +0300, Jani Nikula wrote:
> On Mon, 06 Oct 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Fri, Oct 03, 2025 at 12:21:07PM +0300, Jani Nikula wrote:
> >> Let's gradually start calling i915 and xe core drivers from display via
> >> function pointers passed at display probe. For starters, just add a
> >> small feature test hook ->has_flat_ccs.
> >> 
> >> FIXME: "hooks" is a terrible name, both as a parameter and a struct
> >> intel_display member name. It should reflect that we're calling the core
> >> or parent driver. This is a placeholder name for now.
> >> 
> >> FIXME: Initially, one struct is fine... but once it accumulates a lot of
> >> functions, should it have more indirection? Maybe everything should be
> >> in sub-structs, or the top struct should just be a collection of
> >> pointers to hook structs?
> >> 
> >> 	struct intel_core_hooks {
> >> 		const struct intel_rpm_hooks *rpm;
> >> 	};
> >> 
> >> The above would allow having the struct initialization inside the
> >> implementation file, and the functions themselves static. In any case,
> >> it seems best to have just one initialization of the hooks, instead of
> >> one init/register call for each functional area.
> >> 
> >> The downside is of course having to call the functions like:
> >> 
> >> 	display->hooks->rpm->get(display->drm);
> >> 
> >> FIXME: It would really be convenient if we could stop using a display
> >> device with mock_gem_device() in mock_gem_device.c. The purpose of the
> >> mock gem device is to run mock *gem* tests. Could we make it happen
> >> without display?
> >
> > Haven't really thought too much about the display->xe/i915 direction
> > yet, but I was pondering the opposite direction. Should we even use
> > vfuncs there or perhaps just export the symbols from the display side?
> >
> > But symbol name collisions do worry me. There does seem to be a
> > EXPORT_SYMBOL_FOR_MODULES() now, but I have a nagging feeling that it
> > doesn't actually implement symbol namespaces (as in the same symbol
> > name could be defined in multiple namespaces) but rather just prevents
> > modules from loading when they don't have the permission to use a
> > "namespaced" symbol.
> >
> > So with exports we'd probably have to carefully prefix each exported
> > symbol with "intel_display_" (or whatever). We could of course still
> > use EXPORT_SYMBOL_FOR_MODULES() to make sure other stuff can't access
> > those symbols.
> 
> There's EXPORT_SYMBOL_NS() and MODULE_IMPORT_NS() nowadays that should
> help. We should probably still have the prefixes, but it the namespace
> should tackle the collisions.

I just did a quick test and the build already fails at modpost
if two modules try to export the same symbol name with different
"namespaces". So the "namespace" in the name of this feature is
basically a complete lie. All the symbols still live in the same
global namespace and some modules are just forbidden from calling
some of them. Any actual namespace will still have to be baked
into the symbol name at the exporter.

-- 
Ville Syrjälä
Intel