Re: [RESEND PATCH 4/5] drm: Add a drm_drv_enabled() helper function

From: Javier Martinez Canillas
Date: Thu Nov 04 2021 - 08:11:43 EST


Hello Jani,

On 11/4/21 12:10, Jani Nikula wrote:
> On Wed, 03 Nov 2021, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>> Hi
>>
>> Am 03.11.21 um 14:41 schrieb Jani Nikula:
>>> On Wed, 03 Nov 2021, Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote:
>>>> DRM drivers can use this to determine whether they can be enabled or not.
>>>>
>>>> For now it's just a wrapper around drm_modeset_disabled() but having the
>>>> indirection level will allow to add other conditions besides "nomodeset".
>>>>
>>>> Suggested-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>>>> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
>>>
>>> Can't see i915 trivially using this due to the drm_driver
>>> parameter. Please let's not merge helpers like this without actual
>>> users.
>>
>> Can you explain?
>>
>> This would be called on the top of the PCI probe function. The parameter
>> would allow to decide on a per-driver base (e.g., to ignore generic
>> drivers).
>
> Where and how exactly? This is why we need to see the patch using the
> function. A patch is worth a thousand words. ;)
>

Thomas suggested to squash patches #4 and #5 so I'll do that when posting v2.

> See current vgacon_text_force() usage in i915/i915_module.c. It happens
> way before anything related to pci or drm driver. Why bother with the
> complicated setup and teardown of stuff if you can bail out earlier?
>

Yes, most drivers use vgacon_text_force() in their module init callback.

The ones that do in their probe function are drivers that don't have a
module init/exit but just use the module_platform_driver() macro.

I won't modify that and will keep the bail in the same place that the
drivers already do. I hope to have time and post a new revision today.

>
> BR,
> Jani.
>
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat