Hi Am 15.12.21 um 09:50 schrieb Javier Martinez Canillas:
Hello Thomas, On 12/15/21 09:31, Thomas Zimmermann wrote:Hi Javier, nothing wrong with your patches, but I'd like to propose slightly differnt solution. For many USB drivers, you put the drm_firmware_drivers_only() call into the probe function. For registering, these drivers use module_usb_driver(), which expands to generic device-register functions. I'd like to propose a similar macro for DRM drivers that includes the test for drm_firmware_drivers_only(). In drm_drv.h : #if defined(USB) static int drm_usb_register(struct usb_driver *usb) { if (drm_firmware_drivers_only()) return -ENODEV; return usb_register_driver(usb); } #define drm_module_usb_driver(__usb) module_driver(drm_usb_register, usb_deregister) #endif In each of the USB-based DRM drivers, replace module_usb_driver with drm_module_usb_driver. And then there's PCI [3] and platform drivers, [4] which can be handled similarly. Many PCI drivers open-code the module init and deviceThanks for the suggestion. I actually thought about this approach as well, since seems unnecessary to have all that duplicated logic in every driver. But at the end decided to just do the less intrusive change, because can't do any testing for most of the drivers.registering with the driver-specific enable parameter. Maybe adding a driver-specific register function would make sense.If I understood you correctly, suggested something similar in #dri-devel but danvet's suggestion [0] was to keep it simple: 16:32 javierm: danvet: yes, I don't think we would be able to do it at this point. I'll add a function pointer param to the check function so drivers can also define their own check besides nomodeset 16:33 danvet: javierm, uh that sounds a bit like overkill? just keep that part of the check in drivers? 16:33 javierm: danvet: Ok. Wanted to get rid of the duplicated code but I'm indeed over engineering this :)
An extra pointer sounds indeed like over-engineering. If we take ast for example, I'd just do something like this:
static ast_pci_register_driver(struct pci_driver *pci)
{
if (drm_firmware_drivers_only() && ast_modeset == -1)
return -ENODEV;
if (ast_modeset == 0)
return -ENODEV;
return pci_register_driver(pci);
}
module_driver(&ast_pci_driver, ast_pci_register_driver,
pci_unregister_driver)
That removes some of the boiler-plate module code without changing the
driver's behavior.
Best regards Thomas
[0]: https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2021-11-02Best regards ThomasBest regards,
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
