Re: [Intel-gfx] [PATCH v3 3/4] PCI/VGA: only deal with VGA class devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 2023/6/9 03:12, Bjorn Helgaas wrote:
Start with verb and capitalize to match ("Deal only with ...")

On Thu, Jun 08, 2023 at 07:43:21PM +0800, Sui Jingfeng wrote:
From: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>

vgaarb only deal with the VGA devcie(pdev->class == 0x0300), so replace the
pci_get_subsys() function with pci_get_class(). Filter the non pci display
device out. There no need to process the non display PCI device.
s/non pci/non-PCI/
s/non display/non-display/

Will be fixed at next version, thanks for the strict checking,

I will try to bring this rigorous to the other patch of myself in the future.

This is fine, and I'll merge this, but someday I would like to get rid
of the bus_register_notifier() and call vga_arbiter_add_pci_device()
and vga_arbiter_del_pci_device() directly from the PCI core.

Or if you wanted to do that now, that would be even better :)

I think, I probably should only do what I'm good at.


The "good at" here means that It's under prepared,

already matured with thinking(or consideration) by myself.

And also including testing(on two or three architecture).


That idea is belong to you, I would like to see how does it going to happen only.

Bjorn

Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
---
  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 7f0347f4f6fd..b0bf4952a95d 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -756,10 +756,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) {
@@ -1499,7 +1495,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 >> 8) != PCI_CLASS_DISPLAY_VGA)
+		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
@@ -1509,6 +1507,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;
@@ -1533,8 +1533,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(&vga_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

--
Jingfeng




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux