| Message ID | 20211103123206.1041442-1-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Cleanups for the nomodeset kernel command line parameter logic | expand |
On Wed, 03 Nov 2021, Javier Martinez Canillas <javierm@redhat.com> 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@suse.de> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Can't see i915 trivially using this due to the drm_driver parameter. Please let's not merge helpers like this without actual users. BR, Jani. > --- > > drivers/gpu/drm/drm_drv.c | 21 +++++++++++++++++++++ > include/drm/drm_drv.h | 1 + > 2 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 8214a0b1ab7f..70ef08941e06 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -975,6 +975,27 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name) > } > EXPORT_SYMBOL(drm_dev_set_unique); > > +/** > + * drm_drv_enabled - Checks if a DRM driver can be enabled > + * @driver: DRM driver to check > + * > + * Checks whether a DRM driver can be enabled or not. This may be the case > + * if the "nomodeset" kernel command line parameter is used. > + * > + * Returns: > + * True if the DRM driver is enabled and false otherwise. > + */ > +bool drm_drv_enabled(const struct drm_driver *driver) > +{ > + if (drm_modeset_disabled()) { > + DRM_INFO("%s driver is disabled\n", driver->name); > + return false; > + } > + > + return true; > +} > +EXPORT_SYMBOL(drm_drv_enabled); > + > /* > * DRM Core > * The DRM core module initializes all global DRM objects and makes them > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 0cd95953cdf5..48f2b6eec012 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -598,5 +598,6 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev) > > int drm_dev_set_unique(struct drm_device *dev, const char *name); > > +bool drm_drv_enabled(const struct drm_driver *driver); > > #endif
Hi Am 03.11.21 um 14:41 schrieb Jani Nikula: > On Wed, 03 Nov 2021, Javier Martinez Canillas <javierm@redhat.com> 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@suse.de> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > > 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). Best regards Thomas > > BR, > Jani. > > >> --- >> >> drivers/gpu/drm/drm_drv.c | 21 +++++++++++++++++++++ >> include/drm/drm_drv.h | 1 + >> 2 files changed, 22 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index 8214a0b1ab7f..70ef08941e06 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -975,6 +975,27 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name) >> } >> EXPORT_SYMBOL(drm_dev_set_unique); >> >> +/** >> + * drm_drv_enabled - Checks if a DRM driver can be enabled >> + * @driver: DRM driver to check >> + * >> + * Checks whether a DRM driver can be enabled or not. This may be the case >> + * if the "nomodeset" kernel command line parameter is used. >> + * >> + * Returns: >> + * True if the DRM driver is enabled and false otherwise. >> + */ >> +bool drm_drv_enabled(const struct drm_driver *driver) >> +{ >> + if (drm_modeset_disabled()) { >> + DRM_INFO("%s driver is disabled\n", driver->name); >> + return false; >> + } >> + >> + return true; >> +} >> +EXPORT_SYMBOL(drm_drv_enabled); >> + >> /* >> * DRM Core >> * The DRM core module initializes all global DRM objects and makes them >> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h >> index 0cd95953cdf5..48f2b6eec012 100644 >> --- a/include/drm/drm_drv.h >> +++ b/include/drm/drm_drv.h >> @@ -598,5 +598,6 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev) >> >> int drm_dev_set_unique(struct drm_device *dev, const char *name); >> >> +bool drm_drv_enabled(const struct drm_driver *driver); >> >> #endif >
On Wed, 03 Nov 2021, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Hi > > Am 03.11.21 um 14:41 schrieb Jani Nikula: >> On Wed, 03 Nov 2021, Javier Martinez Canillas <javierm@redhat.com> 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@suse.de> >>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> >> >> 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. ;) 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? BR, Jani. > > Best regards > Thomas > >> >> BR, >> Jani. >> >> >>> --- >>> >>> drivers/gpu/drm/drm_drv.c | 21 +++++++++++++++++++++ >>> include/drm/drm_drv.h | 1 + >>> 2 files changed, 22 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >>> index 8214a0b1ab7f..70ef08941e06 100644 >>> --- a/drivers/gpu/drm/drm_drv.c >>> +++ b/drivers/gpu/drm/drm_drv.c >>> @@ -975,6 +975,27 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name) >>> } >>> EXPORT_SYMBOL(drm_dev_set_unique); >>> >>> +/** >>> + * drm_drv_enabled - Checks if a DRM driver can be enabled >>> + * @driver: DRM driver to check >>> + * >>> + * Checks whether a DRM driver can be enabled or not. This may be the case >>> + * if the "nomodeset" kernel command line parameter is used. >>> + * >>> + * Returns: >>> + * True if the DRM driver is enabled and false otherwise. >>> + */ >>> +bool drm_drv_enabled(const struct drm_driver *driver) >>> +{ >>> + if (drm_modeset_disabled()) { >>> + DRM_INFO("%s driver is disabled\n", driver->name); >>> + return false; >>> + } >>> + >>> + return true; >>> +} >>> +EXPORT_SYMBOL(drm_drv_enabled); >>> + >>> /* >>> * DRM Core >>> * The DRM core module initializes all global DRM objects and makes them >>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h >>> index 0cd95953cdf5..48f2b6eec012 100644 >>> --- a/include/drm/drm_drv.h >>> +++ b/include/drm/drm_drv.h >>> @@ -598,5 +598,6 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev) >>> >>> int drm_dev_set_unique(struct drm_device *dev, const char *name); >>> >>> +bool drm_drv_enabled(const struct drm_driver *driver); >>> >>> #endif >>
Hello Jani, On 11/4/21 12:10, Jani Nikula wrote: > On Wed, 03 Nov 2021, Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Hi >> >> Am 03.11.21 um 14:41 schrieb Jani Nikula: >>> On Wed, 03 Nov 2021, Javier Martinez Canillas <javierm@redhat.com> 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@suse.de> >>>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> >>> >>> 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,
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 8214a0b1ab7f..70ef08941e06 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -975,6 +975,27 @@ int drm_dev_set_unique(struct drm_device *dev, const char *name) } EXPORT_SYMBOL(drm_dev_set_unique); +/** + * drm_drv_enabled - Checks if a DRM driver can be enabled + * @driver: DRM driver to check + * + * Checks whether a DRM driver can be enabled or not. This may be the case + * if the "nomodeset" kernel command line parameter is used. + * + * Returns: + * True if the DRM driver is enabled and false otherwise. + */ +bool drm_drv_enabled(const struct drm_driver *driver) +{ + if (drm_modeset_disabled()) { + DRM_INFO("%s driver is disabled\n", driver->name); + return false; + } + + return true; +} +EXPORT_SYMBOL(drm_drv_enabled); + /* * DRM Core * The DRM core module initializes all global DRM objects and makes them diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 0cd95953cdf5..48f2b6eec012 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -598,5 +598,6 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev) int drm_dev_set_unique(struct drm_device *dev, const char *name); +bool drm_drv_enabled(const struct drm_driver *driver); #endif
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@suse.de> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> --- drivers/gpu/drm/drm_drv.c | 21 +++++++++++++++++++++ include/drm/drm_drv.h | 1 + 2 files changed, 22 insertions(+)