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@xxxxxxxxxxxxx>
> Cc: Rongrong Zou <zourongrong@xxxxxxxxx>
> Signed-off-by: Daniel Axtens <dja@xxxxxxxxxx>
> ---
> 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
>
