[PATCH v2 0/2] Add a drm.disable_native_drivers command line option

[PATCH v2 0/2] Add a drm.disable_native_drivers command line option

[Javier Martinez Canillas]

This patch-series add a drm.disable_native_drivers option that can be used
to prevent native DRM drivers to be probed. That way, the simpledrm driver
won't be removed which can be useful to troubleshoot DRM drivers problems.

Patch #1 is just a small preparatory patch that moves the logic to remove
the conflicting fbdev frame buffer to a helper function.

Patch #2 adds the new kernel command line option and if set, prevents the
drm_aperture_remove_conflicting_framebuffers() function to succeed.

This is a v2 that addresses the issues pointed out by Thomas and Neal.

Best regards,
Javier

Changes in v2:
- Rename command line parameter to drm.disable_native_drivers.
- Return -EBUSY instead of -EINVAL when the function fails.
- Invert the parameter logic and make it false by default.

Javier Martinez Canillas (2):
  drm/aperture: Move conflicting fbdev frame buffer removal to a helper
  drm/aperture: Prevent conflicting framebuffers removal if option is
    set

 drivers/gpu/drm/drm_aperture.c | 54 ++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 12 deletions(-)

-- 
2.31.1

[PATCH v2 1/2] drm/aperture: Move conflicting fbdev frame buffer removal to a helper

[Javier Martinez Canillas]

The logic to remove the conflicting frame buffers for fbdev devices that
use a given memory range is only compiled if CONFIG_FB option is enabled.

But having an ifdefery in drm_aperture_remove_conflicting_framebuffers()
makes the function harder to extend. Move the logic into its own helper.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

(no changes since v1)

 drivers/gpu/drm/drm_aperture.c | 39 ++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 74bd4a76b253..1a8ed0c616d6 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -273,6 +273,30 @@ static void drm_aperture_detach_drivers(resource_size_t base, resource_size_t si
 	mutex_unlock(&drm_apertures_lock);
 }
 
+static int drm_aperture_remove_conflicting_fbdev_framebuffers(resource_size_t base,
+							      resource_size_t size, bool primary,
+							      const struct drm_driver *req_driver)
+{
+#if IS_REACHABLE(CONFIG_FB)
+	struct apertures_struct *a;
+	int ret;
+
+	a = alloc_apertures(1);
+	if (!a)
+		return -ENOMEM;
+
+	a->ranges[0].base = base;
+	a->ranges[0].size = size;
+
+	ret = remove_conflicting_framebuffers(a, req_driver->name, primary);
+	kfree(a);
+
+	if (ret)
+		return ret;
+#endif
+	return 0;
+}
+
 /**
  * drm_aperture_remove_conflicting_framebuffers - remove existing framebuffers in the given range
  * @base: the aperture's base address in physical memory
@@ -289,23 +313,12 @@ static void drm_aperture_detach_drivers(resource_size_t base, resource_size_t si
 int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
 						 bool primary, const struct drm_driver *req_driver)
 {
-#if IS_REACHABLE(CONFIG_FB)
-	struct apertures_struct *a;
 	int ret;
 
-	a = alloc_apertures(1);
-	if (!a)
-		return -ENOMEM;
-
-	a->ranges[0].base = base;
-	a->ranges[0].size = size;
-
-	ret = remove_conflicting_framebuffers(a, req_driver->name, primary);
-	kfree(a);
-
+	ret = drm_aperture_remove_conflicting_fbdev_framebuffers(base, size, primary,
+								 req_driver);
 	if (ret)
 		return ret;
-#endif
 
 	drm_aperture_detach_drivers(base, size);
 
-- 
2.31.1

[PATCH v2 2/2] drm/aperture: Prevent conflicting framebuffers removal if option is set

[Javier Martinez Canillas]

The simpledrm driver allows to use the frame buffer that was set-up by the
firmware. This gives early video output before the platform DRM driver is
probed and takes over.

But it would be useful to have a way to disable this take over by native
DRM drivers. For example, there may be bugs in the DRM drivers that could
cause the display output to not work correctly.

For those cases, it would be good to keep the simpledrm driver instead and
at least get a working display as set-up by the firmware.

Let's add a drm.disable_native_drivers kernel command line parameter, that
when set to true it prevents the conflicting framebuffers to being removed.

Since the drivers call drm_aperture_remove_conflicting_framebuffers() very
early in their probe callback, this will cause the drivers' probe to fail.

Thanks to Neal Gompa for the suggestion and Thomas Zimmermann for the idea
on how this could be implemented.

Suggested-by: Neal Gompa <ngompa13@gmail.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v2:
- Rename command line parameter to drm.disable_native_drivers.
- Return -EBUSY instead of -EINVAL when the function fails.
- Invert the parameter logic and make it false by default.

 drivers/gpu/drm/drm_aperture.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 1a8ed0c616d6..6ae20b6d6499 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -14,6 +14,11 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_print.h>
 
+static bool drm_disable_native_drivers;
+module_param_named(disable_native_drivers, drm_disable_native_drivers, bool, 0600);
+MODULE_PARM_DESC(disable_native_drivers,
+		 "Disable native DRM drivers probing [default=false]");
+
 /**
  * DOC: overview
  *
@@ -307,6 +312,9 @@ static int drm_aperture_remove_conflicting_fbdev_framebuffers(resource_size_t ba
  * This function removes graphics device drivers which use memory range described by
  * @base and @size.
  *
+ * The conflicting framebuffers removal does not happen when drm.disable_native_drivers=1 is
+ * set. When this option is enabled, the function will return an -EBUSY errno code instead.
+ *
  * Returns:
  * 0 on success, or a negative errno code otherwise
  */
@@ -315,6 +323,9 @@ int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_
 {
 	int ret;
 
+	if (drm_disable_native_drivers)
+		return -EBUSY;
+
 	ret = drm_aperture_remove_conflicting_fbdev_framebuffers(base, size, primary,
 								 req_driver);
 	if (ret)
@@ -335,6 +346,9 @@ EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
  * for any of @pdev's memory bars. The function assumes that PCI device with
  * shadowed ROM drives a primary display and so kicks out vga16fb.
  *
+ * The conflicting framebuffers removal does not happen when drm.disable_native_drivers=1 is
+ * set. When this option is enabled, the function will return an -EBUSY errno code instead.
+ *
  * Returns:
  * 0 on success, or a negative errno code otherwise
  */
@@ -344,6 +358,9 @@ int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
 	resource_size_t base, size;
 	int bar, ret = 0;
 
+	if (drm_disable_native_drivers)
+		return -EBUSY;
+
 	for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
 		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
 			continue;
-- 
2.31.1

Re: [PATCH v2 1/2] drm/aperture: Move conflicting fbdev frame buffer removal to a helper

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2840 bytes --]



Am 25.10.21 um 09:57 schrieb Javier Martinez Canillas:
> The logic to remove the conflicting frame buffers for fbdev devices that
> use a given memory range is only compiled if CONFIG_FB option is enabled.
> 
> But having an ifdefery in drm_aperture_remove_conflicting_framebuffers()
> makes the function harder to extend. Move the logic into its own helper.
> 
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
> 
> (no changes since v1)
> 
>   drivers/gpu/drm/drm_aperture.c | 39 ++++++++++++++++++++++------------
>   1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
> index 74bd4a76b253..1a8ed0c616d6 100644
> --- a/drivers/gpu/drm/drm_aperture.c
> +++ b/drivers/gpu/drm/drm_aperture.c
> @@ -273,6 +273,30 @@ static void drm_aperture_detach_drivers(resource_size_t base, resource_size_t si
>   	mutex_unlock(&drm_apertures_lock);
>   }
>   
> +static int drm_aperture_remove_conflicting_fbdev_framebuffers(resource_size_t base,
> +							      resource_size_t size, bool primary,
> +							      const struct drm_driver *req_driver)
> +{
> +#if IS_REACHABLE(CONFIG_FB)
> +	struct apertures_struct *a;
> +	int ret;
> +
> +	a = alloc_apertures(1);
> +	if (!a)
> +		return -ENOMEM;
> +
> +	a->ranges[0].base = base;
> +	a->ranges[0].size = size;
> +
> +	ret = remove_conflicting_framebuffers(a, req_driver->name, primary);
> +	kfree(a);
> +
> +	if (ret)
> +		return ret;
> +#endif
> +	return 0;
> +}
> +
>   /**
>    * drm_aperture_remove_conflicting_framebuffers - remove existing framebuffers in the given range
>    * @base: the aperture's base address in physical memory
> @@ -289,23 +313,12 @@ static void drm_aperture_detach_drivers(resource_size_t base, resource_size_t si
>   int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
>   						 bool primary, const struct drm_driver *req_driver)
>   {
> -#if IS_REACHABLE(CONFIG_FB)
> -	struct apertures_struct *a;
>   	int ret;
>   
> -	a = alloc_apertures(1);
> -	if (!a)
> -		return -ENOMEM;
> -
> -	a->ranges[0].base = base;
> -	a->ranges[0].size = size;
> -
> -	ret = remove_conflicting_framebuffers(a, req_driver->name, primary);
> -	kfree(a);
> -
> +	ret = drm_aperture_remove_conflicting_fbdev_framebuffers(base, size, primary,
> +								 req_driver);
>   	if (ret)
>   		return ret;
> -#endif
>   
>   	drm_aperture_detach_drivers(base, size);
>   
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]