| Message ID | 20140625005501.7ff7e982@neptune.home (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wed, 2014-06-25 at 00:55 +0200, Bruno Prémont wrote: > With introduction of sysfb/simplefb/simpledrm efifb is getting obsolete > while having native drivers for the GPU also makes selecting > sysfb/efifb optional. > Both look good to me. Acked-by: Matthew Garrett <matthew.garrett@nebula.com> -- Matthew Garrett <matthew.garrett@nebula.com>
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 > -- 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
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 -- 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
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. -- 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
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 -- 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
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 -- 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
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; }
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(-)