From 09d62646a4050c044e2f1d75a03040b081cf712c Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 8 Feb 2022 11:42:02 +0100 Subject: [PATCH] backport simplefb/fbdev memory regio release improvements Signed-off-by: Thomas Lamprecht --- ...-firmware-fb-devices-on-forced-remov.patch | 117 ++++++++++++++ ...-Don-t-mark-as-busy-the-simple-frame.patch | 34 +++++ ...edrm-Request-memory-region-in-driver.patch | 63 ++++++++ ...lefb-Request-memory-region-in-driver.patch | 144 ++++++++++++++++++ 4 files changed, 358 insertions(+) create mode 100644 patches/kernel/0010-fbdev-Hot-unplug-firmware-fb-devices-on-forced-remov.patch create mode 100644 patches/kernel/0011-drivers-firmware-Don-t-mark-as-busy-the-simple-frame.patch create mode 100644 patches/kernel/0012-drm-simpledrm-Request-memory-region-in-driver.patch create mode 100644 patches/kernel/0013-fbdev-simplefb-Request-memory-region-in-driver.patch diff --git a/patches/kernel/0010-fbdev-Hot-unplug-firmware-fb-devices-on-forced-remov.patch b/patches/kernel/0010-fbdev-Hot-unplug-firmware-fb-devices-on-forced-remov.patch new file mode 100644 index 0000000..ebc354c --- /dev/null +++ b/patches/kernel/0010-fbdev-Hot-unplug-firmware-fb-devices-on-forced-remov.patch @@ -0,0 +1,117 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Thomas Zimmermann +Date: Tue, 25 Jan 2022 10:12:18 +0100 +Subject: [PATCH] fbdev: Hot-unplug firmware fb devices on forced removal + +Hot-unplug all firmware-framebuffer devices as part of removing +them via remove_conflicting_framebuffers() et al. Releases all +memory regions to be acquired by native drivers. + +Firmware, such as EFI, install a framebuffer while posting the +computer. After removing the firmware-framebuffer device from fbdev, +a native driver takes over the hardware and the firmware framebuffer +becomes invalid. + +Firmware-framebuffer drivers, specifically simplefb, don't release +their device from Linux' device hierarchy. It still owns the firmware +framebuffer and blocks the native drivers from loading. This has been +observed in the vmwgfx driver. [1] + +Initiating a device removal (i.e., hot unplug) as part of +remove_conflicting_framebuffers() removes the underlying device and +returns the memory range to the system. + +[1] https://lore.kernel.org/dri-devel/20220117180359.18114-1-zack@kde.org/ + +v2: + * rename variable 'dev' to 'device' (Javier) + +Signed-off-by: Thomas Zimmermann +Reported-by: Zack Rusin +Reviewed-by: Javier Martinez Canillas +Reviewed-by: Zack Rusin +CC: stable@vger.kernel.org # v5.11+ +Signed-off-by: Thomas Lamprecht +--- + drivers/video/fbdev/core/fbmem.c | 29 ++++++++++++++++++++++++++--- + include/linux/fb.h | 1 + + 2 files changed, 27 insertions(+), 3 deletions(-) + +diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c +index 7bd5e2a4a9da..91145d93990a 100644 +--- a/drivers/video/fbdev/core/fbmem.c ++++ b/drivers/video/fbdev/core/fbmem.c +@@ -25,6 +25,7 @@ + #include + #include + #include ++#include + #include + #include + #include +@@ -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 *device; + + if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE)) + continue; + + gen_aper = registered_fb[i]->apertures; ++ device = 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(device)) { ++ registered_fb[i]->forced_out = true; ++ platform_device_unregister(to_platform_device(device)); ++ } else { ++ pr_warn("fb%d: cannot remove device\n", i); ++ do_unregister_framebuffer(registered_fb[i]); ++ } + } + } + } +@@ -1895,9 +1914,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/include/linux/fb.h b/include/linux/fb.h +index 02f362c661c8..3d7306c9a706 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) { diff --git a/patches/kernel/0011-drivers-firmware-Don-t-mark-as-busy-the-simple-frame.patch b/patches/kernel/0011-drivers-firmware-Don-t-mark-as-busy-the-simple-frame.patch new file mode 100644 index 0000000..449edd5 --- /dev/null +++ b/patches/kernel/0011-drivers-firmware-Don-t-mark-as-busy-the-simple-frame.patch @@ -0,0 +1,34 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Javier Martinez Canillas +Date: Tue, 25 Jan 2022 10:12:19 +0100 +Subject: [PATCH] drivers/firmware: Don't mark as busy the simple-framebuffer + IO resource + +The sysfb_create_simplefb() function requests a IO memory resource for the +simple-framebuffer platform device, but it also marks it as busy which can +lead to drivers requesting the same memory resource to fail. + +Let's drop the IORESOURCE_BUSY flag and let drivers to request it as busy +instead. + +Signed-off-by: Javier Martinez Canillas +Reviewed-by: Zack Rusin +Reviewed-by: Thomas Zimmermann +Signed-off-by: Thomas Lamprecht +--- + drivers/firmware/sysfb_simplefb.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c +index 303a491e520d..76c4abc42a30 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/patches/kernel/0012-drm-simpledrm-Request-memory-region-in-driver.patch b/patches/kernel/0012-drm-simpledrm-Request-memory-region-in-driver.patch new file mode 100644 index 0000000..5bf59ac --- /dev/null +++ b/patches/kernel/0012-drm-simpledrm-Request-memory-region-in-driver.patch @@ -0,0 +1,63 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Thomas Zimmermann +Date: Tue, 25 Jan 2022 10:12:20 +0100 +Subject: [PATCH] drm/simpledrm: Request memory region in driver + +Requesting the framebuffer memory in simpledrm marks the memory +range as busy. This used to be done by the firmware sysfb code, +but the driver is the correct place. + +v2: + * use I/O memory if request_mem_region() fails (Jocelyn) + +Signed-off-by: Thomas Zimmermann +Reviewed-by: Javier Martinez Canillas +Reviewed-by: Jocelyn Falempe +Signed-off-by: Thomas Lamprecht +--- + drivers/gpu/drm/tiny/simpledrm.c | 22 +++++++++++++++++----- + 1 file changed, 17 insertions(+), 5 deletions(-) + +diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c +index 5a6e89825bc2..6da507960af6 100644 +--- a/drivers/gpu/drm/tiny/simpledrm.c ++++ b/drivers/gpu/drm/tiny/simpledrm.c +@@ -525,21 +525,33 @@ 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) { ++ /* ++ * We cannot make this fatal. Sometimes this comes from magic ++ * spaces our resource handlers simply don't know about. Use ++ * the I/O-memory resource as-is and try to map that instead. ++ */ ++ drm_warn(dev, "could not acquire memory region %pr\n", res); ++ mem = res; ++ } ++ + screen_base = devm_ioremap_wc(&pdev->dev, mem->start, + resource_size(mem)); + if (!screen_base) diff --git a/patches/kernel/0013-fbdev-simplefb-Request-memory-region-in-driver.patch b/patches/kernel/0013-fbdev-simplefb-Request-memory-region-in-driver.patch new file mode 100644 index 0000000..dde776e --- /dev/null +++ b/patches/kernel/0013-fbdev-simplefb-Request-memory-region-in-driver.patch @@ -0,0 +1,144 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Thomas Zimmermann +Date: Tue, 25 Jan 2022 10:12:21 +0100 +Subject: [PATCH] fbdev/simplefb: Request memory region in driver + +Requesting the framebuffer memory in simpledrm marks the memory +range as busy. This used to be done by the firmware sysfb code, +but the driver is the correct place. + +v2: + * store memory region in struct for later cleanup (Javier) + +Signed-off-by: Thomas Zimmermann +Reviewed-by: Javier Martinez Canillas +Signed-off-by: Thomas Lamprecht +--- + drivers/video/fbdev/simplefb.c | 65 +++++++++++++++++++++++----------- + 1 file changed, 45 insertions(+), 20 deletions(-) + +diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c +index b63074fd892e..6885ac0203de 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]; ++ struct resource *mem; ++#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 ++}; ++ + 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; ++ struct resource *mem = par->mem; ++ + simplefb_regulators_destroy(info->par); + simplefb_clocks_destroy(info->par); + if (info->screen_base) + iounmap(info->screen_base); ++ ++ if (mem) ++ release_mem_region(mem->start, resource_size(mem)); + } + + 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. +@@ -405,7 +411,7 @@ static int simplefb_probe(struct platform_device *pdev) + struct simplefb_params params; + struct fb_info *info; + struct simplefb_par *par; +- struct resource *mem; ++ struct resource *res, *mem; + + /* + * Generic drivers must not be registered if a framebuffer exists. +@@ -430,15 +436,28 @@ static int simplefb_probe(struct platform_device *pdev) + if (ret) + return ret; + +- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); +- if (!mem) { ++ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); ++ if (!res) { + dev_err(&pdev->dev, "No memory resource\n"); + return -EINVAL; + } + ++ mem = request_mem_region(res->start, resource_size(res), "simplefb"); ++ if (!mem) { ++ /* ++ * We cannot make this fatal. Sometimes this comes from magic ++ * spaces our resource handlers simply don't know about. Use ++ * the I/O-memory resource as-is and try to map that instead. ++ */ ++ dev_warn(&pdev->dev, "simplefb: cannot reserve video memory at %pR\n", res); ++ mem = res; ++ } ++ + 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 +514,9 @@ static int simplefb_probe(struct platform_device *pdev) + info->var.xres, info->var.yres, + info->var.bits_per_pixel, info->fix.line_length); + ++ if (mem != res) ++ par->mem = mem; /* release in clean-up handler */ ++ + ret = register_framebuffer(info); + if (ret < 0) { + dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret); +@@ -513,6 +535,9 @@ static int simplefb_probe(struct platform_device *pdev) + iounmap(info->screen_base); + error_fb_release: + framebuffer_release(info); ++error_release_mem_region: ++ if (mem != res) ++ release_mem_region(mem->start, resource_size(mem)); + return ret; + } +