Re: [PATCH v2 01/37] drm: Add drm_module_{pci,platform}_driver() helper macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Javier,

looks good already. Some comments are below.

Am 17.12.21 um 01:37 schrieb Javier Martinez Canillas:
According to disable Documentation/admin-guide/kernel-parameters.txt, the
nomodeset parameter can be used to disable kernel modesetting.

DRM drivers will not perform display-mode changes or accelerated rendering
and only the system framebuffer will be available if it was set-up.

But only a few DRM drivers currently check for nomodeset, so let's add two
helper macros that can be used by DRM drivers for PCI and platform devices
to have module init functions that checks if the drivers could be loaded.

Suggested-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
---

(no changes since v1)

  include/drm/drm_drv.h | 50 +++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 50 insertions(+)

diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h

I worked on a similar patch today and found that drm_drv.h is actually not a good place. Half of DRM includes this file and now it all depends on linux/pci.h and linux/platform.h (and probably other later).

I propose to put the module helpers into <drm/drm_module.h> and include it where necessary.

index f6159acb8856..4001d73428c5 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -29,6 +29,8 @@
#include <linux/list.h>
  #include <linux/irqreturn.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
#include <drm/drm_device.h> @@ -604,4 +606,52 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name); extern bool drm_firmware_drivers_only(void); +/**
+ * drm_pci_register_driver() - register a DRM driver for PCI devices
+ * @drv: PCI driver structure
+ *
+ * Returns zero on success or a negative errno code on failure.
+ */
+static inline int drm_pci_register_driver(struct pci_driver *drv)

This should be declared as __init, so it goes into a separate section of the module. IIRC the page in the init section are released after the module has been loaded.

I'd either not document the register functions, or explicitly say that the module macros are the preferred way of using them.

+{
+	if (drm_firmware_drivers_only())
+		return -ENODEV;
+
+	return pci_register_driver(drv);
+}
+
+/**
+ * drm_module_pci_driver() - helper macro for registering a DRM PCI driver

Docs for the __pci_driver argument

+ *
+ * Helper macro for DRM PCI drivers which do not do anything special in their
+ * module init/exit and just need the DRM specific module init.
+ */
+#define drm_module_pci_driver(__pci_driver) \
+	module_driver(__pci_driver, drm_pci_register_driver, \
+		      pci_unregister_driver)
+
+/**
+ * drm_platform_driver_register - register a DRM driver for platform devices
+ * @drv: platform driver structure
+ *
+ * Returns zero on success or a negative errno code on failure.
+ */
+static inline int drm_platform_driver_register(struct platform_driver *drv)


+{
+	if (drm_firmware_drivers_only())
+		return -ENODEV;
+
+	return platform_driver_register(drv);
+}
+
+/**
+ * drm_module_platform_driver() - helper macro for registering a DRM platform driver

Docs for the __platform_driver argument

Best regards
Thomas

+ *
+ * Helper macro for DRM platform drivers which do not do anything special in their
+ * module init/exit and just need the DRM specific module init.
+ */
+#define drm_module_platform_driver(__platform_driver) \
+	module_driver(__platform_driver, drm_platform_driver_register, \
+		      platform_driver_unregister)
+
  #endif


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


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux