Hi,
On 4/15/20 1:39 PM, Hans de Goede wrote:
<snip>
/* Add comment explaining why we need this messy stuff here */
const char * const shadow_providers[] = {
#ifdef CONFIG_THINKPAD_ACPI_MODULE
"thinkpad_acpi",
#endif
#ifdef CONFIG_OTHER_MODULE
"other",
#endif
NULL
};
int module_init(void)
{
/* do actual setup of the ?class? */
for (i = 0; shadow_providers[i]; i++)
request_module(shadow_providers[i]);
return 0;
}
Hm I think explicitly loading drivers feels very much not device model
like. Don't these drivers auto-load, matching on acpi functions?
thinkpad_acpi does autoload based on a number of ACPI device-ids,
the idea behind the above request_module is to avoid the need
to have the acpi-match function you mentioned above.
Basically what would happen is e.g. :
1. i915 loads, calls lcdshadow_get(dev, "eDP-1");
2. if this is the first lcdshadow_get() call then
the lcdshadow core will do the request_module calls,
allowing any of these modules to get loaded + probed and
call e.g. lcdshadow_register(&mylcdshadowdev, <gfx-adapter-dev-name>, "eDP-1");
3. After the request modules the lcdshadow_get() will walk over
the list of registered devices, including the ones just registered
by the request_module calls and then hopefully find a match
So by doing the request-module calls before checking for
a matching lcdshadow dev, we avoid the need of having some of
the knowledge currently abstracted away in the thinkpad_acpi driver
duplicated inside the drm code somewhere.
I guess if that doesn't exist, then we'd need to fix that one first :-/
In general no request_module please, that shouldn't be needed either.
The trouble with request_module is also that (afaiui) it doesn't
really work well with parallel module load and all that, for
EPROBE_DEFER to work we do need to be able to answer "should we have a
driver?" independently of whether that driver has loaded already or
not.
The idea here is to avoid using EPROBE_DEFER (on x86 at least)
and either directly return the lcdshadow_dev or ENOENT. Also
see below.
<snip>
Assuming we are going to add some device/model specific
lcdshadow knowledge inside the lcdshadow core as you
suggested with your "small acpi match function" above,
we could do something similar to what the vga_switcheroo
code is doing for this and have a lcdshadow_defer_probe()
helper and call that really early in i915_pci_probe(),
which currently already has this for the vgaswitcheroo case:
if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
So thinking more about this and given the total lack of
EPROBE_DEFER handling in the 3 major X86 GPU/kms drivers
I think that adding a lcdshadow_defer_probe() helper is
the way to go. This will also avoid the need for duplicating
the lcdshadow detect functionality in the small ACPI-match
functions you mentioned (although that might still be
interesting to speedup the boot).
When everything is builtin then each enabled "module"-s
module_init function will get called, we can call a
lcdshadow_probe_done("module-name") function from those
and the lcdshadow core can then track if all potential
lcdhadow providers have initialized before it stops
returning non 0 from lcdshadow_defer_probe().
Or if we still do the small match functions it could
be even smarter with this...
And for the modular case it can call request_module on
all (enabled as module) potential lcdhadow providers
(or again we could rely on the small match function
instead).
Then (on x86 at least) we can have lcdshadow_get never
return -EPROBE_DEFER and avoid the need to solve the
lack of EPROBE_DEFER support in the 3 major x86 drivers.
And this is all kernel internal, so if that lack of
EPROBE_DEFER support ever gets fixed then we can drop
the lcdshadow_defer_probe() hack and make
lcdshadow_get also return -EPROBE_DEFER on x86 in
some cases.
Regards,
Hans
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
