Re: [PATCH v2] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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
>> 



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux