[PATCH] agp/amd64: Bind to unsupported devices only if AGP is present
Lukas Wunner
lukas at wunner.de
Tue Jul 1 18:18:56 UTC 2025
On Tue, Jun 24, 2025 at 11:54:46PM +0200, Ben Hutchings wrote:
> On Sat, 2025-06-21 at 16:05 +0200, Lukas Wunner wrote:
> > So please propose a more accurate explanation.
>
> Something like "The driver iterates over all PCI devices, checking for
> an AGP capability. Since commit 6fd024893911 ("amd64-agp: Probe unknown
> AGP devices the right way") this is done with driver_attach() and a
> wildcard PCI ID table, and the preparation for probing the IOMMU device
> produces this error message."
Thanks, I will respin and amend the commit message.
> Thinking about this further:
>
> - Why *does* the IOMMU device have resources assigned but no driver
> bound? Is that the real bug?
I don't really know, I was hoping the AMD IOMMU maintainers could
comment on that.
> - If not, and there's a general problem with this promiscuous probing,
> would it make more sense to:
> 1. Restore the search for an AGP capability in agp_amd64_init().
> 2. If and only if an AGP device is found, poke the appropriate device
> ID into agp_amd64_pci_promisc_table and then call driver_attach().
> ?
I like the idea. I've realized that we've got pci_add_dynid() for
just this sort of thing. It avoids the need to poke device IDs
into an array at runtime. The (as yet completely untested) patch
below should do the trick.
-- >8 --
diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index bf49096..9b9c473 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -720,11 +720,6 @@ static int agp_amd64_resume(struct device *dev)
MODULE_DEVICE_TABLE(pci, agp_amd64_pci_table);
-static const struct pci_device_id agp_amd64_pci_promisc_table[] = {
- { PCI_DEVICE_CLASS(0, 0) },
- { }
-};
-
static DEFINE_SIMPLE_DEV_PM_OPS(agp_amd64_pm_ops, NULL, agp_amd64_resume);
static struct pci_driver agp_amd64_pci_driver = {
@@ -739,7 +734,8 @@ static int agp_amd64_resume(struct device *dev)
/* Not static due to IOMMU code calling it early. */
int __init agp_amd64_init(void)
{
- int err = 0;
+ struct pci_dev *pdev = NULL;
+ int err, ret;
if (agp_off)
return -EINVAL;
@@ -767,8 +763,15 @@ int __init agp_amd64_init(void)
}
/* Look for any AGP bridge */
- agp_amd64_pci_driver.id_table = agp_amd64_pci_promisc_table;
- err = driver_attach(&agp_amd64_pci_driver.driver);
+ for_each_pci_dev(pdev)
+ if (pci_find_capability(pdev, PCI_CAP_ID_AGP)) {
+ ret = pci_add_dynid(&agp_amd64_pci_driver,
+ pdev->vendor, pdev->device,
+ pdev->subvendor,
+ pdev->subdevice, 0, 0, 0);
+ if (ret)
+ err = ret;
+ }
if (err == 0 && agp_bridges_found == 0) {
pci_unregister_driver(&agp_amd64_pci_driver);
err = -ENODEV;
More information about the dri-devel
mailing list