PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Subsystems (3)
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) that are not spec-compliant: the VGA Enable bit is hardwired to 0 and writes do not change it.
The HiSilicon engineers report that the bridge does not forward the 0xa0000-0xbffff mem range and the 0x3b0-0x3bb and 0x3c0-0x3df I/O ranges.
Because the VGA Enable bit is hardwired to 0, the VGA arbiter refuses to mark any card behind it as the boot device. This breaks Xorg auto-detection.
However, the hibmc VGA card (PCI ID 19e5:1711) has been tested and is known to work when behind these bridges. (It does not require the legacy resources to operate.)
Provide a quirk so that this combination of bridge and card is eligible to be the default VGA card. This fixes Xorg auto-detection on the D05.
---
v3: fix commit message
v4: fix comment (forgot to git add/git commit, sorry for the noise)
--- drivers/pci/quirks.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
drivers/pci/quirks.c +50 -0
@@ -25,6 +25,7 @@#include <linux/sched.h> #include <linux/ktime.h> #include <linux/mm.h> +#include <linux/vgaarb.h> #include <asm/dma.h> /* isa_dma_bridge_buggy */ #include "pci.h"
@@ -4664,3 +4665,52 @@ static void quirk_intel_no_flr(struct pci_dev *dev) }DECLARE_PCI_FIXUP_EARLY (PCI_VENDOR_ID_INTEL ,0x1502 , quirk_intel_no_flr);DECLARE_PCI_FIXUP_EARLY (PCI_VENDOR_ID_INTEL ,0x1503 , quirk_intel_no_flr); + +/* +* The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) +* that are not spec-compliant: the VGA Enable bit is hardwired to 0 +* and writes do not change it. The bridge does not forward legacy +* memory or I/O resources. +* +* Because the VGA Enable bit is hardwired to 0, the VGA arbiter +* refuses to mark any card behind it as the boot device. However, the +* hibmc VGA card (PCI ID 19e5:1711) has been tested and is known to +* work when behind these bridges. +* +* If we have this bridge, this card, and no default card already, +* mark the card as default. +*/ +static void hibmc_fixup_vgaarb (struct pci_dev *pdev) +{ +struct pci_dev *bridge; +struct pci_bus *bus; +u16 config; + + bus = pdev->bus; + bridge = bus->self; +if (!bridge) +return ; + +if (!pci_is_bridge (bridge)) +return ; + +if (bridge->vendor !=PCI_VENDOR_ID_HUAWEI || + bridge->device !=0x1610 ) +return ; + +pci_read_config_word (bridge,PCI_BRIDGE_CONTROL , + &config); +if (config &PCI_BRIDGE_CTL_VGA ) { +/* +* Weirdly, this bridge *is* spec compliant, so bail +* and let vgaarb do its job +*/ +return ; + } + +if (vga_default_device ()) +return ; + +vga_set_default_device (pdev); +} +DECLARE_PCI_FIXUP_FINAL (PCI_VENDOR_ID_HUAWEI ,0x1711 , hibmc_fixup_vgaarb);
Signature · 1 line
2.11.0
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
[+cc Catalin, Will, linux-arm-kernel]
Quoted 86 lines · Daniel Axtens
The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) that are not spec-compliant: the VGA Enable bit is hardwired to 0 and writes do not change it.
The HiSilicon engineers report that the bridge does not forward the 0xa0000-0xbffff mem range and the 0x3b0-0x3bb and 0x3c0-0x3df I/O ranges.
Because the VGA Enable bit is hardwired to 0, the VGA arbiter refuses to mark any card behind it as the boot device. This breaks Xorg auto-detection.
However, the hibmc VGA card (PCI ID 19e5:1711) has been tested and is known to work when behind these bridges. (It does not require the legacy resources to operate.)
Provide a quirk so that this combination of bridge and card is eligible to be the default VGA card. This fixes Xorg auto-detection on the D05.
Cc Xinliang Liu <[email protected]>Cc Rongrong Zou <[email protected]>Signed-off-by Daniel Axtens <[email protected]>---
v3: fix commit message
v4: fix comment (forgot to git add/git commit, sorry for the noise)--- drivers/pci/quirks.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)drivers/pci/quirks.c +40
-0@@ -25,6 +25,7 @@#include <linux/sched.h> #include <linux/ktime.h> #include <linux/mm.h> +#include <linux/vgaarb.h> #include <asm/dma.h> /* isa_dma_bridge_buggy */ #include "pci.h" @@ -4664,3 +4665,52 @@ static void quirk_intel_no_flr(struct pci_dev *dev) }DECLARE_PCI_FIXUP_EARLY (PCI_VENDOR_ID_INTEL ,0x1502 , quirk_intel_no_flr);DECLARE_PCI_FIXUP_EARLY (PCI_VENDOR_ID_INTEL ,0x1503 , quirk_intel_no_flr); + +/* +* The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) +* that are not spec-compliant: the VGA Enable bit is hardwired to 0 +* and writes do not change it. The bridge does not forward legacy +* memory or I/O resources. +* +* Because the VGA Enable bit is hardwired to 0, the VGA arbiter +* refuses to mark any card behind it as the boot device. However, the +* hibmc VGA card (PCI ID 19e5:1711) has been tested and is known to +* work when behind these bridges. +* +* If we have this bridge, this card, and no default card already, +* mark the card as default. +*/ +static void hibmc_fixup_vgaarb (struct pci_dev *pdev) +{ +struct pci_dev *bridge; +struct pci_bus *bus; +u16 config; + + bus = pdev->bus; + bridge = bus->self; +if (!bridge) +return ; + +if (!pci_is_bridge (bridge)) +return ; + +if (bridge->vendor !=PCI_VENDOR_ID_HUAWEI || + bridge->device !=0x1610 ) +return ; + +pci_read_config_word (bridge,PCI_BRIDGE_CONTROL , + &config); +if (config &PCI_BRIDGE_CTL_VGA ) { +/* +* Weirdly, this bridge *is* spec compliant, so bail +* and let vgaarb do its job
drivers/pci/quirks.c static void quirk_intel_no_flr(struct pci_dev *dev) · line 4716
Quoted 10 lines
drivers/pci/quirks.c +10
-0+*/ +return ; + } + +if (vga_default_device ()) +return ; + +vga_set_default_device (pdev); +} +DECLARE_PCI_FIXUP_FINAL (PCI_VENDOR_ID_HUAWEI ,0x1711 , hibmc_fixup_vgaarb);
Is this quirk useful on any arch other than arm64? Per drivers/pci/dwc/Kconfig, CONFIG_PCI_HISI depends on CONFIG_ARM64.
Would it make sense to put this quirk in arch/arm64/kernel/pci.c?
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Hi Bjorn, Daniel
[...]
Quoted 5 lines
Is this quirk useful on any arch other than arm64? Per
drivers/pci/dwc/Kconfig,CONFIG_PCI_HISIdepends onCONFIG_ARM64.Would it make sense to put this quirk in
arch/arm64/kernel/pci.c?
Indeed our host controller depends on ARM64 so maybe it would make sense to move the quirk arch/arm64/kernel/pci.c; however regardless why is it strictly required for a VGA device to be legacy one in order to make it the default boot device?
i.e. couldn't we have:
drivers/gpu/vga/vgaarb.c +1 -2
@@ -667,8 +667,7 @@ 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 */ -if (vga_default ==NULL && - ((vgadev->owns &VGA_RSRC_LEGACY_MASK ) ==VGA_RSRC_LEGACY_MASK )) { +if (vga_default ==NULL ) {vgaarb_info (&pdev->dev,"setting as boot VGA device\n" );vga_set_default_device (pdev); }
?
Thanks
Gab
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
[+cc Ben, David, Daniel, Alex]
Quoted 21 lines · Gabriele Paoloni
Hi Bjorn, Daniel
[...]
Quoted 5 lines
Is this quirk useful on any arch other than arm64? Per
drivers/pci/dwc/Kconfig,CONFIG_PCI_HISIdepends onCONFIG_ARM64.Would it make sense to put this quirk in
arch/arm64/kernel/pci.c?Indeed our host controller depends on ARM64 so maybe it would make sense to move the quirk
arch/arm64/kernel/pci.c; however regardless why is it strictly required for a VGA device to be legacy one in order to make it the default boot device?
i.e. couldn't we have:drivers/gpu/vga/vgaarb.c +0
-0
drivers/gpu/vga/vgaarb.c static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) · line 673
Quoted 9 lines
drivers/gpu/vga/vgaarb.c +1
-2@@ -667,8 +667,7 @@ 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 */ -if (vga_default ==NULL && - ((vgadev->owns &VGA_RSRC_LEGACY_MASK ) ==VGA_RSRC_LEGACY_MASK )) { +if (vga_default ==NULL ) {vgaarb_info (&pdev->dev,"setting as boot VGA device\n" );vga_set_default_device (pdev); }
I don't know enough about the VGA arbiter to answer this. This test was part of the initial implementation: deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux") by Ben.
Bjorn
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Quoted 16 lines · Bjorn Helgaas
Quoted 16 lines
Indeed our host controller depends on ARM64 so maybe it would make sense to move the quirk
arch/arm64/kernel/pci.c; however regardless why is it strictly required for a VGA device to be legacy one in order to make it the default boot device?
i.e. couldn't we have:drivers/gpu/vga/vgaarb.c +0
-0@@ -667,8 +667,7 @@ 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
??????? */
-?????if (vga_default== NULL &&
-???????? ((vgadev->owns&VGA_RSRC_LEGACY_MASK) ==VGA_RSRC_LEGACY_MASK)) {
Quoted 8 lines
Quoted 4 lines
+?????if (
vga_default== NULL) { ???????????????vgaarb_info(&pdev->dev, "setting as boot VGA device\n"); ???????????????vga_set_default_device(pdev);
???????}I don't know enough about the VGA arbiter to answer this.? This test was part of the initial implementation:
deb2d2ecd43d("PCI/GPU: implement VGA arbitration on Linux") by Ben.
The above simply uses the first device that has memory and IO enabled as the default device (you don't need to have a default device).
This is essentially picking up whatever device had been initialized by the BIOS/firmware as default. This is needed for example on x86 where the BIOS tends to only initialize one device.
I'm not sure what problem you are trying to solve here ?
Cheers,
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Hi Ben
Quoted 43 lines
-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh at kernel.crashing.org]
Sent: 13 July 2017 21:45
To: Bjorn Helgaas; Gabriele Paoloni
Cc: Daniel Axtens; linux-pci at vger.kernel.org; Liuxinliang (Matthew Liu); Rongrong Zou; Catalin Marinas; Will Deacon; linux-arm- kernel at lists.infradead.org; David Airlie; Daniel Vetter; Alex Williamson
Subject: Re: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridgeQuoted 3 lines · Bjorn Helgaas
Quoted 3 lines
Indeed our host controller depends on ARM64 so maybe it would make sense to move the quirk
arch/arm64/kernel/pci.c; however regardless why is it strictly required for a VGA device to be legacy one inorder
Quoted 8 lines
drivers/gpu/vga/vgaarb.c static bool vga_arbiter_add_pci_device(struct
Quoted 8 lines
to make it the default boot device?
i.e. couldn't we have:drivers/gpu/vga/vgaarb.c +0
-0@@ -667,8 +667,7 @@ static bool vga_arbiter_add_pci_device(struct
pci_dev*pdev)Quoted 5 lines
Quoted 5 lines
???????/* Deal with VGA default device. Use first enabled one ??????? * 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)) {Quoted 2 lines
Quoted 2 lines
+?????if (
vga_default== NULL) { ???????????????vgaarb_info(&pdev->dev, "setting as boot VGAdevice\n");
Quoted 4 lines
Quoted 2 lines
???????????????vga_set_default_device(pdev);
???????}I don't know enough about the VGA arbiter to answer this.? This test
was
Quoted 1 line
part of the initial implementation:
deb2d2ecd43d("PCI/GPU: implementVGA
Quoted 1 line
arbitration on Linux") by Ben.
Quoted 8 lines
The above simply uses the first device that has memory and IO enabled as the default device (you don't need to have a default device).
This is essentially picking up whatever device had been initialized by the BIOS/firmware as default. This is needed for example on x86 where the BIOS tends to only initialize one device.
I'm not sure what problem you are trying to solve here ?
Well our host platform does not support legacy devices and therefore we find ourselves without a default VGA device...
I was trying to understand why a default device has to be legacy...but I think this was answered by both you and Alex in other follows-up...
Thanks
Gab
Quoted 3 lines
Cheers,
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
In reply to Bjorn HelgaasQuoted 19 lines · On Thu, 13 Jul 2017 06:29:38 -0500 Bjorn Helgaas <[email protected]> wrote:
[+cc Ben, David, Daniel, Alex]
Quoted 16 lines · Gabriele Paoloni
Hi Bjorn, Daniel
[...]
Quoted 5 lines
Is this quirk useful on any arch other than arm64? Per
drivers/pci/dwc/Kconfig,CONFIG_PCI_HISIdepends onCONFIG_ARM64.Would it make sense to put this quirk in
arch/arm64/kernel/pci.c?Indeed our host controller depends on ARM64 so maybe it would make sense to move the quirk
arch/arm64/kernel/pci.c; however regardless why is it strictly required for a VGA device to be legacy one in order to make it the default boot device?
i.e. couldn't we have:
Quoted 14 lines
Quoted 5 lines
drivers/gpu/vga/vgaarb.c +0
-0drivers/gpu/vga/vgaarb.c static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) · line 673
Quoted 9 lines
drivers/gpu/vga/vgaarb.c +1
-2@@ -667,8 +667,7 @@ 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 */ -if (vga_default ==NULL && - ((vgadev->owns &VGA_RSRC_LEGACY_MASK ) ==VGA_RSRC_LEGACY_MASK )) { +if (vga_default ==NULL ) {vgaarb_info (&pdev->dev,"setting as boot VGA device\n" );vga_set_default_device (pdev); }
"Legacy" is the breadcrumb we use to try to pick the same device for default VGA as the system firmware used as the boot VGA. There can be multiple VGA devices in the system, the change above would simply make the first discovered device be the default VGA. That would break many, many systems. If legacy VGA ranges mean nothing on ARM64, then follow the powerpc lead and make an arch quirk that simply selects the first enabled VGA device as the default. VGA routing is part of the PCI spec though, so the default of selecting the device with routing enabled makes sense. Thanks,
Alex
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Quoted 9 lines · Alex Williamson
Quoted 7 lines
Quoted 7 lines
????? */
-???if (vga_default== NULL &&
-?????? ((vgadev->owns&VGA_RSRC_LEGACY_MASK) ==VGA_RSRC_LEGACY_MASK)) { +???if (vga_default== NULL) { ?????????????vgaarb_info(&pdev->dev, "setting as boot VGA device\n"); ?????????????vga_set_default_device(pdev);
?????}?
Quoted 9 lines
"Legacy" is the breadcrumb we use to try to pick the same device for default VGA as the system firmware used as the boot VGA.? There can be multiple VGA devices in the system, the change above would simply make the first discovered device be the default VGA.? That would break many, many systems.? If legacy VGA ranges mean nothing on ARM64, then follow the powerpc lead and make an arch quirk that simply selects the first enabled VGA device as the default.? VGA routing is part of the PCI spec though, so the default of selecting the device with routing enabled makes sense.? Thanks,
"Legacy" there iirc also means that it has decoding enabled at boot. If you pick the first one you may pick a device that doesn't and hasn't been initialized by any driver (good old BIOS-initialized text mode).
Cheers,
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Hi Ben, Alex
Quoted 33 lines
-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh at kernel.crashing.org]
Sent: 13 July 2017 22:21
To: Alex Williamson; Bjorn Helgaas
Cc: Gabriele Paoloni; Daniel Axtens; linux-pci at vger.kernel.org; Liuxinliang (Matthew Liu); Rongrong Zou; Catalin Marinas; Will Deacon; linux-arm-kernel at lists.infradead.org; David Airlie; Daniel Vetter
Subject: Re: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridgeQuoted 3 lines · Alex Williamson
Quoted 3 lines
Quoted 3 lines
????? */
-???if (vga_default== NULL &&
-?????? ((vgadev->owns&VGA_RSRC_LEGACY_MASK) ==
VGA_RSRC_LEGACY_MASK)) {Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
+???if (
vga_default== NULL) { ?????????????vgaarb_info(&pdev->dev, "setting as boot VGAdevice\n");
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
?????????????vga_set_default_device(pdev);
?????}"Legacy" is the breadcrumb we use to try to pick the same device for default VGA as the system firmware used as the boot VGA.? There can
be
Quoted 1 line
multiple VGA devices in the system, the change above would simply
make
Quoted 1 line
the first discovered device be the default VGA.? That would break
many,
Quoted 1 line
many systems.? If legacy VGA ranges mean nothing on ARM64, then
follow
Quoted 2 lines
the powerpc lead and make an arch quirk that simply selects the first enabled VGA device as the default.? VGA routing is part of the PCI
Quoted 7 lines
spec
Quoted 2 lines
though, so the default of selecting the device with routing enabled makes sense.? Thanks,
"Legacy" there iirc also means that it has decoding enabled at boot. If you pick the first one you may pick a device that doesn't and hasn't been initialized by any driver (good old BIOS-initialized text mode).
Right so effectively if there is a legacy dev we should pick that up; however what about the following code:
drivers/gpu/vga/vgaarb.c +8 -0
@@ -1424,6 +1424,14 @@ static int __init vga_arb_device_init(void)list_for_each_entry (vgadev, &vga_list, list) {struct device *dev = &vgadev->pdev->dev; + +/* if no legacy device has been set as default VGA +* device, just pick up the first one in the list */ +if (vga_default ==NULL ) { +vgaarb_info (dev,"setting as boot VGA device\n" ); +vga_set_default_device (vgadev->pdev); + } +#if defined(CONFIG_X86 ) || defined(CONFIG_IA64 )/* * Override vga_arbiter_add_pci_device()'s I/O based detection
Above after we have filled the list of VGA devices by iterating over all PCI devices we check if no legacy one has been set as default VGA device
yet: therefore we set the first VGA device in the list as default one...
Do you think it would work?
Thanks
Gab
Quoted 3 lines
Cheers,
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Quoted 20 lines · Gabriele Paoloni
drivers/gpu/vga/vgaarb.c +0
-0@@ -1424,6 +1424,14 @@ static int __init vga_arb_device_init(void)?
????????list_for_each_entry(vgadev,&vga_list, list) { ????????????????struct device *dev =&vgadev->pdev->dev;drivers/gpu/vga/vgaarb.c +8
-0+ +???????????????/* if no legacy device has been set as default VGA +??????????????? * device, just pick up the first one in the list */ +???????????????if (vga_default == NULL) { +???????????????????????vgaarb_info(dev, "setting as boot VGA device\n"); +???????????????????????vga_set_default_device(vgadev->pdev); +???????????????} +?#if
defined(CONFIG_X86)||defined(CONFIG_IA64)????????????????/*
???????????????? * Override vga_arbiter_add_pci_device()'s I/O based detection
Quoted 5 lines
Above after we have filled the list of VGA devices by iterating over all PCI devices we check if no legacy one has been set as default VGA device
yet: therefore we set the first VGA device in the list as default one...Do you think it would work?
I honestly don't remember all of the details of the arbiter but just make sure that it won't think that device is enabled for things like VGA etc... if it's memory/IO decode weren't enabled.
I'd rather we have no default device until a driver actually picks up though, and then, if we still have no default, use the first driver to pick up.
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Hi Alex, Ben
Quoted 37 lines
-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh at kernel.crashing.org]
Sent: 14 July 2017 14:50
To: Gabriele Paoloni; Alex Williamson; Bjorn Helgaas
Cc: Daniel Axtens; linux-pci at vger.kernel.org; Liuxinliang (Matthew Liu); Rongrong Zou; Catalin Marinas; Will Deacon; linux-arm- kernel at lists.infradead.org; David Airlie; Daniel Vetter
Subject: Re: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridgedrivers/gpu/vga/vgaarb.c static int __init vga_arb_device_init(void) · line 1431
Quoted 13 lines · Gabriele Paoloni
drivers/gpu/vga/vgaarb.c +5
-0@@ -1424,6 +1424,14 @@ static int __init vga_arb_device_init(void) ????????list_for_each_entry(vgadev, &vga_list, list) { ????????????????struct device *dev = &vgadev->pdev->dev; + +???????????????/* if no legacy device has been set as default VGA +??????????????? * device, just pick up the first one in the list */ +???????????????if (vga_default == NULL) { +???????????????????????vgaarb_info(dev, "setting as boot VGAdevice\n");
Quoted 6 lines
drivers/gpu/vga/vgaarb.c +3
-0+???????????????????????vga_set_default_device(vgadev->pdev); +???????????????} + ?#if defined(CONFIG_X86) || defined(CONFIG_IA64)????????????????/*
???????????????? * Override vga_arbiter_add_pci_device()'s I/O baseddetection
Quoted 2 lines
Above after we have filled the list of VGA devices by iterating over
all
Quoted 1 line
PCI devices we check if no legacy one has been set as default VGA
device
Quoted 9 lines
Quoted 1 line
yet: therefore we set the first VGA device in the list as default
one...
Quoted 2 lines
Do you think it would work?
I honestly don't remember all of the details of the arbiter but just make sure that it won't think that device is enabled for things like VGA etc... if it's memory/IO decode weren't enabled.
I think the following one should make sure that MEM/IO response are enabled (also as suggested by Alex)
drivers/gpu/vga/vgaarb.c +15 -0
@@ -1424,6 +1424,21 @@ static int __init vga_arb_device_init(void)list_for_each_entry (vgadev, &vga_list, list) {struct device *dev = &vgadev->pdev->dev; + +/* if no legacy device has been set as default VGA device, +* justpick up the first one in the list capable of responding to +* mem and io requests*/ +if (vga_default ==NULL ) { +u16 cmd; + +pci_read_config_word (vgadev->pdev,PCI_COMMAND , &cmd); +if ((cmd & (PCI_COMMAND_IO |PCI_COMMAND_MEMORY )) || + !vga_default_device ()) { +vga_set_default_device (vgadev->pdev); +vgaarb_info (dev,"setting as boot VGA device\n" ); + } + } +#if defined(CONFIG_X86 ) || defined(CONFIG_IA64 )/* * Override vga_arbiter_add_pci_device()'s I/O based detection
Also the above one could allow us to get rid of fixup_vga in powerpc....
Quoted 3 lines
I'd rather we have no default device until a driver actually picks up though, and then, if we still have no default, use the first driver to pick up.
Well from my understanding the PCI host controller driver will enumerate all the devices in the PCI hierarchy and call pci_device_add() for each of them, that in turn will call device_add(), at this stage if there is a driver available for the device such driver will probe otherwise it will not.
Are you suggesting to add the code above in pci_device_add() after device_add() and after checking that a driver has been bound for such dev?
Thanks
Gab
Quoted 2 lines
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Quoted 11 lines · Gabriele Paoloni
Quoted 3 lines
I'd rather we have no default device until a driver actually picks up though, and then, if we still have no default, use the first driver to pick up.
Well from my understanding the PCI host controller driver will enumerate all the devices in the PCI hierarchy and call
pci_device_add()for each of them, that in turn will calldevice_add(), at this stage if there is a driver available for the device such driver will probe otherwise it will not.Are you suggesting to add the code above in
pci_device_add()afterdevice_add()and after checking that a driver has been bound for such dev?
I don't like us turning on MEM/IO decoding on a device that has potentially not been initialized by its driver.
Ben.
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
In reply to Gabriele PaoloniQuoted 66 lines · On Fri, 14 Jul 2017 12:26:05 +0000 Gabriele Paoloni…
Hi Ben, Alex
Quoted 33 lines
-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh at kernel.crashing.org]
Sent: 13 July 2017 22:21
To: Alex Williamson; Bjorn Helgaas
Cc: Gabriele Paoloni; Daniel Axtens; linux-pci at vger.kernel.org; Liuxinliang (Matthew Liu); Rongrong Zou; Catalin Marinas; Will Deacon; linux-arm-kernel at lists.infradead.org; David Airlie; Daniel Vetter
Subject: Re: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridgeQuoted 3 lines · Alex Williamson
Quoted 3 lines
Quoted 3 lines
????? */
-???if (vga_default== NULL &&
-?????? ((vgadev->owns&VGA_RSRC_LEGACY_MASK) ==
VGA_RSRC_LEGACY_MASK)) {Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
+???if (
vga_default== NULL) { ?????????????vgaarb_info(&pdev->dev, "setting as boot VGAdevice\n");
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
?????????????vga_set_default_device(pdev);
?????}"Legacy" is the breadcrumb we use to try to pick the same device for default VGA as the system firmware used as the boot VGA.? There can
be
Quoted 1 line
multiple VGA devices in the system, the change above would simply
make
Quoted 1 line
the first discovered device be the default VGA.? That would break
many,
Quoted 1 line
many systems.? If legacy VGA ranges mean nothing on ARM64, then
follow
Quoted 2 lines
the powerpc lead and make an arch quirk that simply selects the first enabled VGA device as the default.? VGA routing is part of the PCI
Quoted 7 lines
spec
Quoted 2 lines
though, so the default of selecting the device with routing enabled makes sense.? Thanks,
"Legacy" there iirc also means that it has decoding enabled at boot. If you pick the first one you may pick a device that doesn't and hasn't been initialized by any driver (good old BIOS-initialized text mode).
Right so effectively if there is a legacy dev we should pick that up; however what about the following code:
drivers/gpu/vga/vgaarb.c +8
-0@@ -1424,6 +1424,14 @@ static int __init vga_arb_device_init(void)list_for_each_entry (vgadev, &vga_list, list) {struct device *dev = &vgadev->pdev->dev; + +/* if no legacy device has been set as default VGA +* device, just pick up the first one in the list */ +if (vga_default ==NULL ) { +vgaarb_info (dev,"setting as boot VGA device\n" ); +vga_set_default_device (vgadev->pdev); + } +#if defined(CONFIG_X86 ) || defined(CONFIG_IA64 )/* * Override vga_arbiter_add_pci_device()'s I/O based detection
Quoted 5 lines
Above after we have filled the list of VGA devices by iterating over all PCI devices we check if no legacy one has been set as default VGA device
yet: therefore we set the first VGA device in the list as default one...Do you think it would work?
I'd suggest at least looking for an enabled device as fixup_vga() does, perhaps that would even be enough to remove that arch quirk. Thanks,
Alex
PATCH v4 1/1 PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
In reply to Bjorn HelgaasHi Bjorn,
Quoted 103 lines · Bjorn Helgaas
[+cc Catalin, Will, linux-arm-kernel]
Quoted 86 lines · Daniel Axtens
The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) that are not spec-compliant: the VGA Enable bit is hardwired to 0 and writes do not change it.
The HiSilicon engineers report that the bridge does not forward the 0xa0000-0xbffff mem range and the 0x3b0-0x3bb and 0x3c0-0x3df I/O ranges.
Because the VGA Enable bit is hardwired to 0, the VGA arbiter refuses to mark any card behind it as the boot device. This breaks Xorg auto-detection.
However, the hibmc VGA card (PCI ID 19e5:1711) has been tested and is known to work when behind these bridges. (It does not require the legacy resources to operate.)
Provide a quirk so that this combination of bridge and card is eligible to be the default VGA card. This fixes Xorg auto-detection on the D05.
Cc Xinliang Liu <[email protected]>Cc Rongrong Zou <[email protected]>Signed-off-by Daniel Axtens <[email protected]>---
v3: fix commit message
v4: fix comment (forgot to git add/git commit, sorry for the noise)--- drivers/pci/quirks.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)drivers/pci/quirks.c +40
-0@@ -25,6 +25,7 @@#include <linux/sched.h> #include <linux/ktime.h> #include <linux/mm.h> +#include <linux/vgaarb.h> #include <asm/dma.h> /* isa_dma_bridge_buggy */ #include "pci.h" @@ -4664,3 +4665,52 @@ static void quirk_intel_no_flr(struct pci_dev *dev) }DECLARE_PCI_FIXUP_EARLY (PCI_VENDOR_ID_INTEL ,0x1502 , quirk_intel_no_flr);DECLARE_PCI_FIXUP_EARLY (PCI_VENDOR_ID_INTEL ,0x1503 , quirk_intel_no_flr); + +/* +* The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) +* that are not spec-compliant: the VGA Enable bit is hardwired to 0 +* and writes do not change it. The bridge does not forward legacy +* memory or I/O resources. +* +* Because the VGA Enable bit is hardwired to 0, the VGA arbiter +* refuses to mark any card behind it as the boot device. However, the +* hibmc VGA card (PCI ID 19e5:1711) has been tested and is known to +* work when behind these bridges. +* +* If we have this bridge, this card, and no default card already, +* mark the card as default. +*/ +static void hibmc_fixup_vgaarb (struct pci_dev *pdev) +{ +struct pci_dev *bridge; +struct pci_bus *bus; +u16 config; + + bus = pdev->bus; + bridge = bus->self; +if (!bridge) +return ; + +if (!pci_is_bridge (bridge)) +return ; + +if (bridge->vendor !=PCI_VENDOR_ID_HUAWEI || + bridge->device !=0x1610 ) +return ; + +pci_read_config_word (bridge,PCI_BRIDGE_CONTROL , + &config); +if (config &PCI_BRIDGE_CTL_VGA ) { +/* +* Weirdly, this bridge *is* spec compliant, so bail +* and let vgaarb do its job drivers/pci/quirks.c static void quirk_intel_no_flr(struct pci_dev *dev) · line 4716
Quoted 10 lines
drivers/pci/quirks.c +10
-0+*/ +return ; + } + +if (vga_default_device ()) +return ; + +vga_set_default_device (pdev); +} +DECLARE_PCI_FIXUP_FINAL (PCI_VENDOR_ID_HUAWEI ,0x1711 , hibmc_fixup_vgaarb);Is this quirk useful on any arch other than arm64? Per
drivers/pci/dwc/Kconfig,CONFIG_PCI_HISIdepends onCONFIG_ARM64.
Quoted 1 line
Would it make sense to put this quirk in
arch/arm64/kernel/pci.c?
We've resisted adding PCI quirks in the arch directory so far, so I'd rather it lived in the PCI code if you don't have a strong objection to that. Hardware vendors have a tendency to reuse broken IP, so it's not beyond the realms of imagination that this quirk might be needed for another part some day (e.g. based on 32-bit ARM).
Will