* [Patch] x86, ia64, efifb: Move boot_vga fixup from pci to vgaarb
@ 2014-05-14 20:43 Bruno Prémont
2014-05-27 23:42 ` Bjorn Helgaas
0 siblings, 1 reply; 38+ messages in thread
From: Bruno Prémont @ 2014-05-14 20:43 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: DRI mailing list, Linux PCI, Dave Airlie
With commit b4aa0163056b6c70029b6e8619ce07c274351f42 Matthew Garret
introduced a efifb vga_default_device() so that EFI systems that do not
load shadow VBIOS or setup VGA get proper value for boot_vga PCI sysfs
attribute on the corresponding PCI device.
Xorg is refusing to autodetect devices when boot_vga=0 which is the case
on some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
the dri device but then bails out with "no devices detected".
With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
while having native drivers for the GPU also makes selecting
sysfb/efifb optional.
Remove the efifb implementation of vga_default_device() and initialize
vgaarb's vga_default_device() with the PCI GPU that matches boot
screen_info in pci_fixup_video() [x86 and ia64 pci_fixup_video merged
into common function in vgaarb.c].
Other architectures with PCI GPU might need a similar fixup.
Note: If CONFIG_VGA_ARB is unset vga_default_device() is only available
as a stub that returns NULL, making this adjustment insufficient.
In addition, with the merge of x86/ia64 fixup code, this would
also result in disabled fixup.
Unsetting CONFIG_VGA_ARB requires CONFIG_EXPERT=y though.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
This is ported to changes in pci/fixup.c that landed in 3.15-rcs.
Is this fine to go in, if so who will take it?
I got no feedback on my last respin (on top of 3.14 a couple of weeks ago,
which added moving the ia64 pci boot_vga fixup).
I've been running this revision for a week or so on 3.15-rc kernels here
(mostly EFI-enabled system, the mentioned MBA as wells as non-Apple systems
where the patch was not required).
arch/ia64/pci/fixup.c | 56 ++-------------------------------
arch/x86/include/asm/vga.h | 6 ----
arch/x86/pci/fixup.c | 55 +--------------------------------
drivers/gpu/vga/vgaarb.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
drivers/video/fbdev/efifb.c | 38 -----------------------
include/linux/vgaarb.h | 37 ++++++++++++++++++++++
6 files changed, 116 insertions(+), 151 deletions(-)
diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index eee069a..5df22f9 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -9,64 +9,14 @@
#include <asm/machvec.h>
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. Before marking the device
- * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
- * by either arch cde or vga-arbitration, if so only apply the fixup to this
- * already determined primary video card.
- */
-
-static void pci_fixup_video(struct pci_dev *pdev)
+static void pci_ia64_fixup_video(struct pci_dev *pdev)
{
- struct pci_dev *bridge;
- struct pci_bus *bus;
- u16 config;
-
if ((strcmp(ia64_platform_name, "dig") != 0)
&& (strcmp(ia64_platform_name, "hpzx1") != 0))
return;
/* Maybe, this machine supports legacy memory map. */
- /* Is VGA routed to us? */
- bus = pdev->bus;
- while (bus) {
- bridge = bus->self;
-
- /*
- * From information provided by
- * "David Miller" <davem@davemloft.net>
- * The bridge control register is valid for PCI header
- * type BRIDGE, or CARDBUS. Host to PCI controllers use
- * PCI header type NORMAL.
- */
- if (bridge
- &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
- ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &config);
- if (!(config & PCI_BRIDGE_CTL_VGA))
- return;
- }
- bus = bus->parent;
- }
- if (!vga_default_device() || pdev == vga_default_device()) {
- pci_read_config_word(pdev, PCI_COMMAND, &config);
- if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
- }
- }
+ pci_fixup_video(pdev);
}
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
- PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
+ PCI_CLASS_DISPLAY_VGA, 8, pci_ia64_fixup_video);
diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
index 44282fb..c4b9dc2 100644
--- a/arch/x86/include/asm/vga.h
+++ b/arch/x86/include/asm/vga.h
@@ -17,10 +17,4 @@
#define vga_readb(x) (*(x))
#define vga_writeb(x, y) (*(y) = (x))
-#ifdef CONFIG_FB_EFI
-#define __ARCH_HAS_VGA_DEFAULT_DEVICE
-extern struct pci_dev *vga_default_device(void);
-extern void vga_set_default_device(struct pci_dev *pdev);
-#endif
-
#endif /* _ASM_X86_VGA_H */
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 94ae9ae..b599847 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -302,60 +302,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PB1, pcie_r
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC, pcie_rootport_aspm_quirk);
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC1, pcie_rootport_aspm_quirk);
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. Before marking the device
- * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
- * by either arch cde or vga-arbitration, if so only apply the fixup to this
- * already determined primary video card.
- */
-
-static void pci_fixup_video(struct pci_dev *pdev)
-{
- struct pci_dev *bridge;
- struct pci_bus *bus;
- u16 config;
-
- /* Is VGA routed to us? */
- bus = pdev->bus;
- while (bus) {
- bridge = bus->self;
-
- /*
- * From information provided by
- * "David Miller" <davem@davemloft.net>
- * The bridge control register is valid for PCI header
- * type BRIDGE, or CARDBUS. Host to PCI controllers use
- * PCI header type NORMAL.
- */
- if (bridge
- && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
- || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &config);
- if (!(config & PCI_BRIDGE_CTL_VGA))
- return;
- }
- bus = bus->parent;
- }
- if (!vga_default_device() || pdev == vga_default_device()) {
- pci_read_config_word(pdev, PCI_COMMAND, &config);
- if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
- }
- }
-}
+/* pci_fixup_video shared in vgaarb.c */
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index af02597..f0fbdf6 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -149,6 +149,81 @@ void vga_set_default_device(struct pci_dev *pdev)
}
#endif
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
+ */
+
+void pci_fixup_video(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge;
+ struct pci_bus *bus;
+ u16 config;
+
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
+ /* Is VGA routed to us? */
+ bus = pdev->bus;
+ while (bus) {
+ bridge = bus->self;
+
+ /*
+ * From information provided by
+ * "David Miller" <davem@davemloft.net>
+ * The bridge control register is valid for PCI header
+ * type BRIDGE, or CARDBUS. Host to PCI controllers use
+ * PCI header type NORMAL.
+ */
+ if (bridge
+ && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+ || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+ pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+ &config);
+ if (!(config & PCI_BRIDGE_CTL_VGA))
+ return;
+ }
+ bus = bus->parent;
+ }
+ if (!vga_default_device() || pdev == vga_default_device()) {
+ pci_read_config_word(pdev, PCI_COMMAND, &config);
+ if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+ pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+ dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+ vga_set_default_device(pdev);
+ }
+ }
+}
+
static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
{
if (vgadev->irq_set_state)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index ae9618f..a033180 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -19,8 +19,6 @@
static bool request_mem_succeeded = false;
-static struct pci_dev *default_vga;
-
static struct fb_var_screeninfo efifb_defined = {
.activate = FB_ACTIVATE_NOW,
.height = -1,
@@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
.fb_imageblit = cfb_imageblit,
};
-struct pci_dev *vga_default_device(void)
-{
- return default_vga;
-}
-
-EXPORT_SYMBOL_GPL(vga_default_device);
-
-void vga_set_default_device(struct pci_dev *pdev)
-{
- default_vga = pdev;
-}
-
static int efifb_setup(char *options)
{
char *this_opt;
@@ -126,30 +112,6 @@ static int efifb_setup(char *options)
}
}
- for_each_pci_dev(dev) {
- int i;
-
- if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- continue;
-
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- resource_size_t start, end;
-
- if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(dev, i);
- end = pci_resource_end(dev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- default_vga = dev;
- }
- }
-
return 0;
}
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index 2c02f3a..6518460 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -162,6 +162,43 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
#define vga_put(pdev, rsrc)
#endif
+/**
+ * pci_fixup_video
+ *
+ * This can be called by arch PCI to fixup boot VGA tagging
+ * of VGA PCI devices (e.g. for X86, IA64)
+ *
+ * This code was initially spread/duplicated across:
+ * - X86 PCI-fixup
+ * - IA64 PCI-fixup
+ * - EFI_FB
+ *
+ * * PCI-fixup part:
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ *
+ * * EFI_FB part:
+ * Some EFI-based system (e.g. Intel-Macs from Apple) do not setup
+ * shadow Video-BIOS and thus can only be detected by framebuffer
+ * IO memory range. Flag the corresponding GPU as boot_vga.
+ */
+
+#if defined(CONFIG_VGA_ARB)
+void pci_fixup_video(struct pci_dev *pdev);
+#else
+static inline void pci_fixup_video(struct pci_dev *pdev) { }
+#endif
/**
* vga_default_device
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Patch] x86, ia64, efifb: Move boot_vga fixup from pci to vgaarb
2014-05-14 20:43 [Patch] x86, ia64, efifb: Move boot_vga fixup from pci to vgaarb Bruno Prémont
@ 2014-05-27 23:42 ` Bjorn Helgaas
2014-06-02 18:16 ` Bruno Prémont
0 siblings, 1 reply; 38+ messages in thread
From: Bjorn Helgaas @ 2014-05-27 23:42 UTC (permalink / raw)
To: Bruno Prémont; +Cc: DRI mailing list, Linux PCI, Dave Airlie
On Wed, May 14, 2014 at 10:43:39PM +0200, Bruno Prémont wrote:
> With commit b4aa0163056b6c70029b6e8619ce07c274351f42 Matthew Garret
I think there's an emerging convention to use reference commits as
<12-char-SHA1> ("subject"), i.e.,
b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
for this case. Also, s/Garret/Garrett/.
> introduced a efifb vga_default_device() so that EFI systems that do not
> load shadow VBIOS or setup VGA get proper value for boot_vga PCI sysfs
> attribute on the corresponding PCI device.
>
> Xorg is refusing to autodetect devices when boot_vga=0 which is the case
> on some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> the dri device but then bails out with "no devices detected".
>
> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> while having native drivers for the GPU also makes selecting
> sysfb/efifb optional.
>
> Remove the efifb implementation of vga_default_device() and initialize
> vgaarb's vga_default_device() with the PCI GPU that matches boot
> screen_info in pci_fixup_video() [x86 and ia64 pci_fixup_video merged
> into common function in vgaarb.c].
I think it would be good to split this into two patches:
1) Merge the x86 and ia64 pci_fixup_video() implementations. This should
have no functional change at all.
2) Whatever else you need to actually fix the bug. This will be much
smaller, and the actual bug fix will be easier to see.
It would also be nice to have a URL for a bugzilla or mailing list report
of the problem, where there might be dmesg and/or Xorg logs.
This sounds like it might be applicable for the stable kernels. If so, you
probably should order these so the small bug-fix comes first (even though
this may mean applying the same bugfix for x86 and ia64), and the merge
second. That way the fix can be applied to the stable kernels without the
merge.
> Other architectures with PCI GPU might need a similar fixup.
>
> Note: If CONFIG_VGA_ARB is unset vga_default_device() is only available
> as a stub that returns NULL, making this adjustment insufficient.
> In addition, with the merge of x86/ia64 fixup code, this would
> also result in disabled fixup.
> Unsetting CONFIG_VGA_ARB requires CONFIG_EXPERT=y though.
I'm not quite sure what this means, or if this is hint that something is
still wrong even with this patch. Do you mean that if CONFIG_VGA_ARB is
unset, something still doesn't work?
Maybe this will become obvious to me when you split up the patch.
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
> This is ported to changes in pci/fixup.c that landed in 3.15-rcs.
>
> Is this fine to go in, if so who will take it?
>
> I got no feedback on my last respin (on top of 3.14 a couple of weeks ago,
> which added moving the ia64 pci boot_vga fixup).
> I've been running this revision for a week or so on 3.15-rc kernels here
> (mostly EFI-enabled system, the mentioned MBA as wells as non-Apple systems
> where the patch was not required).
>
>
> arch/ia64/pci/fixup.c | 56 ++-------------------------------
> arch/x86/include/asm/vga.h | 6 ----
> arch/x86/pci/fixup.c | 55 +--------------------------------
> drivers/gpu/vga/vgaarb.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/video/fbdev/efifb.c | 38 -----------------------
> include/linux/vgaarb.h | 37 ++++++++++++++++++++++
> 6 files changed, 116 insertions(+), 151 deletions(-)
>
> diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
> index eee069a..5df22f9 100644
> --- a/arch/ia64/pci/fixup.c
> +++ b/arch/ia64/pci/fixup.c
> @@ -9,64 +9,14 @@
>
> #include <asm/machvec.h>
>
> -/*
> - * Fixup to mark boot BIOS video selected by BIOS before it changes
> - *
> - * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
> - *
> - * The standard boot ROM sequence for an x86 machine uses the BIOS
> - * to select an initial video card for boot display. This boot video
> - * card will have it's BIOS copied to C0000 in system RAM.
> - * IORESOURCE_ROM_SHADOW is used to associate the boot video
> - * card with this copy. On laptops this copy has to be used since
> - * the main ROM may be compressed or combined with another image.
> - * See pci_map_rom() for use of this flag. Before marking the device
> - * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
> - * by either arch cde or vga-arbitration, if so only apply the fixup to this
> - * already determined primary video card.
> - */
> -
> -static void pci_fixup_video(struct pci_dev *pdev)
> +static void pci_ia64_fixup_video(struct pci_dev *pdev)
> {
> - struct pci_dev *bridge;
> - struct pci_bus *bus;
> - u16 config;
> -
> if ((strcmp(ia64_platform_name, "dig") != 0)
> && (strcmp(ia64_platform_name, "hpzx1") != 0))
> return;
> /* Maybe, this machine supports legacy memory map. */
>
> - /* Is VGA routed to us? */
> - bus = pdev->bus;
> - while (bus) {
> - bridge = bus->self;
> -
> - /*
> - * From information provided by
> - * "David Miller" <davem@davemloft.net>
> - * The bridge control register is valid for PCI header
> - * type BRIDGE, or CARDBUS. Host to PCI controllers use
> - * PCI header type NORMAL.
> - */
> - if (bridge
> - &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> - ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
> - pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
> - &config);
> - if (!(config & PCI_BRIDGE_CTL_VGA))
> - return;
> - }
> - bus = bus->parent;
> - }
> - if (!vga_default_device() || pdev == vga_default_device()) {
> - pci_read_config_word(pdev, PCI_COMMAND, &config);
> - if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> - pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
> - dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
> - vga_set_default_device(pdev);
> - }
> - }
> + pci_fixup_video(pdev);
> }
> DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> - PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
> + PCI_CLASS_DISPLAY_VGA, 8, pci_ia64_fixup_video);
> diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
> index 44282fb..c4b9dc2 100644
> --- a/arch/x86/include/asm/vga.h
> +++ b/arch/x86/include/asm/vga.h
> @@ -17,10 +17,4 @@
> #define vga_readb(x) (*(x))
> #define vga_writeb(x, y) (*(y) = (x))
>
> -#ifdef CONFIG_FB_EFI
> -#define __ARCH_HAS_VGA_DEFAULT_DEVICE
> -extern struct pci_dev *vga_default_device(void);
> -extern void vga_set_default_device(struct pci_dev *pdev);
> -#endif
> -
> #endif /* _ASM_X86_VGA_H */
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 94ae9ae..b599847 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -302,60 +302,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PB1, pcie_r
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC, pcie_rootport_aspm_quirk);
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC1, pcie_rootport_aspm_quirk);
>
> -/*
> - * Fixup to mark boot BIOS video selected by BIOS before it changes
> - *
> - * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
> - *
> - * The standard boot ROM sequence for an x86 machine uses the BIOS
> - * to select an initial video card for boot display. This boot video
> - * card will have it's BIOS copied to C0000 in system RAM.
> - * IORESOURCE_ROM_SHADOW is used to associate the boot video
> - * card with this copy. On laptops this copy has to be used since
> - * the main ROM may be compressed or combined with another image.
> - * See pci_map_rom() for use of this flag. Before marking the device
> - * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
> - * by either arch cde or vga-arbitration, if so only apply the fixup to this
> - * already determined primary video card.
> - */
> -
> -static void pci_fixup_video(struct pci_dev *pdev)
> -{
> - struct pci_dev *bridge;
> - struct pci_bus *bus;
> - u16 config;
> -
> - /* Is VGA routed to us? */
> - bus = pdev->bus;
> - while (bus) {
> - bridge = bus->self;
> -
> - /*
> - * From information provided by
> - * "David Miller" <davem@davemloft.net>
> - * The bridge control register is valid for PCI header
> - * type BRIDGE, or CARDBUS. Host to PCI controllers use
> - * PCI header type NORMAL.
> - */
> - if (bridge
> - && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> - || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
> - pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
> - &config);
> - if (!(config & PCI_BRIDGE_CTL_VGA))
> - return;
> - }
> - bus = bus->parent;
> - }
> - if (!vga_default_device() || pdev == vga_default_device()) {
> - pci_read_config_word(pdev, PCI_COMMAND, &config);
> - if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> - pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
> - dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
> - vga_set_default_device(pdev);
> - }
> - }
> -}
> +/* pci_fixup_video shared in vgaarb.c */
> DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
>
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index af02597..f0fbdf6 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -149,6 +149,81 @@ void vga_set_default_device(struct pci_dev *pdev)
> }
> #endif
>
> +/*
> + * Fixup to mark boot BIOS video selected by BIOS before it changes
> + *
> + * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
> + *
> + * The standard boot ROM sequence for an x86 machine uses the BIOS
> + * to select an initial video card for boot display. This boot video
> + * card will have it's BIOS copied to C0000 in system RAM.
> + * IORESOURCE_ROM_SHADOW is used to associate the boot video
> + * card with this copy. On laptops this copy has to be used since
> + * the main ROM may be compressed or combined with another image.
> + * See pci_map_rom() for use of this flag. Before marking the device
> + * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
> + * by either arch cde or vga-arbitration, if so only apply the fixup to this
> + * already determined primary video card.
> + */
> +
> +void pci_fixup_video(struct pci_dev *pdev)
> +{
> + struct pci_dev *bridge;
> + struct pci_bus *bus;
> + u16 config;
> +
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> + /* Is VGA routed to us? */
> + bus = pdev->bus;
> + while (bus) {
> + bridge = bus->self;
> +
> + /*
> + * From information provided by
> + * "David Miller" <davem@davemloft.net>
> + * The bridge control register is valid for PCI header
> + * type BRIDGE, or CARDBUS. Host to PCI controllers use
> + * PCI header type NORMAL.
> + */
> + if (bridge
> + && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> + || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
I have a patch in my pci/pci_is_bridge branch that will conflict with this,
but I'll take care of sorting that out. You can continue to base your
patch on v3.15-rc1 or whatever you're using.
> + pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
> + &config);
> + if (!(config & PCI_BRIDGE_CTL_VGA))
> + return;
> + }
> + bus = bus->parent;
> + }
> + if (!vga_default_device() || pdev == vga_default_device()) {
> + pci_read_config_word(pdev, PCI_COMMAND, &config);
> + if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> + pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
> + dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
> + vga_set_default_device(pdev);
> + }
> + }
> +}
> +
> static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
> {
> if (vgadev->irq_set_state)
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index ae9618f..a033180 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -19,8 +19,6 @@
>
> static bool request_mem_succeeded = false;
>
> -static struct pci_dev *default_vga;
> -
> static struct fb_var_screeninfo efifb_defined = {
> .activate = FB_ACTIVATE_NOW,
> .height = -1,
> @@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
> .fb_imageblit = cfb_imageblit,
> };
>
> -struct pci_dev *vga_default_device(void)
> -{
> - return default_vga;
> -}
> -
> -EXPORT_SYMBOL_GPL(vga_default_device);
> -
> -void vga_set_default_device(struct pci_dev *pdev)
> -{
> - default_vga = pdev;
> -}
> -
> static int efifb_setup(char *options)
> {
> char *this_opt;
> @@ -126,30 +112,6 @@ static int efifb_setup(char *options)
> }
> }
>
> - for_each_pci_dev(dev) {
> - int i;
> -
> - if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> - continue;
> -
> - for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> - resource_size_t start, end;
> -
> - if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
> - continue;
> -
> - start = pci_resource_start(dev, i);
> - end = pci_resource_end(dev, i);
> -
> - if (!start || !end)
> - continue;
> -
> - if (screen_info.lfb_base >= start &&
> - (screen_info.lfb_base + screen_info.lfb_size) < end)
> - default_vga = dev;
> - }
> - }
> -
> return 0;
> }
>
> diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
> index 2c02f3a..6518460 100644
> --- a/include/linux/vgaarb.h
> +++ b/include/linux/vgaarb.h
> @@ -162,6 +162,43 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
> #define vga_put(pdev, rsrc)
> #endif
>
> +/**
> + * pci_fixup_video
> + *
> + * This can be called by arch PCI to fixup boot VGA tagging
> + * of VGA PCI devices (e.g. for X86, IA64)
> + *
> + * This code was initially spread/duplicated across:
> + * - X86 PCI-fixup
> + * - IA64 PCI-fixup
> + * - EFI_FB
> + *
> + * * PCI-fixup part:
> + * Fixup to mark boot BIOS video selected by BIOS before it changes
> + *
> + * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
> + *
> + * The standard boot ROM sequence for an x86 machine uses the BIOS
> + * to select an initial video card for boot display. This boot video
> + * card will have it's BIOS copied to C0000 in system RAM.
> + * IORESOURCE_ROM_SHADOW is used to associate the boot video
> + * card with this copy. On laptops this copy has to be used since
> + * the main ROM may be compressed or combined with another image.
> + * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
> + * is marked here since the boot video device will be the only enabled
> + * video device at this point.
> + *
> + * * EFI_FB part:
> + * Some EFI-based system (e.g. Intel-Macs from Apple) do not setup
> + * shadow Video-BIOS and thus can only be detected by framebuffer
> + * IO memory range. Flag the corresponding GPU as boot_vga.
> + */
I would fold this information into the comment at the function definition.
It looks like pci_fixup_video() is only called as a quirk, so there aren't
going to be random callers that will look for information at this
declaration.
> +
> +#if defined(CONFIG_VGA_ARB)
> +void pci_fixup_video(struct pci_dev *pdev);
> +#else
> +static inline void pci_fixup_video(struct pci_dev *pdev) { }
> +#endif
>
> /**
> * vga_default_device
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Patch] x86, ia64, efifb: Move boot_vga fixup from pci to vgaarb
2014-05-27 23:42 ` Bjorn Helgaas
@ 2014-06-02 18:16 ` Bruno Prémont
2014-06-02 18:19 ` [PATCH 2/2] x86, ia64: Merge common vga fixup code Bruno Prémont
2014-06-02 18:19 ` [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Bruno Prémont
0 siblings, 2 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-02 18:16 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: DRI mailing list, Linux PCI, Dave Airlie
On Tue, 27 May 2014 Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, May 14, 2014 at 10:43:39PM +0200, Bruno Prémont wrote:
> > With commit b4aa0163056b6c70029b6e8619ce07c274351f42 Matthew Garret
>
> I think there's an emerging convention to use reference commits as
> <12-char-SHA1> ("subject"), i.e.,
>
> b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
>
> for this case. Also, s/Garret/Garrett/.
Adjusted
> > introduced a efifb vga_default_device() so that EFI systems that do not
> > load shadow VBIOS or setup VGA get proper value for boot_vga PCI sysfs
> > attribute on the corresponding PCI device.
> >
> > Xorg is refusing to autodetect devices when boot_vga=0 which is the case
> > on some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> > the dri device but then bails out with "no devices detected".
> >
> > With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> > while having native drivers for the GPU also makes selecting
> > sysfb/efifb optional.
> >
> > Remove the efifb implementation of vga_default_device() and initialize
> > vgaarb's vga_default_device() with the PCI GPU that matches boot
> > screen_info in pci_fixup_video() [x86 and ia64 pci_fixup_video merged
> > into common function in vgaarb.c].
>
> I think it would be good to split this into two patches:
>
> 1) Merge the x86 and ia64 pci_fixup_video() implementations. This should
> have no functional change at all.
>
> 2) Whatever else you need to actually fix the bug. This will be much
> smaller, and the actual bug fix will be easier to see.
>
> It would also be nice to have a URL for a bugzilla or mailing list report
> of the problem, where there might be dmesg and/or Xorg logs.
>
> This sounds like it might be applicable for the stable kernels. If so, you
> probably should order these so the small bug-fix comes first (even though
> this may mean applying the same bugfix for x86 and ia64), and the merge
> second. That way the fix can be applied to the stable kernels without the
> merge.
Split up patches come in reply to this mail
> > Other architectures with PCI GPU might need a similar fixup.
> >
> > Note: If CONFIG_VGA_ARB is unset vga_default_device() is only available
> > as a stub that returns NULL, making this adjustment insufficient.
> > In addition, with the merge of x86/ia64 fixup code, this would
> > also result in disabled fixup.
> > Unsetting CONFIG_VGA_ARB requires CONFIG_EXPERT=y though.
>
> I'm not quite sure what this means, or if this is hint that something is
> still wrong even with this patch. Do you mean that if CONFIG_VGA_ARB is
> unset, something still doesn't work?
>
> Maybe this will become obvious to me when you split up the patch.
This means that if someone unsets CONFIG_VGA_ARB (he need to set CONFIG_EXPERT
to do so) the fixup will not happen at all.
Without fixup the systems that need to set the IORESOURCE_ROM_SHADOW
flag will not have it, and boot_vga pci device attribute will be 0
more often as well.
Moving the unified function somewhere else than vgaarb.c would fix the new
IORESOURCE_ROM_SHADOW fixup dependency on VGA_ARB, though not prevent the
issue I try to fix as vga_default_device() always returns NULL without VGA_ARB.
One possible location would be drivers/pci/quirks.c.
Bruno
> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> > ---
> > This is ported to changes in pci/fixup.c that landed in 3.15-rcs.
> >
> > Is this fine to go in, if so who will take it?
> >
> > I got no feedback on my last respin (on top of 3.14 a couple of weeks ago,
> > which added moving the ia64 pci boot_vga fixup).
> > I've been running this revision for a week or so on 3.15-rc kernels here
> > (mostly EFI-enabled system, the mentioned MBA as wells as non-Apple systems
> > where the patch was not required).
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 2/2] x86, ia64: Merge common vga fixup code
2014-06-02 18:16 ` Bruno Prémont
@ 2014-06-02 18:19 ` Bruno Prémont
2014-06-02 18:19 ` [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Bruno Prémont
1 sibling, 0 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-02 18:19 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: DRI mailing list, Linux PCI, Dave Airlie
The fixup code for PCI VGA devices in ia64 and x86 is mostly identical.
Merge it into a single function called from both sides.
The common code is moved to vgaarb.c which makes in dependant on
CONFIG_VGA_ARB.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
arch/ia64/pci/fixup.c | 77 ++----------------------------------------------
arch/x86/pci/fixup.c | 76 +----------------------------------------------
drivers/gpu/vga/vgaarb.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/vgaarb.h | 37 +++++++++++++++++++++++
4 files changed, 116 insertions(+), 149 deletions(-)
diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 9ed5bef..5df22f9 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -9,85 +9,14 @@
#include <asm/machvec.h>
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. Before marking the device
- * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
- * by either arch cde or vga-arbitration, if so only apply the fixup to this
- * already determined primary video card.
- */
-
-static void pci_fixup_video(struct pci_dev *pdev)
+static void pci_ia64_fixup_video(struct pci_dev *pdev)
{
- struct pci_dev *bridge;
- struct pci_bus *bus;
- u16 config;
-
if ((strcmp(ia64_platform_name, "dig") != 0)
&& (strcmp(ia64_platform_name, "hpzx1") != 0))
return;
/* Maybe, this machine supports legacy memory map. */
- if (!vga_default_device()) {
- resource_size_t start, end;
- int i;
-
- /* Does firmware framebuffer belong to us? */
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(pdev, i);
- end = pci_resource_end(pdev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- vga_set_default_device(pdev);
- }
- }
-
- /* Is VGA routed to us? */
- bus = pdev->bus;
- while (bus) {
- bridge = bus->self;
-
- /*
- * From information provided by
- * "David Miller" <davem@davemloft.net>
- * The bridge control register is valid for PCI header
- * type BRIDGE, or CARDBUS. Host to PCI controllers use
- * PCI header type NORMAL.
- */
- if (bridge
- &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
- ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &config);
- if (!(config & PCI_BRIDGE_CTL_VGA))
- return;
- }
- bus = bus->parent;
- }
- if (!vga_default_device() || pdev == vga_default_device()) {
- pci_read_config_word(pdev, PCI_COMMAND, &config);
- if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
- }
- }
+ pci_fixup_video(pdev);
}
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
- PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
+ PCI_CLASS_DISPLAY_VGA, 8, pci_ia64_fixup_video);
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 7246cf2..b599847 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -302,81 +302,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PB1, pcie_r
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC, pcie_rootport_aspm_quirk);
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC1, pcie_rootport_aspm_quirk);
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. Before marking the device
- * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
- * by either arch cde or vga-arbitration, if so only apply the fixup to this
- * already determined primary video card.
- */
-
-static void pci_fixup_video(struct pci_dev *pdev)
-{
- struct pci_dev *bridge;
- struct pci_bus *bus;
- u16 config;
-
- if (!vga_default_device()) {
- resource_size_t start, end;
- int i;
-
- /* Does firmware framebuffer belong to us? */
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(pdev, i);
- end = pci_resource_end(pdev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- vga_set_default_device(pdev);
- }
- }
-
- /* Is VGA routed to us? */
- bus = pdev->bus;
- while (bus) {
- bridge = bus->self;
-
- /*
- * From information provided by
- * "David Miller" <davem@davemloft.net>
- * The bridge control register is valid for PCI header
- * type BRIDGE, or CARDBUS. Host to PCI controllers use
- * PCI header type NORMAL.
- */
- if (bridge
- && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
- || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &config);
- if (!(config & PCI_BRIDGE_CTL_VGA))
- return;
- }
- bus = bus->parent;
- }
- if (!vga_default_device() || pdev == vga_default_device()) {
- pci_read_config_word(pdev, PCI_COMMAND, &config);
- if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
- }
- }
-}
+/* pci_fixup_video shared in vgaarb.c */
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index af02597..f0fbdf6 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -149,6 +149,81 @@ void vga_set_default_device(struct pci_dev *pdev)
}
#endif
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
+ */
+
+void pci_fixup_video(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge;
+ struct pci_bus *bus;
+ u16 config;
+
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
+ /* Is VGA routed to us? */
+ bus = pdev->bus;
+ while (bus) {
+ bridge = bus->self;
+
+ /*
+ * From information provided by
+ * "David Miller" <davem@davemloft.net>
+ * The bridge control register is valid for PCI header
+ * type BRIDGE, or CARDBUS. Host to PCI controllers use
+ * PCI header type NORMAL.
+ */
+ if (bridge
+ && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+ || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+ pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+ &config);
+ if (!(config & PCI_BRIDGE_CTL_VGA))
+ return;
+ }
+ bus = bus->parent;
+ }
+ if (!vga_default_device() || pdev == vga_default_device()) {
+ pci_read_config_word(pdev, PCI_COMMAND, &config);
+ if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+ pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+ dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+ vga_set_default_device(pdev);
+ }
+ }
+}
+
static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
{
if (vgadev->irq_set_state)
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index 2c02f3a..6518460 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -162,6 +162,43 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
#define vga_put(pdev, rsrc)
#endif
+/**
+ * pci_fixup_video
+ *
+ * This can be called by arch PCI to fixup boot VGA tagging
+ * of VGA PCI devices (e.g. for X86, IA64)
+ *
+ * This code was initially spread/duplicated across:
+ * - X86 PCI-fixup
+ * - IA64 PCI-fixup
+ * - EFI_FB
+ *
+ * * PCI-fixup part:
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ *
+ * * EFI_FB part:
+ * Some EFI-based system (e.g. Intel-Macs from Apple) do not setup
+ * shadow Video-BIOS and thus can only be detected by framebuffer
+ * IO memory range. Flag the corresponding GPU as boot_vga.
+ */
+
+#if defined(CONFIG_VGA_ARB)
+void pci_fixup_video(struct pci_dev *pdev);
+#else
+static inline void pci_fixup_video(struct pci_dev *pdev) { }
+#endif
/**
* vga_default_device
--
1.8.5.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-02 18:16 ` Bruno Prémont
2014-06-02 18:19 ` [PATCH 2/2] x86, ia64: Merge common vga fixup code Bruno Prémont
@ 2014-06-02 18:19 ` Bruno Prémont
2014-06-17 22:35 ` Bjorn Helgaas
` (2 more replies)
1 sibling, 3 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-02 18:19 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: DRI mailing list, Linux PCI, Dave Airlie
With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
Matthew Garrett introduced a efifb vga_default_device() so that EFI
systems that do not load shadow VBIOS or setup VGA get proper value for
boot_vga PCI sysfs attribute on the corresponding PCI device.
Xorg is refusing to detect devices when boot_vga=0 which is the case on
some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
the dri device but then bails out with "no devices detected".
Note: When vga_default_device() is set boot_vga PCI sysfs attribute
reflects its state. When unset this attribute is 1 whenever
IORESOURCE_ROM_SHADOW flag is set.
With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
while having native drivers for the GPU also makes selecting
sysfb/efifb optional.
Remove the efifb implementation of vga_default_device() and initialize
vgaarb's vga_default_device() with the PCI GPU that matches boot
screen_info in pci_fixup_video().
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
arch/x86/include/asm/vga.h | 6 ------
arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
drivers/video/fbdev/efifb.c | 38 --------------------------------------
4 files changed, 42 insertions(+), 44 deletions(-)
diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index eee069a..9ed5bef 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
return;
/* Maybe, this machine supports legacy memory map. */
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ /* Does firmware framebuffer belong to us? */
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
/* Is VGA routed to us? */
bus = pdev->bus;
while (bus) {
diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
index 44282fb..c4b9dc2 100644
--- a/arch/x86/include/asm/vga.h
+++ b/arch/x86/include/asm/vga.h
@@ -17,10 +17,4 @@
#define vga_readb(x) (*(x))
#define vga_writeb(x, y) (*(y) = (x))
-#ifdef CONFIG_FB_EFI
-#define __ARCH_HAS_VGA_DEFAULT_DEVICE
-extern struct pci_dev *vga_default_device(void);
-extern void vga_set_default_device(struct pci_dev *pdev);
-#endif
-
#endif /* _ASM_X86_VGA_H */
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 94ae9ae..7246cf2 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
struct pci_bus *bus;
u16 config;
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ /* Does firmware framebuffer belong to us? */
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
/* Is VGA routed to us? */
bus = pdev->bus;
while (bus) {
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index ae9618f..a033180 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -19,8 +19,6 @@
static bool request_mem_succeeded = false;
-static struct pci_dev *default_vga;
-
static struct fb_var_screeninfo efifb_defined = {
.activate = FB_ACTIVATE_NOW,
.height = -1,
@@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
.fb_imageblit = cfb_imageblit,
};
-struct pci_dev *vga_default_device(void)
-{
- return default_vga;
-}
-
-EXPORT_SYMBOL_GPL(vga_default_device);
-
-void vga_set_default_device(struct pci_dev *pdev)
-{
- default_vga = pdev;
-}
-
static int efifb_setup(char *options)
{
char *this_opt;
@@ -126,30 +112,6 @@ static int efifb_setup(char *options)
}
}
- for_each_pci_dev(dev) {
- int i;
-
- if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- continue;
-
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- resource_size_t start, end;
-
- if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(dev, i);
- end = pci_resource_end(dev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- default_vga = dev;
- }
- }
-
return 0;
}
--
1.8.5.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-02 18:19 ` [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Bruno Prémont
@ 2014-06-17 22:35 ` Bjorn Helgaas
2014-06-18 6:09 ` Bruno Prémont
2014-06-24 20:41 ` Bruno Prémont
2014-06-24 22:55 ` [PATCH 1/2 v2] " Bruno Prémont
2 siblings, 1 reply; 38+ messages in thread
From: Bjorn Helgaas @ 2014-06-17 22:35 UTC (permalink / raw)
To: Bruno Prémont; +Cc: DRI mailing list, Linux PCI, Dave Airlie
On Mon, Jun 02, 2014 at 08:19:26PM +0200, Bruno Prémont wrote:
> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
> Matthew Garrett introduced a efifb vga_default_device() so that EFI
> systems that do not load shadow VBIOS or setup VGA get proper value for
> boot_vga PCI sysfs attribute on the corresponding PCI device.
>
> Xorg is refusing to detect devices when boot_vga=0 which is the case on
> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> the dri device but then bails out with "no devices detected".
>
> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
> reflects its state. When unset this attribute is 1 whenever
> IORESOURCE_ROM_SHADOW flag is set.
>
> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> while having native drivers for the GPU also makes selecting
> sysfb/efifb optional.
>
> Remove the efifb implementation of vga_default_device() and initialize
> vgaarb's vga_default_device() with the PCI GPU that matches boot
> screen_info in pci_fixup_video().
>
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
> arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
> arch/x86/include/asm/vga.h | 6 ------
> arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
> drivers/video/fbdev/efifb.c | 38 --------------------------------------
> 4 files changed, 42 insertions(+), 44 deletions(-)
Something went wrong here. It seems like the [2/2] patch should have
been [1/2], and this one should be [2/2]. And this one modifies both
arch/ia64/pci/fixup.c and arch/x86/pci/fixup.c, but the other patch
mostly combines them, so I don't see how this one applies.
And there were unrelated (trivial) changes to these files, so they
need to be rebased to v3.16-rc1. I'd take care of the rebase, but
I don't understand the other stuff I mentioned.
Bjorn
> diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
> index eee069a..9ed5bef 100644
> --- a/arch/ia64/pci/fixup.c
> +++ b/arch/ia64/pci/fixup.c
> @@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> return;
> /* Maybe, this machine supports legacy memory map. */
>
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + /* Does firmware framebuffer belong to us? */
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> /* Is VGA routed to us? */
> bus = pdev->bus;
> while (bus) {
> diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
> index 44282fb..c4b9dc2 100644
> --- a/arch/x86/include/asm/vga.h
> +++ b/arch/x86/include/asm/vga.h
> @@ -17,10 +17,4 @@
> #define vga_readb(x) (*(x))
> #define vga_writeb(x, y) (*(y) = (x))
>
> -#ifdef CONFIG_FB_EFI
> -#define __ARCH_HAS_VGA_DEFAULT_DEVICE
> -extern struct pci_dev *vga_default_device(void);
> -extern void vga_set_default_device(struct pci_dev *pdev);
> -#endif
> -
> #endif /* _ASM_X86_VGA_H */
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 94ae9ae..7246cf2 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> struct pci_bus *bus;
> u16 config;
>
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + /* Does firmware framebuffer belong to us? */
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> /* Is VGA routed to us? */
> bus = pdev->bus;
> while (bus) {
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index ae9618f..a033180 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -19,8 +19,6 @@
>
> static bool request_mem_succeeded = false;
>
> -static struct pci_dev *default_vga;
> -
> static struct fb_var_screeninfo efifb_defined = {
> .activate = FB_ACTIVATE_NOW,
> .height = -1,
> @@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
> .fb_imageblit = cfb_imageblit,
> };
>
> -struct pci_dev *vga_default_device(void)
> -{
> - return default_vga;
> -}
> -
> -EXPORT_SYMBOL_GPL(vga_default_device);
> -
> -void vga_set_default_device(struct pci_dev *pdev)
> -{
> - default_vga = pdev;
> -}
> -
> static int efifb_setup(char *options)
> {
> char *this_opt;
> @@ -126,30 +112,6 @@ static int efifb_setup(char *options)
> }
> }
>
> - for_each_pci_dev(dev) {
> - int i;
> -
> - if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> - continue;
> -
> - for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> - resource_size_t start, end;
> -
> - if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
> - continue;
> -
> - start = pci_resource_start(dev, i);
> - end = pci_resource_end(dev, i);
> -
> - if (!start || !end)
> - continue;
> -
> - if (screen_info.lfb_base >= start &&
> - (screen_info.lfb_base + screen_info.lfb_size) < end)
> - default_vga = dev;
> - }
> - }
> -
> return 0;
> }
>
> --
> 1.8.5.5
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-17 22:35 ` Bjorn Helgaas
@ 2014-06-18 6:09 ` Bruno Prémont
0 siblings, 0 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-18 6:09 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: DRI mailing list, Linux PCI, Dave Airlie
On Tue, 17 Jun 2014 16:35:42 -0600 Bjorn Helgaas wrote:
> On Mon, Jun 02, 2014 at 08:19:26PM +0200, Bruno Prémont wrote:
> > With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
> > Matthew Garrett introduced a efifb vga_default_device() so that EFI
> > systems that do not load shadow VBIOS or setup VGA get proper value for
> > boot_vga PCI sysfs attribute on the corresponding PCI device.
> >
> > Xorg is refusing to detect devices when boot_vga=0 which is the case on
> > some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> > the dri device but then bails out with "no devices detected".
> >
> > Note: When vga_default_device() is set boot_vga PCI sysfs attribute
> > reflects its state. When unset this attribute is 1 whenever
> > IORESOURCE_ROM_SHADOW flag is set.
> >
> > With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> > while having native drivers for the GPU also makes selecting
> > sysfb/efifb optional.
> >
> > Remove the efifb implementation of vga_default_device() and initialize
> > vgaarb's vga_default_device() with the PCI GPU that matches boot
> > screen_info in pci_fixup_video().
> >
> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> > ---
> > arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
> > arch/x86/include/asm/vga.h | 6 ------
> > arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
> > drivers/video/fbdev/efifb.c | 38 --------------------------------------
> > 4 files changed, 42 insertions(+), 44 deletions(-)
>
> Something went wrong here. It seems like the [2/2] patch should have
> been [1/2], and this one should be [2/2]. And this one modifies both
> arch/ia64/pci/fixup.c and arch/x86/pci/fixup.c, but the other patch
> mostly combines them, so I don't see how this one applies.
I ordered both patches the other way around from your explicit ordering
proposal following the stable kernel suggestion following it.
My patch 1/2 fixes the encountered issue and is the one that may go
stable while my patch 2/2 performs unification of common code.
The unification will need to be placed somewhere else than in vgaarb.c
if one wants to keep current fixup working with CONFIG_VGA_ARB=n.
> And there were unrelated (trivial) changes to these files, so they
> need to be rebased to v3.16-rc1. I'd take care of the rebase, but
> I don't understand the other stuff I mentioned.
Ok
Bruno
> Bjorn
>
> > diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
> > index eee069a..9ed5bef 100644
> > --- a/arch/ia64/pci/fixup.c
> > +++ b/arch/ia64/pci/fixup.c
> > @@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> > return;
> > /* Maybe, this machine supports legacy memory map. */
> >
> > + if (!vga_default_device()) {
> > + resource_size_t start, end;
> > + int i;
> > +
> > + /* Does firmware framebuffer belong to us? */
> > + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> > + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> > + continue;
> > +
> > + start = pci_resource_start(pdev, i);
> > + end = pci_resource_end(pdev, i);
> > +
> > + if (!start || !end)
> > + continue;
> > +
> > + if (screen_info.lfb_base >= start &&
> > + (screen_info.lfb_base + screen_info.lfb_size) < end)
> > + vga_set_default_device(pdev);
> > + }
> > + }
> > +
> > /* Is VGA routed to us? */
> > bus = pdev->bus;
> > while (bus) {
> > diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
> > index 44282fb..c4b9dc2 100644
> > --- a/arch/x86/include/asm/vga.h
> > +++ b/arch/x86/include/asm/vga.h
> > @@ -17,10 +17,4 @@
> > #define vga_readb(x) (*(x))
> > #define vga_writeb(x, y) (*(y) = (x))
> >
> > -#ifdef CONFIG_FB_EFI
> > -#define __ARCH_HAS_VGA_DEFAULT_DEVICE
> > -extern struct pci_dev *vga_default_device(void);
> > -extern void vga_set_default_device(struct pci_dev *pdev);
> > -#endif
> > -
> > #endif /* _ASM_X86_VGA_H */
> > diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> > index 94ae9ae..7246cf2 100644
> > --- a/arch/x86/pci/fixup.c
> > +++ b/arch/x86/pci/fixup.c
> > @@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> > struct pci_bus *bus;
> > u16 config;
> >
> > + if (!vga_default_device()) {
> > + resource_size_t start, end;
> > + int i;
> > +
> > + /* Does firmware framebuffer belong to us? */
> > + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> > + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> > + continue;
> > +
> > + start = pci_resource_start(pdev, i);
> > + end = pci_resource_end(pdev, i);
> > +
> > + if (!start || !end)
> > + continue;
> > +
> > + if (screen_info.lfb_base >= start &&
> > + (screen_info.lfb_base + screen_info.lfb_size) < end)
> > + vga_set_default_device(pdev);
> > + }
> > + }
> > +
> > /* Is VGA routed to us? */
> > bus = pdev->bus;
> > while (bus) {
> > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> > index ae9618f..a033180 100644
> > --- a/drivers/video/fbdev/efifb.c
> > +++ b/drivers/video/fbdev/efifb.c
> > @@ -19,8 +19,6 @@
> >
> > static bool request_mem_succeeded = false;
> >
> > -static struct pci_dev *default_vga;
> > -
> > static struct fb_var_screeninfo efifb_defined = {
> > .activate = FB_ACTIVATE_NOW,
> > .height = -1,
> > @@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
> > .fb_imageblit = cfb_imageblit,
> > };
> >
> > -struct pci_dev *vga_default_device(void)
> > -{
> > - return default_vga;
> > -}
> > -
> > -EXPORT_SYMBOL_GPL(vga_default_device);
> > -
> > -void vga_set_default_device(struct pci_dev *pdev)
> > -{
> > - default_vga = pdev;
> > -}
> > -
> > static int efifb_setup(char *options)
> > {
> > char *this_opt;
> > @@ -126,30 +112,6 @@ static int efifb_setup(char *options)
> > }
> > }
> >
> > - for_each_pci_dev(dev) {
> > - int i;
> > -
> > - if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> > - continue;
> > -
> > - for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> > - resource_size_t start, end;
> > -
> > - if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
> > - continue;
> > -
> > - start = pci_resource_start(dev, i);
> > - end = pci_resource_end(dev, i);
> > -
> > - if (!start || !end)
> > - continue;
> > -
> > - if (screen_info.lfb_base >= start &&
> > - (screen_info.lfb_base + screen_info.lfb_size) < end)
> > - default_vga = dev;
> > - }
> > - }
> > -
> > return 0;
> > }
> >
> > --
> > 1.8.5.5
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-02 18:19 ` [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Bruno Prémont
2014-06-17 22:35 ` Bjorn Helgaas
@ 2014-06-24 20:41 ` Bruno Prémont
2014-06-24 22:55 ` [PATCH 1/2 v2] " Bruno Prémont
2 siblings, 0 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-24 20:41 UTC (permalink / raw)
To: Bjorn Helgaas, Dave Airlie
Cc: DRI mailing list, Linux PCI, Anibal Francisco Martinez Cortina, nouveau
On Mon, 02 June 2014 Bruno Prémont <bonbons@linux-vserver.org> wrote:
> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
> Matthew Garrett introduced a efifb vga_default_device() so that EFI
> systems that do not load shadow VBIOS or setup VGA get proper value for
> boot_vga PCI sysfs attribute on the corresponding PCI device.
>
> Xorg is refusing to detect devices when boot_vga=0 which is the case on
> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> the dri device but then bails out with "no devices detected".
>
> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
> reflects its state. When unset this attribute is 1 whenever
> IORESOURCE_ROM_SHADOW flag is set.
>
> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> while having native drivers for the GPU also makes selecting
> sysfb/efifb optional.
>
> Remove the efifb implementation of vga_default_device() and initialize
> vgaarb's vga_default_device() with the PCI GPU that matches boot
> screen_info in pci_fixup_video().
Quite a few people trip over this issue, apparently concentrated on
Apple Macs (EFI) with Nvidia graphics.
It has shown up in at least two bugzilla reports (as side-issue):
https://bugs.freedesktop.org/show_bug.cgi?id=58556#c16
https://bugs.freedesktop.org/show_bug.cgi?id=76732#c34
My initial report and discussion on freenode, #nouveau:
http://people.freedesktop.org/~cbrill/dri-log/?channel=nouveau&date=2013-11-23
followed by initial patch proposal:
http://permalink.gmane.org/gmane.comp.video.dri.devel/95943
>From today on #nouveau:
Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
As reported on #nouveau, every now and then someone trips over this issue
and falls back to enabling efifb so X server accepts to start without
explicit configuration.
It would also be worth to add for first patch:
Cc: stable@vger.kernel.org
Bruno
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
> arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
> arch/x86/include/asm/vga.h | 6 ------
> arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
> drivers/video/fbdev/efifb.c | 38 --------------------------------------
> 4 files changed, 42 insertions(+), 44 deletions(-)
>
> diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
> index eee069a..9ed5bef 100644
> --- a/arch/ia64/pci/fixup.c
> +++ b/arch/ia64/pci/fixup.c
> @@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> return;
> /* Maybe, this machine supports legacy memory map. */
>
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + /* Does firmware framebuffer belong to us? */
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> /* Is VGA routed to us? */
> bus = pdev->bus;
> while (bus) {
> diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
> index 44282fb..c4b9dc2 100644
> --- a/arch/x86/include/asm/vga.h
> +++ b/arch/x86/include/asm/vga.h
> @@ -17,10 +17,4 @@
> #define vga_readb(x) (*(x))
> #define vga_writeb(x, y) (*(y) = (x))
>
> -#ifdef CONFIG_FB_EFI
> -#define __ARCH_HAS_VGA_DEFAULT_DEVICE
> -extern struct pci_dev *vga_default_device(void);
> -extern void vga_set_default_device(struct pci_dev *pdev);
> -#endif
> -
> #endif /* _ASM_X86_VGA_H */
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 94ae9ae..7246cf2 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> struct pci_bus *bus;
> u16 config;
>
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + /* Does firmware framebuffer belong to us? */
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> /* Is VGA routed to us? */
> bus = pdev->bus;
> while (bus) {
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index ae9618f..a033180 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -19,8 +19,6 @@
>
> static bool request_mem_succeeded = false;
>
> -static struct pci_dev *default_vga;
> -
> static struct fb_var_screeninfo efifb_defined = {
> .activate = FB_ACTIVATE_NOW,
> .height = -1,
> @@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
> .fb_imageblit = cfb_imageblit,
> };
>
> -struct pci_dev *vga_default_device(void)
> -{
> - return default_vga;
> -}
> -
> -EXPORT_SYMBOL_GPL(vga_default_device);
> -
> -void vga_set_default_device(struct pci_dev *pdev)
> -{
> - default_vga = pdev;
> -}
> -
> static int efifb_setup(char *options)
> {
> char *this_opt;
> @@ -126,30 +112,6 @@ static int efifb_setup(char *options)
> }
> }
>
> - for_each_pci_dev(dev) {
> - int i;
> -
> - if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> - continue;
> -
> - for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> - resource_size_t start, end;
> -
> - if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
> - continue;
> -
> - start = pci_resource_start(dev, i);
> - end = pci_resource_end(dev, i);
> -
> - if (!start || !end)
> - continue;
> -
> - if (screen_info.lfb_base >= start &&
> - (screen_info.lfb_base + screen_info.lfb_size) < end)
> - default_vga = dev;
> - }
> - }
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-02 18:19 ` [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Bruno Prémont
2014-06-17 22:35 ` Bjorn Helgaas
2014-06-24 20:41 ` Bruno Prémont
@ 2014-06-24 22:55 ` Bruno Prémont
2014-06-24 22:58 ` [PATCH 2/2 v2] x86, ia64: Merge common vga fixup code Bruno Prémont
` (2 more replies)
2 siblings, 3 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-24 22:55 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett
With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
Matthew Garrett introduced a efifb vga_default_device() so that EFI
systems that do not load shadow VBIOS or setup VGA get proper value for
boot_vga PCI sysfs attribute on the corresponding PCI device.
Xorg is refusing to detect devices when boot_vga=0 which is the case on
some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
the dri device but then bails out with "no devices detected".
Note: When vga_default_device() is set boot_vga PCI sysfs attribute
reflects its state. When unset this attribute is 1 whenever
IORESOURCE_ROM_SHADOW flag is set.
With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
while having native drivers for the GPU also makes selecting
sysfb/efifb optional.
Remove the efifb implementation of vga_default_device() and initialize
vgaarb's vga_default_device() with the PCI GPU that matches boot
screen_info in pci_fixup_video().
Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
Cc: Matthew Garrett <matthew.garrett@nebula.com>
Cc: stable@vger.kernel.org
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
Changes since v1:
Added Tested-by, Cc
arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
arch/x86/include/asm/vga.h | 6 ------
arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
drivers/video/fbdev/efifb.c | 38 --------------------------------------
4 files changed, 42 insertions(+), 44 deletions(-)
diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index eee069a..9ed5bef 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
return;
/* Maybe, this machine supports legacy memory map. */
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ /* Does firmware framebuffer belong to us? */
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
/* Is VGA routed to us? */
bus = pdev->bus;
while (bus) {
diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
index 44282fb..c4b9dc2 100644
--- a/arch/x86/include/asm/vga.h
+++ b/arch/x86/include/asm/vga.h
@@ -17,10 +17,4 @@
#define vga_readb(x) (*(x))
#define vga_writeb(x, y) (*(y) = (x))
-#ifdef CONFIG_FB_EFI
-#define __ARCH_HAS_VGA_DEFAULT_DEVICE
-extern struct pci_dev *vga_default_device(void);
-extern void vga_set_default_device(struct pci_dev *pdev);
-#endif
-
#endif /* _ASM_X86_VGA_H */
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 94ae9ae..7246cf2 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
struct pci_bus *bus;
u16 config;
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ /* Does firmware framebuffer belong to us? */
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
/* Is VGA routed to us? */
bus = pdev->bus;
while (bus) {
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index ae9618f..a033180 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -19,8 +19,6 @@
static bool request_mem_succeeded = false;
-static struct pci_dev *default_vga;
-
static struct fb_var_screeninfo efifb_defined = {
.activate = FB_ACTIVATE_NOW,
.height = -1,
@@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
.fb_imageblit = cfb_imageblit,
};
-struct pci_dev *vga_default_device(void)
-{
- return default_vga;
-}
-
-EXPORT_SYMBOL_GPL(vga_default_device);
-
-void vga_set_default_device(struct pci_dev *pdev)
-{
- default_vga = pdev;
-}
-
static int efifb_setup(char *options)
{
char *this_opt;
@@ -126,30 +112,6 @@ static int efifb_setup(char *options)
}
}
- for_each_pci_dev(dev) {
- int i;
-
- if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- continue;
-
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- resource_size_t start, end;
-
- if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(dev, i);
- end = pci_resource_end(dev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- default_vga = dev;
- }
- }
-
return 0;
}
--
1.8.5.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 2/2 v2] x86, ia64: Merge common vga fixup code
2014-06-24 22:55 ` [PATCH 1/2 v2] " Bruno Prémont
@ 2014-06-24 22:58 ` Bruno Prémont
2014-06-24 23:02 ` [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Matthew Garrett
2014-07-05 17:15 ` Bjorn Helgaas
2 siblings, 0 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-06-24 22:58 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett,
Anibal Francisco Martinez Cortina
The fixup code for PCI VGA devices in ia64 and x86 is mostly identical.
Merge it into a single function called from both sides.
The common code is moved to vgaarb.c which makes in dependant on
CONFIG_VGA_ARB.
Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
Cc: Matthew Garrett <matthew.garrett@nebula.com>
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
Changes since v1:
Added Tested-by, Cc
arch/ia64/pci/fixup.c | 77 ++----------------------------------------------
arch/x86/pci/fixup.c | 76 +----------------------------------------------
drivers/gpu/vga/vgaarb.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/vgaarb.h | 37 +++++++++++++++++++++++
4 files changed, 116 insertions(+), 149 deletions(-)
diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 9ed5bef..5df22f9 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -9,85 +9,14 @@
#include <asm/machvec.h>
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. Before marking the device
- * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
- * by either arch cde or vga-arbitration, if so only apply the fixup to this
- * already determined primary video card.
- */
-
-static void pci_fixup_video(struct pci_dev *pdev)
+static void pci_ia64_fixup_video(struct pci_dev *pdev)
{
- struct pci_dev *bridge;
- struct pci_bus *bus;
- u16 config;
-
if ((strcmp(ia64_platform_name, "dig") != 0)
&& (strcmp(ia64_platform_name, "hpzx1") != 0))
return;
/* Maybe, this machine supports legacy memory map. */
- if (!vga_default_device()) {
- resource_size_t start, end;
- int i;
-
- /* Does firmware framebuffer belong to us? */
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(pdev, i);
- end = pci_resource_end(pdev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- vga_set_default_device(pdev);
- }
- }
-
- /* Is VGA routed to us? */
- bus = pdev->bus;
- while (bus) {
- bridge = bus->self;
-
- /*
- * From information provided by
- * "David Miller" <davem@davemloft.net>
- * The bridge control register is valid for PCI header
- * type BRIDGE, or CARDBUS. Host to PCI controllers use
- * PCI header type NORMAL.
- */
- if (bridge
- &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
- ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &config);
- if (!(config & PCI_BRIDGE_CTL_VGA))
- return;
- }
- bus = bus->parent;
- }
- if (!vga_default_device() || pdev == vga_default_device()) {
- pci_read_config_word(pdev, PCI_COMMAND, &config);
- if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
- }
- }
+ pci_fixup_video(pdev);
}
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
- PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
+ PCI_CLASS_DISPLAY_VGA, 8, pci_ia64_fixup_video);
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 7246cf2..b599847 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -302,81 +302,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PB1, pcie_r
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC, pcie_rootport_aspm_quirk);
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC1, pcie_rootport_aspm_quirk);
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. Before marking the device
- * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
- * by either arch cde or vga-arbitration, if so only apply the fixup to this
- * already determined primary video card.
- */
-
-static void pci_fixup_video(struct pci_dev *pdev)
-{
- struct pci_dev *bridge;
- struct pci_bus *bus;
- u16 config;
-
- if (!vga_default_device()) {
- resource_size_t start, end;
- int i;
-
- /* Does firmware framebuffer belong to us? */
- for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
- if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
- continue;
-
- start = pci_resource_start(pdev, i);
- end = pci_resource_end(pdev, i);
-
- if (!start || !end)
- continue;
-
- if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
- vga_set_default_device(pdev);
- }
- }
-
- /* Is VGA routed to us? */
- bus = pdev->bus;
- while (bus) {
- bridge = bus->self;
-
- /*
- * From information provided by
- * "David Miller" <davem@davemloft.net>
- * The bridge control register is valid for PCI header
- * type BRIDGE, or CARDBUS. Host to PCI controllers use
- * PCI header type NORMAL.
- */
- if (bridge
- && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
- || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &config);
- if (!(config & PCI_BRIDGE_CTL_VGA))
- return;
- }
- bus = bus->parent;
- }
- if (!vga_default_device() || pdev == vga_default_device()) {
- pci_read_config_word(pdev, PCI_COMMAND, &config);
- if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
- }
- }
-}
+/* pci_fixup_video shared in vgaarb.c */
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index af02597..f0fbdf6 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -149,6 +149,81 @@ void vga_set_default_device(struct pci_dev *pdev)
}
#endif
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
+ */
+
+void pci_fixup_video(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge;
+ struct pci_bus *bus;
+ u16 config;
+
+ if (!vga_default_device()) {
+ resource_size_t start, end;
+ int i;
+
+ for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
+ continue;
+
+ start = pci_resource_start(pdev, i);
+ end = pci_resource_end(pdev, i);
+
+ if (!start || !end)
+ continue;
+
+ if (screen_info.lfb_base >= start &&
+ (screen_info.lfb_base + screen_info.lfb_size) < end)
+ vga_set_default_device(pdev);
+ }
+ }
+
+ /* Is VGA routed to us? */
+ bus = pdev->bus;
+ while (bus) {
+ bridge = bus->self;
+
+ /*
+ * From information provided by
+ * "David Miller" <davem@davemloft.net>
+ * The bridge control register is valid for PCI header
+ * type BRIDGE, or CARDBUS. Host to PCI controllers use
+ * PCI header type NORMAL.
+ */
+ if (bridge
+ && ((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+ || (bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+ pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+ &config);
+ if (!(config & PCI_BRIDGE_CTL_VGA))
+ return;
+ }
+ bus = bus->parent;
+ }
+ if (!vga_default_device() || pdev == vga_default_device()) {
+ pci_read_config_word(pdev, PCI_COMMAND, &config);
+ if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+ pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+ dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+ vga_set_default_device(pdev);
+ }
+ }
+}
+
static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
{
if (vgadev->irq_set_state)
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index 2c02f3a..6518460 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -162,6 +162,43 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
#define vga_put(pdev, rsrc)
#endif
+/**
+ * pci_fixup_video
+ *
+ * This can be called by arch PCI to fixup boot VGA tagging
+ * of VGA PCI devices (e.g. for X86, IA64)
+ *
+ * This code was initially spread/duplicated across:
+ * - X86 PCI-fixup
+ * - IA64 PCI-fixup
+ * - EFI_FB
+ *
+ * * PCI-fixup part:
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ *
+ * * EFI_FB part:
+ * Some EFI-based system (e.g. Intel-Macs from Apple) do not setup
+ * shadow Video-BIOS and thus can only be detected by framebuffer
+ * IO memory range. Flag the corresponding GPU as boot_vga.
+ */
+
+#if defined(CONFIG_VGA_ARB)
+void pci_fixup_video(struct pci_dev *pdev);
+#else
+static inline void pci_fixup_video(struct pci_dev *pdev) { }
+#endif
/**
* vga_default_device
--
1.8.5.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-24 22:55 ` [PATCH 1/2 v2] " Bruno Prémont
2014-06-24 22:58 ` [PATCH 2/2 v2] x86, ia64: Merge common vga fixup code Bruno Prémont
@ 2014-06-24 23:02 ` Matthew Garrett
2014-07-05 17:15 ` Bjorn Helgaas
2 siblings, 0 replies; 38+ messages in thread
From: Matthew Garrett @ 2014-06-24 23:02 UTC (permalink / raw)
To: bonbons; +Cc: dri-devel, linux-pci, bhelgaas, airlied
T24gV2VkLCAyMDE0LTA2LTI1IGF0IDAwOjU1ICswMjAwLCBCcnVubyBQcsOpbW9udCB3cm90ZToN
Cg0KPiBXaXRoIGludHJvZHVjdGlvbiBvZiBzeXNmYi9zaW1wbGVmYi9zaW1wbGVkcm0gZWZpZmIg
aXMgZ2V0dGluZyBvYnNvbGV0ZQ0KPiB3aGlsZSBoYXZpbmcgbmF0aXZlIGRyaXZlcnMgZm9yIHRo
ZSBHUFUgYWxzbyBtYWtlcyBzZWxlY3RpbmcNCj4gc3lzZmIvZWZpZmIgb3B0aW9uYWwuDQo+IA0K
DQpCb3RoIGxvb2sgZ29vZCB0byBtZS4NCg0KQWNrZWQtYnk6IE1hdHRoZXcgR2FycmV0dCA8bWF0
dGhldy5nYXJyZXR0QG5lYnVsYS5jb20+DQoNCi0tIA0KTWF0dGhldyBHYXJyZXR0IDxtYXR0aGV3
LmdhcnJldHRAbmVidWxhLmNvbT4NCg==
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-06-24 22:55 ` [PATCH 1/2 v2] " Bruno Prémont
2014-06-24 22:58 ` [PATCH 2/2 v2] x86, ia64: Merge common vga fixup code Bruno Prémont
2014-06-24 23:02 ` [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Matthew Garrett
@ 2014-07-05 17:15 ` Bjorn Helgaas
2014-08-10 0:21 ` Andreas Noever
2 siblings, 1 reply; 38+ messages in thread
From: Bjorn Helgaas @ 2014-07-05 17:15 UTC (permalink / raw)
To: Bruno Prémont
Cc: DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett
On Wed, Jun 25, 2014 at 12:55:01AM +0200, Bruno Prémont wrote:
> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
> Matthew Garrett introduced a efifb vga_default_device() so that EFI
> systems that do not load shadow VBIOS or setup VGA get proper value for
> boot_vga PCI sysfs attribute on the corresponding PCI device.
>
> Xorg is refusing to detect devices when boot_vga=0 which is the case on
> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> the dri device but then bails out with "no devices detected".
>
> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
> reflects its state. When unset this attribute is 1 whenever
> IORESOURCE_ROM_SHADOW flag is set.
>
> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> while having native drivers for the GPU also makes selecting
> sysfb/efifb optional.
>
> Remove the efifb implementation of vga_default_device() and initialize
> vgaarb's vga_default_device() with the PCI GPU that matches boot
> screen_info in pci_fixup_video().
>
> Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
> Cc: Matthew Garrett <matthew.garrett@nebula.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
I applied both with Matthew's ack to pci/misc for v3.17, thanks!
> ---
>
> Changes since v1:
> Added Tested-by, Cc
>
> arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
> arch/x86/include/asm/vga.h | 6 ------
> arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
> drivers/video/fbdev/efifb.c | 38 --------------------------------------
> 4 files changed, 42 insertions(+), 44 deletions(-)
>
> diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
> index eee069a..9ed5bef 100644
> --- a/arch/ia64/pci/fixup.c
> +++ b/arch/ia64/pci/fixup.c
> @@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> return;
> /* Maybe, this machine supports legacy memory map. */
>
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + /* Does firmware framebuffer belong to us? */
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> /* Is VGA routed to us? */
> bus = pdev->bus;
> while (bus) {
> diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
> index 44282fb..c4b9dc2 100644
> --- a/arch/x86/include/asm/vga.h
> +++ b/arch/x86/include/asm/vga.h
> @@ -17,10 +17,4 @@
> #define vga_readb(x) (*(x))
> #define vga_writeb(x, y) (*(y) = (x))
>
> -#ifdef CONFIG_FB_EFI
> -#define __ARCH_HAS_VGA_DEFAULT_DEVICE
> -extern struct pci_dev *vga_default_device(void);
> -extern void vga_set_default_device(struct pci_dev *pdev);
> -#endif
> -
> #endif /* _ASM_X86_VGA_H */
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 94ae9ae..7246cf2 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
> struct pci_bus *bus;
> u16 config;
>
> + if (!vga_default_device()) {
> + resource_size_t start, end;
> + int i;
> +
> + /* Does firmware framebuffer belong to us? */
> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, i);
> + end = pci_resource_end(pdev, i);
> +
> + if (!start || !end)
> + continue;
> +
> + if (screen_info.lfb_base >= start &&
> + (screen_info.lfb_base + screen_info.lfb_size) < end)
> + vga_set_default_device(pdev);
> + }
> + }
> +
> /* Is VGA routed to us? */
> bus = pdev->bus;
> while (bus) {
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index ae9618f..a033180 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -19,8 +19,6 @@
>
> static bool request_mem_succeeded = false;
>
> -static struct pci_dev *default_vga;
> -
> static struct fb_var_screeninfo efifb_defined = {
> .activate = FB_ACTIVATE_NOW,
> .height = -1,
> @@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
> .fb_imageblit = cfb_imageblit,
> };
>
> -struct pci_dev *vga_default_device(void)
> -{
> - return default_vga;
> -}
> -
> -EXPORT_SYMBOL_GPL(vga_default_device);
> -
> -void vga_set_default_device(struct pci_dev *pdev)
> -{
> - default_vga = pdev;
> -}
> -
> static int efifb_setup(char *options)
> {
> char *this_opt;
> @@ -126,30 +112,6 @@ static int efifb_setup(char *options)
> }
> }
>
> - for_each_pci_dev(dev) {
> - int i;
> -
> - if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> - continue;
> -
> - for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
> - resource_size_t start, end;
> -
> - if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
> - continue;
> -
> - start = pci_resource_start(dev, i);
> - end = pci_resource_end(dev, i);
> -
> - if (!start || !end)
> - continue;
> -
> - if (screen_info.lfb_base >= start &&
> - (screen_info.lfb_base + screen_info.lfb_size) < end)
> - default_vga = dev;
> - }
> - }
> -
> return 0;
> }
>
> --
> 1.8.5.5
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-07-05 17:15 ` Bjorn Helgaas
@ 2014-08-10 0:21 ` Andreas Noever
2014-08-10 0:36 ` Andreas Noever
0 siblings, 1 reply; 38+ messages in thread
From: Andreas Noever @ 2014-08-10 0:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bruno Prémont, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett
On Sat, Jul 5, 2014 at 7:15 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Jun 25, 2014 at 12:55:01AM +0200, Bruno Prémont wrote:
>> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
>> Matthew Garrett introduced a efifb vga_default_device() so that EFI
>> systems that do not load shadow VBIOS or setup VGA get proper value for
>> boot_vga PCI sysfs attribute on the corresponding PCI device.
>>
>> Xorg is refusing to detect devices when boot_vga=0 which is the case on
>> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
>> the dri device but then bails out with "no devices detected".
>>
>> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
>> reflects its state. When unset this attribute is 1 whenever
>> IORESOURCE_ROM_SHADOW flag is set.
>>
>> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
>> while having native drivers for the GPU also makes selecting
>> sysfb/efifb optional.
>>
>> Remove the efifb implementation of vga_default_device() and initialize
>> vgaarb's vga_default_device() with the PCI GPU that matches boot
>> screen_info in pci_fixup_video().
>>
>> Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
>> Cc: Matthew Garrett <matthew.garrett@nebula.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
>
> I applied both with Matthew's ack to pci/misc for v3.17, thanks!
I just tried to run the latest kernel. It failed to boot and git
bisect points to this commit (MacBookPro10,1 with Nvidia&Intel
graphics).
The (now removed) code in efifb_setup did always set default_vga, even
if it had already been set earlier. The new code in pci_fixup_video
runs only if vga_default_device() is NULL. Removing the check fixes
the regression.
The following calls to vga_set_default_device are made during boot:
vga_arbiter_add_pci_device -> vga_set_default_device(intel)
pci_fixup_video -> vga_set_default_device(intel) (there are two calls
in pci_fixup_video, this one is the one near "Boot video device")
pci_fixup_video -> vga_set_default_device(nvidia) (from the "Does
firmware framebuffer belong to us?" loop, only if I remove the check)
vga_arbiter_add_pci_device chooses intel simply because it is the
first device. Next pci_fixup_video(intel) sees that it is the default
device, sets the IORESOURCE_ROM_SHADOW flag and calls
vga_set_default_device again. And finally (if the check is removed)
pci_fixup_video(nvidia) sees that it owns the framebuffer and sets
itself as the default device which allows the system to boot again.
Does setting the ROM_SHADOW flag on (possibly) the wrong device have
any effect? Maybe we should also move this whole detection logic to
some earlier stage to make sure that the default device is correct
from the beginning and doesn't change?
>
>> ---
>>
>> Changes since v1:
>> Added Tested-by, Cc
>>
>> arch/ia64/pci/fixup.c | 21 +++++++++++++++++++++
>> arch/x86/include/asm/vga.h | 6 ------
>> arch/x86/pci/fixup.c | 21 +++++++++++++++++++++
>> drivers/video/fbdev/efifb.c | 38 --------------------------------------
>> 4 files changed, 42 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
>> index eee069a..9ed5bef 100644
>> --- a/arch/ia64/pci/fixup.c
>> +++ b/arch/ia64/pci/fixup.c
>> @@ -37,6 +37,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
>> return;
>> /* Maybe, this machine supports legacy memory map. */
>>
>> + if (!vga_default_device()) {
>> + resource_size_t start, end;
>> + int i;
>> +
>> + /* Does firmware framebuffer belong to us? */
>> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
>> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
>> + continue;
>> +
>> + start = pci_resource_start(pdev, i);
>> + end = pci_resource_end(pdev, i);
>> +
>> + if (!start || !end)
>> + continue;
>> +
>> + if (screen_info.lfb_base >= start &&
>> + (screen_info.lfb_base + screen_info.lfb_size) < end)
>> + vga_set_default_device(pdev);
>> + }
>> + }
>> +
>> /* Is VGA routed to us? */
>> bus = pdev->bus;
>> while (bus) {
>> diff --git a/arch/x86/include/asm/vga.h b/arch/x86/include/asm/vga.h
>> index 44282fb..c4b9dc2 100644
>> --- a/arch/x86/include/asm/vga.h
>> +++ b/arch/x86/include/asm/vga.h
>> @@ -17,10 +17,4 @@
>> #define vga_readb(x) (*(x))
>> #define vga_writeb(x, y) (*(y) = (x))
>>
>> -#ifdef CONFIG_FB_EFI
>> -#define __ARCH_HAS_VGA_DEFAULT_DEVICE
>> -extern struct pci_dev *vga_default_device(void);
>> -extern void vga_set_default_device(struct pci_dev *pdev);
>> -#endif
>> -
>> #endif /* _ASM_X86_VGA_H */
>> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
>> index 94ae9ae..7246cf2 100644
>> --- a/arch/x86/pci/fixup.c
>> +++ b/arch/x86/pci/fixup.c
>> @@ -325,6 +325,27 @@ static void pci_fixup_video(struct pci_dev *pdev)
>> struct pci_bus *bus;
>> u16 config;
>>
>> + if (!vga_default_device()) {
>> + resource_size_t start, end;
>> + int i;
>> +
>> + /* Does firmware framebuffer belong to us? */
>> + for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
>> + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
>> + continue;
>> +
>> + start = pci_resource_start(pdev, i);
>> + end = pci_resource_end(pdev, i);
>> +
>> + if (!start || !end)
>> + continue;
>> +
>> + if (screen_info.lfb_base >= start &&
>> + (screen_info.lfb_base + screen_info.lfb_size) < end)
>> + vga_set_default_device(pdev);
>> + }
>> + }
>> +
>> /* Is VGA routed to us? */
>> bus = pdev->bus;
>> while (bus) {
>> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
>> index ae9618f..a033180 100644
>> --- a/drivers/video/fbdev/efifb.c
>> +++ b/drivers/video/fbdev/efifb.c
>> @@ -19,8 +19,6 @@
>>
>> static bool request_mem_succeeded = false;
>>
>> -static struct pci_dev *default_vga;
>> -
>> static struct fb_var_screeninfo efifb_defined = {
>> .activate = FB_ACTIVATE_NOW,
>> .height = -1,
>> @@ -84,18 +82,6 @@ static struct fb_ops efifb_ops = {
>> .fb_imageblit = cfb_imageblit,
>> };
>>
>> -struct pci_dev *vga_default_device(void)
>> -{
>> - return default_vga;
>> -}
>> -
>> -EXPORT_SYMBOL_GPL(vga_default_device);
>> -
>> -void vga_set_default_device(struct pci_dev *pdev)
>> -{
>> - default_vga = pdev;
>> -}
>> -
>> static int efifb_setup(char *options)
>> {
>> char *this_opt;
>> @@ -126,30 +112,6 @@ static int efifb_setup(char *options)
>> }
>> }
>>
>> - for_each_pci_dev(dev) {
>> - int i;
>> -
>> - if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
>> - continue;
>> -
>> - for (i=0; i < DEVICE_COUNT_RESOURCE; i++) {
>> - resource_size_t start, end;
>> -
>> - if (!(pci_resource_flags(dev, i) & IORESOURCE_MEM))
>> - continue;
>> -
>> - start = pci_resource_start(dev, i);
>> - end = pci_resource_end(dev, i);
>> -
>> - if (!start || !end)
>> - continue;
>> -
>> - if (screen_info.lfb_base >= start &&
>> - (screen_info.lfb_base + screen_info.lfb_size) < end)
>> - default_vga = dev;
>> - }
>> - }
>> -
>> return 0;
>> }
>>
>> --
>> 1.8.5.5
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-10 0:21 ` Andreas Noever
@ 2014-08-10 0:36 ` Andreas Noever
2014-08-10 9:26 ` Bruno Prémont
0 siblings, 1 reply; 38+ messages in thread
From: Andreas Noever @ 2014-08-10 0:36 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bruno Prémont, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett
On Sun, Aug 10, 2014 at 2:21 AM, Andreas Noever
<andreas.noever@gmail.com> wrote:
> On Sat, Jul 5, 2014 at 7:15 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Wed, Jun 25, 2014 at 12:55:01AM +0200, Bruno Prémont wrote:
>>> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
>>> Matthew Garrett introduced a efifb vga_default_device() so that EFI
>>> systems that do not load shadow VBIOS or setup VGA get proper value for
>>> boot_vga PCI sysfs attribute on the corresponding PCI device.
>>>
>>> Xorg is refusing to detect devices when boot_vga=0 which is the case on
>>> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
>>> the dri device but then bails out with "no devices detected".
>>>
>>> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
>>> reflects its state. When unset this attribute is 1 whenever
>>> IORESOURCE_ROM_SHADOW flag is set.
>>>
>>> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
>>> while having native drivers for the GPU also makes selecting
>>> sysfb/efifb optional.
>>>
>>> Remove the efifb implementation of vga_default_device() and initialize
>>> vgaarb's vga_default_device() with the PCI GPU that matches boot
>>> screen_info in pci_fixup_video().
>>>
>>> Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
>>> Cc: Matthew Garrett <matthew.garrett@nebula.com>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
>>
>> I applied both with Matthew's ack to pci/misc for v3.17, thanks!
>
> I just tried to run the latest kernel. It failed to boot and git
> bisect points to this commit (MacBookPro10,1 with Nvidia&Intel
> graphics).
>
> The (now removed) code in efifb_setup did always set default_vga, even
> if it had already been set earlier. The new code in pci_fixup_video
> runs only if vga_default_device() is NULL. Removing the check fixes
> the regression.
>
>
> The following calls to vga_set_default_device are made during boot:
>
> vga_arbiter_add_pci_device -> vga_set_default_device(intel)
> pci_fixup_video -> vga_set_default_device(intel) (there are two calls
> in pci_fixup_video, this one is the one near "Boot video device")
> pci_fixup_video -> vga_set_default_device(nvidia) (from the "Does
> firmware framebuffer belong to us?" loop, only if I remove the check)
>
> vga_arbiter_add_pci_device chooses intel simply because it is the
> first device. Next pci_fixup_video(intel) sees that it is the default
> device, sets the IORESOURCE_ROM_SHADOW flag and calls
> vga_set_default_device again. And finally (if the check is removed)
> pci_fixup_video(nvidia) sees that it owns the framebuffer and sets
> itself as the default device which allows the system to boot again.
>
> Does setting the ROM_SHADOW flag on (possibly) the wrong device have
> any effect?
Yes it does. Removing the line changes a long standing
i915 0000:00:02.0: Invalid ROM contents
into a
i915 0000:00:02.0: BAR 6: can't assign [??? 0x00000000 flags
0x20000000] (bogus alignment).
The first is logged at KERN_ERR and the second one only at KERN_INFO.
We are making progress.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-10 0:36 ` Andreas Noever
@ 2014-08-10 9:26 ` Bruno Prémont
2014-08-10 9:56 ` Andreas Noever
0 siblings, 1 reply; 38+ messages in thread
From: Bruno Prémont @ 2014-08-10 9:26 UTC (permalink / raw)
To: Andreas Noever
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett
On Sun, 10 August 2014 Andreas Noever <andreas.noever@gmail.com> wrote:
> On Sun, Aug 10, 2014 at 2:21 AM, Andreas Noever
> <andreas.noever@gmail.com> wrote:
> > On Sat, Jul 5, 2014 at 7:15 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> On Wed, Jun 25, 2014 at 12:55:01AM +0200, Bruno Prémont wrote:
> >>> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
> >>> Matthew Garrett introduced a efifb vga_default_device() so that EFI
> >>> systems that do not load shadow VBIOS or setup VGA get proper value for
> >>> boot_vga PCI sysfs attribute on the corresponding PCI device.
> >>>
> >>> Xorg is refusing to detect devices when boot_vga=0 which is the case on
> >>> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
> >>> the dri device but then bails out with "no devices detected".
> >>>
> >>> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
> >>> reflects its state. When unset this attribute is 1 whenever
> >>> IORESOURCE_ROM_SHADOW flag is set.
> >>>
> >>> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
> >>> while having native drivers for the GPU also makes selecting
> >>> sysfb/efifb optional.
> >>>
> >>> Remove the efifb implementation of vga_default_device() and initialize
> >>> vgaarb's vga_default_device() with the PCI GPU that matches boot
> >>> screen_info in pci_fixup_video().
> >>>
> >>> Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
> >>> Cc: Matthew Garrett <matthew.garrett@nebula.com>
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> >>
> >> I applied both with Matthew's ack to pci/misc for v3.17, thanks!
> >
> > I just tried to run the latest kernel. It failed to boot and git
> > bisect points to this commit (MacBookPro10,1 with Nvidia&Intel
> > graphics).
> >
> > The (now removed) code in efifb_setup did always set default_vga, even
> > if it had already been set earlier. The new code in pci_fixup_video
> > runs only if vga_default_device() is NULL. Removing the check fixes
> > the regression.
> >
> >
> > The following calls to vga_set_default_device are made during boot:
> >
> > vga_arbiter_add_pci_device -> vga_set_default_device(intel)
> > pci_fixup_video -> vga_set_default_device(intel) (there are two calls
> > in pci_fixup_video, this one is the one near "Boot video device")
> > pci_fixup_video -> vga_set_default_device(nvidia) (from the "Does
> > firmware framebuffer belong to us?" loop, only if I remove the check)
> >
> > vga_arbiter_add_pci_device chooses intel simply because it is the
> > first device. Next pci_fixup_video(intel) sees that it is the default
> > device, sets the IORESOURCE_ROM_SHADOW flag and calls
> > vga_set_default_device again. And finally (if the check is removed)
> > pci_fixup_video(nvidia) sees that it owns the framebuffer and sets
> > itself as the default device which allows the system to boot again.
> >
> > Does setting the ROM_SHADOW flag on (possibly) the wrong device have
> > any effect?
> Yes it does. Removing the line changes a long standing
> i915 0000:00:02.0: Invalid ROM contents
> into a
> i915 0000:00:02.0: BAR 6: can't assign [??? 0x00000000 flags
> 0x20000000] (bogus alignment).
>
> The first is logged at KERN_ERR and the second one only at KERN_INFO.
> We are making progress.
How does your system behave if you change vga_arbiter_add_pci_device()
not to set vga_set_default_device() with the first device registered?
That is remove the
#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
code block in vga_arbiter_add_pci_device().
How did your system behave in the past if you did not enable efifb?
Bruno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-10 9:26 ` Bruno Prémont
@ 2014-08-10 9:56 ` Andreas Noever
2014-08-10 16:34 ` Bruno Prémont
0 siblings, 1 reply; 38+ messages in thread
From: Andreas Noever @ 2014-08-10 9:56 UTC (permalink / raw)
To: Bruno Prémont
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett
On Sun, Aug 10, 2014 at 11:26 AM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> On Sun, 10 August 2014 Andreas Noever <andreas.noever@gmail.com> wrote:
>
>> On Sun, Aug 10, 2014 at 2:21 AM, Andreas Noever
>> <andreas.noever@gmail.com> wrote:
>> > On Sat, Jul 5, 2014 at 7:15 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> >> On Wed, Jun 25, 2014 at 12:55:01AM +0200, Bruno Prémont wrote:
>> >>> With commit b4aa0163056b ("efifb: Implement vga_default_device() (v2)")
>> >>> Matthew Garrett introduced a efifb vga_default_device() so that EFI
>> >>> systems that do not load shadow VBIOS or setup VGA get proper value for
>> >>> boot_vga PCI sysfs attribute on the corresponding PCI device.
>> >>>
>> >>> Xorg is refusing to detect devices when boot_vga=0 which is the case on
>> >>> some EFI system (e.g. MacBookAir2,1). Xorg detects the GPU and finds
>> >>> the dri device but then bails out with "no devices detected".
>> >>>
>> >>> Note: When vga_default_device() is set boot_vga PCI sysfs attribute
>> >>> reflects its state. When unset this attribute is 1 whenever
>> >>> IORESOURCE_ROM_SHADOW flag is set.
>> >>>
>> >>> With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete
>> >>> while having native drivers for the GPU also makes selecting
>> >>> sysfb/efifb optional.
>> >>>
>> >>> Remove the efifb implementation of vga_default_device() and initialize
>> >>> vgaarb's vga_default_device() with the PCI GPU that matches boot
>> >>> screen_info in pci_fixup_video().
>> >>>
>> >>> Tested-by: Anibal Francisco Martinez Cortina <linuxkid.zeuz@gmail.com>
>> >>> Cc: Matthew Garrett <matthew.garrett@nebula.com>
>> >>> Cc: stable@vger.kernel.org
>> >>> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
>> >>
>> >> I applied both with Matthew's ack to pci/misc for v3.17, thanks!
>> >
>> > I just tried to run the latest kernel. It failed to boot and git
>> > bisect points to this commit (MacBookPro10,1 with Nvidia&Intel
>> > graphics).
>> >
>> > The (now removed) code in efifb_setup did always set default_vga, even
>> > if it had already been set earlier. The new code in pci_fixup_video
>> > runs only if vga_default_device() is NULL. Removing the check fixes
>> > the regression.
>> >
>> >
>> > The following calls to vga_set_default_device are made during boot:
>> >
>> > vga_arbiter_add_pci_device -> vga_set_default_device(intel)
>> > pci_fixup_video -> vga_set_default_device(intel) (there are two calls
>> > in pci_fixup_video, this one is the one near "Boot video device")
>> > pci_fixup_video -> vga_set_default_device(nvidia) (from the "Does
>> > firmware framebuffer belong to us?" loop, only if I remove the check)
>> >
>> > vga_arbiter_add_pci_device chooses intel simply because it is the
>> > first device. Next pci_fixup_video(intel) sees that it is the default
>> > device, sets the IORESOURCE_ROM_SHADOW flag and calls
>> > vga_set_default_device again. And finally (if the check is removed)
>> > pci_fixup_video(nvidia) sees that it owns the framebuffer and sets
>> > itself as the default device which allows the system to boot again.
>> >
>> > Does setting the ROM_SHADOW flag on (possibly) the wrong device have
>> > any effect?
>> Yes it does. Removing the line changes a long standing
>> i915 0000:00:02.0: Invalid ROM contents
>> into a
>> i915 0000:00:02.0: BAR 6: can't assign [??? 0x00000000 flags
>> 0x20000000] (bogus alignment).
>>
>> The first is logged at KERN_ERR and the second one only at KERN_INFO.
>> We are making progress.
>
> How does your system behave if you change vga_arbiter_add_pci_device()
> not to set vga_set_default_device() with the first device registered?
>
> That is remove the
> #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> code block in vga_arbiter_add_pci_device().
The system does not boot. The Intel device is still set as the
default device in pci_fixup_video (near "Boot video device") and
prevents the nvidia device (which is initialized later) from becoming
the default one.
> How did your system behave in the past if you did not enable efifb?
I don't think that I ever did not enable efifb. It seems to have been
around for quite a while?
Andreas
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-10 9:56 ` Andreas Noever
@ 2014-08-10 16:34 ` Bruno Prémont
2014-08-14 0:40 ` Andreas Noever
0 siblings, 1 reply; 38+ messages in thread
From: Bruno Prémont @ 2014-08-10 16:34 UTC (permalink / raw)
To: Andreas Noever
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett
On Sun, 10 August 2014 Andreas Noever <andreas.noever@gmail.com> wrote:
> On Sun, Aug 10, 2014 at 11:26 AM, Bruno Prémont wrote:
> > On Sun, 10 August 2014 Andreas Noever wrote:
> >
> >> On Sun, Aug 10, 2014 at 2:21 AM, Andreas Noeverwrote:
> >> > I just tried to run the latest kernel. It failed to boot and git
> >> > bisect points to this commit (MacBookPro10,1 with Nvidia&Intel
> >> > graphics).
> >> >
> >> > The (now removed) code in efifb_setup did always set default_vga, even
> >> > if it had already been set earlier. The new code in pci_fixup_video
> >> > runs only if vga_default_device() is NULL. Removing the check fixes
> >> > the regression.
> >> >
> >> >
> >> > The following calls to vga_set_default_device are made during boot:
> >> >
> >> > vga_arbiter_add_pci_device -> vga_set_default_device(intel)
> >> > pci_fixup_video -> vga_set_default_device(intel) (there are two calls
> >> > in pci_fixup_video, this one is the one near "Boot video device")
> >> > pci_fixup_video -> vga_set_default_device(nvidia) (from the "Does
> >> > firmware framebuffer belong to us?" loop, only if I remove the check)
> >> >
> >> > vga_arbiter_add_pci_device chooses intel simply because it is the
> >> > first device. Next pci_fixup_video(intel) sees that it is the default
> >> > device, sets the IORESOURCE_ROM_SHADOW flag and calls
> >> > vga_set_default_device again. And finally (if the check is removed)
> >> > pci_fixup_video(nvidia) sees that it owns the framebuffer and sets
> >> > itself as the default device which allows the system to boot again.
> >> >
> >> > Does setting the ROM_SHADOW flag on (possibly) the wrong device have
> >> > any effect?
> >> Yes it does. Removing the line changes a long standing
> >> i915 0000:00:02.0: Invalid ROM contents
> >> into a
> >> i915 0000:00:02.0: BAR 6: can't assign [??? 0x00000000 flags
> >> 0x20000000] (bogus alignment).
> >>
> >> The first is logged at KERN_ERR and the second one only at KERN_INFO.
> >> We are making progress.
> >
> > How does your system behave if you change vga_arbiter_add_pci_device()
> > not to set vga_set_default_device() with the first device registered?
> >
> > That is remove the
> > #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> > code block in vga_arbiter_add_pci_device().
> The system does not boot. The Intel device is still set as the
> default device in pci_fixup_video (near "Boot video device") and
> prevents the nvidia device (which is initialized later) from becoming
> the default one.
>
> > How did your system behave in the past if you did not enable efifb?
> I don't think that I ever did not enable efifb. It seems to have been
> around for quite a while?
The question here is if you system just refuses to boot as well.
The aim of my patch is to make system work (properly) when efifb is not used
(why use efifb if built-in drm drivers handle the GPU(s)?)
If you decided to replace efifb with platform-framebuffer+simplefb/simpledrm
you would probably see the same issue as that would be no efifb as well.
The presence of efifb should not be mandatory for successfully booting
and running Xorg.
> Andreas
How does you system behave with below patch on top of Linus tree?
Bruno
---
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index c61ea57..15d0068 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -367,7 +367,7 @@ static void pci_fixup_video(struct pci_dev *pdev)
}
bus = bus->parent;
}
- if (!vga_default_device() || pdev == vga_default_device()) {
+ if (pdev == vga_default_device()) {
pci_read_config_word(pdev, PCI_COMMAND, &config);
if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index d2077f0..ac44d87 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -112,10 +112,8 @@ both:
return 1;
}
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
/* this is only used a cookie - it should not be dereferenced */
static struct pci_dev *vga_default;
-#endif
static void vga_arb_device_card_gone(struct pci_dev *pdev);
@@ -131,7 +129,6 @@ static struct vga_device *vgadev_find(struct pci_dev *pdev)
}
/* Returns the default VGA device (vgacon's babe) */
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
struct pci_dev *vga_default_device(void)
{
return vga_default;
@@ -147,7 +144,6 @@ void vga_set_default_device(struct pci_dev *pdev)
pci_dev_put(vga_default);
vga_default = pci_dev_get(pdev);
}
-#endif
static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
{
@@ -580,10 +576,10 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
bus = bus->parent;
}
+#if 0
/* Deal with VGA default device. Use first enabled one
* by default if arch doesn't have it's own hook
*/
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
if (vga_default == NULL &&
((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK))
vga_set_default_device(pdev);
@@ -621,10 +617,8 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
goto bail;
}
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
if (vga_default == pdev)
vga_set_default_device(NULL);
-#endif
if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
vga_decode_count--;
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index 2c02f3a..c37bd4d 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -182,7 +182,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
* vga_get()...
*/
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
#ifdef CONFIG_VGA_ARB
extern struct pci_dev *vga_default_device(void);
extern void vga_set_default_device(struct pci_dev *pdev);
@@ -190,7 +189,6 @@ extern void vga_set_default_device(struct pci_dev *pdev);
static inline struct pci_dev *vga_default_device(void) { return NULL; };
static inline void vga_set_default_device(struct pci_dev *pdev) { };
#endif
-#endif
/**
* vga_conflicts
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-10 16:34 ` Bruno Prémont
@ 2014-08-14 0:40 ` Andreas Noever
2014-08-16 17:21 ` [PATCH 0/2] " Bruno Prémont
0 siblings, 1 reply; 38+ messages in thread
From: Andreas Noever @ 2014-08-14 0:40 UTC (permalink / raw)
To: Bruno Prémont
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett
On Sun, Aug 10, 2014 at 6:34 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> On Sun, 10 August 2014 Andreas Noever <andreas.noever@gmail.com> wrote:
>> On Sun, Aug 10, 2014 at 11:26 AM, Bruno Prémont wrote:
>> > On Sun, 10 August 2014 Andreas Noever wrote:
>> >
>> >> On Sun, Aug 10, 2014 at 2:21 AM, Andreas Noeverwrote:
>> >> > I just tried to run the latest kernel. It failed to boot and git
>> >> > bisect points to this commit (MacBookPro10,1 with Nvidia&Intel
>> >> > graphics).
>> >> >
>> >> > The (now removed) code in efifb_setup did always set default_vga, even
>> >> > if it had already been set earlier. The new code in pci_fixup_video
>> >> > runs only if vga_default_device() is NULL. Removing the check fixes
>> >> > the regression.
>> >> >
>> >> >
>> >> > The following calls to vga_set_default_device are made during boot:
>> >> >
>> >> > vga_arbiter_add_pci_device -> vga_set_default_device(intel)
>> >> > pci_fixup_video -> vga_set_default_device(intel) (there are two calls
>> >> > in pci_fixup_video, this one is the one near "Boot video device")
>> >> > pci_fixup_video -> vga_set_default_device(nvidia) (from the "Does
>> >> > firmware framebuffer belong to us?" loop, only if I remove the check)
>> >> >
>> >> > vga_arbiter_add_pci_device chooses intel simply because it is the
>> >> > first device. Next pci_fixup_video(intel) sees that it is the default
>> >> > device, sets the IORESOURCE_ROM_SHADOW flag and calls
>> >> > vga_set_default_device again. And finally (if the check is removed)
>> >> > pci_fixup_video(nvidia) sees that it owns the framebuffer and sets
>> >> > itself as the default device which allows the system to boot again.
>> >> >
>> >> > Does setting the ROM_SHADOW flag on (possibly) the wrong device have
>> >> > any effect?
>> >> Yes it does. Removing the line changes a long standing
>> >> i915 0000:00:02.0: Invalid ROM contents
>> >> into a
>> >> i915 0000:00:02.0: BAR 6: can't assign [??? 0x00000000 flags
>> >> 0x20000000] (bogus alignment).
>> >>
>> >> The first is logged at KERN_ERR and the second one only at KERN_INFO.
>> >> We are making progress.
>> >
>> > How does your system behave if you change vga_arbiter_add_pci_device()
>> > not to set vga_set_default_device() with the first device registered?
>> >
>> > That is remove the
>> > #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
>> > code block in vga_arbiter_add_pci_device().
>> The system does not boot. The Intel device is still set as the
>> default device in pci_fixup_video (near "Boot video device") and
>> prevents the nvidia device (which is initialized later) from becoming
>> the default one.
>>
>> > How did your system behave in the past if you did not enable efifb?
>> I don't think that I ever did not enable efifb. It seems to have been
>> around for quite a while?
>
> The question here is if you system just refuses to boot as well.
I have just tested 3.16 without FB_EFI and it refuses to boot as well.
> The aim of my patch is to make system work (properly) when efifb is not used
> (why use efifb if built-in drm drivers handle the GPU(s)?)
>
> If you decided to replace efifb with platform-framebuffer+simplefb/simpledrm
> you would probably see the same issue as that would be no efifb as well.
>
> The presence of efifb should not be mandatory for successfully booting
> and running Xorg.
>
>> Andreas
>
> How does you system behave with below patch on top of Linus tree?
This patch fixes the problem (with and without FB_EFI).
Andreas
> Bruno
>
>
>
> ---
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index c61ea57..15d0068 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -367,7 +367,7 @@ static void pci_fixup_video(struct pci_dev *pdev)
> }
> bus = bus->parent;
> }
> - if (!vga_default_device() || pdev == vga_default_device()) {
> + if (pdev == vga_default_device()) {
> pci_read_config_word(pdev, PCI_COMMAND, &config);
> if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
> pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index d2077f0..ac44d87 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -112,10 +112,8 @@ both:
> return 1;
> }
>
> -#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> /* this is only used a cookie - it should not be dereferenced */
> static struct pci_dev *vga_default;
> -#endif
>
> static void vga_arb_device_card_gone(struct pci_dev *pdev);
>
> @@ -131,7 +129,6 @@ static struct vga_device *vgadev_find(struct pci_dev *pdev)
> }
>
> /* Returns the default VGA device (vgacon's babe) */
> -#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> struct pci_dev *vga_default_device(void)
> {
> return vga_default;
> @@ -147,7 +144,6 @@ void vga_set_default_device(struct pci_dev *pdev)
> pci_dev_put(vga_default);
> vga_default = pci_dev_get(pdev);
> }
> -#endif
>
> static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
> {
> @@ -580,10 +576,10 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
> bus = bus->parent;
> }
>
> +#if 0
> /* Deal with VGA default device. Use first enabled one
> * by default if arch doesn't have it's own hook
> */
> -#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> if (vga_default == NULL &&
> ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK))
> vga_set_default_device(pdev);
> @@ -621,10 +617,8 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
> goto bail;
> }
>
> -#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> if (vga_default == pdev)
> vga_set_default_device(NULL);
> -#endif
>
> if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
> vga_decode_count--;
> diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
> index 2c02f3a..c37bd4d 100644
> --- a/include/linux/vgaarb.h
> +++ b/include/linux/vgaarb.h
> @@ -182,7 +182,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
> * vga_get()...
> */
>
> -#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
> #ifdef CONFIG_VGA_ARB
> extern struct pci_dev *vga_default_device(void);
> extern void vga_set_default_device(struct pci_dev *pdev);
> @@ -190,7 +189,6 @@ extern void vga_set_default_device(struct pci_dev *pdev);
> static inline struct pci_dev *vga_default_device(void) { return NULL; };
> static inline void vga_set_default_device(struct pci_dev *pdev) { };
> #endif
> -#endif
>
> /**
> * vga_conflicts
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-14 0:40 ` Andreas Noever
@ 2014-08-16 17:21 ` Bruno Prémont
2014-08-16 17:25 ` [PATCH 1/2] vgaarb: Drop obsolete #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE Bruno Prémont
` (2 more replies)
0 siblings, 3 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-08-16 17:21 UTC (permalink / raw)
To: Andreas Noever, Bjorn Helgaas
Cc: DRI mailing list, Linux PCI, Dave Airlie, Matthew Garrett,
Greg Kroah-Hartman
This series improves on commit 20cde694027e (x86, ia64: Move EFI_FB
vga_default_device() initialization to pci_vga_fixup()):
- cleanup remaining but always-true #ifndefs
- fix boot regression on dual-GPU Macs
Andreas, can you please test this series? It is a modification from
previous testing patch that should still work fine for you.
That testing patch would have been failing X startup on old BIOS systems
booted with vga=normal (or otherwise in VGA text mode).
Greg, in case you have scheduled above-mentioned commit for your next
stable iteration, please hold it back in the queue until this follow-up
has landed and can be included within the same stable update as alone
that patch regresses for Macs with dual-GPU and using efifb.
Bruno
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 1/2] vgaarb: Drop obsolete #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
2014-08-16 17:21 ` [PATCH 0/2] " Bruno Prémont
@ 2014-08-16 17:25 ` Bruno Prémont
2014-08-16 17:30 ` [PATCH 2/2] x86, ia64: Don't default to first video device Bruno Prémont
2014-08-19 15:45 ` [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Andreas Noever
2 siblings, 0 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-08-16 17:25 UTC (permalink / raw)
To: Andreas Noever
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett, Greg Kroah-Hartman
With commit 20cde694027e boot video device detection was moved from
efifb to x86 and ia64 pci/fixup.c.
Remove the left-over #ifndef check that will allways match since
the corresponding arch-specific define is gone with above patch.
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
CC: Matthew Garrett <matthew.garrett@nebula.com>
CC: stable@vger.kernel.org # v3.5+
---
May be applied to stable as cleanup for upstream commit
20cde694027e7477cc532833e38ab9fcaa83fb64 which is marked for
stable.
drivers/gpu/vga/vgaarb.c | 8 --------
include/linux/vgaarb.h | 2 --
2 files changed, 10 deletions(-)
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index d2077f0..257674d 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -112,10 +112,8 @@ both:
return 1;
}
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
/* this is only used a cookie - it should not be dereferenced */
static struct pci_dev *vga_default;
-#endif
static void vga_arb_device_card_gone(struct pci_dev *pdev);
@@ -131,7 +129,6 @@ static struct vga_device *vgadev_find(struct pci_dev *pdev)
}
/* Returns the default VGA device (vgacon's babe) */
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
struct pci_dev *vga_default_device(void)
{
return vga_default;
@@ -147,7 +144,6 @@ void vga_set_default_device(struct pci_dev *pdev)
pci_dev_put(vga_default);
vga_default = pci_dev_get(pdev);
}
-#endif
static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
{
@@ -583,11 +579,9 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
/* Deal with VGA default device. Use first enabled one
* by default if arch doesn't have it's own hook
*/
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
if (vga_default == NULL &&
((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK))
vga_set_default_device(pdev);
-#endif
vga_arbiter_check_bridge_sharing(vgadev);
@@ -621,10 +615,8 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
goto bail;
}
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
if (vga_default == pdev)
vga_set_default_device(NULL);
-#endif
if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
vga_decode_count--;
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index 2c02f3a..c37bd4d 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -182,7 +182,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
* vga_get()...
*/
-#ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE
#ifdef CONFIG_VGA_ARB
extern struct pci_dev *vga_default_device(void);
extern void vga_set_default_device(struct pci_dev *pdev);
@@ -190,7 +189,6 @@ extern void vga_set_default_device(struct pci_dev *pdev);
static inline struct pci_dev *vga_default_device(void) { return NULL; };
static inline void vga_set_default_device(struct pci_dev *pdev) { };
#endif
-#endif
/**
* vga_conflicts
--
1.8.5.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 2/2] x86, ia64: Don't default to first video device
2014-08-16 17:21 ` [PATCH 0/2] " Bruno Prémont
2014-08-16 17:25 ` [PATCH 1/2] vgaarb: Drop obsolete #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE Bruno Prémont
@ 2014-08-16 17:30 ` Bruno Prémont
2014-08-19 15:45 ` [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Andreas Noever
2 siblings, 0 replies; 38+ messages in thread
From: Bruno Prémont @ 2014-08-16 17:30 UTC (permalink / raw)
To: Andreas Noever
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett, Greg Kroah-Hartman
With commit 20cde694027e boot video device detection was moved from
efifb to x86 and ia64 pci/fixup.c.
For dual-GPU Apple computers above above change represents a regression
as code in efifb did forcefully override vga_default_device while the
merge did not (changed ordering of actions).
This stops setting default_vga_device when applying
IORESOURCE_ROM_SHADOW (only doing so for the detected boot GPU) and
updates logging of boot video device selection, in vgaarb which covers
VGA text-mode booting and first half of pci_fixup_video which covers
framebuffer mode (EFI, VESA).
By setting IORESOURCE_ROM_SHADOW only on effective boot GPU we also
corrects a longstanding complaint from intel driver as reported by
Andreas:
> Does setting the ROM_SHADOW flag on (possibly) the wrong device
> have any effect?
Yes it does. Removing the line changes a long standing
i915 0000:00:02.0: Invalid ROM contents
into a
i915 0000:00:02.0: BAR 6: can't assign [??? 0x00000000 flags
0x20000000] (bogus alignment).
The first is logged at KERN_ERR while the second is at KERN_INFO.
Reported-By: Andreas Noever <andreas.noever@gmail.com>
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
CC: Matthew Garrett <matthew.garrett@nebula.com>
CC: stable@vger.kernel.org # v3.5+
---
Must be applied to stable when upstream commit
20cde694027e7477cc532833e38ab9fcaa83fb64, which is marked for
stable, gets applied.
Can be applied without patch 1/2 from this series though dropped
#ifndefs will cause this patch not to apply cleanly.
arch/ia64/pci/fixup.c | 9 +++++----
arch/x86/pci/fixup.c | 9 +++++----
drivers/gpu/vga/vgaarb.c | 4 +++-
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index ec73b2c..05198f8 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -54,8 +54,10 @@ static void pci_fixup_video(struct pci_dev *pdev)
continue;
if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
+ (screen_info.lfb_base + screen_info.lfb_size) < end) {
+ dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
vga_set_default_device(pdev);
+ }
}
}
@@ -79,12 +81,11 @@ static void pci_fixup_video(struct pci_dev *pdev)
}
bus = bus->parent;
}
- if (!vga_default_device() || pdev == vga_default_device()) {
+ if (pdev == vga_default_device()) {
pci_read_config_word(pdev, PCI_COMMAND, &config);
if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
+ dev_printk(KERN_DEBUG, &pdev->dev, "Video device with shadowed ROM\n");
}
}
}
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index c61ea57..5b392d2 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -342,8 +342,10 @@ static void pci_fixup_video(struct pci_dev *pdev)
continue;
if (screen_info.lfb_base >= start &&
- (screen_info.lfb_base + screen_info.lfb_size) < end)
+ (screen_info.lfb_base + screen_info.lfb_size) < end) {
+ dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
vga_set_default_device(pdev);
+ }
}
}
@@ -367,12 +369,11 @@ static void pci_fixup_video(struct pci_dev *pdev)
}
bus = bus->parent;
}
- if (!vga_default_device() || pdev == vga_default_device()) {
+ if (pdev == vga_default_device()) {
pci_read_config_word(pdev, PCI_COMMAND, &config);
if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
- dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
- vga_set_default_device(pdev);
+ dev_printk(KERN_DEBUG, &pdev->dev, "Video device with shadowed ROM\n");
}
}
}
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 257674d..c6eeed5 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -580,8 +580,10 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
* by default if arch doesn't have it's own hook
*/
if (vga_default == NULL &&
- ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK))
+ ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK)) {
+ pr_info("vgaarb: Boot video device: PCI:%s\n", pci_name(pdev));
vga_set_default_device(pdev);
+ }
vga_arbiter_check_bridge_sharing(vgadev);
--
1.8.5.5
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-16 17:21 ` [PATCH 0/2] " Bruno Prémont
2014-08-16 17:25 ` [PATCH 1/2] vgaarb: Drop obsolete #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE Bruno Prémont
2014-08-16 17:30 ` [PATCH 2/2] x86, ia64: Don't default to first video device Bruno Prémont
@ 2014-08-19 15:45 ` Andreas Noever
2014-08-20 5:55 ` Bruno Prémont
2 siblings, 1 reply; 38+ messages in thread
From: Andreas Noever @ 2014-08-19 15:45 UTC (permalink / raw)
To: Bruno Prémont
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett, Greg Kroah-Hartman
On Sat, Aug 16, 2014 at 7:21 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> This series improves on commit 20cde694027e (x86, ia64: Move EFI_FB
> vga_default_device() initialization to pci_vga_fixup()):
> - cleanup remaining but always-true #ifndefs
> - fix boot regression on dual-GPU Macs
>
> Andreas, can you please test this series? It is a modification from
> previous testing patch that should still work fine for you.
> That testing patch would have been failing X startup on old BIOS systems
> booted with vga=normal (or otherwise in VGA text mode).
>
>
> Greg, in case you have scheduled above-mentioned commit for your next
> stable iteration, please hold it back in the queue until this follow-up
> has landed and can be included within the same stable update as alone
> that patch regresses for Macs with dual-GPU and using efifb.
>
> Bruno
Fails again (with and without efifb).
The vga_set_default_device in vga_arbiter_add_pci_device is at fault.
It sets the boot video device to intel. Removing it makes the system
bootable again.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-19 15:45 ` [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Andreas Noever
@ 2014-08-20 5:55 ` Bruno Prémont
2014-08-20 7:11 ` Bruno Prémont
0 siblings, 1 reply; 38+ messages in thread
From: Bruno Prémont @ 2014-08-20 5:55 UTC (permalink / raw)
To: Andreas Noever
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett, Greg Kroah-Hartman
On Tue, 19 Aug 2014 17:45:00 +0200 Andreas Noever wrote:
> On Sat, Aug 16, 2014 at 7:21 PM, Bruno Prémont wrote:
> > This series improves on commit 20cde694027e (x86, ia64: Move EFI_FB
> > vga_default_device() initialization to pci_vga_fixup()):
> > - cleanup remaining but always-true #ifndefs
> > - fix boot regression on dual-GPU Macs
> >
> > Andreas, can you please test this series? It is a modification from
> > previous testing patch that should still work fine for you.
> > That testing patch would have been failing X startup on old BIOS systems
> > booted with vga=normal (or otherwise in VGA text mode).
> >
> >
> > Greg, in case you have scheduled above-mentioned commit for your next
> > stable iteration, please hold it back in the queue until this follow-up
> > has landed and can be included within the same stable update as alone
> > that patch regresses for Macs with dual-GPU and using efifb.
> >
> > Bruno
>
> Fails again (with and without efifb).
>
> The vga_set_default_device in vga_arbiter_add_pci_device is at fault.
> It sets the boot video device to intel. Removing it makes the system
> bootable again.
Could you provide your whole kernel log? I would like to understand
how your vga devices are setup and why it starts the wrong way.
If you can grab kernel log from both working and failing setups it
would be even better. The failing one is interesting for where exactly it
starts failing at boot.
Bruno
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup()
2014-08-20 5:55 ` Bruno Prémont
@ 2014-08-20 7:11 ` Bruno Prémont
[not found] ` <CAMxnaaUodONkqmdPWedN-Q1qheHiJAScFcG_XbX1--ZmiOQZDg@mail.gmail.com>
0 siblings, 1 reply; 38+ messages in thread
From: Bruno Prémont @ 2014-08-20 7:11 UTC (permalink / raw)
To: Andreas Noever
Cc: Bjorn Helgaas, DRI mailing list, Linux PCI, Dave Airlie,
Matthew Garrett, Greg Kroah-Hartman
On Wed, 20 Aug 2014 07:55:08 +0200 Bruno Prémont wrote:
> On Tue, 19 Aug 2014 17:45:00 +0200 Andreas Noever wrote:
> > On Sat, Aug 16, 2014 at 7:21 PM, Bruno Prémont wrote:
> > > This series improves on commit 20cde694027e (x86, ia64: Move EFI_FB
> > > vga_default_device() initialization to pci_vga_fixup()):
> > > - cleanup remaining but always-true #ifndefs
> > > - fix boot regression on dual-GPU Macs
> > >
> > > Andreas, can you please test this series? It is a modification from
> > > previous testing patch that should still work fine for you.
> > > That testing patch would have been failing X startup on old BIOS systems
> > > booted with vga=normal (or otherwise in VGA text mode).
> > >
> > >
> > > Greg, in case you have scheduled above-mentioned commit for your next
> > > stable iteration, please hold it back in the queue until this follow-up
> > > has landed and can be included within the same stable update as alone
> > > that patch regresses for Macs with dual-GPU and using efifb.
> > >
> > > Bruno
> >
> > Fails again (with and without efifb).
> >
> > The vga_set_default_device in vga_arbiter_add_pci_device is at fault.
> > It sets the boot video device to intel. Removing it makes the system
> > bootable again.
>
> Could you provide your whole kernel log? I would like to understand
> how your vga devices are setup and why it starts the wrong way.
>
> If you can grab kernel log from both working and failing setups it
> would be even better. The failing one is interesting for where exactly it
> starts failing at boot.
While collecting debug logs, please apply following patch to get
PCI command and bridge control registers as configured when vgaarb looks
at them.
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index af02597..8c8e7af 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -554,6 +554,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
* clear that below if the bridge isn't forwarding
*/
pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+ pr_info("vgaarb: PCI:%s PCI_COMMAND=%04x\n", pci_name(pdev), (unsigned int)cmd);
if (cmd & PCI_COMMAND_IO)
vgadev->owns |= VGA_RSRC_LEGACY_IO;
if (cmd & PCI_COMMAND_MEMORY)
@@ -567,6 +568,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
u16 l;
pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
&l);
+ pr_info("vgaarb: PCI:%s, bridge PCI:%s PCI_BRIDGE_CONTROL=%04x\n", pci_name(pdev), pci_name(bridge), (unsigned int)l);
if (!(l & PCI_BRIDGE_CTL_VGA)) {
vgadev->owns = 0;
break;
^ permalink raw reply related [flat|nested] 38+ messages in thread
end of thread, other threads:[~2014-09-19 5:18 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-14 20:43 [Patch] x86, ia64, efifb: Move boot_vga fixup from pci to vgaarb Bruno Prémont
2014-05-27 23:42 ` Bjorn Helgaas
2014-06-02 18:16 ` Bruno Prémont
2014-06-02 18:19 ` [PATCH 2/2] x86, ia64: Merge common vga fixup code Bruno Prémont
2014-06-02 18:19 ` [PATCH 1/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Bruno Prémont
2014-06-17 22:35 ` Bjorn Helgaas
2014-06-18 6:09 ` Bruno Prémont
2014-06-24 20:41 ` Bruno Prémont
2014-06-24 22:55 ` [PATCH 1/2 v2] " Bruno Prémont
2014-06-24 22:58 ` [PATCH 2/2 v2] x86, ia64: Merge common vga fixup code Bruno Prémont
2014-06-24 23:02 ` [PATCH 1/2 v2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Matthew Garrett
2014-07-05 17:15 ` Bjorn Helgaas
2014-08-10 0:21 ` Andreas Noever
2014-08-10 0:36 ` Andreas Noever
2014-08-10 9:26 ` Bruno Prémont
2014-08-10 9:56 ` Andreas Noever
2014-08-10 16:34 ` Bruno Prémont
2014-08-14 0:40 ` Andreas Noever
2014-08-16 17:21 ` [PATCH 0/2] " Bruno Prémont
2014-08-16 17:25 ` [PATCH 1/2] vgaarb: Drop obsolete #ifndef __ARCH_HAS_VGA_DEFAULT_DEVICE Bruno Prémont
2014-08-16 17:30 ` [PATCH 2/2] x86, ia64: Don't default to first video device Bruno Prémont
2014-08-19 15:45 ` [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Andreas Noever
2014-08-20 5:55 ` Bruno Prémont
2014-08-20 7:11 ` Bruno Prémont
[not found] ` <CAMxnaaUodONkqmdPWedN-Q1qheHiJAScFcG_XbX1--ZmiOQZDg@mail.gmail.com>
2014-08-21 21:34 ` Bruno Prémont
2014-08-22 4:39 ` Bjorn Helgaas
2014-08-22 6:23 ` Bruno Prémont
[not found] ` <CAMxnaaU9EiMcne-aQjS1sY1Orn6xGbVHEnd057ogcZ77p74Y=Q@mail.gmail.com>
2014-08-23 11:06 ` Bruno Prémont
2014-08-24 21:09 ` [PATCH 1/2 v2] vgaarb: Don't default exclusively to first video device with mem+io Bruno Prémont
2014-08-26 15:32 ` Andreas Noever
2014-08-28 20:47 ` Bruno Prémont
2014-09-12 11:19 ` Bruno Prémont
2014-09-12 14:28 ` Bjorn Helgaas
2014-09-18 23:26 ` Dave Airlie
2014-09-19 5:18 ` Bjorn Helgaas
2014-08-24 21:13 ` [PATCH 2/2 v2] vgaarb: Drop obsolete #ifndef Bruno Prémont
2014-08-25 12:16 ` [PATCH 0/2] x86, ia64: Move EFI_FB vga_default_device() initialization to pci_vga_fixup() Daniel Vetter
2014-08-25 12:39 ` Bruno Prémont
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).