Hi Am 20.01.22 um 05:06 schrieb Zack Rusin: [...]
kernel: fb0: switching to vmwgfx from simple kernel: Console: switching to colour dummy device 80x25 kernel: vmwgfx 0000:00:0f.0: BAR 2: can't reserve [mem 0x78000000- 0x7fffffff 64bit pref] kernel: vmwgfx: probe of 0000:00:0f.0 failed with error -16 leaving the system without a fb driver.OK, I suspect that it would work if you use simpledrm instead of into the kernel binary.Yes, simpledrm works fine. BTW, is there any remaining work before distros can enable it by default?
It's here and ready for use.Simpledrm requires DRM to be linked in the kernel. We're modularizing DRM helpers to reduce the binary size.
On the distro side, Javier and me are sorting out issue. But it's fairly quite and not many problems show up.
If that works, would you consider protecting pci_request_region() with #if not defined(CONFIG_FB_SIMPLE) #endif with a FIXME comment?Yes, I think that's a good compromise. I'll respin the patch with that.
Before you do that, I have one more patch for you to try. It's all the changes as before, but now fbdev hot-unplugs the underlying platform device from the device hierarchy. The BOOTFB will not consume parts of vmwgfx's display memory range any longer. It's now the same behavior as with simpledrm.
This works for me with simplefb and efifb on i915 hardware. Best regards Thomas
z
-- 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
From dfd482bfc143b9ec0d8b103a56f6576acef8a860 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@xxxxxxx>
Date: Wed, 19 Jan 2022 10:23:07 +0100
Subject: [PATCH v2] [RFC] drop IORESOURCE_BUSY from sysfb code and request
memory in drivers
Drop the IORESOURCE_BUSY flag when creating a simple-framebuffer
device. Instead acquire the memory in drivers. Also hot-unplug the
platform device.
---
drivers/firmware/sysfb_simplefb.c | 2 +-
drivers/gpu/drm/tiny/simpledrm.c | 17 ++++++---
drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++--
drivers/video/fbdev/simplefb.c | 57 ++++++++++++++++++++++---------
include/linux/fb.h | 1 +
5 files changed, 80 insertions(+), 26 deletions(-)
diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c
index b86761904949..179e9d0ef3e9 100644
--- a/drivers/firmware/sysfb_simplefb.c
+++ b/drivers/firmware/sysfb_simplefb.c
@@ -99,7 +99,7 @@ __init int sysfb_create_simplefb(const struct screen_info *si,
/* setup IORESOURCE_MEM as framebuffer memory */
memset(&res, 0, sizeof(res));
- res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+ res.flags = IORESOURCE_MEM;
res.name = simplefb_resname;
res.start = base;
res.end = res.start + length - 1;
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 04146da2d1d8..32550697b1a7 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -526,21 +526,28 @@ static int simpledrm_device_init_mm(struct simpledrm_device *sdev)
{
struct drm_device *dev = &sdev->dev;
struct platform_device *pdev = sdev->pdev;
- struct resource *mem;
+ struct resource *res, *mem;
void __iomem *screen_base;
int ret;
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem)
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
return -EINVAL;
- ret = devm_aperture_acquire_from_firmware(dev, mem->start, resource_size(mem));
+ ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res));
if (ret) {
drm_err(dev, "could not acquire memory range %pr: error %d\n",
- mem, ret);
+ res, ret);
return ret;
}
+ mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
+ sdev->dev.driver->name);
+ if (!mem) {
+ drm_err(dev, "could not acquire memory region %pr\n", res);
+ return -EBUSY;
+ }
+
screen_base = devm_ioremap_wc(&pdev->dev, mem->start,
resource_size(mem));
if (!screen_base)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0fa7ede94fa6..2db31ea7cbb4 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -25,6 +25,7 @@
#include <linux/init.h>
#include <linux/linux_logo.h>
#include <linux/proc_fs.h>
+#include <linux/platform_device.h>
#include <linux/seq_file.h>
#include <linux/console.h>
#include <linux/kmod.h>
@@ -1557,18 +1558,36 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
/* check all firmware fbs and kick off if the base addr overlaps */
for_each_registered_fb(i) {
struct apertures_struct *gen_aper;
+ struct device *dev;
if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
continue;
gen_aper = registered_fb[i]->apertures;
+ dev = registered_fb[i]->device;
if (fb_do_apertures_overlap(gen_aper, a) ||
(primary && gen_aper && gen_aper->count &&
gen_aper->ranges[0].base == VGA_FB_PHYS)) {
printk(KERN_INFO "fb%d: switching to %s from %s\n",
i, name, registered_fb[i]->fix.id);
- do_unregister_framebuffer(registered_fb[i]);
+
+ /*
+ * If we kick-out a firmware driver, we also want to remove
+ * the underlying platform device, such as simple-framebuffer,
+ * VESA, EFI, etc. A native driver will then be able to
+ * allocate the memory range.
+ *
+ * If it's not a platform device, at least print a warning. A
+ * fix would add code to remove the device from the system.
+ */
+ if (dev_is_platform(dev)) {
+ registered_fb[i]->forced_out = true;
+ platform_device_unregister(to_platform_device(dev));
+ } else {
+ printk(KERN_WARNING "fb%d: cannot remove device\n", i);
+ do_unregister_framebuffer(registered_fb[i]);
+ }
}
}
}
@@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer);
void
unregister_framebuffer(struct fb_info *fb_info)
{
- mutex_lock(®istration_lock);
+ bool forced_out = fb_info->forced_out;
+
+ if (!forced_out)
+ mutex_lock(®istration_lock);
do_unregister_framebuffer(fb_info);
- mutex_unlock(®istration_lock);
+ if (!forced_out)
+ mutex_unlock(®istration_lock);
}
EXPORT_SYMBOL(unregister_framebuffer);
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 57541887188b..7b9072f07337 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -66,16 +66,36 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
return 0;
}
-struct simplefb_par;
+struct simplefb_par {
+ u32 palette[PSEUDO_PALETTE_SIZE];
+#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
+ bool clks_enabled;
+ unsigned int clk_count;
+ struct clk **clks;
+#endif
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+ bool regulators_enabled;
+ u32 regulator_count;
+ struct regulator **regulators;
+#endif
+ bool release_mem_region;
+};
+
static void simplefb_clocks_destroy(struct simplefb_par *par);
static void simplefb_regulators_destroy(struct simplefb_par *par);
static void simplefb_destroy(struct fb_info *info)
{
+ struct simplefb_par *par = info->par;
+
simplefb_regulators_destroy(info->par);
simplefb_clocks_destroy(info->par);
if (info->screen_base)
iounmap(info->screen_base);
+
+ if (par->release_mem_region)
+ release_mem_region(info->apertures->ranges[0].base,
+ info->apertures->ranges[0].size);
}
static const struct fb_ops simplefb_ops = {
@@ -169,20 +189,6 @@ static int simplefb_parse_pd(struct platform_device *pdev,
return 0;
}
-struct simplefb_par {
- u32 palette[PSEUDO_PALETTE_SIZE];
-#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
- bool clks_enabled;
- unsigned int clk_count;
- struct clk **clks;
-#endif
-#if defined CONFIG_OF && defined CONFIG_REGULATOR
- bool regulators_enabled;
- u32 regulator_count;
- struct regulator **regulators;
-#endif
-};
-
#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
/*
* Clock handling code.
@@ -401,6 +407,7 @@ static void simplefb_regulators_destroy(struct simplefb_par *par) { }
static int simplefb_probe(struct platform_device *pdev)
{
+ bool request_mem_succeeded = false;
int ret;
struct simplefb_params params;
struct fb_info *info;
@@ -436,9 +443,20 @@ static int simplefb_probe(struct platform_device *pdev)
return -EINVAL;
}
+ if (request_mem_region(mem->start, resource_size(mem), "simplefb")) {
+ request_mem_succeeded = true;
+ } else {
+ /* We cannot make this fatal. Sometimes this comes from magic
+ spaces our resource handlers simply don't know about */
+ dev_warn(&pdev->dev, "simplefb: cannot reserve video memory at %pR\n",
+ mem);
+ }
+
info = framebuffer_alloc(sizeof(struct simplefb_par), &pdev->dev);
- if (!info)
- return -ENOMEM;
+ if (!info) {
+ ret = -ENOMEM;
+ goto error_release_mem_region;
+ }
platform_set_drvdata(pdev, info);
par = info->par;
@@ -495,6 +513,8 @@ static int simplefb_probe(struct platform_device *pdev)
info->var.xres, info->var.yres,
info->var.bits_per_pixel, info->fix.line_length);
+ par->release_mem_region = request_mem_succeeded;
+
ret = register_framebuffer(info);
if (ret < 0) {
dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret);
@@ -513,6 +533,9 @@ static int simplefb_probe(struct platform_device *pdev)
iounmap(info->screen_base);
error_fb_release:
framebuffer_release(info);
+error_release_mem_region:
+ if (request_mem_succeeded)
+ release_mem_region(mem->start, resource_size(mem));
return ret;
}
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 3da95842b207..9a14f3f8a329 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -502,6 +502,7 @@ struct fb_info {
} *apertures;
bool skip_vt_switch; /* no VT switch on suspend/resume required */
+ bool forced_out; /* set when being removed by another driver */
};
static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
--
2.34.1
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
