[PATCH] x86, ia64: Do not lose track of the EFI default VGA device

[PATCH] x86, ia64: Do not lose track of the EFI default VGA device

[Henrik Rydberg]

Since commit 20cde694027e ("x86, ia64: Move EFI_FB
vga_default_device() initialization to pci_vga_fixup()") in the 3.17
merge window, the EFI framebuffer depends on the VGA arbitration
layer. However, the configuration does not reflect this, which leads
to a hard-to-find bug when FB_EFI is configured without VGA_ARB. Add a
select clause to remedy this.

Cc: Bruno Prémont <bonbons@linux-vserver.org>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
Hi Peter,

I stumbled upon this bug from the 3.17 merge window when updating to
Linus's 3.18 git head yesterday. The patch has been tested on two
different EFI machines; one that needs the patch and one that does not.

Thanks,
Henrik

 drivers/video/fbdev/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index c7bf606..81b21bc 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -761,6 +761,7 @@ config FB_EFI
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VGA_ARB
 	help
 	  This is the EFI frame buffer device driver. If the firmware on
 	  your platform is EFI 1.10 or UEFI 2.0, select Y to add support for
-- 
2.1.3

Re: [PATCH] x86, ia64: Do not lose track of the EFI default VGA device

[Bruno Prémont]

On Fri, 14 Nov 2014 13:53:30 +0100 Henrik Rydberg wrote:
> Since commit 20cde694027e ("x86, ia64: Move EFI_FB
> vga_default_device() initialization to pci_vga_fixup()") in the 3.17
> merge window, the EFI framebuffer depends on the VGA arbitration
> layer. However, the configuration does not reflect this, which leads
> to a hard-to-find bug when FB_EFI is configured without VGA_ARB. Add a
> select clause to remedy this.

Could you be more verbose in why it depends on/needs VGA_ARB?

With EFI starting to show up on ARM this is not necessarily true
(no PCI -> no VGA_ARB arbitration).


So it would need to at least be select VGA_ARB if (PCI && !S390)
in order to not have broken kernel configuration (in more or less
exotic cases) while depends on VGA_ARB would be the only correct option
if the rule 'select only allowed for leafs' is enforced.

Bruno

> Cc: Bruno Prémont <bonbons@linux-vserver.org>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> Hi Peter,
> 
> I stumbled upon this bug from the 3.17 merge window when updating to
> Linus's 3.18 git head yesterday. The patch has been tested on two
> different EFI machines; one that needs the patch and one that does not.
> 
> Thanks,
> Henrik
> 
>  drivers/video/fbdev/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index c7bf606..81b21bc 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -761,6 +761,7 @@ config FB_EFI
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> +	select VGA_ARB
>  	help
>  	  This is the EFI frame buffer device driver. If the firmware on
>  	  your platform is EFI 1.10 or UEFI 2.0, select Y to add support for

Re: [PATCH] x86, ia64: Do not lose track of the EFI default VGA device

[Henrik Rydberg]

On 11/14/2014 03:42 PM, Bruno Prémont wrote:
> On Fri, 14 Nov 2014 13:53:30 +0100 Henrik Rydberg wrote:
>> Since commit 20cde694027e ("x86, ia64: Move EFI_FB
>> vga_default_device() initialization to pci_vga_fixup()") in the 3.17
>> merge window, the EFI framebuffer depends on the VGA arbitration
>> layer. However, the configuration does not reflect this, which leads
>> to a hard-to-find bug when FB_EFI is configured without VGA_ARB. Add a
>> select clause to remedy this.
> 
> Could you be more verbose in why it depends on/needs VGA_ARB?

When the EFI framebuffer is configured but not VGA_ARB, the kernel manages to
lose track of the default VGA device in some cases. As a result, the X11
nouveau driver fails on my MacBookAir3,1 (GeForce 320M, nv50, 0xaf), which
is booting in EFI_STUB mode.

The code to select the right PCI device was literally moved from efifb.c to the
internals of vgaarb. The PCI sysfs layer seems to depend on
vga_default_device(), which is only defined when VGA_ARB is set.

> With EFI starting to show up on ARM this is not necessarily true
> (no PCI -> no VGA_ARB arbitration).
> 
> So it would need to at least be select VGA_ARB if (PCI && !S390)
> in order to not have broken kernel configuration (in more or less
> exotic cases) while depends on VGA_ARB would be the only correct option
> if the rule 'select only allowed for leafs' is enforced.

I agree that the tie probably should be somewhere else. I am fine with any
combination of flags that respects the fact that efifb used to define
vga_default_device(), apparently for good reason.

Thanks,
Henrik

Re: [PATCH] x86, ia64: Do not lose track of the EFI default VGA device

[Henrik Rydberg]

Hi Bruno,

> So it would need to at least be select VGA_ARB if (PCI && !S390)
> in order to not have broken kernel configuration (in more or less
> exotic cases) while depends on VGA_ARB would be the only correct option
> if the rule 'select only allowed for leafs' is enforced.

Here is a tested patch that does just that, thanks for the suggestion.

Henrik

From 43c16bbc7adbcb17aac73d09f046bf2779771c4c Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <rydberg@euromail.se>
Date: Fri, 14 Nov 2014 20:01:21 +0100
Subject: [PATCH v2] x86, ia64: Do not lose track of the EFI default VGA device
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since commit 20cde694 in the 3.17 merge window, the EFI framebuffer
depends on the VGA arbitration layer. However, the configuration does
not reflect this, which leads to a hard-to-find bug when FB_EFI is
configured without VGA_ARB. Add a select clause to remedy this.

Cc: Bruno Prémont <bonbons@linux-vserver.org>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/video/fbdev/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index c7bf606..1615a1b 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -761,6 +761,7 @@ config FB_EFI
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VGA_ARB if (PCI && !S390)
 	help
 	  This is the EFI frame buffer device driver. If the firmware on
 	  your platform is EFI 1.10 or UEFI 2.0, select Y to add support for
-- 
2.1.3

Re: [PATCH] x86, ia64: Do not lose track of the EFI default VGA device

[Bruno Prémont]

Hi Hendrick,

On Fri, 14 November 2014 Henrik Rydberg <rydberg@euromail.se> wrote:
> > So it would need to at least be select VGA_ARB if (PCI && !S390)
> > in order to not have broken kernel configuration (in more or less
> > exotic cases) while depends on VGA_ARB would be the only correct option
> > if the rule 'select only allowed for leafs' is enforced.
> 
> Here is a tested patch that does just that, thanks for the suggestion.

Sure it will cover your case but with a few issues.

As you might have noted, VGA_ARB can only be disabled if you select
EXPERT. In that case you kind of give up warranty.

In addition, prior to the patches that landed in 3.17, just selecting
X86_SYSFB would runtime-disable EFI_FB disabling boot_vga tagging
based on screen_info as well.

Stating in the commit message that EFI_FB depends on VGA_ARB is pretty
wrong. A more correct phrasing would be that before the commit EFI_FB
included boot_vga detection which has been moved around, but that
feature is not inherent to EFI_FB. Doing the select only keeps
`make oldconfig` provide the same feature set.




As boot_vga is a PCI thing maybe the whole detection should effectively
be fully dissociated from runtime GPU switching and be adjusted to
really only flag the boot GPU (even when there is no VGA around).

Unfortunately this attribute is not explicitly documented so drawing the
line one way or another may trip on some user's feet.

We have some more or less conflicting information in commit messages though:

217f45de3d2 by Dave Airlie introducing boot_vgain 2009:
    PCI: expose boot VGA device via sysfs.
    
    X really would like to know which VGA device was considered the boot
    device by the system. The x86 PCI fixups have support for discovering
    this but we provide no way to expose it to userspace.
    
    This adds a sysfs file per VGA class device which has the value 0 for
    non the boot device or unknown, and 1 if the VGA device is the boot
    device.

1a39b310e92 by Matthew Garrett making it variable in 2012:
    vgaarb: Add support for setting the default video device (v2)
    
    The default VGA device is a somewhat fluid concept on platforms with
    multiple GPUs. Add support for setting it so switching code can update
    things appropriately, and make sure that the sysfs code returns the right
    device if it's changed.

So should boot_vga now represent the main GPU of the system or should
it represent the one(s) that were used for booting? (I've not heard
of systems with more than one active firmware GPU... but why not!

Matthew covered the case where multiple GPUs are in a set where only one
of them can be active as in driving displays. Maybe this would need some
alternative sysfs attribute kind of "preferred_gpu".

Bruno


> From 43c16bbc7adbcb17aac73d09f046bf2779771c4c Mon Sep 17 00:00:00 2001
> From: Henrik Rydberg <rydberg@euromail.se>
> Date: Fri, 14 Nov 2014 20:01:21 +0100
> Subject: [PATCH v2] x86, ia64: Do not lose track of the EFI default VGA device
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Since commit 20cde694 in the 3.17 merge window, the EFI framebuffer
> depends on the VGA arbitration layer. However, the configuration does
> not reflect this, which leads to a hard-to-find bug when FB_EFI is
> configured without VGA_ARB. Add a select clause to remedy this.
> 
> Cc: Bruno Prémont <bonbons@linux-vserver.org>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/video/fbdev/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index c7bf606..1615a1b 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -761,6 +761,7 @@ config FB_EFI
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> +	select VGA_ARB if (PCI && !S390)
>  	help
>  	  This is the EFI frame buffer device driver. If the firmware on
>  	  your platform is EFI 1.10 or UEFI 2.0, select Y to add support for