| Message ID | 20170620013302.528-1-dja@axtens.net (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi Daniel, s/arbitrartion/arbitration/ below On Tue, Jun 20, 2017 at 11:33:02AM +1000, Daniel Axtens wrote: > The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) that > are not spec-compliant: the VGA Enable bit is set to 0 in hardware > and writes do not change it. > > This stops VGA arbitrartion from marking a VGA card behind the bridge > as a boot device, and therefore breaks Xorg auto-configuration. > > The hibmc VGA card (PCI ID 19e5:1711) is known to work when behind > these bridges. > > Provide a quirk so that this combination of bridge and card is eligible > to be the default VGA card. > > This fixes Xorg auto-detection. How exactly is this bridge broken? Apparently the PCI_BRIDGE_CTL_VGA is a read-only zero. Does the bridge then *always* forward the 0xa0000-0xbffff mem range and the 0x3b0-0x3bb and 0x3c0-0x3df I/O ranges? (This is from the PCI-to-PCI Bridge Spec, r1.2, sec 3.2.5.18). I don't know whether the hibmc VGA card requires those legacy VGA resources, so the fact that the card works doesn't answer the question -- the driver may be able to operate the card without those legacy resources. But if the system contains multiple VGA devices, we may not be able to arbitrate between them correctly if this bit doesn't work. For example, if the bridge always forwards the legacy VGA resources, and a second VGA devices behind a different bridge requires those resources, we'll have a problem: a read may receive two responses. The fact that this patch doesn't save any kind of "vga_broken" bit for later use by the VGA arbiter makes me suspect that we're making the device work in the current configuration, but things might break if we add another VGA card. > Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> > Cc: Rongrong Zou <zourongrong@gmail.com> > Signed-off-by: Daniel Axtens <dja@axtens.net> > --- > drivers/pci/quirks.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 16e6cd86ad71..86d7c848f3e2 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -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,48 @@ 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 hibmc card on a HiSilicon D05 board sits behind a non-compliant > + * bridge. The bridge has the PCI_BRIDGE_CTL_VGA config bit fixed at 0 > + * in hardware. This prevents the vgaarb from marking a card behind it > + * as boot VGA device. > + * > + * However, the hibmc card is known to still work, so if we have that > + * card behind that particular bridge (19e5:1610), mark it as the > + * default device if none has been detected. > + */ > +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); > -- > 2.11.0 >
Hi Bjorn, > How exactly is this bridge broken? Apparently the PCI_BRIDGE_CTL_VGA > is a read-only zero. Does the bridge then *always* forward the > 0xa0000-0xbffff mem range and the 0x3b0-0x3bb and 0x3c0-0x3df I/O > ranges? (This is from the PCI-to-PCI Bridge Spec, r1.2, sec > 3.2.5.18). Good question. I will forward this to the HiSilicon hardware engineers and get their input. This may take a couple of days as there's a language barrier. > I don't know whether the hibmc VGA card requires those legacy VGA > resources, so the fact that the card works doesn't answer the > question -- the driver may be able to operate the card without those > legacy resources. My understanding is that the hibmc card does not require the legacy VGA resources. > But if the system contains multiple VGA devices, we may not be able to > arbitrate between them correctly if this bit doesn't work. For > example, if the bridge always forwards the legacy VGA resources, and a > second VGA devices behind a different bridge requires those resources, > we'll have a problem: a read may receive two responses. I see. > The fact that this patch doesn't save any kind of "vga_broken" bit for > later use by the VGA arbiter makes me suspect that we're making the > device work in the current configuration, but things might break if we > add another VGA card. I'll have a think about that and get back to you when I hear back from HiSilicon. Regards, Daniel >> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> >> Cc: Rongrong Zou <zourongrong@gmail.com> >> Signed-off-by: Daniel Axtens <dja@axtens.net> >> --- >> drivers/pci/quirks.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index 16e6cd86ad71..86d7c848f3e2 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -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,48 @@ 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 hibmc card on a HiSilicon D05 board sits behind a non-compliant >> + * bridge. The bridge has the PCI_BRIDGE_CTL_VGA config bit fixed at 0 >> + * in hardware. This prevents the vgaarb from marking a card behind it >> + * as boot VGA device. >> + * >> + * However, the hibmc card is known to still work, so if we have that >> + * card behind that particular bridge (19e5:1610), mark it as the >> + * default device if none has been detected. >> + */ >> +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); >> -- >> 2.11.0 >>
Hi Bjorn, > How exactly is this bridge broken? Apparently the PCI_BRIDGE_CTL_VGA > is a read-only zero. Does the bridge then *always* forward the > 0xa0000-0xbffff mem range and the 0x3b0-0x3bb and 0x3c0-0x3df I/O > ranges? (This is from the PCI-to-PCI Bridge Spec, r1.2, sec > 3.2.5.18). So I asked the HiSilicon hardware folk, and they said "no". My understanding is that the resources are never forwarded. The justification I was given is that "this legacy mem range and IO are not used in ARM". (The D05 is an arm64 system.) > I don't know whether the hibmc VGA card requires those legacy VGA > resources, so the fact that the card works doesn't answer the > question -- the driver may be able to operate the card without those > legacy resources. I also asked the HiSilicon folk; and yes, the driver operates the card without the legacy resources. > But if the system contains multiple VGA devices, we may not be able to > arbitrate between them correctly if this bit doesn't work. For > example, if the bridge always forwards the legacy VGA resources, and a > second VGA devices behind a different bridge requires those resources, > we'll have a problem: a read may receive two responses. > > The fact that this patch doesn't save any kind of "vga_broken" bit for > later use by the VGA arbiter makes me suspect that we're making the > device work in the current configuration, but things might break if we > add another VGA card. All the PCI ports on the system I have access to are behind these bridges, so: - it looks like VGA cards will only work if their drivers support operating the card without legacy resources. - the vga default device should be set only when this quirk activates: normal VGA devices will fail the bridge capability test in vgaarb.c. If there are mutiple BMC cards then the check for an existing default device will ensure the default device is only set once. Having said that, I am happy to write a patch to disable the arbiter if you want - I'm just not sure quite how that would work. Regards, Daniel > >> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> >> Cc: Rongrong Zou <zourongrong@gmail.com> >> Signed-off-by: Daniel Axtens <dja@axtens.net> >> --- >> drivers/pci/quirks.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index 16e6cd86ad71..86d7c848f3e2 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -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,48 @@ 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 hibmc card on a HiSilicon D05 board sits behind a non-compliant >> + * bridge. The bridge has the PCI_BRIDGE_CTL_VGA config bit fixed at 0 >> + * in hardware. This prevents the vgaarb from marking a card behind it >> + * as boot VGA device. >> + * >> + * However, the hibmc card is known to still work, so if we have that >> + * card behind that particular bridge (19e5:1610), mark it as the >> + * default device if none has been detected. >> + */ >> +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); >> -- >> 2.11.0 >>
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 16e6cd86ad71..86d7c848f3e2 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -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,48 @@ 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 hibmc card on a HiSilicon D05 board sits behind a non-compliant + * bridge. The bridge has the PCI_BRIDGE_CTL_VGA config bit fixed at 0 + * in hardware. This prevents the vgaarb from marking a card behind it + * as boot VGA device. + * + * However, the hibmc card is known to still work, so if we have that + * card behind that particular bridge (19e5:1610), mark it as the + * default device if none has been detected. + */ +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);
The HiSilicon D05 board has some PCI bridges (PCI ID 19e5:1610) that are not spec-compliant: the VGA Enable bit is set to 0 in hardware and writes do not change it. This stops VGA arbitrartion from marking a VGA card behind the bridge as a boot device, and therefore breaks Xorg auto-configuration. The hibmc VGA card (PCI ID 19e5:1711) is known to work when behind these bridges. Provide a quirk so that this combination of bridge and card is eligible to be the default VGA card. This fixes Xorg auto-detection. Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> Cc: Rongrong Zou <zourongrong@gmail.com> Signed-off-by: Daniel Axtens <dja@axtens.net> --- drivers/pci/quirks.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)