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

BR,
Jani.

-- 
Jani Nikula, Intel