vgaarb: Fixes for partial VGA opt-out
Subsystems (2)
cover vgaarb: Fixes for partial VGA opt-out
I'm trying to add support for VGA arbitration on newer Intel graphics devices. The existing code attempts to do this, but appear to have not been updated since GMCH devices roamed the Earth. On newer devices like Haswell, we can disable VGA memory through an MSR on the device, but we rely on the VGA arbiter to manage VGA IO using the PCI COMMAND register. In trying to unregister legacy VGA memory, I found that the VGA arbiter still wanted to disable both memory and IO on the device and that it forgot to actually program the device to disable IO when the decoding is updated. This series attempts to fix both of those. Thanks,
Alex
---
Alex Williamson (2):
vgaarb: Don't disable resources that are not owned
vgaarb: Fix VGA decodes changes
drivers/gpu/vga/vgaarb.c | 50 ++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 28 deletions(-)
PATCH 1/2 vgaarb: Don't disable resources that are not owned
If a device does not own a resource then we don't need to disable it. This resolves the case where an Intel IGD device can be configured to disable decode of VGA memory but we still need the arbiter to handle VGA I/O port routing. When the IGD device is in conflict, only PCI_COMMAND_IO should be disabled since VGA memory does not require arbitration on this device.
--- drivers/gpu/vga/vgaarb.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/gpu/vga/vgaarb.c +5 -5
@@ -257,9 +257,9 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,if (!conflict->bridge_has_one_vga) {vga_irq_set_state (conflict, false); flags |=PCI_VGA_STATE_CHANGE_DECODES ; -if (lwants & (VGA_RSRC_LEGACY_MEM |VGA_RSRC_NORMAL_MEM )) +if (match & (VGA_RSRC_LEGACY_MEM |VGA_RSRC_NORMAL_MEM )) pci_bits |=PCI_COMMAND_MEMORY ; -if (lwants & (VGA_RSRC_LEGACY_IO |VGA_RSRC_NORMAL_IO )) +if (match & (VGA_RSRC_LEGACY_IO |VGA_RSRC_NORMAL_IO )) pci_bits |=PCI_COMMAND_IO ; }
@@ -267,11 +267,11 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev, flags |=PCI_VGA_STATE_CHANGE_BRIDGE ;pci_set_vga_state (conflict->pdev, false, pci_bits, flags); - conflict->owns &= ~lwants; + conflict->owns &= ~match;/* If he also owned non-legacy, that is no longer the case */ -if (lwants &VGA_RSRC_LEGACY_MEM ) +if (match &VGA_RSRC_LEGACY_MEM ) conflict->owns &= ~VGA_RSRC_NORMAL_MEM ; -if (lwants &VGA_RSRC_LEGACY_IO ) +if (match &VGA_RSRC_LEGACY_IO ) conflict->owns &= ~VGA_RSRC_NORMAL_IO ; }
PATCH 2/2 vgaarb: Fix VGA decodes changes
In reply to Alex WilliamsonWhen VGA decodes change we need to do a bit more evaluation of exactly what has changed. We don't necessarily give up all the old owns resources and we need to account for resources with locks. The new algorithm is: If something is added, update decodes. If legacy resources were added and none were there before, we have a new participant. If something is removed, update decodes. If we previously owned it, we no longer own it. If it was previously locked, invalidate all locks and release it. If legacy resources were removed and none are left, remove the participant from VGA arbitration.
Previously we updated decodes, released ownership of everything that was previously decoded, ignored all locks, and went off looking for another device to transfer VGA to. In a test case where Intel IGD removes only legacy VGA memory decoding, this left the arbiter switching to discrete graphics without actually disabling legacy VGA IO from the IGD. As a bonus, we bumped up the count of VGA arbitration participants for no good reason.
--- drivers/gpu/vga/vgaarb.c | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-)
drivers/gpu/vga/vgaarb.c +17 -23
@@ -644,10 +644,13 @@ bail:static inline void vga_update_device_decodes (struct vga_device *vgadev,int new_decodes) { -int old_decodes; +int old_decodes, decodes_removed, decodes_unlocked;struct vga_device *new_vgadev, *conflict; old_decodes = vgadev->decodes; + decodes_removed = ~new_decodes & old_decodes; + decodes_unlocked = vgadev->locks & decodes_removed; + vgadev->owns &= ~decodes_removed; vgadev->decodes = new_decodes; pr_info("vgaarb: device changed decodes: PCI:%s,olddecodes=%s,decodes=%s:owns=%s\n",
@@ -656,31 +659,22 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev,vga_iostate_to_str (vgadev->decodes), vga_iostate_to_str(vgadev->owns)); - -/* if we own the decodes we should move them along to -another card */ -if ((vgadev->owns & old_decodes) && (vga_count >1 )) { -/* set us to own nothing */ - vgadev->owns &= ~old_decodes; -list_for_each_entry (new_vgadev, &vga_list, list) { -if ((new_vgadev != vgadev) && - (new_vgadev->decodes &VGA_RSRC_LEGACY_MASK )) { -pr_info ("vgaarb: transferring owner from PCI:%s to PCI:%s\n" ,pci_name (vgadev->pdev),pci_name (new_vgadev->pdev)); - conflict =__vga_tryget (new_vgadev,VGA_RSRC_LEGACY_MASK ); -if (!conflict) -__vga_put (new_vgadev,VGA_RSRC_LEGACY_MASK ); -break ; - } - } +/* if we removed locked decodes, lock count goes to zero, and release */ +if (decodes_unlocked) { +if (decodes_unlocked &VGA_RSRC_LEGACY_IO ) + vgadev->io_lock_cnt =0 ; +if (decodes_unlocked &VGA_RSRC_LEGACY_MEM ) + vgadev->mem_lock_cnt =0 ; +__vga_put (vgadev, decodes_unlocked); }/* change decodes counter */ -if (old_decodes != new_decodes) { -if (new_decodes & (VGA_RSRC_LEGACY_IO |VGA_RSRC_LEGACY_MEM )) - vga_decode_count++; -else - vga_decode_count--; - } +if (old_decodes &VGA_RSRC_LEGACY_MASK && + !(new_decodes &VGA_RSRC_LEGACY_MASK )) + vga_decode_count--; +if (!(old_decodes &VGA_RSRC_LEGACY_MASK ) && + new_decodes &VGA_RSRC_LEGACY_MASK ) + vga_decode_count++;pr_debug ("vgaarb: decoding count now is: %d\n" , vga_decode_count); }
Quoted 10 lines · Alex Williamson
I'm trying to add support for VGA arbitration on newer Intel graphics devices. The existing code attempts to do this, but appear to have not been updated since GMCH devices roamed the Earth. On newer devices like Haswell, we can disable VGA memory through an MSR on the device, but we rely on the VGA arbiter to manage VGA IO using the PCI COMMAND register. In trying to unregister legacy VGA memory, I found that the VGA arbiter still wanted to disable both memory and IO on the device and that it forgot to actually program the device to disable IO when the decoding is updated. This series attempts to fix both of those. Thanks,
The series looks good to me.
Quoted 12 lines
Alex
---
Alex Williamson (2):
vgaarb: Don't disable resources that are not owned
vgaarb: Fix VGA decodes changesdrivers/gpu/vga/vgaarb.c | 50 ++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 28 deletions(-)
Signature · 2 lines
Ville Syrjälä Intel OTC
Daniel Vetter
AppliedQuoted 12 lines · Ville Syrjälä
Quoted 10 lines · Alex Williamson
I'm trying to add support for VGA arbitration on newer Intel graphics devices. The existing code attempts to do this, but appear to have not been updated since GMCH devices roamed the Earth. On newer devices like Haswell, we can disable VGA memory through an MSR on the device, but we rely on the VGA arbiter to manage VGA IO using the PCI COMMAND register. In trying to unregister legacy VGA memory, I found that the VGA arbiter still wanted to disable both memory and IO on the device and that it forgot to actually program the device to disable IO when the decoding is updated. This series attempts to fix both of those. Thanks,
Quoted 3 lines
The series looks good to me.
Reviewed-by Ville Syrjälä <[email protected]>
Merged to dinq with Dave's irc-ack. I'll shuffle my tree a bit so that this will be part of my 3.12 latecomers pull request to Dave. -Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch