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 at linux-vserver.org>
> > ---
> > 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));
