Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi,
On 2023/6/16 22:34, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 10:22 AM Sui Jingfeng wrote:
On 2023/6/16 21:41, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote:
Hi,
On 2023/6/16 05:11, Alex Deucher wrote:
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
as its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY0x03
#define PCI_CLASS_DISPLAY_VGA0x0300
#define PCI_CLASS_DISPLAY_XGA0x0301
#define PCI_CLASS_DISPLAY_3D0x0302
#define PCI_CLASS_DISPLAY_OTHER0x0380
```
Any ideas ?
I'm not quite sure what you are asking about here.
To be honest, I'm worried about the PCI devices which has a
PCI_CLASS_DISPLAY_XGA as its PCI class number.
As those devices are very uncommon in the real world.
$ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
there no code reference this macro. So I think it seems safe to ignore
the XGA ?
PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
the render-only GPU.
And render-only GPU can't decode the fixed VGA address space, it is safe
to ignore them.
For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them.
However, as VGA gets deprecated,
We need the vgaarb for a system with multiple video card.
Not only because some Legacy VGA devices implemented
on PCI will typically have the same "hard-decoded" addresses;
But also these video card need to participate in the arbitration,
determine the default boot device.
But couldn't the boot device be determined via what whatever resources
were used by the pre-OS console?
I don't know what you are refer to by saying pre-OS console, UEFI
SHELL, UEFI GOP or something like that.
Right. Before the OS loads the platform firmware generally sets up
something for display. That could be GOP or vesa or some other
platform specific protocol.
If you are referring to the framebuffer driver which light up the screen
before the Linux kernel is loaded .
Then, what you have said is true, the boot device is determined by the
pre-OS console.
But the problem is how does the Linux kernel(vgaarb) could know which
one is the default boot device
on a multiple GPU machine. Relaying on the firmware fb's address and
size is what the mechanism
we already in using.
Right. It shouldn't need to depend on vgaarb.
I feel like that should be separate from vgaarb.
Emm, this really deserved another patch, please ?
vgaarb should handle PCI VGA routing and some other
mechanism should be used to determine what device provided the pre-OS
console.
If the new mechanism need the firmware changed, then this probably break
the old machine.
Also, this probably will get all arch involved. to get the new mechanism
supported.
The testing pressure and review power needed is quite large.
drm/amdgpu and drm/radeon already being used on X86, ARM64, Mips and
more arch...
The reviewing process will became quite difficult then.
vgaarb is really what we already in use, and being
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi,
On 2023/6/16 22:34, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 10:22 AM Sui Jingfeng wrote:
On 2023/6/16 21:41, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote:
Hi,
On 2023/6/16 05:11, Alex Deucher wrote:
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
as its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY0x03
#define PCI_CLASS_DISPLAY_VGA0x0300
#define PCI_CLASS_DISPLAY_XGA0x0301
#define PCI_CLASS_DISPLAY_3D0x0302
#define PCI_CLASS_DISPLAY_OTHER0x0380
```
Any ideas ?
I'm not quite sure what you are asking about here.
To be honest, I'm worried about the PCI devices which has a
PCI_CLASS_DISPLAY_XGA as its PCI class number.
As those devices are very uncommon in the real world.
$ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
there no code reference this macro. So I think it seems safe to ignore
the XGA ?
PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
the render-only GPU.
And render-only GPU can't decode the fixed VGA address space, it is safe
to ignore them.
For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them.
However, as VGA gets deprecated,
We need the vgaarb for a system with multiple video card.
Not only because some Legacy VGA devices implemented
on PCI will typically have the same "hard-decoded" addresses;
But also these video card need to participate in the arbitration,
determine the default boot device.
But couldn't the boot device be determined via what whatever resources
were used by the pre-OS console?
I don't know what you are refer to by saying pre-OS console, UEFI
SHELL, UEFI GOP or something like that.
Right. Before the OS loads the platform firmware generally sets up
something for display. That could be GOP or vesa or some other
platform specific protocol.
If you are referring to the framebuffer driver which light up the screen
before the Linux kernel is loaded .
Then, what you have said is true, the boot device is determined by the
pre-OS console.
But the problem is how does the Linux kernel(vgaarb) could know which
one is the default boot device
on a multiple GPU machine. Relaying on the firmware fb's address and
size is what the mechanism
we already in using.
Right. It shouldn't need to depend on vgaarb.
I feel like that should be separate from vgaarb.
Emm, this really deserved another patch, please ?
vgaarb should handle PCI VGA routing and some other
mechanism should be used to determine what device provided the pre-OS
console.
If the new mechanism need the firmware changed, then this probably break
the old machine.
Also, this probably will get all arch involved. to get the new mechanism
supported.
The testing pressure and review power needed is quite large.
drm/amdgpu and drm/radeon already being used on X86, ARM64, Mips and
more arch...
The reviewing process will became quite difficult then.
vgaarb is really what we already in use, and being
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On Tue, 13 Jun 2023, Sui Jingfeng wrote: > Deal only with the VGA devcie(pdev->class == 0x0300), so replace the Typo here: s/devcie/device/. > pci_get_subsys() function with pci_get_class(). Filter the non-PCI display > device(pdev->class != 0x0300) out. There no need to process the non-display > PCI device. I've only come across this patch series now. Without diving into what this code actually does I have just one question as a matter of interest. > diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c > index c1bc6c983932..22a505e877dc 100644 > --- a/drivers/pci/vgaarb.c > +++ b/drivers/pci/vgaarb.c > @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, > unsigned long action, > struct pci_dev *pdev = to_pci_dev(dev); > bool notify = false; > > - vgaarb_dbg(dev, "%s\n", __func__); > + /* Only deal with VGA class devices */ > + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8) > + return 0; Hmm, shouldn't this also handle PCI_CLASS_NOT_DEFINED_VGA? As far as I know it is the equivalent of PCI_CLASS_DISPLAY_VGA for PCI <= 2.0 devices that were implemented before the idea of PCI device classes has developed into its current form. I may have such a VGA device somewhere. Maciej
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi, On 2023/6/19 11:02, Maciej W. Rozycki wrote: On Tue, 13 Jun 2023, Sui Jingfeng wrote: Deal only with the VGA devcie(pdev->class == 0x0300), so replace the Typo here: s/devcie/device/. Thanks a lot, pci_get_subsys() function with pci_get_class(). Filter the non-PCI display device(pdev->class != 0x0300) out. There no need to process the non-display PCI device. I've only come across this patch series now. Without diving into what this code actually does I have just one question as a matter of interest. diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c index c1bc6c983932..22a505e877dc 100644 --- a/drivers/pci/vgaarb.c +++ b/drivers/pci/vgaarb.c @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned long action, struct pci_dev *pdev = to_pci_dev(dev); bool notify = false; - vgaarb_dbg(dev, "%s\n", __func__); + /* Only deal with VGA class devices */ + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8) + return 0; Hmm, shouldn't this also handle PCI_CLASS_NOT_DEFINED_VGA? If your machine have only one such a VGA card, it probably don't hurt. But, such a card will also get ignored originally (before applying this patch). As far as I know it is the equivalent of PCI_CLASS_DISPLAY_VGA for PCI <= 2.0 devices that were implemented before the idea of PCI device classes has developed into its current form. If multiple video card problems on your machine is matter, then I think it do deserve another patch to clarify this issue and to explain the rationale. I may have such a VGA device somewhere. Maciej
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On 2023/6/18 20:11, Sui Jingfeng wrote: And screen_info is more about video specifci thing. screen_info is something more about video specific thing.
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi, On 2023/6/18 20:11, Sui Jingfeng wrote: call back to use if successful Call back to us if the drm device driver bound to the PCI GPU successfully
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi,
On 2023/6/16 22:34, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 10:22 AM Sui Jingfeng wrote:
On 2023/6/16 21:41, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote:
Hi,
On 2023/6/16 05:11, Alex Deucher wrote:
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
as its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY0x03
#define PCI_CLASS_DISPLAY_VGA0x0300
#define PCI_CLASS_DISPLAY_XGA0x0301
#define PCI_CLASS_DISPLAY_3D0x0302
#define PCI_CLASS_DISPLAY_OTHER0x0380
```
Any ideas ?
I'm not quite sure what you are asking about here.
To be honest, I'm worried about the PCI devices which has a
PCI_CLASS_DISPLAY_XGA as its PCI class number.
As those devices are very uncommon in the real world.
$ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
there no code reference this macro. So I think it seems safe to ignore
the XGA ?
PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
the render-only GPU.
And render-only GPU can't decode the fixed VGA address space, it is safe
to ignore them.
For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them.
However, as VGA gets deprecated,
We need the vgaarb for a system with multiple video card.
Not only because some Legacy VGA devices implemented
on PCI will typically have the same "hard-decoded" addresses;
But also these video card need to participate in the arbitration,
determine the default boot device.
But couldn't the boot device be determined via what whatever resources
were used by the pre-OS console?
I don't know what you are refer to by saying pre-OS console, UEFI
SHELL, UEFI GOP or something like that.
Right. Before the OS loads the platform firmware generally sets up
something for display. That could be GOP or vesa or some other
platform specific protocol.
If you are referring to the framebuffer driver which light up the screen
before the Linux kernel is loaded .
Then, what you have said is true, the boot device is determined by the
pre-OS console.
But the problem is how does the Linux kernel(vgaarb) could know which
one is the default boot device
on a multiple GPU machine. Relaying on the firmware fb's address and
size is what the mechanism
we already in using.
Right. It shouldn't need to depend on vgaarb.
I feel like that should be separate from vgaarb.
Emm, this really deserved another patch, please ?
vgaarb should handle PCI VGA routing and some other
mechanism should be used to determine what device provided the pre-OS
console.
If the new mechanism need the firmware changed, then this probably break
the old machine.
Also, this probably will get all arch involved. to get the new mechanism
supported.
The testing pressure and review power needed is quite large.
drm/amdgpu and drm/radeon already being used on X86, ARM64, Mips and
more arch...
The reviewing process will became quite difficult then.
vgaarb is really what we already in use, and being
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi,
On 2023/6/16 22:34, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 10:22 AM Sui Jingfeng wrote:
On 2023/6/16 21:41, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote:
Hi,
On 2023/6/16 05:11, Alex Deucher wrote:
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
as its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY0x03
#define PCI_CLASS_DISPLAY_VGA0x0300
#define PCI_CLASS_DISPLAY_XGA0x0301
#define PCI_CLASS_DISPLAY_3D0x0302
#define PCI_CLASS_DISPLAY_OTHER0x0380
```
Any ideas ?
I'm not quite sure what you are asking about here.
To be honest, I'm worried about the PCI devices which has a
PCI_CLASS_DISPLAY_XGA as its PCI class number.
As those devices are very uncommon in the real world.
$ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
there no code reference this macro. So I think it seems safe to ignore
the XGA ?
PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
the render-only GPU.
And render-only GPU can't decode the fixed VGA address space, it is safe
to ignore them.
For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them.
However, as VGA gets deprecated,
We need the vgaarb for a system with multiple video card.
Not only because some Legacy VGA devices implemented
on PCI will typically have the same "hard-decoded" addresses;
But also these video card need to participate in the arbitration,
determine the default boot device.
But couldn't the boot device be determined via what whatever resources
were used by the pre-OS console?
I don't know what you are refer to by saying pre-OS console, UEFI
SHELL, UEFI GOP or something like that.
Right. Before the OS loads the platform firmware generally sets up
something for display. That could be GOP or vesa or some other
platform specific protocol.
If you are referring to the framebuffer driver which light up the screen
before the Linux kernel is loaded .
Then, what you have said is true, the boot device is determined by the
pre-OS console.
But the problem is how does the Linux kernel(vgaarb) could know which
one is the default boot device
on a multiple GPU machine. Relaying on the firmware fb's address and
size is what the mechanism
we already in using.
Right. It shouldn't need to depend on vgaarb.
I feel like that should be separate from vgaarb.
Emm, this really deserved another patch, please ?
vgaarb should handle PCI VGA routing and some other
mechanism should be used to determine what device provided the pre-OS
console.
If the new mechanism need the firmware changed, then this probably break
the old machine.
Also, this probably will get all arch involved. to get the new mechanism
supported.
The testing pressure and review power needed is quite large.
drm/amdgpu and drm/radeon already being used on X86, ARM64, Mips and
more arch...
The reviewing process will became quite difficult then.
vgaarb is really what we already in use, and being
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On Fri, Jun 16, 2023 at 10:22 AM Sui Jingfeng wrote:
>
>
> On 2023/6/16 21:41, Alex Deucher wrote:
> > On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng
> > wrote:
> >> Hi,
> >>
> >> On 2023/6/16 05:11, Alex Deucher wrote:
> >>> On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng
> >>> wrote:
> Hi,
>
> On 2023/6/13 11:01, Sui Jingfeng wrote:
> > From: Sui Jingfeng
> >
> > Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
> > pci_get_subsys() function with pci_get_class(). Filter the non-PCI
> > display
> > device(pdev->class != 0x0300) out. There no need to process the
> > non-display
> > PCI device.
> >
> > Cc: Bjorn Helgaas
> > Signed-off-by: Sui Jingfeng
> > ---
> > drivers/pci/vgaarb.c | 22 --
> > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> > index c1bc6c983932..22a505e877dc 100644
> > --- a/drivers/pci/vgaarb.c
> > +++ b/drivers/pci/vgaarb.c
> > @@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct
> > pci_dev *pdev)
> > struct pci_dev *bridge;
> > u16 cmd;
> >
> > - /* Only deal with VGA class devices */
> > - if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> > - return false;
> > -
> Hi, here is probably a bug fixing.
>
> For an example, nvidia render only GPU typically has 0x0380.
>
> as its PCI class number, but render only GPU should not participate in
> the arbitration.
>
> As it shouldn't snoop the legacy fixed VGA address.
>
> It(render only GPU) can not display anything.
>
>
> But 0x0380 >> 8 = 0x03, the filter failed.
>
>
> > /* Allocate structure */
> > vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
> > if (vgadev == NULL) {
> > @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb,
> > unsigned long action,
> > struct pci_dev *pdev = to_pci_dev(dev);
> > bool notify = false;
> >
> > - vgaarb_dbg(dev, "%s\n", __func__);
> > + /* Only deal with VGA class devices */
> > + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
> > + return 0;
> So here we only care 0x0300, my initial intent is to make an
> optimization,
>
> nowadays sane display graphic card should all has 0x0300 as its PCI
> class number, is this complete right?
>
> ```
>
> #define PCI_BASE_CLASS_DISPLAY0x03
> #define PCI_CLASS_DISPLAY_VGA0x0300
> #define PCI_CLASS_DISPLAY_XGA0x0301
> #define PCI_CLASS_DISPLAY_3D0x0302
> #define PCI_CLASS_DISPLAY_OTHER0x0380
>
> ```
>
> Any ideas ?
> >>> I'm not quite sure what you are asking about here.
> >> To be honest, I'm worried about the PCI devices which has a
> >>
> >> PCI_CLASS_DISPLAY_XGA as its PCI class number.
> >>
> >> As those devices are very uncommon in the real world.
> >>
> >>
> >> $ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
> >>
> >>
> >> Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
> >>
> >> there no code reference this macro. So I think it seems safe to ignore
> >> the XGA ?
> >>
> >>
> >> PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
> >> the render-only GPU.
> >>
> >> And render-only GPU can't decode the fixed VGA address space, it is safe
> >> to ignore them.
> >>
> >>
> >>>For vga_arb, we
> >>> only care about VGA class devices since those should be on the only
> >>> ones that might have VGA routed to them.
> >>>However, as VGA gets deprecated,
> >> We need the vgaarb for a system with multiple video card.
> >>
> >> Not only because some Legacy VGA devices implemented
> >>
> >> on PCI will typically have the same "hard-decoded" addresses;
> >>
> >> But also these video card need to participate in the arbitration,
> >>
> >> determine the default boot device.
> > But couldn't the boot device be determined via what whatever resources
> > were used by the pre-OS console?
>
> I don't know what you are refer to by saying pre-OS console, UEFI
> SHELL, UEFI GOP or something like that.
>
Right. Before the OS loads the platform firmware generally sets up
something for display. That could be GOP or vesa or some other
platform specific protocol.
> If you are referring to the framebuffer driver which light up the screen
> before the Linux kernel is loaded .
>
>
> Then, what you have said is true, the boot device is determined by the
> pre-OS console.
>
> But the problem is how does the Linux kernel(vgaarb) could know which
> one is the default boot device
>
> on a multiple GPU machine. Relaying on the firmware fb's address and
> size is what the mechanism
>
> we already in
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On 2023/6/16 21:41, Alex Deucher wrote:
On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote:
Hi,
On 2023/6/16 05:11, Alex Deucher wrote:
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
as its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY0x03
#define PCI_CLASS_DISPLAY_VGA0x0300
#define PCI_CLASS_DISPLAY_XGA0x0301
#define PCI_CLASS_DISPLAY_3D0x0302
#define PCI_CLASS_DISPLAY_OTHER0x0380
```
Any ideas ?
I'm not quite sure what you are asking about here.
To be honest, I'm worried about the PCI devices which has a
PCI_CLASS_DISPLAY_XGA as its PCI class number.
As those devices are very uncommon in the real world.
$ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
there no code reference this macro. So I think it seems safe to ignore
the XGA ?
PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
the render-only GPU.
And render-only GPU can't decode the fixed VGA address space, it is safe
to ignore them.
For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them.
However, as VGA gets deprecated,
We need the vgaarb for a system with multiple video card.
Not only because some Legacy VGA devices implemented
on PCI will typically have the same "hard-decoded" addresses;
But also these video card need to participate in the arbitration,
determine the default boot device.
But couldn't the boot device be determined via what whatever resources
were used by the pre-OS console?
I don't know what you are refer to by saying pre-OS console, UEFI
SHELL, UEFI GOP or something like that.
If you are referring to the framebuffer driver which light up the screen
before the Linux kernel is loaded .
Then, what you have said is true, the boot device is determined by the
pre-OS console.
But the problem is how does the Linux kernel(vgaarb) could know which
one is the default boot device
on a multiple GPU machine. Relaying on the firmware fb's address and
size is what the mechanism
we already in using.
I feel like that should be separate from vgaarb.
Emm, this really deserved another patch, please ?
vgaarb should handle PCI VGA routing and some other
mechanism should be used to determine what device provided the pre-OS
console.
If the new mechanism need the firmware changed, then this probably break
the old machine.
Also, this probably will get all arch involved. to get the new mechanism
supported.
The testing pressure and review power needed is quite large.
drm/amdgpu and drm/radeon already being used on X86, ARM64, Mips and
more arch...
The reviewing process will became quite difficult then.
vgaarb is really what we already in use, and being used more than ten
years ...
Alex
Nowadays, the 'VGA devices' here is stand for the Graphics card
which is capable of display something on the screen.
We still need vgaarb to select the default boot device.
you'll have more non VGA PCI classes for devices which
could be the pre-OS console
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote:
>
> Hi,
>
> On 2023/6/16 05:11, Alex Deucher wrote:
> > On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng
> > wrote:
> >> Hi,
> >>
> >> On 2023/6/13 11:01, Sui Jingfeng wrote:
> >>> From: Sui Jingfeng
> >>>
> >>> Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
> >>> pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
> >>> device(pdev->class != 0x0300) out. There no need to process the
> >>> non-display
> >>> PCI device.
> >>>
> >>> Cc: Bjorn Helgaas
> >>> Signed-off-by: Sui Jingfeng
> >>> ---
> >>>drivers/pci/vgaarb.c | 22 --
> >>>1 file changed, 12 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> >>> index c1bc6c983932..22a505e877dc 100644
> >>> --- a/drivers/pci/vgaarb.c
> >>> +++ b/drivers/pci/vgaarb.c
> >>> @@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct
> >>> pci_dev *pdev)
> >>>struct pci_dev *bridge;
> >>>u16 cmd;
> >>>
> >>> - /* Only deal with VGA class devices */
> >>> - if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> >>> - return false;
> >>> -
> >> Hi, here is probably a bug fixing.
> >>
> >> For an example, nvidia render only GPU typically has 0x0380.
> >>
> >> as its PCI class number, but render only GPU should not participate in
> >> the arbitration.
> >>
> >> As it shouldn't snoop the legacy fixed VGA address.
> >>
> >> It(render only GPU) can not display anything.
> >>
> >>
> >> But 0x0380 >> 8 = 0x03, the filter failed.
> >>
> >>
> >>>/* Allocate structure */
> >>>vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
> >>>if (vgadev == NULL) {
> >>> @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb,
> >>> unsigned long action,
> >>>struct pci_dev *pdev = to_pci_dev(dev);
> >>>bool notify = false;
> >>>
> >>> - vgaarb_dbg(dev, "%s\n", __func__);
> >>> + /* Only deal with VGA class devices */
> >>> + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
> >>> + return 0;
> >> So here we only care 0x0300, my initial intent is to make an optimization,
> >>
> >> nowadays sane display graphic card should all has 0x0300 as its PCI
> >> class number, is this complete right?
> >>
> >> ```
> >>
> >> #define PCI_BASE_CLASS_DISPLAY0x03
> >> #define PCI_CLASS_DISPLAY_VGA0x0300
> >> #define PCI_CLASS_DISPLAY_XGA0x0301
> >> #define PCI_CLASS_DISPLAY_3D0x0302
> >> #define PCI_CLASS_DISPLAY_OTHER0x0380
> >>
> >> ```
> >>
> >> Any ideas ?
> > I'm not quite sure what you are asking about here.
>
> To be honest, I'm worried about the PCI devices which has a
>
> PCI_CLASS_DISPLAY_XGA as its PCI class number.
>
> As those devices are very uncommon in the real world.
>
>
> $ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
>
>
> Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
>
> there no code reference this macro. So I think it seems safe to ignore
> the XGA ?
>
>
> PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
> the render-only GPU.
>
> And render-only GPU can't decode the fixed VGA address space, it is safe
> to ignore them.
>
>
> > For vga_arb, we
> > only care about VGA class devices since those should be on the only
> > ones that might have VGA routed to them.
>
> > However, as VGA gets deprecated,
>
> We need the vgaarb for a system with multiple video card.
>
> Not only because some Legacy VGA devices implemented
>
> on PCI will typically have the same "hard-decoded" addresses;
>
> But also these video card need to participate in the arbitration,
>
> determine the default boot device.
But couldn't the boot device be determined via what whatever resources
were used by the pre-OS console? I feel like that should be separate
from vgaarb. vgaarb should handle PCI VGA routing and some other
mechanism should be used to determine what device provided the pre-OS
console.
Alex
>
>
> Nowadays, the 'VGA devices' here is stand for the Graphics card
>
> which is capable of display something on the screen.
>
> We still need vgaarb to select the default boot device.
>
>
> > you'll have more non VGA PCI classes for devices which
> > could be the pre-OS console device.
>
> Ah, we still want do this(by applying this patch) first,
>
> and then we will have the opportunity to see who will crying if
> something is broken. Will know more then.
>
> But drop this patch or revise it with more consideration is also
> acceptable.
>
>
> I asking about suggestion and/or review.
>
> > Alex
> >
> >>>/* For now we're only intereted in devices added and removed. I
> >>> didn't
> >>> * test this thing here, so someone needs to double check for the
> >>> @@ -1510,6 +1508,8 @@ static int pci_notify(struct notifier_block *nb,
> >>> unsigned long action,
> >>>else if (action ==
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi,
On 2023/6/16 05:11, Alex Deucher wrote:
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
as its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY0x03
#define PCI_CLASS_DISPLAY_VGA0x0300
#define PCI_CLASS_DISPLAY_XGA0x0301
#define PCI_CLASS_DISPLAY_3D0x0302
#define PCI_CLASS_DISPLAY_OTHER0x0380
```
Any ideas ?
I'm not quite sure what you are asking about here.
To be honest, I'm worried about the PCI devices which has a
PCI_CLASS_DISPLAY_XGA as its PCI class number.
As those devices are very uncommon in the real world.
$ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA"
Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO,
there no code reference this macro. So I think it seems safe to ignore
the XGA ?
PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate
the render-only GPU.
And render-only GPU can't decode the fixed VGA address space, it is safe
to ignore them.
For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them.
However, as VGA gets deprecated,
We need the vgaarb for a system with multiple video card.
Not only because some Legacy VGA devices implemented
on PCI will typically have the same "hard-decoded" addresses;
But also these video card need to participate in the arbitration,
determine the default boot device.
Nowadays, the 'VGA devices' here is stand for the Graphics card
which is capable of display something on the screen.
We still need vgaarb to select the default boot device.
you'll have more non VGA PCI classes for devices which
could be the pre-OS console device.
Ah, we still want do this(by applying this patch) first,
and then we will have the opportunity to see who will crying if
something is broken. Will know more then.
But drop this patch or revise it with more consideration is also
acceptable.
I asking about suggestion and/or review.
Alex
/* For now we're only intereted in devices added and removed. I didn't
* test this thing here, so someone needs to double check for the
@@ -1510,6 +1508,8 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
else if (action == BUS_NOTIFY_DEL_DEVICE)
notify = vga_arbiter_del_pci_device(pdev);
+ vgaarb_dbg(dev, "%s: action = %lu\n", __func__, action);
+
if (notify)
vga_arbiter_notify_clients();
return 0;
@@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
static int __init vga_arb_device_init(void)
{
+ struct pci_dev *pdev = NULL;
int rc;
- struct pci_dev *pdev;
rc = misc_register(_arb_device);
if (rc < 0)
@@ -1545,11 +1545,13 @@ static int __init vga_arb_device_init(void)
/* We add all PCI devices satisfying VGA class in the arbiter by
* default */
- pdev = NULL;
- while ((pdev =
- pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
-PCI_ANY_ID, pdev)) != NULL)
+ while (1) {
+ pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8,
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote:
>
> Hi,
>
> On 2023/6/13 11:01, Sui Jingfeng wrote:
> > From: Sui Jingfeng
> >
> > Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
> > pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
> > device(pdev->class != 0x0300) out. There no need to process the non-display
> > PCI device.
> >
> > Cc: Bjorn Helgaas
> > Signed-off-by: Sui Jingfeng
> > ---
> > drivers/pci/vgaarb.c | 22 --
> > 1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> > index c1bc6c983932..22a505e877dc 100644
> > --- a/drivers/pci/vgaarb.c
> > +++ b/drivers/pci/vgaarb.c
> > @@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
> > *pdev)
> > struct pci_dev *bridge;
> > u16 cmd;
> >
> > - /* Only deal with VGA class devices */
> > - if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> > - return false;
> > -
>
> Hi, here is probably a bug fixing.
>
> For an example, nvidia render only GPU typically has 0x0380.
>
> at its PCI class number, but render only GPU should not participate in
> the arbitration.
>
> As it shouldn't snoop the legacy fixed VGA address.
>
> It(render only GPU) can not display anything.
>
>
> But 0x0380 >> 8 = 0x03, the filter failed.
>
>
> > /* Allocate structure */
> > vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
> > if (vgadev == NULL) {
> > @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb,
> > unsigned long action,
> > struct pci_dev *pdev = to_pci_dev(dev);
> > bool notify = false;
> >
> > - vgaarb_dbg(dev, "%s\n", __func__);
> > + /* Only deal with VGA class devices */
> > + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
> > + return 0;
>
> So here we only care 0x0300, my initial intent is to make an optimization,
>
> nowadays sane display graphic card should all has 0x0300 as its PCI
> class number, is this complete right?
>
> ```
>
> #define PCI_BASE_CLASS_DISPLAY0x03
> #define PCI_CLASS_DISPLAY_VGA0x0300
> #define PCI_CLASS_DISPLAY_XGA0x0301
> #define PCI_CLASS_DISPLAY_3D0x0302
> #define PCI_CLASS_DISPLAY_OTHER0x0380
>
> ```
>
> Any ideas ?
I'm not quite sure what you are asking about here. For vga_arb, we
only care about VGA class devices since those should be on the only
ones that might have VGA routed to them. However, as VGA gets
deprecated, you'll have more non VGA PCI classes for devices which
could be the pre-OS console device.
Alex
>
> > /* For now we're only intereted in devices added and removed. I didn't
> >* test this thing here, so someone needs to double check for the
> > @@ -1510,6 +1508,8 @@ static int pci_notify(struct notifier_block *nb,
> > unsigned long action,
> > else if (action == BUS_NOTIFY_DEL_DEVICE)
> > notify = vga_arbiter_del_pci_device(pdev);
> >
> > + vgaarb_dbg(dev, "%s: action = %lu\n", __func__, action);
> > +
> > if (notify)
> > vga_arbiter_notify_clients();
> > return 0;
> > @@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
> >
> > static int __init vga_arb_device_init(void)
> > {
> > + struct pci_dev *pdev = NULL;
> > int rc;
> > - struct pci_dev *pdev;
> >
> > rc = misc_register(_arb_device);
> > if (rc < 0)
> > @@ -1545,11 +1545,13 @@ static int __init vga_arb_device_init(void)
> >
> > /* We add all PCI devices satisfying VGA class in the arbiter by
> >* default */
> > - pdev = NULL;
> > - while ((pdev =
> > - pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> > -PCI_ANY_ID, pdev)) != NULL)
> > + while (1) {
> > + pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev);
> > + if (!pdev)
> > + break;
> > +
> > vga_arbiter_add_pci_device(pdev);
> > + }
> >
> > pr_info("loaded\n");
> > return rc;
>
> --
> Jingfeng
>
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi,
On 2023/6/13 11:01, Sui Jingfeng wrote:
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
Hi, here is probably a bug fixing.
For an example, nvidia render only GPU typically has 0x0380.
at its PCI class number, but render only GPU should not participate in
the arbitration.
As it shouldn't snoop the legacy fixed VGA address.
It(render only GPU) can not display anything.
But 0x0380 >> 8 = 0x03, the filter failed.
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
So here we only care 0x0300, my initial intent is to make an optimization,
nowadays sane display graphic card should all has 0x0300 as its PCI
class number, is this complete right?
```
#define PCI_BASE_CLASS_DISPLAY 0x03
#define PCI_CLASS_DISPLAY_VGA 0x0300
#define PCI_CLASS_DISPLAY_XGA 0x0301
#define PCI_CLASS_DISPLAY_3D 0x0302
#define PCI_CLASS_DISPLAY_OTHER 0x0380
```
Any ideas ?
/* For now we're only intereted in devices added and removed. I didn't
* test this thing here, so someone needs to double check for the
@@ -1510,6 +1508,8 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
else if (action == BUS_NOTIFY_DEL_DEVICE)
notify = vga_arbiter_del_pci_device(pdev);
+ vgaarb_dbg(dev, "%s: action = %lu\n", __func__, action);
+
if (notify)
vga_arbiter_notify_clients();
return 0;
@@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
static int __init vga_arb_device_init(void)
{
+ struct pci_dev *pdev = NULL;
int rc;
- struct pci_dev *pdev;
rc = misc_register(_arb_device);
if (rc < 0)
@@ -1545,11 +1545,13 @@ static int __init vga_arb_device_init(void)
/* We add all PCI devices satisfying VGA class in the arbiter by
* default */
- pdev = NULL;
- while ((pdev =
- pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
- PCI_ANY_ID, pdev)) != NULL)
+ while (1) {
+ pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev);
+ if (!pdev)
+ break;
+
vga_arbiter_add_pci_device(pdev);
+ }
pr_info("loaded\n");
return rc;
--
Jingfeng
[PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
From: Sui Jingfeng
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
/* For now we're only intereted in devices added and removed. I didn't
* test this thing here, so someone needs to double check for the
@@ -1510,6 +1508,8 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
else if (action == BUS_NOTIFY_DEL_DEVICE)
notify = vga_arbiter_del_pci_device(pdev);
+ vgaarb_dbg(dev, "%s: action = %lu\n", __func__, action);
+
if (notify)
vga_arbiter_notify_clients();
return 0;
@@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
static int __init vga_arb_device_init(void)
{
+ struct pci_dev *pdev = NULL;
int rc;
- struct pci_dev *pdev;
rc = misc_register(_arb_device);
if (rc < 0)
@@ -1545,11 +1545,13 @@ static int __init vga_arb_device_init(void)
/* We add all PCI devices satisfying VGA class in the arbiter by
* default */
- pdev = NULL;
- while ((pdev =
- pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
- PCI_ANY_ID, pdev)) != NULL)
+ while (1) {
+ pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev);
+ if (!pdev)
+ break;
+
vga_arbiter_add_pci_device(pdev);
+ }
pr_info("loaded\n");
return rc;
--
2.25.1
[PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Deal only with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non-PCI display
device(pdev->class != 0x0300) out. There no need to process the non-display
PCI device.
Cc: Bjorn Helgaas
Signed-off-by: Sui Jingfeng
---
drivers/pci/vgaarb.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c1bc6c983932..22a505e877dc 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev
*pdev)
struct pci_dev *bridge;
u16 cmd;
- /* Only deal with VGA class devices */
- if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
- return false;
-
/* Allocate structure */
vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL);
if (vgadev == NULL) {
@@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
struct pci_dev *pdev = to_pci_dev(dev);
bool notify = false;
- vgaarb_dbg(dev, "%s\n", __func__);
+ /* Only deal with VGA class devices */
+ if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8)
+ return 0;
/* For now we're only intereted in devices added and removed. I didn't
* test this thing here, so someone needs to double check for the
@@ -1510,6 +1508,8 @@ static int pci_notify(struct notifier_block *nb, unsigned
long action,
else if (action == BUS_NOTIFY_DEL_DEVICE)
notify = vga_arbiter_del_pci_device(pdev);
+ vgaarb_dbg(dev, "%s: action = %lu\n", __func__, action);
+
if (notify)
vga_arbiter_notify_clients();
return 0;
@@ -1534,8 +1534,8 @@ static struct miscdevice vga_arb_device = {
static int __init vga_arb_device_init(void)
{
+ struct pci_dev *pdev = NULL;
int rc;
- struct pci_dev *pdev;
rc = misc_register(_arb_device);
if (rc < 0)
@@ -1545,11 +1545,13 @@ static int __init vga_arb_device_init(void)
/* We add all PCI devices satisfying VGA class in the arbiter by
* default */
- pdev = NULL;
- while ((pdev =
- pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
- PCI_ANY_ID, pdev)) != NULL)
+ while (1) {
+ pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev);
+ if (!pdev)
+ break;
+
vga_arbiter_add_pci_device(pdev);
+ }
pr_info("loaded\n");
return rc;
--
2.25.1
