Message-ID: <20251229215906.3688205-2-zack.rusin@broadcom.com>
Date: Mon, 29 Dec 2025 16:58:07 -0500
From: Zack Rusin <zack.rusin@...adcom.com>
To: dri-devel@...ts.freedesktop.org
Cc: Ard Biesheuvel <ardb@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Javier Martinez Canillas <javierm@...hat.com>,
	Helge Deller <deller@....de>,
	linux-efi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-fbdev@...r.kernel.org
Subject: [PATCH 01/12] video/aperture: Add sysfb restore on DRM probe failure

When a DRM driver calls aperture_remove_conflicting_pci_devices(), the
firmware framebuffer (EFI, VESA, etc.) is disabled and its platform
device is unregistered. If the DRM driver's probe subsequently fails,
the user is left with no display output.

Add sysfb_restore() to re-enable the Generic System Framebuffers
support and re-create the platform device that was previously
unregistered by sysfb_disable().

Add devm_aperture_remove_conflicting_pci_devices() which wraps the
existing function and registers a devm action to automatically call
sysfb_restore() if the driver's probe fails or the driver is unloaded.
Drivers can call devm_aperture_remove_conflicting_pci_devices_done()
after successful probe to cancel the automatic restore.

Refactor sysfb_init() to use a shared __sysfb_create_device() helper
that can be called from both sysfb_init() and sysfb_restore(). Add a
quirks_applied flag to handle the edge case where a driver calls
sysfb_disable() before sysfb_init() runs, in this case sysfb_restore()
defers device creation to sysfb_init() since the __init quirk functions
cannot be called after init memory is freed.

Signed-off-by: Zack Rusin <zack.rusin@...adcom.com>
Cc: Ard Biesheuvel <ardb@...nel.org>
Cc: Thomas Zimmermann <tzimmermann@...e.de>
Cc: Javier Martinez Canillas <javierm@...hat.com>
Cc: Helge Deller <deller@....de>
Cc: linux-efi@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Cc: dri-devel@...ts.freedesktop.org
Cc: linux-fbdev@...r.kernel.org
---
 drivers/firmware/efi/sysfb_efi.c  |   2 +-
 drivers/firmware/sysfb.c          | 191 +++++++++++++++++++-----------
 drivers/firmware/sysfb_simplefb.c |  10 +-
 drivers/video/aperture.c          |  54 +++++++++
 include/linux/aperture.h          |  14 +++
 include/linux/sysfb.h             |   6 +
 6 files changed, 201 insertions(+), 76 deletions(-)

diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 1e509595ac03..3fe7c57ad849 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -365,7 +365,7 @@ __init void sysfb_apply_efi_quirks(void)
 	}
 }
 
-__init void sysfb_set_efifb_fwnode(struct platform_device *pd)
+void sysfb_set_efifb_fwnode(struct platform_device *pd)
 {
 	if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI && IS_ENABLED(CONFIG_PCI)) {
 		fwnode_init(&efifb_fwnode, &efifb_fwnode_ops);
diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 889e5b05c739..c45b6f487103 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -38,6 +38,7 @@
 static struct platform_device *pd;
 static DEFINE_MUTEX(disable_lock);
 static bool disabled;
+static bool quirks_applied;
 
 static struct device *sysfb_parent_dev(const struct screen_info *si);
 
@@ -79,6 +80,121 @@ void sysfb_disable(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(sysfb_disable);
 
+/* Caller must hold disable_lock */
+static int __sysfb_create_device(bool restore)
+{
+	struct screen_info *si = &screen_info;
+	struct device *parent;
+	unsigned int type;
+	struct simplefb_platform_data mode;
+	const char *name;
+	bool compatible;
+	int ret = 0;
+
+	if (!IS_ERR_OR_NULL(pd))
+		return 0;
+
+	/*
+	 * If quirks haven't been applied yet, sysfb_init() hasn't run.
+	 * Don't create the device now - let sysfb_init() do it after
+	 * applying the necessary fixups and quirks. We can't call
+	 * sysfb_apply_efi_quirks() here because it's __init.
+	 */
+	if (!quirks_applied)
+		return 0;
+
+	parent = sysfb_parent_dev(si);
+	if (IS_ERR(parent))
+		return PTR_ERR(parent);
+
+	type = screen_info_video_type(si);
+
+	/* try to create a simple-framebuffer device */
+	compatible = sysfb_parse_mode(si, &mode);
+	if (compatible) {
+		pd = sysfb_create_simplefb(si, &mode, parent);
+		if (!IS_ERR(pd)) {
+			if (restore)
+				pr_info("sysfb: restored simple-framebuffer device\n");
+			goto put_device;
+		}
+	}
+
+	/* if the FB is incompatible, create a legacy framebuffer device */
+	switch (type) {
+	case VIDEO_TYPE_EGAC:
+		name = "ega-framebuffer";
+		break;
+	case VIDEO_TYPE_VGAC:
+		name = "vga-framebuffer";
+		break;
+	case VIDEO_TYPE_VLFB:
+		name = "vesa-framebuffer";
+		break;
+	case VIDEO_TYPE_EFI:
+		name = "efi-framebuffer";
+		break;
+	default:
+		name = "platform-framebuffer";
+		break;
+	}
+
+	pd = platform_device_alloc(name, 0);
+	if (!pd) {
+		ret = -ENOMEM;
+		goto put_device;
+	}
+
+	pd->dev.parent = parent;
+
+	sysfb_set_efifb_fwnode(pd);
+
+	ret = platform_device_add_data(pd, si, sizeof(*si));
+	if (ret)
+		goto err;
+
+	ret = platform_device_add(pd);
+	if (ret)
+		goto err;
+
+	if (restore)
+		pr_info("sysfb: restored %s device\n", name);
+	goto put_device;
+err:
+	platform_device_put(pd);
+	pd = NULL;
+put_device:
+	put_device(parent);
+	return ret;
+}
+
+/**
+ * sysfb_restore() - restore the Generic System Framebuffer
+ *
+ * This function re-enables the Generic System Framebuffers support and
+ * re-creates the platform device that was previously unregistered by
+ * sysfb_disable(). This is intended for use by DRM drivers that need to
+ * restore the fallback framebuffer when their probe fails after having
+ * called aperture_remove_conflicting_devices() or similar.
+ *
+ * Context: The function can sleep. A @disable_lock mutex is acquired.
+ *
+ * Returns:
+ * 0 on success, or a negative errno value otherwise.
+ */
+int sysfb_restore(void)
+{
+	int ret;
+
+	mutex_lock(&disable_lock);
+	disabled = false;
+	ret = __sysfb_create_device(true);
+	mutex_unlock(&disable_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sysfb_restore);
+
 /**
  * sysfb_handles_screen_info() - reports if sysfb handles the global screen_info
  *
@@ -141,82 +257,17 @@ static struct device *sysfb_parent_dev(const struct screen_info *si)
 
 static __init int sysfb_init(void)
 {
-	struct screen_info *si = &screen_info;
-	struct device *parent;
-	unsigned int type;
-	struct simplefb_platform_data mode;
-	const char *name;
-	bool compatible;
 	int ret = 0;
 
 	screen_info_apply_fixups();
-
-	mutex_lock(&disable_lock);
-	if (disabled)
-		goto unlock_mutex;
-
 	sysfb_apply_efi_quirks();
 
-	parent = sysfb_parent_dev(si);
-	if (IS_ERR(parent)) {
-		ret = PTR_ERR(parent);
-		goto unlock_mutex;
-	}
-
-	/* try to create a simple-framebuffer device */
-	compatible = sysfb_parse_mode(si, &mode);
-	if (compatible) {
-		pd = sysfb_create_simplefb(si, &mode, parent);
-		if (!IS_ERR(pd))
-			goto put_device;
-	}
-
-	type = screen_info_video_type(si);
-
-	/* if the FB is incompatible, create a legacy framebuffer device */
-	switch (type) {
-	case VIDEO_TYPE_EGAC:
-		name = "ega-framebuffer";
-		break;
-	case VIDEO_TYPE_VGAC:
-		name = "vga-framebuffer";
-		break;
-	case VIDEO_TYPE_VLFB:
-		name = "vesa-framebuffer";
-		break;
-	case VIDEO_TYPE_EFI:
-		name = "efi-framebuffer";
-		break;
-	default:
-		name = "platform-framebuffer";
-		break;
-	}
-
-	pd = platform_device_alloc(name, 0);
-	if (!pd) {
-		ret = -ENOMEM;
-		goto put_device;
-	}
-
-	pd->dev.parent = parent;
-
-	sysfb_set_efifb_fwnode(pd);
-
-	ret = platform_device_add_data(pd, si, sizeof(*si));
-	if (ret)
-		goto err;
-
-	ret = platform_device_add(pd);
-	if (ret)
-		goto err;
-
-	goto put_device;
-err:
-	platform_device_put(pd);
-put_device:
-	put_device(parent);
-unlock_mutex:
+	mutex_lock(&disable_lock);
+	quirks_applied = true;
+	if (!disabled)
+		ret = __sysfb_create_device(false);
 	mutex_unlock(&disable_lock);
+
 	return ret;
 }
 
diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c
index 592d8a644619..6fcbc3ae17d5 100644
--- a/drivers/firmware/sysfb_simplefb.c
+++ b/drivers/firmware/sysfb_simplefb.c
@@ -24,8 +24,8 @@ static const char simplefb_resname[] = "BOOTFB";
 static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
 
 /* try parsing screen_info into a simple-framebuffer mode struct */
-__init bool sysfb_parse_mode(const struct screen_info *si,
-			     struct simplefb_platform_data *mode)
+bool sysfb_parse_mode(const struct screen_info *si,
+		      struct simplefb_platform_data *mode)
 {
 	__u8 type;
 	u32 bits_per_pixel;
@@ -61,9 +61,9 @@ __init bool sysfb_parse_mode(const struct screen_info *si,
 	return false;
 }
 
-__init struct platform_device *sysfb_create_simplefb(const struct screen_info *si,
-						     const struct simplefb_platform_data *mode,
-						     struct device *parent)
+struct platform_device *sysfb_create_simplefb(const struct screen_info *si,
+					      const struct simplefb_platform_data *mode,
+					      struct device *parent)
 {
 	struct platform_device *pd;
 	struct resource res;
diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 2b5a1e666e9b..4de6dc04a3fd 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -372,3 +372,57 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
 
 }
 EXPORT_SYMBOL(aperture_remove_conflicting_pci_devices);
+
+static void devm_aperture_restore_sysfb(void *unused)
+{
+	sysfb_restore();
+}
+
+/**
+ * devm_aperture_remove_conflicting_pci_devices - remove existing framebuffers
+ *                                                with sysfb restore on failure
+ * @pdev: PCI device
+ * @name: a descriptive name of the requesting driver
+ *
+ * This function removes devices that own apertures within any of @pdev's
+ * memory bars, similar to aperture_remove_conflicting_pci_devices().
+ *
+ * Additionally, it registers a devm action that will restore the system
+ * framebuffer if the driver's probe fails or the driver is unloaded. This
+ * ensures the user doesn't lose display output if the DRM driver probe fails
+ * after removing the firmware framebuffer.
+ *
+ * This function should be called early in the driver's probe function. The
+ * driver must call devm_aperture_remove_conflicting_pci_devices_done() after
+ * successfully completing probe to cancel the automatic restore.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise
+ */
+int devm_aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
+						 const char *name)
+{
+	int ret;
+
+	ret = aperture_remove_conflicting_pci_devices(pdev, name);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(&pdev->dev, devm_aperture_restore_sysfb,
+					NULL);
+}
+EXPORT_SYMBOL(devm_aperture_remove_conflicting_pci_devices);
+
+/**
+ * devm_aperture_remove_conflicting_pci_devices_done - cancel sysfb restore
+ * @pdev: PCI device
+ *
+ * Cancels the automatic sysfb restore action registered by
+ * devm_aperture_remove_conflicting_pci_devices(). Call this after the
+ * driver has successfully completed probe and registered its display.
+ */
+void devm_aperture_remove_conflicting_pci_devices_done(struct pci_dev *pdev)
+{
+	devm_remove_action(&pdev->dev, devm_aperture_restore_sysfb, NULL);
+}
+EXPORT_SYMBOL(devm_aperture_remove_conflicting_pci_devices_done);
diff --git a/include/linux/aperture.h b/include/linux/aperture.h
index 1a9a88b11584..ea0ece7f777e 100644
--- a/include/linux/aperture.h
+++ b/include/linux/aperture.h
@@ -19,6 +19,10 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
 int __aperture_remove_legacy_vga_devices(struct pci_dev *pdev);
 
 int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name);
+
+int devm_aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
+						 const char *name);
+void devm_aperture_remove_conflicting_pci_devices_done(struct pci_dev *pdev);
 #else
 static inline int devm_aperture_acquire_for_platform_device(struct platform_device *pdev,
 							    resource_size_t base,
@@ -42,6 +46,16 @@ static inline int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
 {
 	return 0;
 }
+
+static inline int devm_aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
+							       const char *name)
+{
+	return 0;
+}
+
+static inline void devm_aperture_remove_conflicting_pci_devices_done(struct pci_dev *pdev)
+{
+}
 #endif
 
 /**
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index b449665c686a..c0ade38bcf99 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -63,6 +63,7 @@ struct efifb_dmi_info {
 #ifdef CONFIG_SYSFB
 
 void sysfb_disable(struct device *dev);
+int sysfb_restore(void);
 
 bool sysfb_handles_screen_info(void);
 
@@ -72,6 +73,11 @@ static inline void sysfb_disable(struct device *dev)
 {
 }
 
+static inline int sysfb_restore(void)
+{
+	return -ENODEV;
+}
+
 static inline bool sysfb_handles_screen_info(void)
 {
 	return false;
-- 
2.48.1

Powered by blists - more mailing lists