Allow non-legacy cards to be vgaarb default

Some powerpc devices provide a PCI display that isn't picked up by the VGA arbiter, presumably because it doesn't support the PCI legacy VGA ranges.

Commit c2e1d84523ad ("powerpc: Set default VGA device") introduced an arch quirk to mark these devices as default to fix X autoconfig.

The commit message stated that the patch:

Ensures a default VGA is always set if a graphics adapter is present, even if firmware did not initialize it. If more than one graphics adapter is present, ensure the one initialized by firmware is set as the default VGA device.

The patch used the following test to decide whether or not to mark a device as default:

  pci_read_config_word(pdev, PCI_COMMAND, &cmd);
  if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
          vga_set_default_device(pdev);

This doesn't seem like it works quite as intended. Because of the logical OR, the default device will be set in 2 cases:

1) if there is no default device
OR
2) if this device has normal memory/IO decoding turned on

This will work as intended if there is only one device, but if there are multiple devices, we may override the device the VGA arbiter picked.

Instead, set a device as default if there is no default device AND this device decodes.

This will not change behaviour on single-headed systems.

Cc Brian King <[email protected]>

Signed-off-by Daniel Axtens <[email protected]>

---

Tested in TCG (the card provided by qemu doesn't automatically register with vgaarb, so the relevant code path has been tested) but I would appreciate any tests on real hardware.

---
 arch/powerpc/kernel/pci-common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/powerpc/kernel/pci-common.c +4 -1

index 341a7469cab8..c95fdda3a2dc 100644
@@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
 {
 	u16 cmd;
 
+	if (vga_default_device())
+		return;
+
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
+	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
 		vga_set_default_device(pdev);
 
 }

Hi all,

Previously I posted a patch that provided a quirk for a hibmc card behind a particular Huawei bridge that allowed it to be marked as the default device in the VGA arbiter.[0] This lead to some discussion.[1] It was broadly suggested that a more generic solution would be better, something in the style of powerpc's fixup_vga() quirk.

Here is my suggested solution:

- Create a Kconfig option ARCH_WANT_VGA_ARB_FALLBACK

- if an arch selects that option, install PCI_FIXUP_CLASS_ENABLE hook. This hook fires when a card is enabled, which will require that a driver has been bound.

- if there is no default device when the hook fires, and the device can control memory and I/O, mark it as default.

The patches are as follows:

(1) powerpc: simplify and fix VGA default device behaviour

This cleans up some quirks in the powerpc implementation of the vga_fixup. It should make the behaviour match the original intention.

(2) vgaarb: allow non-legacy cards to be marked as default

Add the Kconfig option, and create the fixup in vgaarb.c gated behind the option. Nothing happens at this stage because no arch has selected the option yet.

(3) powerpc: replace vga_fixup() with generic code

Select the option on powerpc and remove the old code. The only change is that it moves from being a final fixup to an enable fixup.

(4) arm64: allow non-legacy VGA devices to be default

Select the option on arm64. This solves my problem with the D05, but may cause other cards to be marked as default on other boards. This shouldn't cause any real issues but is worth being aware of.

Regards,
Daniel

[0]: https://patchwork.ozlabs.org/patch/787003/
[1]: https://www.spinics.net/lists/arm-kernel/msg593656.html

Daniel Axtens (4):
powerpc: simplify and fix VGA default device behaviour
vgaarb: allow non-legacy cards to be marked as default
powerpc: replace vga_fixup() with generic code
arm64: allow non-legacy VGA devices to be default

 arch/arm64/Kconfig               |  1 +
 arch/powerpc/Kconfig             |  1 +
 arch/powerpc/kernel/pci-common.c | 12 ------------
 drivers/gpu/vga/Kconfig          |  7 +++++++
 drivers/gpu/vga/vgaarb.c         | 21 +++++++++++++++++++++
 5 files changed, 30 insertions(+), 12 deletions(-)

Signature · 1 line

2.11.0

Hi all,

Previously I posted a patch that provided a quirk for a hibmc card behind a particular Huawei bridge that allowed it to be marked as the default device in the VGA arbiter.[0] This lead to some discussion.[1] It was broadly suggested that a more generic solution would be better, something in the style of powerpc's fixup_vga() quirk.

Here is my suggested solution:

- Create a Kconfig option ARCH_WANT_VGA_ARB_FALLBACK

- if an arch selects that option, install PCI_FIXUP_CLASS_ENABLE hook. This hook fires when a card is enabled, which will require that a driver has been bound.

- if there is no default device when the hook fires, and the device can control memory and I/O, mark it as default.

The patches are as follows:

(1) powerpc: simplify and fix VGA default device behaviour

This cleans up some quirks in the powerpc implementation of the vga_fixup. It should make the behaviour match the original intention.

(2) vgaarb: allow non-legacy cards to be marked as default

Add the Kconfig option, and create the fixup in vgaarb.c gated behind the option. Nothing happens at this stage because no arch has selected the option yet.

(3) powerpc: replace vga_fixup() with generic code

Select the option on powerpc and remove the old code. The only change is that it moves from being a final fixup to an enable fixup.

(4) arm64: allow non-legacy VGA devices to be default

Select the option on arm64. This solves my problem with the D05, but may cause other cards to be marked as default on other boards. This shouldn't cause any real issues but is worth being aware of.

(+ Laszlo)

Quoted 39 lines · Daniel Axtens

Hi all,

Previously I posted a patch that provided a quirk for a hibmc card behind a particular Huawei bridge that allowed it to be marked as the default device in the VGA arbiter.[0] This lead to some discussion.[1] It was broadly suggested that a more generic solution would be better, something in the style of powerpc's fixup_vga() quirk.

Here is my suggested solution:

- Create a Kconfig option ARCH_WANT_VGA_ARB_FALLBACK

- if an arch selects that option, install PCI_FIXUP_CLASS_ENABLE hook. This hook fires when a card is enabled, which will require that a driver has been bound.

- if there is no default device when the hook fires, and the device can control memory and I/O, mark it as default.

The patches are as follows:

(1) powerpc: simplify and fix VGA default device behaviour

This cleans up some quirks in the powerpc implementation of the vga_fixup. It should make the behaviour match the original intention.

(2) vgaarb: allow non-legacy cards to be marked as default

Add the Kconfig option, and create the fixup in vgaarb.c gated behind the option. Nothing happens at this stage because no arch has selected the option yet.

(3) powerpc: replace vga_fixup() with generic code

Select the option on powerpc and remove the old code. The only change is that it moves from being a final fixup to an enable fixup.

Quoted 7 lines

(4) arm64: allow non-legacy VGA devices to be default

Select the option on arm64. This solves my problem with the D05, but may cause other cards to be marked as default on other boards. This shouldn't cause any real issues but is worth being aware of.

Hi Daniel,

Given that the whole point of the VGA arbiter is the ability to share the legacy mem+io ranges between different cards, why do we care about the VGA arbiter in the first place on arm64?

AFAIK, there have been some recent changes in Xorg to address the auto-detection problem. I don't remember the exact details, but I have added Laszlo, who was involved with this at the time.

Signature · 2 lines

-- 
Ard.

Hi Ard,

Quoted 52 lines

(+ Laszlo)

Quoted 39 lines · Daniel Axtens

Hi all,

Previously I posted a patch that provided a quirk for a hibmc card behind a particular Huawei bridge that allowed it to be marked as the default device in the VGA arbiter.[0] This lead to some discussion.[1] It was broadly suggested that a more generic solution would be better, something in the style of powerpc's fixup_vga() quirk.

Here is my suggested solution:

- Create a Kconfig option ARCH_WANT_VGA_ARB_FALLBACK

- if an arch selects that option, install PCI_FIXUP_CLASS_ENABLE hook. This hook fires when a card is enabled, which will require that a driver has been bound.

- if there is no default device when the hook fires, and the device can control memory and I/O, mark it as default.

The patches are as follows:

(1) powerpc: simplify and fix VGA default device behaviour

This cleans up some quirks in the powerpc implementation of the vga_fixup. It should make the behaviour match the original intention.

(2) vgaarb: allow non-legacy cards to be marked as default

Add the Kconfig option, and create the fixup in vgaarb.c gated behind the option. Nothing happens at this stage because no arch has selected the option yet.

(3) powerpc: replace vga_fixup() with generic code

Select the option on powerpc and remove the old code. The only change is that it moves from being a final fixup to an enable fixup.

Quoted 7 lines

(4) arm64: allow non-legacy VGA devices to be default

Select the option on arm64. This solves my problem with the D05, but may cause other cards to be marked as default on other boards. This shouldn't cause any real issues but is worth being aware of.

Hi Daniel,

Quoted 7 lines

Given that the whole point of the VGA arbiter is the ability to share the legacy mem+io ranges between different cards, why do we care about the VGA arbiter in the first place on arm64?

AFAIK, there have been some recent changes in Xorg to address the auto-detection problem. I don't remember the exact details, but I have added Laszlo, who was involved with this at the time.

I haven't been able to locate those changes - I remember that the call to pci_device_is_boot_vga() in xf86pciBus.c [0] was critical and that is still there in the latest git.

Indeed, the reason we care about the vga arbiter at all is because of that Xorg dependency on the boot VGA card. pci_device_is_boot_vga() reads a sysfs file, and that sysfs file is populated based on the vga_default_driver(), so it's very difficult to extricate ourselves from the vga arbiter and its concept of the default device.

We could make this method an 'either/or' rather than a fallback - so platforms who didn't care about legacy resources didn't bother with those tests, but I'm not sure what benefit that would give and I find it harder to be confident of an absence of unexpected consequences.

But the fact remains that we are going about this the wrong way. Whether a graphics card decodes legacy VGA ranges or not has *nothing* to do with whether or not it is in fact the primary device on a non-x86 system, and so I still think the VGA arbiter should be omitted entirely for such platforms, and Xorg should be fixed instead.

Hi Ard,

Quoted 5 lines

But the fact remains that we are going about this the wrong way. Whether a graphics card decodes legacy VGA ranges or not has *nothing* to do with whether or not it is in fact the primary device on a non-x86 system, and so I still think the VGA arbiter should be omitted entirely for such platforms, and Xorg should be fixed instead.

OK, I see where you're coming from. I've been trying to keep my changes small as I don't want to end up on the hook for the almost limitless range of problems that changing this sort of code can have, but I do take your point that it's a bit of an ugly hack of a solution.

Say we were to change Xorg instead. What would correct Xorg behaviour look like? Xorg would need to honour the boot_vga file if it existed so as not to break x86, etc. So your proposed Xorg - if it couldn't find a default card that way, and if there was no helpful config file info, would arbitrarily pick a card that has an Xorg driver? In other words, much like the proposed kernel approach but at a different level of the stack?

Are there other graphical applications we care about (other than Xorg) that would need to be patched? I'm happy to do the Xorg patch, but I don't know if anything other than Xorg keys off the boot_vga file.

I'm not fundamentally opposed to this approach if the Xorg community is happy with it, the kernel community is happy with it, and no-one expects me to provide patches to any other user-space applications that depend on boot_vga.

Having practically zero background in gfx development (either kernel or Xorg), I think the problem is that vga_default_device() / vga_set_default_device(), which -- apparently -- "boot_vga" is based upon, come from "drivers/gpu/vga/vgaarb.c". Namely, the concept of "primary / boot display device" is tied to the VGA arbiter, plus only a PCI device can currently be marked as primary/boot display device.

Can these concepts be split from each other? (I can fully imagine that this would result in a userspace visible interface change (or addition),
so that e.g. "/sys/devices/**/boot_gpu" would have to be consulted by display servers.)

(Sorry if I'm totally wrong.)

... Hm, reading the thread starter at
<https://www.mail-archive.com/linuxppc-
dev at lists.ozlabs.org/msg120851.html>,
and the references within... It looks like this work is motivated by hardware that is supposed to be PCI, but actually breaks the specs. Is that correct? If so, then I don't think I can suggest anything useful.

Specs exist so that hardware vendors and software authors follow them. If hardware does not conform, then software should either refuse to work
with it, or handle it with quirks, on a case-by-case basis. I guess this
means that I don't agree with the

broad[] suggest[ion] that a more generic solution would be better

which seems to disqualify me from the discussion, as it must have been suggested by people with incomparably more experience than what I have :)

Thanks
Laszlo

Hi Laszlo

[...]

Quoted 14 lines

Having practically zero background in gfx development (either kernel or Xorg), I think the problem is that vga_default_device() / vga_set_default_device(), which -- apparently -- "boot_vga" is based upon, come from "drivers/gpu/vga/vgaarb.c". Namely, the concept of "primary / boot display device" is tied to the VGA arbiter, plus only a PCI device can currently be marked as primary/boot display device.

Can these concepts be split from each other? (I can fully imagine that this would result in a userspace visible interface change (or addition),
so that e.g. "/sys/devices/**/boot_gpu" would have to be consulted by display servers.)

Quoted 8 lines

(Sorry if I'm totally wrong.)

... Hm, reading the thread starter at
<https://www.mail-archive.com/linuxppc-
dev at lists.ozlabs.org/msg120851.html>,
and the references within... It looks like this work is motivated by hardware that is supposed to be PCI, but actually breaks the specs. Is that correct? If so, then I don't think I can suggest anything useful.

My understanding is that the current PCIe HW is specs compliant but the vgaarb, in order to make a VGA device the default one, requires all the bridges on top of such device to have the "VGA Enable" bit set (optional bit in the PCI Express? to PCI/PCI-X Bridge Spec). I.e. all the bridges on top have to support legacy VGA devices; and this is not mandatory from the specs...right?

BTW my VGA experience is limited too...this is just my understanding...

Gab

Quoted 15 lines

Specs exist so that hardware vendors and software authors follow them. If hardware does not conform, then software should either refuse to work
with it, or handle it with quirks, on a case-by-case basis. I guess this
means that I don't agree with the

broad[] suggest[ion] that a more generic solution would be better

which seems to disqualify me from the discussion, as it must have been suggested by people with incomparably more experience than what I have :)

Thanks
Laszlo

Quoted 13 lines

Having practically zero background in gfx development (either kernel or Xorg), I think the problem is that vga_default_device() / vga_set_default_device(), which -- apparently -- "boot_vga" is based upon, come from "drivers/gpu/vga/vgaarb.c". Namely, the concept of "primary / boot display device" is tied to the VGA arbiter, plus only a PCI device can currently be marked as primary/boot display device.

Can these concepts be split from each other? (I can fully imagine that this would result in a userspace visible interface change (or addition),
so that e.g. "/sys/devices/**/boot_gpu" would have to be consulted by display servers.)

Quoted 8 lines

(Sorry if I'm totally wrong.)

... Hm, reading the thread starter at
<https://www.mail-archive.com/linuxppc-
dev at lists.ozlabs.org/msg120851.html>,
and the references within... It looks like this work is motivated by hardware that is supposed to be PCI, but actually breaks the specs. Is that correct? If so, then I don't think I can suggest anything useful.

My understanding is that the current PCIe HW is specs compliant but the vgaarb, in order to make a VGA device the default one, requires all the bridges on top of such device to have the "VGA Enable" bit set (optional bit in the PCI Express? to PCI/PCI-X Bridge Spec). I.e. all the bridges on top have to support legacy VGA devices; and this is not mandatory from the specs...right?

Quoted 8 lines

Are there other graphical applications we care about (other than Xorg) that would need to be patched? I'm happy to do the Xorg patch, but I don't know if anything other than Xorg keys off the boot_vga file.

I'm not fundamentally opposed to this approach if the Xorg community is happy with it, the kernel community is happy with it, and no-one expects me to provide patches to any other user-space applications that depend on boot_vga.

Ard both identified the Xorg commit that I would have, and CC'd Hans which I would have recommended as well.

I assume the symptom is that now there's a class of platform GPU devices that is neither PCI nor legacy VGA, so neither the kernel's boot_vga logic matches it, nor Xorg's commit in question.

I agree that it should be possible to add more logic to Xorg to detect this kind device as primary. However, I share Daniel's worry that it wouldn't cover all user space apps -- I see "Wayland this, Wayland that" on reddit every week.

Having practically zero background in gfx development (either kernel or Xorg), I think the problem is that vga_default_device() / vga_set_default_device(), which -- apparently -- "boot_vga" is based upon, come from "drivers/gpu/vga/vgaarb.c". Namely, the concept of "primary / boot display device" is tied to the VGA arbiter, plus only a PCI device can currently be marked as primary/boot display device.

Can these concepts be split from each other? (I can fully imagine that this would result in a userspace visible interface change (or addition), so that e.g. "/sys/devices/**/boot_gpu" would have to be consulted by display servers.)

(Sorry if I'm totally wrong.)

... Hm, reading the thread starter at
<https://www.mail-archive.com/[email protected]/msg120851.html>,
and the references within... It looks like this work is motivated by hardware that is supposed to be PCI, but actually breaks the specs. Is that correct? If so, then I don't think I can suggest anything useful. Specs exist so that hardware vendors and software authors follow them. If hardware does not conform, then software should either refuse to work with it, or handle it with quirks, on a case-by-case basis. I guess this means that I don't agree with the

broad[] suggest[ion] that a more generic solution would be better

which seems to disqualify me from the discussion, as it must have been suggested by people with incomparably more experience than what I have :)

Thanks
Laszlo