* [PATCH 0/2] vgaarb: Select fallback default VGA device
@ 2017-10-06 22:24 Bjorn Helgaas
2017-10-06 22:24 ` [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA Bjorn Helgaas
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2017-10-06 22:24 UTC (permalink / raw)
To: Daniel Vetter
Cc: Lorenzo Pieralisi, Benjamin Herrenschmidt, Gabriele Paoloni,
Ard Biesheuvel, linux-pci, Will Deacon, dri-devel, Xinliang Liu,
Alex Williamson, Lukas Wunner, Catalin Marinas, Rongrong Zou,
Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens
These patches are supposed to fix a problem Daniel Axtens found on the
HiSilicon D05 board. The VGA device there is behind a bridge that doesn't
support PCI_BRIDGE_CTL_VGA, so the arbiter never selects the device as the
default.
The first patch extends the arbiter so that if it can't find an enabled VGA
device with legacy resources, it selects the first enabled device *without*
legacy resources (this is what fixes the D05). If that fails, it selects
the first device that isn't enabled. The combination of both changes
should make the current powerpc fixup_vga() quirk unnecessary.
N.B. It changes the powerpc behavior: if there are several enabled VGA
devices, the current quirk selects the last one, while this patch selects
the first one. If this is a problem, I can drop that part of the patch and
keep the quirk.
The second patch pulls out this fallback device detection (and the EFI
override) from vga_arb_device_init() to make it easier to read.
---
Bjorn Helgaas (2):
vgaarb: Select a default VGA device even if there's no legacy VGA
vgaarb: Factor out EFI and fallback default device selection
arch/powerpc/kernel/pci-common.c | 12 ------
drivers/gpu/vga/vgaarb.c | 72 +++++++++++++++++++++++++++++---------
2 files changed, 55 insertions(+), 29 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA 2017-10-06 22:24 [PATCH 0/2] vgaarb: Select fallback default VGA device Bjorn Helgaas @ 2017-10-06 22:24 ` Bjorn Helgaas 2017-10-12 11:24 ` Julien Thierry 2017-10-06 22:24 ` [PATCH 2/2] vgaarb: Factor out EFI and fallback default device selection Bjorn Helgaas 2017-10-12 10:35 ` [PATCH 0/2] vgaarb: Select fallback default VGA device Sherlock Wang 2 siblings, 1 reply; 8+ messages in thread From: Bjorn Helgaas @ 2017-10-06 22:24 UTC (permalink / raw) To: Daniel Vetter Cc: Lorenzo Pieralisi, Gabriele Paoloni, Ard Biesheuvel, linux-pci, Catalin Marinas, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, Benjamin Herrenschmidt, Rongrong Zou, Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens From: Bjorn Helgaas <bhelgaas@google.com> Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter never selects it as the default, which means Xorg auto-detection doesn't work. VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g., [mem 0xa0000-0xbffff], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are not configurable by BARs. Consequently, multiple VGA devices can conflict with each other. The VGA arbiter avoids conflicts by ensuring that those legacy resources are only routed to one VGA device at a time. The arbiter identifies the "default VGA" device, i.e., a legacy VGA device that was used by boot firmware. It selects the first device that: - is of PCI_CLASS_DISPLAY_VGA, - has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and - has PCI_BRIDGE_CTL_VGA set in all upstream bridges. Some systems don't have such a device. For example, if a host bridge doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any devices below it. Or, as on the HiSilicon D05, the VGA device may be behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the legacy VGA resources will never reach the device. This patch extends the arbiter so that if it doesn't find a device that meets all the above criteria, it selects the first device that: - is of PCI_CLASS_DISPLAY_VGA and - has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled If it doesn't find even that, it selects the first device that: - is of class PCI_CLASS_DISPLAY_VGA. Such a device may not be able to use the legacy VGA resources, but most drivers can operate the device without those. Setting it as the default device means its "boot_vga" sysfs file will contain "1", which Xorg (via libpciaccess) uses to help select its default output device. This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in particular; see the link below). It also replaces the powerpc fixup_vga() quirk, albeit with slightly different semantics: the quirk selected the first VGA device we found, and overrode that selection with any enabled VGA device we found. If there were several enabled VGA devices, the *last* one we found would become the default. The code here instead selects the *first* enabled VGA device we find, and if none are enabled, the first VGA device we find. Link: http://lkml.kernel.org/r/20170901072744.2409-1-dja@axtens.net Tested-by: Daniel Axtens <dja@axtens.net> # arm64, ppc64-qemu-tcg Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- arch/powerpc/kernel/pci-common.c | 12 ------------ drivers/gpu/vga/vgaarb.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 02831a396419..0ac7aa346c69 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl); DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl); - -static void fixup_vga(struct pci_dev *pdev) -{ - u16 cmd; - - pci_read_config_word(pdev, PCI_COMMAND, &cmd); - if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device()) - vga_set_default_device(pdev); - -} -DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_DISPLAY_VGA, 8, fixup_vga); diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c index 76875f6299b8..aeb41f793ed4 100644 --- a/drivers/gpu/vga/vgaarb.c +++ b/drivers/gpu/vga/vgaarb.c @@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void) vgaarb_info(dev, "no bridge control possible\n"); } + if (!vga_default_device()) { + list_for_each_entry(vgadev, &vga_list, list) { + struct device *dev = &vgadev->pdev->dev; + u16 cmd; + + pdev = vgadev->pdev; + pci_read_config_word(pdev, PCI_COMMAND, &cmd); + if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { + vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); + vga_set_default_device(pdev); + break; + } + } + } + + if (!vga_default_device()) { + vgadev = list_first_entry_or_null(&vga_list, + struct vga_device, list); + if (vgadev) { + struct device *dev = &vgadev->pdev->dev; + vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); + vga_set_default_device(pdev); + } + } + pr_info("loaded\n"); return rc; } ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA 2017-10-06 22:24 ` [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA Bjorn Helgaas @ 2017-10-12 11:24 ` Julien Thierry 2017-10-12 12:05 ` Lothar Waßmann 2017-10-13 3:44 ` Bjorn Helgaas 0 siblings, 2 replies; 8+ messages in thread From: Julien Thierry @ 2017-10-12 11:24 UTC (permalink / raw) To: Bjorn Helgaas, Daniel Vetter Cc: Lorenzo Pieralisi, Benjamin Herrenschmidt, Gabriele Paoloni, Ard Biesheuvel, linux-pci, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, Catalin Marinas, Rongrong Zou, Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens Hi Bjorn, On 06/10/17 23:24, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is > behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter > never selects it as the default, which means Xorg auto-detection doesn't > work. > > VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g., > [mem 0xa0000-0xbffff], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are > not configurable by BARs. Consequently, multiple VGA devices can conflict > with each other. The VGA arbiter avoids conflicts by ensuring that those > legacy resources are only routed to one VGA device at a time. > > The arbiter identifies the "default VGA" device, i.e., a legacy VGA device > that was used by boot firmware. It selects the first device that: > > - is of PCI_CLASS_DISPLAY_VGA, > - has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and > - has PCI_BRIDGE_CTL_VGA set in all upstream bridges. > > Some systems don't have such a device. For example, if a host bridge > doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any > devices below it. Or, as on the HiSilicon D05, the VGA device may be > behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the > legacy VGA resources will never reach the device. > > This patch extends the arbiter so that if it doesn't find a device that > meets all the above criteria, it selects the first device that: > > - is of PCI_CLASS_DISPLAY_VGA and > - has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled > > If it doesn't find even that, it selects the first device that: > > - is of class PCI_CLASS_DISPLAY_VGA. > > Such a device may not be able to use the legacy VGA resources, but most > drivers can operate the device without those. Setting it as the default > device means its "boot_vga" sysfs file will contain "1", which Xorg (via > libpciaccess) uses to help select its default output device. > > This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in > particular; see the link below). > > It also replaces the powerpc fixup_vga() quirk, albeit with slightly > different semantics: the quirk selected the first VGA device we found, and > overrode that selection with any enabled VGA device we found. If there > were several enabled VGA devices, the *last* one we found would become the > default. > > The code here instead selects the *first* enabled VGA device we find, and > if none are enabled, the first VGA device we find. > > Link: http://lkml.kernel.org/r/20170901072744.2409-1-dja@axtens.net > Tested-by: Daniel Axtens <dja@axtens.net> # arm64, ppc64-qemu-tcg > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > --- > arch/powerpc/kernel/pci-common.c | 12 ------------ > drivers/gpu/vga/vgaarb.c | 25 +++++++++++++++++++++++++ > 2 files changed, 25 insertions(+), 12 deletions(-) > > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > index 02831a396419..0ac7aa346c69 100644 > --- a/arch/powerpc/kernel/pci-common.c > +++ b/arch/powerpc/kernel/pci-common.c > @@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev) > } > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl); > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl); > - > -static void fixup_vga(struct pci_dev *pdev) > -{ > - u16 cmd; > - > - pci_read_config_word(pdev, PCI_COMMAND, &cmd); > - if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device()) > - vga_set_default_device(pdev); > - > -} > -DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_DISPLAY_VGA, 8, fixup_vga); > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > index 76875f6299b8..aeb41f793ed4 100644 > --- a/drivers/gpu/vga/vgaarb.c > +++ b/drivers/gpu/vga/vgaarb.c > @@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void) > vgaarb_info(dev, "no bridge control possible\n"); > } > > + if (!vga_default_device()) { > + list_for_each_entry(vgadev, &vga_list, list) { > + struct device *dev = &vgadev->pdev->dev; > + u16 cmd; > + > + pdev = vgadev->pdev; > + pci_read_config_word(pdev, PCI_COMMAND, &cmd); > + if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { > + vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); > + vga_set_default_device(pdev); > + break; > + } > + } > + } > + > + if (!vga_default_device()) { > + vgadev = list_first_entry_or_null(&vga_list, > + struct vga_device, list); > + if (vgadev) { > + struct device *dev = &vgadev->pdev->dev; > + vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); > + vga_set_default_device(pdev); Isn't 'pdev' NULL here? shouldn't it be vgadev->pdev instead? Cheers, -- Julien Thierry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA 2017-10-12 11:24 ` Julien Thierry @ 2017-10-12 12:05 ` Lothar Waßmann 2017-10-12 12:56 ` Julien Thierry 2017-10-13 3:44 ` Bjorn Helgaas 1 sibling, 1 reply; 8+ messages in thread From: Lothar Waßmann @ 2017-10-12 12:05 UTC (permalink / raw) To: Julien Thierry Cc: Lorenzo Pieralisi, Dave Airlie, Gabriele Paoloni, Ard Biesheuvel, linux-pci, Catalin Marinas, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, Bjorn Helgaas, Benjamin Herrenschmidt, Rongrong Zou, Daniel Vetter, linuxppc-dev, linux-arm-kernel, Daniel Axtens SGksCgpPbiBUaHUsIDEyIE9jdCAyMDE3IDEyOjI0OjEwICswMTAwIEp1bGllbiBUaGllcnJ5IHdy b3RlOgo+IEhpIEJqb3JuLAo+IAo+IE9uIDA2LzEwLzE3IDIzOjI0LCBCam9ybiBIZWxnYWFzIHdy b3RlOgo+ID4gRnJvbTogQmpvcm4gSGVsZ2FhcyA8YmhlbGdhYXNAZ29vZ2xlLmNvbT4KPiA+IAo+ ID4gRGFuaWVsIEF4dGVucyByZXBvcnRlZCB0aGF0IG9uIHRoZSBIaVNpbGljb24gRDA1IGJvYXJk LCB0aGUgVkdBIGRldmljZSBpcwo+ID4gYmVoaW5kIGEgYnJpZGdlIHRoYXQgZG9lc24ndCBzdXBw b3J0IFBDSV9CUklER0VfQ1RMX1ZHQSwgc28gdGhlIFZHQSBhcmJpdGVyCj4gPiBuZXZlciBzZWxl Y3RzIGl0IGFzIHRoZSBkZWZhdWx0LCB3aGljaCBtZWFucyBYb3JnIGF1dG8tZGV0ZWN0aW9uIGRv ZXNuJ3QKPiA+IHdvcmsuCj4gPiAKPiA+IFZHQSBpcyBhIGxlZ2FjeSBQQ0kgZmVhdHVyZTogYSBW R0EgZGV2aWNlIGNhbiByZXNwb25kIHRvIGFkZHJlc3NlcywgZS5nLiwKPiA+IFttZW0gMHhhMDAw MC0weGJmZmZmXSwgW2lvIDB4M2IwLTB4M2JiXSwgW2lvIDB4M2MwLTB4M2RmXSwgZXRjLiwgdGhh dCBhcmUKPiA+IG5vdCBjb25maWd1cmFibGUgYnkgQkFScy4gIENvbnNlcXVlbnRseSwgbXVsdGlw bGUgVkdBIGRldmljZXMgY2FuIGNvbmZsaWN0Cj4gPiB3aXRoIGVhY2ggb3RoZXIuICBUaGUgVkdB IGFyYml0ZXIgYXZvaWRzIGNvbmZsaWN0cyBieSBlbnN1cmluZyB0aGF0IHRob3NlCj4gPiBsZWdh Y3kgcmVzb3VyY2VzIGFyZSBvbmx5IHJvdXRlZCB0byBvbmUgVkdBIGRldmljZSBhdCBhIHRpbWUu Cj4gPiAKPiA+IFRoZSBhcmJpdGVyIGlkZW50aWZpZXMgdGhlICJkZWZhdWx0IFZHQSIgZGV2aWNl LCBpLmUuLCBhIGxlZ2FjeSBWR0EgZGV2aWNlCj4gPiB0aGF0IHdhcyB1c2VkIGJ5IGJvb3QgZmly bXdhcmUuICBJdCBzZWxlY3RzIHRoZSBmaXJzdCBkZXZpY2UgdGhhdDoKPiA+IAo+ID4gICAgLSBp cyBvZiBQQ0lfQ0xBU1NfRElTUExBWV9WR0EsCj4gPiAgICAtIGhhcyBib3RoIFBDSV9DT01NQU5E X0lPIGFuZCBQQ0lfQ09NTUFORF9NRU1PUlkgZW5hYmxlZCwgYW5kCj4gPiAgICAtIGhhcyBQQ0lf QlJJREdFX0NUTF9WR0Egc2V0IGluIGFsbCB1cHN0cmVhbSBicmlkZ2VzLgo+ID4gCj4gPiBTb21l IHN5c3RlbXMgZG9uJ3QgaGF2ZSBzdWNoIGEgZGV2aWNlLiAgRm9yIGV4YW1wbGUsIGlmIGEgaG9z dCBicmlkZ2UKPiA+IGRvZXNuJ3Qgc3VwcG9ydCBJL08gc3BhY2UsIFBDSV9DT01NQU5EX0lPIHBy b2JhYmx5IHdvbid0IGJlIGVuYWJsZWQgZm9yIGFueQo+ID4gZGV2aWNlcyBiZWxvdyBpdC4gIE9y LCBhcyBvbiB0aGUgSGlTaWxpY29uIEQwNSwgdGhlIFZHQSBkZXZpY2UgbWF5IGJlCj4gPiBiZWhp bmQgYSBicmlkZ2UgdGhhdCBkb2Vzbid0IHN1cHBvcnQgUENJX0JSSURHRV9DVExfVkdBLCBzbyBh Y2Nlc3NlcyB0byB0aGUKPiA+IGxlZ2FjeSBWR0EgcmVzb3VyY2VzIHdpbGwgbmV2ZXIgcmVhY2gg dGhlIGRldmljZS4KPiA+IAo+ID4gVGhpcyBwYXRjaCBleHRlbmRzIHRoZSBhcmJpdGVyIHNvIHRo YXQgaWYgaXQgZG9lc24ndCBmaW5kIGEgZGV2aWNlIHRoYXQKPiA+IG1lZXRzIGFsbCB0aGUgYWJv dmUgY3JpdGVyaWEsIGl0IHNlbGVjdHMgdGhlIGZpcnN0IGRldmljZSB0aGF0Ogo+ID4gCj4gPiAg ICAtIGlzIG9mIFBDSV9DTEFTU19ESVNQTEFZX1ZHQSBhbmQKPiA+ICAgIC0gaGFzIFBDSV9DT01N QU5EX0lPIG9yIFBDSV9DT01NQU5EX01FTU9SWSBlbmFibGVkCj4gPiAKPiA+IElmIGl0IGRvZXNu J3QgZmluZCBldmVuIHRoYXQsIGl0IHNlbGVjdHMgdGhlIGZpcnN0IGRldmljZSB0aGF0Ogo+ID4g Cj4gPiAgICAtIGlzIG9mIGNsYXNzIFBDSV9DTEFTU19ESVNQTEFZX1ZHQS4KPiA+IAo+ID4gU3Vj aCBhIGRldmljZSBtYXkgbm90IGJlIGFibGUgdG8gdXNlIHRoZSBsZWdhY3kgVkdBIHJlc291cmNl cywgYnV0IG1vc3QKPiA+IGRyaXZlcnMgY2FuIG9wZXJhdGUgdGhlIGRldmljZSB3aXRob3V0IHRo b3NlLiAgU2V0dGluZyBpdCBhcyB0aGUgZGVmYXVsdAo+ID4gZGV2aWNlIG1lYW5zIGl0cyAiYm9v dF92Z2EiIHN5c2ZzIGZpbGUgd2lsbCBjb250YWluICIxIiwgd2hpY2ggWG9yZyAodmlhCj4gPiBs aWJwY2lhY2Nlc3MpIHVzZXMgdG8gaGVscCBzZWxlY3QgaXRzIGRlZmF1bHQgb3V0cHV0IGRldmlj ZS4KPiA+IAo+ID4gVGhpcyBmaXhlcyBYb3JnIGF1dG8tZGV0ZWN0aW9uIG9uIHNvbWUgYXJtNjQg c3lzdGVtcyAoSGlTaWxpY29uIEQwNSBpbgo+ID4gcGFydGljdWxhcjsgc2VlIHRoZSBsaW5rIGJl bG93KS4KPiA+IAo+ID4gSXQgYWxzbyByZXBsYWNlcyB0aGUgcG93ZXJwYyBmaXh1cF92Z2EoKSBx dWlyaywgYWxiZWl0IHdpdGggc2xpZ2h0bHkKPiA+IGRpZmZlcmVudCBzZW1hbnRpY3M6IHRoZSBx dWlyayBzZWxlY3RlZCB0aGUgZmlyc3QgVkdBIGRldmljZSB3ZSBmb3VuZCwgYW5kCj4gPiBvdmVy cm9kZSB0aGF0IHNlbGVjdGlvbiB3aXRoIGFueSBlbmFibGVkIFZHQSBkZXZpY2Ugd2UgZm91bmQu ICBJZiB0aGVyZQo+ID4gd2VyZSBzZXZlcmFsIGVuYWJsZWQgVkdBIGRldmljZXMsIHRoZSAqbGFz dCogb25lIHdlIGZvdW5kIHdvdWxkIGJlY29tZSB0aGUKPiA+IGRlZmF1bHQuCj4gPiAKPiA+IFRo ZSBjb2RlIGhlcmUgaW5zdGVhZCBzZWxlY3RzIHRoZSAqZmlyc3QqIGVuYWJsZWQgVkdBIGRldmlj ZSB3ZSBmaW5kLCBhbmQKPiA+IGlmIG5vbmUgYXJlIGVuYWJsZWQsIHRoZSBmaXJzdCBWR0EgZGV2 aWNlIHdlIGZpbmQuCj4gPiAKPiA+IExpbms6IGh0dHA6Ly9sa21sLmtlcm5lbC5vcmcvci8yMDE3 MDkwMTA3Mjc0NC4yNDA5LTEtZGphQGF4dGVucy5uZXQKPiA+IFRlc3RlZC1ieTogRGFuaWVsIEF4 dGVucyA8ZGphQGF4dGVucy5uZXQ+ICAgICMgYXJtNjQsIHBwYzY0LXFlbXUtdGNnCj4gPiBTaWdu ZWQtb2ZmLWJ5OiBCam9ybiBIZWxnYWFzIDxiaGVsZ2Fhc0Bnb29nbGUuY29tPgo+ID4gLS0tCj4g PiAgIGFyY2gvcG93ZXJwYy9rZXJuZWwvcGNpLWNvbW1vbi5jIHwgICAxMiAtLS0tLS0tLS0tLS0K PiA+ICAgZHJpdmVycy9ncHUvdmdhL3ZnYWFyYi5jICAgICAgICAgfCAgIDI1ICsrKysrKysrKysr KysrKysrKysrKysrKysKPiA+ICAgMiBmaWxlcyBjaGFuZ2VkLCAyNSBpbnNlcnRpb25zKCspLCAx MiBkZWxldGlvbnMoLSkKPiA+IAo+ID4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rZXJuZWwv cGNpLWNvbW1vbi5jIGIvYXJjaC9wb3dlcnBjL2tlcm5lbC9wY2ktY29tbW9uLmMKPiA+IGluZGV4 IDAyODMxYTM5NjQxOS4uMGFjN2FhMzQ2YzY5IDEwMDY0NAo+ID4gLS0tIGEvYXJjaC9wb3dlcnBj L2tlcm5lbC9wY2ktY29tbW9uLmMKPiA+ICsrKyBiL2FyY2gvcG93ZXJwYy9rZXJuZWwvcGNpLWNv bW1vbi5jCj4gPiBAQCAtMTc0MCwxNSArMTc0MCwzIEBAIHN0YXRpYyB2b2lkIGZpeHVwX2hpZGVf aG9zdF9yZXNvdXJjZV9mc2woc3RydWN0IHBjaV9kZXYgKmRldikKPiA+ICAgfQo+ID4gICBERUNM QVJFX1BDSV9GSVhVUF9IRUFERVIoUENJX1ZFTkRPUl9JRF9NT1RPUk9MQSwgUENJX0FOWV9JRCwg Zml4dXBfaGlkZV9ob3N0X3Jlc291cmNlX2ZzbCk7Cj4gPiAgIERFQ0xBUkVfUENJX0ZJWFVQX0hF QURFUihQQ0lfVkVORE9SX0lEX0ZSRUVTQ0FMRSwgUENJX0FOWV9JRCwgZml4dXBfaGlkZV9ob3N0 X3Jlc291cmNlX2ZzbCk7Cj4gPiAtCj4gPiAtc3RhdGljIHZvaWQgZml4dXBfdmdhKHN0cnVjdCBw Y2lfZGV2ICpwZGV2KQo+ID4gLXsKPiA+IC0JdTE2IGNtZDsKPiA+IC0KPiA+IC0JcGNpX3JlYWRf Y29uZmlnX3dvcmQocGRldiwgUENJX0NPTU1BTkQsICZjbWQpOwo+ID4gLQlpZiAoKGNtZCAmIChQ Q0lfQ09NTUFORF9JTyB8IFBDSV9DT01NQU5EX01FTU9SWSkpIHx8ICF2Z2FfZGVmYXVsdF9kZXZp Y2UoKSkKPiA+IC0JCXZnYV9zZXRfZGVmYXVsdF9kZXZpY2UocGRldik7Cj4gPiAtCj4gPiAtfQo+ ID4gLURFQ0xBUkVfUENJX0ZJWFVQX0NMQVNTX0ZJTkFMKFBDSV9BTllfSUQsIFBDSV9BTllfSUQs Cj4gPiAtCQkJICAgICAgUENJX0NMQVNTX0RJU1BMQVlfVkdBLCA4LCBmaXh1cF92Z2EpOwo+ID4g ZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L3ZnYS92Z2FhcmIuYyBiL2RyaXZlcnMvZ3B1L3ZnYS92 Z2FhcmIuYwo+ID4gaW5kZXggNzY4NzVmNjI5OWI4Li5hZWI0MWY3OTNlZDQgMTAwNjQ0Cj4gPiAt LS0gYS9kcml2ZXJzL2dwdS92Z2EvdmdhYXJiLmMKPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L3ZnYS92 Z2FhcmIuYwo+ID4gQEAgLTE0NjgsNiArMTQ2OCwzMSBAQCBzdGF0aWMgaW50IF9faW5pdCB2Z2Ff YXJiX2RldmljZV9pbml0KHZvaWQpCj4gPiAgIAkJCXZnYWFyYl9pbmZvKGRldiwgIm5vIGJyaWRn ZSBjb250cm9sIHBvc3NpYmxlXG4iKTsKPiA+ICAgCX0KPiA+ICAgCj4gPiArCWlmICghdmdhX2Rl ZmF1bHRfZGV2aWNlKCkpIHsKPiA+ICsJCWxpc3RfZm9yX2VhY2hfZW50cnkodmdhZGV2LCAmdmdh X2xpc3QsIGxpc3QpIHsKPiA+ICsJCQlzdHJ1Y3QgZGV2aWNlICpkZXYgPSAmdmdhZGV2LT5wZGV2 LT5kZXY7Cj4gPiArCQkJdTE2IGNtZDsKPiA+ICsKPiA+ICsJCQlwZGV2ID0gdmdhZGV2LT5wZGV2 Owo+ID4gKwkJCXBjaV9yZWFkX2NvbmZpZ193b3JkKHBkZXYsIFBDSV9DT01NQU5ELCAmY21kKTsK PiA+ICsJCQlpZiAoY21kICYgKFBDSV9DT01NQU5EX0lPIHwgUENJX0NPTU1BTkRfTUVNT1JZKSkg ewo+ID4gKwkJCQl2Z2FhcmJfaW5mbyhkZXYsICJzZXR0aW5nIGFzIGJvb3QgZGV2aWNlIChWR0Eg bGVnYWN5IHJlc291cmNlcyBub3QgYXZhaWxhYmxlKVxuIik7Cj4gPiArCQkJCXZnYV9zZXRfZGVm YXVsdF9kZXZpY2UocGRldik7Cj4gPiArCQkJCWJyZWFrOwo+ID4gKwkJCX0KPiA+ICsJCX0KPiA+ ICsJfQo+ID4gKwo+ID4gKwlpZiAoIXZnYV9kZWZhdWx0X2RldmljZSgpKSB7Cj4gPiArCQl2Z2Fk ZXYgPSBsaXN0X2ZpcnN0X2VudHJ5X29yX251bGwoJnZnYV9saXN0LAo+ID4gKwkJCQkJCSAgc3Ry dWN0IHZnYV9kZXZpY2UsIGxpc3QpOwo+ID4gKwkJaWYgKHZnYWRldikgewo+ID4gKwkJCXN0cnVj dCBkZXZpY2UgKmRldiA9ICZ2Z2FkZXYtPnBkZXYtPmRldjsKPiA+ICsJCQl2Z2FhcmJfaW5mbyhk ZXYsICJzZXR0aW5nIGFzIGJvb3QgZGV2aWNlIChWR0EgbGVnYWN5IHJlc291cmNlcyBub3QgYXZh aWxhYmxlKVxuIik7Cj4gPiArCQkJdmdhX3NldF9kZWZhdWx0X2RldmljZShwZGV2KTsKPiAKPiBJ c24ndCAncGRldicgTlVMTCBoZXJlPyBzaG91bGRuJ3QgaXQgYmUgdmdhZGV2LT5wZGV2IGluc3Rl YWQ/Cj4gClRoYXQgY2Fubm90IG5vdCBoYXBwZW4sIHRob3VnaCBpdCBpc24ndCBxdWl0ZSBvYnZp b3VzLgondmdhZGV2JyB3aWxsIG9ubHkgYmUgbm9uLU5VTEwsIHdoZW4gdGhlIHZnYV9saXN0IGlz bid0IGVtcHR5IGFuZCBpbgp0aGF0IGNhc2UgcGRldiBoYXMgYmVlbiBzZXQgdXAgaW4gdGhlIGxp c3RfZm9yX2VhY2hfZW50cnkoKSBsb29wIGFib3ZlLgoKCkxvdGhhciBXYcOfbWFubgoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5l bCBtYWlsaW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6 Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo= ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA 2017-10-12 12:05 ` Lothar Waßmann @ 2017-10-12 12:56 ` Julien Thierry 0 siblings, 0 replies; 8+ messages in thread From: Julien Thierry @ 2017-10-12 12:56 UTC (permalink / raw) To: Lothar Waßmann Cc: Bjorn Helgaas, Daniel Vetter, Lorenzo Pieralisi, Catalin Marinas, Gabriele Paoloni, Ard Biesheuvel, Benjamin Herrenschmidt, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, linux-pci, Rongrong Zou, Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens On 12/10/17 13:05, Lothar Waßmann wrote: > Hi, > > On Thu, 12 Oct 2017 12:24:10 +0100 Julien Thierry wrote: >> Hi Bjorn, >> >> On 06/10/17 23:24, Bjorn Helgaas wrote: >>> From: Bjorn Helgaas <bhelgaas@google.com> >>> >>> Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is >>> behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter >>> never selects it as the default, which means Xorg auto-detection doesn't >>> work. >>> >>> VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g., >>> [mem 0xa0000-0xbffff], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are >>> not configurable by BARs. Consequently, multiple VGA devices can conflict >>> with each other. The VGA arbiter avoids conflicts by ensuring that those >>> legacy resources are only routed to one VGA device at a time. >>> >>> The arbiter identifies the "default VGA" device, i.e., a legacy VGA device >>> that was used by boot firmware. It selects the first device that: >>> >>> - is of PCI_CLASS_DISPLAY_VGA, >>> - has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and >>> - has PCI_BRIDGE_CTL_VGA set in all upstream bridges. >>> >>> Some systems don't have such a device. For example, if a host bridge >>> doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any >>> devices below it. Or, as on the HiSilicon D05, the VGA device may be >>> behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the >>> legacy VGA resources will never reach the device. >>> >>> This patch extends the arbiter so that if it doesn't find a device that >>> meets all the above criteria, it selects the first device that: >>> >>> - is of PCI_CLASS_DISPLAY_VGA and >>> - has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled >>> >>> If it doesn't find even that, it selects the first device that: >>> >>> - is of class PCI_CLASS_DISPLAY_VGA. >>> >>> Such a device may not be able to use the legacy VGA resources, but most >>> drivers can operate the device without those. Setting it as the default >>> device means its "boot_vga" sysfs file will contain "1", which Xorg (via >>> libpciaccess) uses to help select its default output device. >>> >>> This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in >>> particular; see the link below). >>> >>> It also replaces the powerpc fixup_vga() quirk, albeit with slightly >>> different semantics: the quirk selected the first VGA device we found, and >>> overrode that selection with any enabled VGA device we found. If there >>> were several enabled VGA devices, the *last* one we found would become the >>> default. >>> >>> The code here instead selects the *first* enabled VGA device we find, and >>> if none are enabled, the first VGA device we find. >>> >>> Link: http://lkml.kernel.org/r/20170901072744.2409-1-dja@axtens.net >>> Tested-by: Daniel Axtens <dja@axtens.net> # arm64, ppc64-qemu-tcg >>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> >>> --- >>> arch/powerpc/kernel/pci-common.c | 12 ------------ >>> drivers/gpu/vga/vgaarb.c | 25 +++++++++++++++++++++++++ >>> 2 files changed, 25 insertions(+), 12 deletions(-) >>> >>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >>> index 02831a396419..0ac7aa346c69 100644 >>> --- a/arch/powerpc/kernel/pci-common.c >>> +++ b/arch/powerpc/kernel/pci-common.c >>> @@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev) >>> } >>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl); >>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl); >>> - >>> -static void fixup_vga(struct pci_dev *pdev) >>> -{ >>> - u16 cmd; >>> - >>> - pci_read_config_word(pdev, PCI_COMMAND, &cmd); >>> - if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device()) >>> - vga_set_default_device(pdev); >>> - >>> -} >>> -DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_DISPLAY_VGA, 8, fixup_vga); >>> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c >>> index 76875f6299b8..aeb41f793ed4 100644 >>> --- a/drivers/gpu/vga/vgaarb.c >>> +++ b/drivers/gpu/vga/vgaarb.c >>> @@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void) >>> vgaarb_info(dev, "no bridge control possible\n"); >>> } >>> >>> + if (!vga_default_device()) { >>> + list_for_each_entry(vgadev, &vga_list, list) { >>> + struct device *dev = &vgadev->pdev->dev; >>> + u16 cmd; >>> + >>> + pdev = vgadev->pdev; >>> + pci_read_config_word(pdev, PCI_COMMAND, &cmd); >>> + if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { >>> + vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); >>> + vga_set_default_device(pdev); >>> + break; >>> + } >>> + } >>> + } >>> + >>> + if (!vga_default_device()) { >>> + vgadev = list_first_entry_or_null(&vga_list, >>> + struct vga_device, list); >>> + if (vgadev) { >>> + struct device *dev = &vgadev->pdev->dev; >>> + vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); >>> + vga_set_default_device(pdev); >> >> Isn't 'pdev' NULL here? shouldn't it be vgadev->pdev instead? >> > That cannot not happen, though it isn't quite obvious. > 'vgadev' will only be non-NULL, when the vga_list isn't empty and in > that case pdev has been set up in the list_for_each_entry() loop above. > Indeed, it can't be NULL. However, in that case we would be using the pdev from the last vgadev in the list whereas this last case suggest we are picking the first vgadev in the list. So I don't understand why 'pdev' is being used rather than 'vgadev-pdev'. Thanks, -- Julien Thierry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA 2017-10-12 11:24 ` Julien Thierry 2017-10-12 12:05 ` Lothar Waßmann @ 2017-10-13 3:44 ` Bjorn Helgaas 1 sibling, 0 replies; 8+ messages in thread From: Bjorn Helgaas @ 2017-10-13 3:44 UTC (permalink / raw) To: Julien Thierry Cc: Daniel Vetter, Lorenzo Pieralisi, Benjamin Herrenschmidt, Gabriele Paoloni, Ard Biesheuvel, linux-pci, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, Catalin Marinas, Rongrong Zou, Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens On Thu, Oct 12, 2017 at 12:24:10PM +0100, Julien Thierry wrote: > Hi Bjorn, > > On 06/10/17 23:24, Bjorn Helgaas wrote: > >From: Bjorn Helgaas <bhelgaas@google.com> > > > >Daniel Axtens reported that on the HiSilicon D05 board, the VGA device is > >behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so the VGA arbiter > >never selects it as the default, which means Xorg auto-detection doesn't > >work. > > > >VGA is a legacy PCI feature: a VGA device can respond to addresses, e.g., > >[mem 0xa0000-0xbffff], [io 0x3b0-0x3bb], [io 0x3c0-0x3df], etc., that are > >not configurable by BARs. Consequently, multiple VGA devices can conflict > >with each other. The VGA arbiter avoids conflicts by ensuring that those > >legacy resources are only routed to one VGA device at a time. > > > >The arbiter identifies the "default VGA" device, i.e., a legacy VGA device > >that was used by boot firmware. It selects the first device that: > > > > - is of PCI_CLASS_DISPLAY_VGA, > > - has both PCI_COMMAND_IO and PCI_COMMAND_MEMORY enabled, and > > - has PCI_BRIDGE_CTL_VGA set in all upstream bridges. > > > >Some systems don't have such a device. For example, if a host bridge > >doesn't support I/O space, PCI_COMMAND_IO probably won't be enabled for any > >devices below it. Or, as on the HiSilicon D05, the VGA device may be > >behind a bridge that doesn't support PCI_BRIDGE_CTL_VGA, so accesses to the > >legacy VGA resources will never reach the device. > > > >This patch extends the arbiter so that if it doesn't find a device that > >meets all the above criteria, it selects the first device that: > > > > - is of PCI_CLASS_DISPLAY_VGA and > > - has PCI_COMMAND_IO or PCI_COMMAND_MEMORY enabled > > > >If it doesn't find even that, it selects the first device that: > > > > - is of class PCI_CLASS_DISPLAY_VGA. > > > >Such a device may not be able to use the legacy VGA resources, but most > >drivers can operate the device without those. Setting it as the default > >device means its "boot_vga" sysfs file will contain "1", which Xorg (via > >libpciaccess) uses to help select its default output device. > > > >This fixes Xorg auto-detection on some arm64 systems (HiSilicon D05 in > >particular; see the link below). > > > >It also replaces the powerpc fixup_vga() quirk, albeit with slightly > >different semantics: the quirk selected the first VGA device we found, and > >overrode that selection with any enabled VGA device we found. If there > >were several enabled VGA devices, the *last* one we found would become the > >default. > > > >The code here instead selects the *first* enabled VGA device we find, and > >if none are enabled, the first VGA device we find. > > > >Link: http://lkml.kernel.org/r/20170901072744.2409-1-dja@axtens.net > >Tested-by: Daniel Axtens <dja@axtens.net> # arm64, ppc64-qemu-tcg > >Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > >--- > > arch/powerpc/kernel/pci-common.c | 12 ------------ > > drivers/gpu/vga/vgaarb.c | 25 +++++++++++++++++++++++++ > > 2 files changed, 25 insertions(+), 12 deletions(-) > > > >diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c > >index 02831a396419..0ac7aa346c69 100644 > >--- a/arch/powerpc/kernel/pci-common.c > >+++ b/arch/powerpc/kernel/pci-common.c > >@@ -1740,15 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev) > > } > > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl); > > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl); > >- > >-static void fixup_vga(struct pci_dev *pdev) > >-{ > >- u16 cmd; > >- > >- pci_read_config_word(pdev, PCI_COMMAND, &cmd); > >- if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device()) > >- vga_set_default_device(pdev); > >- > >-} > >-DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID, > >- PCI_CLASS_DISPLAY_VGA, 8, fixup_vga); > >diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > >index 76875f6299b8..aeb41f793ed4 100644 > >--- a/drivers/gpu/vga/vgaarb.c > >+++ b/drivers/gpu/vga/vgaarb.c > >@@ -1468,6 +1468,31 @@ static int __init vga_arb_device_init(void) > > vgaarb_info(dev, "no bridge control possible\n"); > > } > >+ if (!vga_default_device()) { > >+ list_for_each_entry(vgadev, &vga_list, list) { > >+ struct device *dev = &vgadev->pdev->dev; > >+ u16 cmd; > >+ > >+ pdev = vgadev->pdev; > >+ pci_read_config_word(pdev, PCI_COMMAND, &cmd); > >+ if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { > >+ vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); > >+ vga_set_default_device(pdev); > >+ break; > >+ } > >+ } > >+ } > >+ > >+ if (!vga_default_device()) { > >+ vgadev = list_first_entry_or_null(&vga_list, > >+ struct vga_device, list); > >+ if (vgadev) { > >+ struct device *dev = &vgadev->pdev->dev; > >+ vgaarb_info(dev, "setting as boot device (VGA legacy resources not available)\n"); > >+ vga_set_default_device(pdev); > > Isn't 'pdev' NULL here? shouldn't it be vgadev->pdev instead? Yes, vgadev->pdev is what I intended, thanks a lot for pointing that out! Bjorn ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] vgaarb: Factor out EFI and fallback default device selection 2017-10-06 22:24 [PATCH 0/2] vgaarb: Select fallback default VGA device Bjorn Helgaas 2017-10-06 22:24 ` [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA Bjorn Helgaas @ 2017-10-06 22:24 ` Bjorn Helgaas 2017-10-12 10:35 ` [PATCH 0/2] vgaarb: Select fallback default VGA device Sherlock Wang 2 siblings, 0 replies; 8+ messages in thread From: Bjorn Helgaas @ 2017-10-06 22:24 UTC (permalink / raw) To: Daniel Vetter Cc: Lorenzo Pieralisi, Gabriele Paoloni, Ard Biesheuvel, linux-pci, Catalin Marinas, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, Benjamin Herrenschmidt, Rongrong Zou, Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens From: Bjorn Helgaas <bhelgaas@google.com> The default VGA device is normally set in vga_arbiter_add_pci_device() when we call it for the first enabled device that can be accessed with the legacy VGA resources ([mem 0xa0000-0xbffff], etc.) That default device can be overridden by an EFI device that owns the boot framebuffer. As a fallback, we can also select a VGA device that can't be accessed via legacy VGA resources, or a VGA device that isn't even enabled. Factor out this EFI and fallback selection from vga_arb_device_init() into a separate vga_arb_select_default_device() function. This doesn't change any behavior, but it untangles the "bridge control possible" checking and messages from the default device selection. Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- drivers/gpu/vga/vgaarb.c | 57 ++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c index aeb41f793ed4..7803e35a3702 100644 --- a/drivers/gpu/vga/vgaarb.c +++ b/drivers/gpu/vga/vgaarb.c @@ -1402,29 +1402,14 @@ static struct miscdevice vga_arb_device = { MISC_DYNAMIC_MINOR, "vga_arbiter", &vga_arb_device_fops }; -static int __init vga_arb_device_init(void) +static void __init vga_arb_select_default_device(void) { - int rc; struct pci_dev *pdev; struct vga_device *vgadev; - rc = misc_register(&vga_arb_device); - if (rc < 0) - pr_err("error %d registering device\n", rc); - - bus_register_notifier(&pci_bus_type, &pci_notifier); - - /* We add all pci devices satisfying vga class in the arbiter by - * default */ - pdev = NULL; - while ((pdev = - pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, - PCI_ANY_ID, pdev)) != NULL) - vga_arbiter_add_pci_device(pdev); - +#if defined(CONFIG_X86) || defined(CONFIG_IA64) list_for_each_entry(vgadev, &vga_list, list) { struct device *dev = &vgadev->pdev->dev; -#if defined(CONFIG_X86) || defined(CONFIG_IA64) /* * Override vga_arbiter_add_pci_device()'s I/O based detection * as it may take the wrong device (e.g. on Apple system under @@ -1461,12 +1446,8 @@ static int __init vga_arb_device_init(void) vgaarb_info(dev, "overriding boot device\n"); vga_set_default_device(vgadev->pdev); } -#endif - if (vgadev->bridge_has_one_vga) - vgaarb_info(dev, "bridge control possible\n"); - else - vgaarb_info(dev, "no bridge control possible\n"); } +#endif if (!vga_default_device()) { list_for_each_entry(vgadev, &vga_list, list) { @@ -1492,6 +1473,38 @@ static int __init vga_arb_device_init(void) vga_set_default_device(pdev); } } +} + +static int __init vga_arb_device_init(void) +{ + int rc; + struct pci_dev *pdev; + struct vga_device *vgadev; + + rc = misc_register(&vga_arb_device); + if (rc < 0) + pr_err("error %d registering device\n", rc); + + bus_register_notifier(&pci_bus_type, &pci_notifier); + + /* We add all PCI devices satisfying VGA class in the arbiter by + * default */ + pdev = NULL; + while ((pdev = + pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, + PCI_ANY_ID, pdev)) != NULL) + vga_arbiter_add_pci_device(pdev); + + list_for_each_entry(vgadev, &vga_list, list) { + struct device *dev = &vgadev->pdev->dev; + + if (vgadev->bridge_has_one_vga) + vgaarb_info(dev, "bridge control possible\n"); + else + vgaarb_info(dev, "no bridge control possible\n"); + } + + vga_arb_select_default_device(); pr_info("loaded\n"); return rc; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] vgaarb: Select fallback default VGA device 2017-10-06 22:24 [PATCH 0/2] vgaarb: Select fallback default VGA device Bjorn Helgaas 2017-10-06 22:24 ` [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA Bjorn Helgaas 2017-10-06 22:24 ` [PATCH 2/2] vgaarb: Factor out EFI and fallback default device selection Bjorn Helgaas @ 2017-10-12 10:35 ` Sherlock Wang 2 siblings, 0 replies; 8+ messages in thread From: Sherlock Wang @ 2017-10-12 10:35 UTC (permalink / raw) To: Bjorn Helgaas Cc: Daniel Vetter, Lorenzo Pieralisi, Gabriele Paoloni, Ard Biesheuvel, linux-pci, Catalin Marinas, Will Deacon, dri-devel, Xinliang Liu, Alex Williamson, Lukas Wunner, Benjamin Herrenschmidt, Rongrong Zou, Dave Airlie, linuxppc-dev, linux-arm-kernel, Daniel Axtens On Fri, Oct 06, 2017 at 05:24:20PM -0500, Bjorn Helgaas wrote: > These patches are supposed to fix a problem Daniel Axtens found on the > HiSilicon D05 board. The VGA device there is behind a bridge that doesn't > support PCI_BRIDGE_CTL_VGA, so the arbiter never selects the device as the > default. > > The first patch extends the arbiter so that if it can't find an enabled VGA > device with legacy resources, it selects the first enabled device *without* > legacy resources (this is what fixes the D05). If that fails, it selects > the first device that isn't enabled. The combination of both changes > should make the current powerpc fixup_vga() quirk unnecessary. > > N.B. It changes the powerpc behavior: if there are several enabled VGA > devices, the current quirk selects the last one, while this patch selects > the first one. If this is a problem, I can drop that part of the patch and > keep the quirk. > > The second patch pulls out this fallback device detection (and the EFI > override) from vga_arb_device_init() to make it easier to read. Hi Bjorn, I tested this series in: D05 board based on HiSilicon SoC Hip07. And HiSilicon SoC Hip08 based board. Both of these two SoCs' PCIe host bridge don't support PCI_BRIDGE_CTL_VGA. One VGA device which does not support legacy resources has been connected to these two PCIe host bridges to test. Default VGA device can be selected in above two systems. Tested-by: Zhou Wang <wangzhou1@hisilicon.com> (sorry for my HiSilicon E-mail box miss this serias, so here I have to use my gmail box) Thanks, Zhou > > --- > > Bjorn Helgaas (2): > vgaarb: Select a default VGA device even if there's no legacy VGA > vgaarb: Factor out EFI and fallback default device selection > > > arch/powerpc/kernel/pci-common.c | 12 ------ > drivers/gpu/vga/vgaarb.c | 72 +++++++++++++++++++++++++++++--------- > 2 files changed, 55 insertions(+), 29 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-13 3:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-06 22:24 [PATCH 0/2] vgaarb: Select fallback default VGA device Bjorn Helgaas 2017-10-06 22:24 ` [PATCH 1/2] vgaarb: Select a default VGA device even if there's no legacy VGA Bjorn Helgaas 2017-10-12 11:24 ` Julien Thierry 2017-10-12 12:05 ` Lothar Waßmann 2017-10-12 12:56 ` Julien Thierry 2017-10-13 3:44 ` Bjorn Helgaas 2017-10-06 22:24 ` [PATCH 2/2] vgaarb: Factor out EFI and fallback default device selection Bjorn Helgaas 2017-10-12 10:35 ` [PATCH 0/2] vgaarb: Select fallback default VGA device Sherlock Wang
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).