Re: [PATCH 11/11] video/aperture: Only remove sysfb on the default vga pci device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/11/23 8:58 AM, Javier Martinez Canillas wrote:
Hello Daniel,

On 1/11/23 16:41, Daniel Vetter wrote:
This fixes a regression introduced by ee7a69aa38d8 ("fbdev: Disable
sysfb device registration when removing conflicting FBs"), where we
remove the sysfb when loading a driver for an unrelated pci device,
resulting in the user loosing their efifb console or similar.

Note that in practice this only is a problem with the nvidia blob,
because that's the only gpu driver people might install which does not
come with an fbdev driver of it's own. For everyone else the real gpu
driver will restor a working console.

restore


Also note that in the referenced bug there's confusion that this same
bug also happens on amdgpu. But that was just another amdgpu specific
regression, which just happened to happen at roughly the same time and
with the same user-observable symptons. That bug is fixed now, see

symptoms

https://bugzilla.kernel.org/show_bug.cgi?id=216331#c15

For the above reasons the cc: stable is just notionally, this patch
will need a backport and that's up to nvidia if they care enough.


Maybe adding a Fixes: ee7a69aa38d8 tag here too ?

References: https://bugzilla.kernel.org/show_bug.cgi?id=216303#c28
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Cc: Aaron Plattner <aplattner@xxxxxxxxxx>
Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
Cc: Helge Deller <deller@xxxxxx>
Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # v5.19+ (if someone else does the backport)
---
  drivers/video/aperture.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index ba565515480d..a1821d369bb1 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -321,15 +321,16 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
primary = pdev == vga_default_device(); + if (primary)
+		sysfb_disable();
+
  	for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
  		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
  			continue;
base = pci_resource_start(pdev, bar);
  		size = pci_resource_len(pdev, bar);
-		ret = aperture_remove_conflicting_devices(base, size, name);
-		if (ret)
-			return ret;
+		aperture_detach_devices(base, size);

Maybe mention in the commit message that you are doing this change, something like:

"Instead of calling aperture_remove_conflicting_devices() to remove the conflicting
devices, just call to aperture_detach_devices() to detach the device that matches
the same PCI BAR / aperture range. Since the former is just a wrapper of the latter
plus a sysfb_disable() call, and now that's done in this function but only for the
primary devices"

Patch looks good to me:

Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

Thanks Daniel and Javier!

I wasn't able to reproduce the original problem on my hybrid laptop since it refuses to boot with the console on an external display, but I was able to reproduce it by switching the configuration around: booting with i915.modeset=0 and with an experimental version of nvidia-drm that registers a framebuffer console. I verified that loading nvidia-drm breaks the efi-firmware framebuffer on Intel on Arch's linux-6.1.4-arch1-1 kernel and that applying this patch series fixes it. So

Tested-by: Aaron Plattner <aplattner@xxxxxxxxxx>

FWIW, the bug ought to be reproducible with i915.modeset=0 + any other drm driver that registers a framebuffer.

-- Aaron



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux