Re: [edk2] [PATCH 2/2] MdeModulePkg/PciBus: Restrict one VGA per HostBridge not Segment
On 2018/8/23 10:53, Ruiyu Ni wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1109
Today's restriction of VGA device is to have only one VGA device
enabled per PCI segment. It's not correct because several segments
may share one IO / MMIO address space.
We should restrict to have one VGA per Host Bridge because each
Host Bridge has its only IO / MMIO address space.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni
Cc: Star Zeng
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 22 +++---
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h | 10 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c| 4 ++--
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index fdec0bcd53..fbcc53e41a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -979,33 +979,33 @@ PciDeviceExisted (
}
/**
- Get the active VGA device on the same segment.
+ Get the active VGA device on the specified Host Bridge.
- @param VgaDevicePCI IO instance for the VGA device.
+ @param HostBridgeHandleHost Bridge handle.
- @return The active VGA device on the same segment.
+ @return The active VGA device on the specified Host Bridge.
**/
PCI_IO_DEVICE *
-ActiveVGADeviceOnTheSameSegment (
- IN PCI_IO_DEVICE*VgaDevice
+LocateVgaDeviceOnHostBridge (
+ IN EFI_HANDLE HostBridgeHandle
)
{
LIST_ENTRY *CurrentLink;
- PCI_IO_DEVICE *Temp;
+ PCI_IO_DEVICE *PciIo;
How about using PciIoDev as the local variable name? PciIo makes me
regard it for PciIo protocol at the first glance. :)
There are still two minor comments for the code pieces below.
CurrentLink = mPciDevicePool.ForwardLink;
while (CurrentLink != NULL && CurrentLink != ) {
-Temp = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
+PciIo = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
-if (Temp->PciRootBridgeIo->SegmentNumber == VgaDevice->PciRootBridgeIo->SegmentNumber) {
+if (PciIo->PciRootBridgeIo->ParentHandle== HostBridgeHandle) {
- Temp = LocateVgaDevice (Temp);
+ PciIo = LocateVgaDevice (PciIo);
- if (Temp != NULL) {
-return Temp;
+ if (PciIo != NULL) {
+return PciIo;
}
}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
index 1ec2178a21..b45d2a5d77 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
@@ -230,16 +230,16 @@ PciDeviceExisted (
);
/**
- Get the active VGA device on the same segment.
+ Get the active VGA device on the specified Host Bridge.
- @param VgaDevicePCI IO instance for the VGA device.
+ @param HostBridgeHandleHost Bridge handle.
- @return The active VGA device on the same segment.
+ @return The active VGA device on the specified Host Bridge.
**/
PCI_IO_DEVICE *
-ActiveVGADeviceOnTheSameSegment (
- IN PCI_IO_DEVICE*VgaDevice
+LocateVgaDeviceOnHostBridge (
+ IN EFI_HANDLE HostBridgeHandle
);
/**
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 291578c63c..333b875ff0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1415,7 +1415,7 @@ SupportPaletteSnoopAttributes (
//
// Get the boot VGA on the same segement
Please also update this code comment.
//
- Temp = ActiveVGADeviceOnTheSameSegment (PciIoDevice);
+ Temp = LocateVgaDeviceOnHostBridge
(PciIoDevice->PciRootBridgeIo->ParentHandle);
if (Temp == NULL) {
//
@@ -1670,7 +1670,7 @@ PciIoAttributes (
//
// Check if there have been an active VGA device on the same segment
Please also update this code comment.
With the new name and code comments updated,
Reviewed-by: Star Zeng
Thanks,
Star
//
-Temp = ActiveVGADeviceOnTheSameSegment (PciIoDevice);
+Temp = LocateVgaDeviceOnHostBridge
(PciIoDevice->PciRootBridgeIo->ParentHandle);
if (Temp != NULL && Temp != PciIoDevice) {
//
// An active VGA has been detected, so can not enable another
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] MdeModulePkg/PciBus: Restrict one VGA per HostBridge not Segment
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1109
Today's restriction of VGA device is to have only one VGA device
enabled per PCI segment. It's not correct because several segments
may share one IO / MMIO address space.
We should restrict to have one VGA per Host Bridge because each
Host Bridge has its only IO / MMIO address space.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni
Cc: Star Zeng
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 22 +++---
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h | 10 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c| 4 ++--
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index fdec0bcd53..fbcc53e41a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -979,33 +979,33 @@ PciDeviceExisted (
}
/**
- Get the active VGA device on the same segment.
+ Get the active VGA device on the specified Host Bridge.
- @param VgaDevicePCI IO instance for the VGA device.
+ @param HostBridgeHandleHost Bridge handle.
- @return The active VGA device on the same segment.
+ @return The active VGA device on the specified Host Bridge.
**/
PCI_IO_DEVICE *
-ActiveVGADeviceOnTheSameSegment (
- IN PCI_IO_DEVICE*VgaDevice
+LocateVgaDeviceOnHostBridge (
+ IN EFI_HANDLE HostBridgeHandle
)
{
LIST_ENTRY *CurrentLink;
- PCI_IO_DEVICE *Temp;
+ PCI_IO_DEVICE *PciIo;
CurrentLink = mPciDevicePool.ForwardLink;
while (CurrentLink != NULL && CurrentLink != ) {
-Temp = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
+PciIo = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
-if (Temp->PciRootBridgeIo->SegmentNumber ==
VgaDevice->PciRootBridgeIo->SegmentNumber) {
+if (PciIo->PciRootBridgeIo->ParentHandle== HostBridgeHandle) {
- Temp = LocateVgaDevice (Temp);
+ PciIo = LocateVgaDevice (PciIo);
- if (Temp != NULL) {
-return Temp;
+ if (PciIo != NULL) {
+return PciIo;
}
}
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
index 1ec2178a21..b45d2a5d77 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.h
@@ -230,16 +230,16 @@ PciDeviceExisted (
);
/**
- Get the active VGA device on the same segment.
+ Get the active VGA device on the specified Host Bridge.
- @param VgaDevicePCI IO instance for the VGA device.
+ @param HostBridgeHandleHost Bridge handle.
- @return The active VGA device on the same segment.
+ @return The active VGA device on the specified Host Bridge.
**/
PCI_IO_DEVICE *
-ActiveVGADeviceOnTheSameSegment (
- IN PCI_IO_DEVICE*VgaDevice
+LocateVgaDeviceOnHostBridge (
+ IN EFI_HANDLE HostBridgeHandle
);
/**
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 291578c63c..333b875ff0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1415,7 +1415,7 @@ SupportPaletteSnoopAttributes (
//
// Get the boot VGA on the same segement
//
- Temp = ActiveVGADeviceOnTheSameSegment (PciIoDevice);
+ Temp = LocateVgaDeviceOnHostBridge
(PciIoDevice->PciRootBridgeIo->ParentHandle);
if (Temp == NULL) {
//
@@ -1670,7 +1670,7 @@ PciIoAttributes (
//
// Check if there have been an active VGA device on the same segment
//
-Temp = ActiveVGADeviceOnTheSameSegment (PciIoDevice);
+Temp = LocateVgaDeviceOnHostBridge
(PciIoDevice->PciRootBridgeIo->ParentHandle);
if (Temp != NULL && Temp != PciIoDevice) {
//
// An active VGA has been detected, so can not enable another
--
2.16.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
