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

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 06 Oct 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

Ugh. *facepalm*.


-- 
Jani Nikula, Intel