Re: RFC: Drm-connector properties managed by another driver / privacy screen support

[Hans de Goede <hdegoede@redhat.com>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel