From 55285e21f04517939480966164a33898c34b2af2 Mon Sep 17 00:00:00 2001 From: Imre Deak Date: Mon, 9 Aug 2021 16:31:46 +0300 Subject: fbdev/efifb: Release PCI device's runtime PM ref during FB destroy Atm the EFI FB platform driver gets a runtime PM reference for the associated GFX PCI device during probing the EFI FB platform device and releases it only when the platform device gets unbound. When fbcon switches to the FB provided by the PCI device's driver (for instance i915/drmfb), the EFI FB will get only unregistered without the EFI FB platform device getting unbound, keeping the runtime PM reference acquired during the platform device probing. This reference will prevent the PCI driver from runtime suspending the device. Fix this by releasing the RPM reference from the EFI FB's destroy hook, called when the FB gets unregistered. While at it assert that pm_runtime_get_sync() didn't fail. v2: - Move pm_runtime_get_sync() before register_framebuffer() to avoid its race wrt. efifb_destroy()->pm_runtime_put(). (Daniel) - Assert that pm_runtime_get_sync() didn't fail. - Clarify commit message wrt. platform/PCI device/driver and driver removal vs. device unbinding. Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0") Cc: Kai-Heng Feng Cc: Daniel Vetter Reviewed-by: Daniel Vetter (v1) Acked-by: Alex Deucher Acked-by: Kai-Heng Feng Signed-off-by: Imre Deak Link: https://patchwork.freedesktop.org/patch/msgid/20210809133146.2478382-1-imre.deak@intel.com --- drivers/video/fbdev/efifb.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index 8ea8f079cde2..edca3703b964 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -47,6 +47,8 @@ static bool use_bgrt = true; static bool request_mem_succeeded = false; static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC; +static struct pci_dev *efifb_pci_dev; /* dev with BAR covering the efifb */ + static struct fb_var_screeninfo efifb_defined = { .activate = FB_ACTIVATE_NOW, .height = -1, @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info *info) {} static void efifb_destroy(struct fb_info *info) { + if (efifb_pci_dev) + pm_runtime_put(&efifb_pci_dev->dev); + if (info->screen_base) { if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC)) iounmap(info->screen_base); @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb); static bool pci_dev_disabled; /* FB base matches BAR of a disabled device */ -static struct pci_dev *efifb_pci_dev; /* dev with BAR covering the efifb */ static struct resource *bar_resource; static u64 bar_offset; @@ -569,17 +573,22 @@ static int efifb_probe(struct platform_device *dev) pr_err("efifb: cannot allocate colormap\n"); goto err_groups; } + + if (efifb_pci_dev) + WARN_ON(pm_runtime_get_sync(&efifb_pci_dev->dev) < 0); + err = register_framebuffer(info); if (err < 0) { pr_err("efifb: cannot register framebuffer\n"); - goto err_fb_dealoc; + goto err_put_rpm_ref; } fb_info(info, "%s frame buffer device\n", info->fix.id); - if (efifb_pci_dev) - pm_runtime_get_sync(&efifb_pci_dev->dev); return 0; -err_fb_dealoc: +err_put_rpm_ref: + if (efifb_pci_dev) + pm_runtime_put(&efifb_pci_dev->dev); + fb_dealloc_cmap(&info->cmap); err_groups: sysfs_remove_groups(&dev->dev.kobj, efifb_groups); @@ -603,8 +612,6 @@ static int efifb_remove(struct platform_device *pdev) unregister_framebuffer(info); sysfs_remove_groups(&pdev->dev.kobj, efifb_groups); framebuffer_release(info); - if (efifb_pci_dev) - pm_runtime_put(&efifb_pci_dev->dev); return 0; } -- cgit v1.2.3 From cd395d529faf46bd7fd799852a659ca1bd650a27 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 18 Sep 2021 11:15:01 -0700 Subject: tgafb: clarify dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TGA boards were based on the DECchip 21030 PCI graphics accelerator used mainly for alpha, and existed in a TURBOchannel (TC) version for the DECstation (MIPS) workstations. However, the config option for the TGA code is a bit confused, and says depends on FB && (ALPHA || TC) because people didn't really want to enable the option for random PCI environments, so the "ALPHA" stands in for that case (while the TC case is then the MIPS DECstation case). So that config dependency is kind of a mixture of architecture and bus choices. But it's incorrect, in that there were non-PCI-based alpha hardware, and then the driver just causes warnings: drivers/video/fbdev/tgafb.c:1532:13: error: ‘tgafb_unregister’ defined but not used [-Werror=unused-function] 1532 | static void tgafb_unregister(struct device *dev) | ^~~~~~~~~~~~~~~~ drivers/video/fbdev/tgafb.c:1387:12: error: ‘tgafb_register’ defined but not used [-Werror=unused-function] 1387 | static int tgafb_register(struct device *dev) | ^~~~~~~~~~~~~~ so let's make the config option dependencies a bit more explict: depends on FB depends on PCI || TC depends on ALPHA || TC where that first "FB" is the software configuration dependency, the second "PCI || TC" is the hardware bus dependency, while that final "ALPHA || TC" dependency is the "don't bother asking except for these situations. We could make that third case have "COMPILE_TEST" as an option, and mark the register/unregister functions as __maybe_unused, but I'm not sure it's really worth it. Signed-off-by: Linus Torvalds --- drivers/video/fbdev/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/video') diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index d33c5cd684c0..b26b79dfcac9 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -582,7 +582,9 @@ config FB_HP300 config FB_TGA tristate "TGA/SFB+ framebuffer support" - depends on FB && (ALPHA || TC) + depends on FB + depends on PCI || TC + depends on ALPHA || TC select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT -- cgit v1.2.3