Skip to content

Commit 3448a19

Browse files
committed
vgaarb: use bridges to control VGA routing where possible.
So in a lot of modern systems, a GPU will always be below a parent bridge that won't share with any other GPUs. This means VGA arbitration on those GPUs can be controlled by using the bridge routing instead of io/mem decodes. The problem is locating which GPUs share which upstream bridges. This patch attempts to identify all the GPUs which can be controlled via bridges, and ones that can't. This patch endeavours to work out the bridge sharing semantics. When disabling GPUs via a bridge, it doesn't do irq callbacks or touch the io/mem decodes for the gpu. Signed-off-by: Dave Airlie <[email protected]>
1 parent 8116188 commit 3448a19

File tree

3 files changed

+118
-27
lines changed

3 files changed

+118
-27
lines changed

drivers/gpu/vga/vgaarb.c

+99-14
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ struct vga_device {
6161
unsigned int mem_lock_cnt; /* legacy MEM lock count */
6262
unsigned int io_norm_cnt; /* normal IO count */
6363
unsigned int mem_norm_cnt; /* normal MEM count */
64-
64+
bool bridge_has_one_vga;
6565
/* allow IRQ enable/disable hook */
6666
void *cookie;
6767
void (*irq_set_state)(void *cookie, bool enable);
@@ -165,6 +165,8 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
165165
unsigned int wants, legacy_wants, match;
166166
struct vga_device *conflict;
167167
unsigned int pci_bits;
168+
u32 flags = 0;
169+
168170
/* Account for "normal" resources to lock. If we decode the legacy,
169171
* counterpart, we need to request it as well
170172
*/
@@ -237,16 +239,23 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
237239
/* looks like he doesn't have a lock, we can steal
238240
* them from him
239241
*/
240-
vga_irq_set_state(conflict, false);
241242

243+
flags = 0;
242244
pci_bits = 0;
243-
if (lwants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM))
244-
pci_bits |= PCI_COMMAND_MEMORY;
245-
if (lwants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO))
246-
pci_bits |= PCI_COMMAND_IO;
247245

248-
pci_set_vga_state(conflict->pdev, false, pci_bits,
249-
change_bridge);
246+
if (!conflict->bridge_has_one_vga) {
247+
vga_irq_set_state(conflict, false);
248+
flags |= PCI_VGA_STATE_CHANGE_DECODES;
249+
if (lwants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM))
250+
pci_bits |= PCI_COMMAND_MEMORY;
251+
if (lwants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO))
252+
pci_bits |= PCI_COMMAND_IO;
253+
}
254+
255+
if (change_bridge)
256+
flags |= PCI_VGA_STATE_CHANGE_BRIDGE;
257+
258+
pci_set_vga_state(conflict->pdev, false, pci_bits, flags);
250259
conflict->owns &= ~lwants;
251260
/* If he also owned non-legacy, that is no longer the case */
252261
if (lwants & VGA_RSRC_LEGACY_MEM)
@@ -261,14 +270,24 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
261270
* also have in "decodes". We can lock resources we don't decode but
262271
* not own them.
263272
*/
273+
flags = 0;
264274
pci_bits = 0;
265-
if (wants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM))
266-
pci_bits |= PCI_COMMAND_MEMORY;
267-
if (wants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO))
268-
pci_bits |= PCI_COMMAND_IO;
269-
pci_set_vga_state(vgadev->pdev, true, pci_bits, !!(wants & VGA_RSRC_LEGACY_MASK));
270275

271-
vga_irq_set_state(vgadev, true);
276+
if (!vgadev->bridge_has_one_vga) {
277+
flags |= PCI_VGA_STATE_CHANGE_DECODES;
278+
if (wants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM))
279+
pci_bits |= PCI_COMMAND_MEMORY;
280+
if (wants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO))
281+
pci_bits |= PCI_COMMAND_IO;
282+
}
283+
if (!!(wants & VGA_RSRC_LEGACY_MASK))
284+
flags |= PCI_VGA_STATE_CHANGE_BRIDGE;
285+
286+
pci_set_vga_state(vgadev->pdev, true, pci_bits, flags);
287+
288+
if (!vgadev->bridge_has_one_vga) {
289+
vga_irq_set_state(vgadev, true);
290+
}
272291
vgadev->owns |= (wants & vgadev->decodes);
273292
lock_them:
274293
vgadev->locks |= (rsrc & VGA_RSRC_LEGACY_MASK);
@@ -421,6 +440,62 @@ void vga_put(struct pci_dev *pdev, unsigned int rsrc)
421440
}
422441
EXPORT_SYMBOL(vga_put);
423442

443+
/* Rules for using a bridge to control a VGA descendant decoding:
444+
if a bridge has only one VGA descendant then it can be used
445+
to control the VGA routing for that device.
446+
It should always use the bridge closest to the device to control it.
447+
If a bridge has a direct VGA descendant, but also have a sub-bridge
448+
VGA descendant then we cannot use that bridge to control the direct VGA descendant.
449+
So for every device we register, we need to iterate all its parent bridges
450+
so we can invalidate any devices using them properly.
451+
*/
452+
static void vga_arbiter_check_bridge_sharing(struct vga_device *vgadev)
453+
{
454+
struct vga_device *same_bridge_vgadev;
455+
struct pci_bus *new_bus, *bus;
456+
struct pci_dev *new_bridge, *bridge;
457+
458+
vgadev->bridge_has_one_vga = true;
459+
460+
if (list_empty(&vga_list))
461+
return;
462+
463+
/* okay iterate the new devices bridge hierarachy */
464+
new_bus = vgadev->pdev->bus;
465+
while (new_bus) {
466+
new_bridge = new_bus->self;
467+
468+
if (new_bridge) {
469+
/* go through list of devices already registered */
470+
list_for_each_entry(same_bridge_vgadev, &vga_list, list) {
471+
bus = same_bridge_vgadev->pdev->bus;
472+
bridge = bus->self;
473+
474+
/* see if the share a bridge with this device */
475+
if (new_bridge == bridge) {
476+
/* if their direct parent bridge is the same
477+
as any bridge of this device then it can't be used
478+
for that device */
479+
same_bridge_vgadev->bridge_has_one_vga = false;
480+
}
481+
482+
/* now iterate the previous devices bridge hierarchy */
483+
/* if the new devices parent bridge is in the other devices
484+
hierarchy then we can't use it to control this device */
485+
while (bus) {
486+
bridge = bus->self;
487+
if (bridge) {
488+
if (bridge == vgadev->pdev->bus->self)
489+
vgadev->bridge_has_one_vga = false;
490+
}
491+
bus = bus->parent;
492+
}
493+
}
494+
}
495+
new_bus = new_bus->parent;
496+
}
497+
}
498+
424499
/*
425500
* Currently, we assume that the "initial" setup of the system is
426501
* not sane, that is we come up with conflicting devices and let
@@ -500,6 +575,8 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
500575
vga_default = pci_dev_get(pdev);
501576
#endif
502577

578+
vga_arbiter_check_bridge_sharing(vgadev);
579+
503580
/* Add to the list */
504581
list_add(&vgadev->list, &vga_list);
505582
vga_count++;
@@ -1222,6 +1299,7 @@ static int __init vga_arb_device_init(void)
12221299
{
12231300
int rc;
12241301
struct pci_dev *pdev;
1302+
struct vga_device *vgadev;
12251303

12261304
rc = misc_register(&vga_arb_device);
12271305
if (rc < 0)
@@ -1238,6 +1316,13 @@ static int __init vga_arb_device_init(void)
12381316
vga_arbiter_add_pci_device(pdev);
12391317

12401318
pr_info("vgaarb: loaded\n");
1319+
1320+
list_for_each_entry(vgadev, &vga_list, list) {
1321+
if (vgadev->bridge_has_one_vga)
1322+
pr_info("vgaarb: bridge control possible %s\n", pci_name(vgadev->pdev));
1323+
else
1324+
pr_info("vgaarb: no bridge control possible %s\n", pci_name(vgadev->pdev));
1325+
}
12411326
return rc;
12421327
}
12431328
subsys_initcall(vga_arb_device_init);

drivers/pci/pci.c

+14-11
Original file line numberDiff line numberDiff line change
@@ -2875,31 +2875,34 @@ static int pci_set_vga_state_arch(struct pci_dev *dev, bool decode,
28752875
* @dev: the PCI device
28762876
* @decode: true = enable decoding, false = disable decoding
28772877
* @command_bits: PCI_COMMAND_IO and/or PCI_COMMAND_MEMORY
2878-
* @change_bridge: traverse ancestors and change bridges
2878+
* @change_bridge_flags: traverse ancestors and change bridges
2879+
* CHANGE_BRIDGE_ONLY / CHANGE_BRIDGE
28792880
*/
28802881
int pci_set_vga_state(struct pci_dev *dev, bool decode,
2881-
unsigned int command_bits, bool change_bridge)
2882+
unsigned int command_bits, u32 flags)
28822883
{
28832884
struct pci_bus *bus;
28842885
struct pci_dev *bridge;
28852886
u16 cmd;
28862887
int rc;
28872888

2888-
WARN_ON(command_bits & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY));
2889+
WARN_ON((flags & PCI_VGA_STATE_CHANGE_DECODES) & (command_bits & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY)));
28892890

28902891
/* ARCH specific VGA enables */
2891-
rc = pci_set_vga_state_arch(dev, decode, command_bits, change_bridge);
2892+
rc = pci_set_vga_state_arch(dev, decode, command_bits, flags);
28922893
if (rc)
28932894
return rc;
28942895

2895-
pci_read_config_word(dev, PCI_COMMAND, &cmd);
2896-
if (decode == true)
2897-
cmd |= command_bits;
2898-
else
2899-
cmd &= ~command_bits;
2900-
pci_write_config_word(dev, PCI_COMMAND, cmd);
2896+
if (flags & PCI_VGA_STATE_CHANGE_DECODES) {
2897+
pci_read_config_word(dev, PCI_COMMAND, &cmd);
2898+
if (decode == true)
2899+
cmd |= command_bits;
2900+
else
2901+
cmd &= ~command_bits;
2902+
pci_write_config_word(dev, PCI_COMMAND, cmd);
2903+
}
29012904

2902-
if (change_bridge == false)
2905+
if (!(flags & PCI_VGA_STATE_CHANGE_BRIDGE))
29032906
return 0;
29042907

29052908
bus = dev->bus;

include/linux/pci.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -915,8 +915,11 @@ int pci_cfg_space_size_ext(struct pci_dev *dev);
915915
int pci_cfg_space_size(struct pci_dev *dev);
916916
unsigned char pci_bus_max_busnr(struct pci_bus *bus);
917917

918+
#define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0)
919+
#define PCI_VGA_STATE_CHANGE_DECODES (1 << 1)
920+
918921
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
919-
unsigned int command_bits, bool change_bridge);
922+
unsigned int command_bits, u32 flags);
920923
/* kmem_cache style wrapper around pci_alloc_consistent() */
921924

922925
#include <linux/pci-dma.h>
@@ -1061,7 +1064,7 @@ static inline int pci_proc_domain(struct pci_bus *bus)
10611064

10621065
/* some architectures require additional setup to direct VGA traffic */
10631066
typedef int (*arch_set_vga_state_t)(struct pci_dev *pdev, bool decode,
1064-
unsigned int command_bits, bool change_bridge);
1067+
unsigned int command_bits, u32 flags);
10651068
extern void pci_register_set_vga_state(arch_set_vga_state_t func);
10661069

10671070
#else /* CONFIG_PCI is not enabled */

0 commit comments

Comments
 (0)