On Mon, 25 November 2013 Bjorn Helgaas wrote: > On Mon, Nov 25, 2013 at 12:54 PM, Bruno Prémont wrote: > > On a MacBookAir2,1, booting to Linux with EFI though having > > no efifb built-in Xorg refuses to start with "no devices detected" > > because for the only VGA device available (NVidia Geforce 9400M) > > the sysfs attribute boot_vga is zero (instead of expected 1). > > > > When CONFIG_FB_EFI is selected, efifb does provide its own > > vga_default_device() to report the PCI device matching global > > screen_info as determined during efifb setup. > > > > Otherwise there is just a dummy or VGA_ARB's vga_default_device() > > that does not provide the right information. > > Wouldn't it be cleaner to fix vga_default_device() so it returns the > correct thing even when CONFIG_FB_EFI=n? I would rather completely drop the vga_default_device() from efifb (CONFIG_FB_EFI) and just keep vga_default_device() for vga-arbitration/vga-switcheroo. The fact that there are currently *two* instances of that function doesn't make life easy for determining who is providing it and when. drivers/gpu/vga/vgaarb.c:135 drivers/video/efifb.c:88 include/linux/vgaarb.h:190 (dummy) > > On the other hand, boot_vga_show() falls back to poking PCI > > resources to flag a PCI device as boot_vga if vga_default_device() > > returned no PCI device (NULL). > > > > To complement this PCI resource poking, this patch copies the > > validation code used to determine which PCI device to report as > > default VGA device by efifb into boot_vga_show(). > > If we do have to use logic like this, I'd like it better if it were > factored into a single function called both here and from > efifb_setup(). As of above, I would preferably drop that part of code from efifb_setup() and have the logic called only in one place and each time the same way. Otherwise efifb versus x86_sysfb+simplefb or directly going to native driver (nvoueau/radeon/...) behave differently without reason. Bruno > > Signed-off-by: Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx> > > --- > > Would it make sense to kill the corresponding code from efifb > > as it covers only a single case? > > > > The other EFI capable system I have (AMD Ilano based, Gigabyte > > mainboard does report boot_vga=1, possibly through the resources > > poking and there Xorg starts properly without efifb built in. > > > > Selecting CONFIG_X86_SYSFB (combined with CONFIG_FB_SIMPLE) does > > not help by itself, patching that one instead of PCI's boot_vga > > attribute directly would still not cover the case when neither > > of them is enabled. > > > > > > drivers/pci/pci-sysfs.c | 21 +++++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c > > index 7128cfd..91cac71 100644 > > --- a/drivers/pci/pci-sysfs.c > > +++ b/drivers/pci/pci-sysfs.c > > @@ -28,6 +28,7 @@ > > #include <linux/pci-aspm.h> > > #include <linux/slab.h> > > #include <linux/vgaarb.h> > > +#include <linux/screen_info.h> > > #include <linux/pm_runtime.h> > > #include "pci.h" > > > > @@ -540,6 +541,26 @@ boot_vga_show(struct device *dev, struct device_attribute *attr, char *buf) > > if (vga_dev) > > return sprintf(buf, "%u\n", (pdev == vga_dev)); > > > > + if ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA) { > > + 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) > > + return sprintf(buf, "1\n"); > > + } > > + } > > + > > return sprintf(buf, "%u\n", > > !!(pdev->resource[PCI_ROM_RESOURCE].flags & > > IORESOURCE_ROM_SHADOW)); -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html