From b3f2333de8e81b089262b26d52272911523e605f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:31 +0100 Subject: drm: restrict the device list for shadow attached drivers There's really no need for the drm core to keep a list of all devices of a given driver - the linux device model keeps perfect track of this already for us. The exception is old legacy ums drivers using pci shadow attaching. So rename the lists to make the use case clearer and rip out everything else. v2: Rebase on top of David Herrmann's drm device register changes. Also drop the bogus dev_set_drvdata for platform drivers that somehow crept into the original version - drivers really should be in full control of that field. v3: Initialize driver->legacy_dev_list outside of the loop, spotted by David Herrmann. v4: Rebase on top of the newly created host1x drm_bus for tegra. Cc: David Herrmann Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 02679793c9e2..efadad850288 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -346,6 +346,11 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, driver->name, driver->major, driver->minor, driver->patchlevel, driver->date, pci_name(pdev), dev->primary->index); + /* No locking needed since shadow-attach is single-threaded since it may + * only be called from the per-driver module init hook. */ + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list); + return 0; err_pci: @@ -375,7 +380,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) DRM_DEBUG("\n"); - INIT_LIST_HEAD(&driver->device_list); driver->kdriver.pci = pdriver; driver->bus = &drm_pci_bus; @@ -383,6 +387,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return pci_register_driver(pdriver); /* If not using KMS, fall back to stealth mode manual scanning. */ + INIT_LIST_HEAD(&driver->legacy_dev_list); for (i = 0; pdriver->id_table[i].vendor != 0; i++) { pid = &pdriver->id_table[i]; @@ -465,8 +470,11 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) if (driver->driver_features & DRIVER_MODESET) { pci_unregister_driver(pdriver); } else { - list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item) + list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list, + legacy_dev_list) { drm_put_dev(dev); + list_del(&dev->legacy_dev_list); + } } DRM_INFO("Module unloaded\n"); } -- cgit v1.2.3 From 24986ee06929a8de3a5b4722ccadf0b85c175264 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:33 +0100 Subject: drm: kill DRIVER_REQUIRE_AGP Only the two intel drivers need this and they can easily check for working agp support in their driver ->load callbacks. This is the only reason why agp initialization could fail, so allows us to rip out a bit of error handling code in the next patch. Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index efadad850288..c99c71b3d220 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -267,11 +267,6 @@ static int drm_pci_agp_init(struct drm_device *dev) if (drm_core_has_AGP(dev)) { if (drm_pci_device_is_agp(dev)) dev->agp = drm_agp_init(dev); - if (drm_core_check_feature(dev, DRIVER_REQUIRE_AGP) - && (dev->agp == NULL)) { - DRM_ERROR("Cannot initialize the agpgart module.\n"); - return -EINVAL; - } if (dev->agp) { dev->agp->agp_mtrr = arch_phys_wc_add( dev->agp->agp_info.aper_base, -- cgit v1.2.3 From 8da79ccd1aaa2efe482b2c555c4684c7b503864a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:34 +0100 Subject: drm: ->agp_init can't fail Thanks to the removal of REQUIRE_AGP we can use a void return value and shed a bit of complexity. Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index c99c71b3d220..d3875e3f9d9c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -262,7 +262,7 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p) return 0; } -static int drm_pci_agp_init(struct drm_device *dev) +static void drm_pci_agp_init(struct drm_device *dev) { if (drm_core_has_AGP(dev)) { if (drm_pci_device_is_agp(dev)) @@ -274,7 +274,6 @@ static int drm_pci_agp_init(struct drm_device *dev) 1024 * 1024); } } - return 0; } static void drm_pci_agp_destroy(struct drm_device *dev) -- cgit v1.2.3 From d9906753bb997d651beaba0e4026a873bd0e8340 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:35 +0100 Subject: drm: rip out drm_core_has_AGP Most place actually want to just check for dev->agp (most do, but a few don't so this fixes a few potential NULL derefs). The only exception is the agp init code which should check for the AGP driver feature flag. Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index d3875e3f9d9c..626e9cfd8aa5 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -264,7 +264,7 @@ static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p) static void drm_pci_agp_init(struct drm_device *dev) { - if (drm_core_has_AGP(dev)) { + if (drm_core_check_feature(dev, DRIVER_USE_AGP)) { if (drm_pci_device_is_agp(dev)) dev->agp = drm_agp_init(dev); if (dev->agp) { @@ -278,7 +278,7 @@ static void drm_pci_agp_init(struct drm_device *dev) static void drm_pci_agp_destroy(struct drm_device *dev) { - if (drm_core_has_AGP(dev) && dev->agp) { + if (dev->agp) { arch_phys_wc_del(dev->agp->agp_mtrr); drm_agp_clear(dev); drm_agp_destroy(dev->agp); -- cgit v1.2.3 From 2c695fa0444273c7139a3ca4c324c95498a0bfed Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:36 +0100 Subject: drm: remove agp_init() bus callback The PCI bus helper is the only user of it. Call it directly before device-registration to get rid of the callback. Note that all drm_agp_*() calls are locked with the drm-global-mutex so we need to explicitly lock it during initialization. It's not really clear why it's needed, but lets be safe. v2: Rebase on top of the agp_init interface change. v3: Remove the rebase-fail where I've accidentally killed the ->irq_by_busid callback a bit too early. Cc: David Herrmann Signed-off-by: David Herrmann (v1) Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 626e9cfd8aa5..2211c4d70c2d 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -293,7 +293,6 @@ static struct drm_bus drm_pci_bus = { .set_busid = drm_pci_set_busid, .set_unique = drm_pci_set_unique, .irq_by_busid = drm_pci_irq_by_busid, - .agp_init = drm_pci_agp_init, .agp_destroy = drm_pci_agp_destroy, }; @@ -332,9 +331,13 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, if (drm_core_check_feature(dev, DRIVER_MODESET)) pci_set_drvdata(pdev, dev); + mutex_lock(&drm_global_mutex); + drm_pci_agp_init(dev); + mutex_unlock(&drm_global_mutex); + ret = drm_dev_register(dev, ent->driver_data); if (ret) - goto err_pci; + goto err_agp; DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n", driver->name, driver->major, driver->minor, driver->patchlevel, @@ -347,7 +350,10 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, return 0; -err_pci: +err_agp: + mutex_lock(&drm_global_mutex); + drm_pci_agp_destroy(dev); + mutex_unlock(&drm_global_mutex); pci_disable_device(pdev); err_free: drm_dev_free(dev); -- cgit v1.2.3 From d6e4b28b60c5dae660aebe5cd731b21d02ca285e Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:37 +0100 Subject: drm: inline drm_agp_destroy Wrapping a kfree is pointless. v2: Add a comment to the kerneldoc for drm_agp_init to explain where the kfree happens as requested by David. Note that for modeset drivers agp cleanup is fairly complicated anyway: The drm_agp_clear is a noop and drivers must call drm_agp_release on their own. Which they all seem to do properly. Cc: David Herrmann Reviewed-by: David Herrmann Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 2211c4d70c2d..f710e3d9d847 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -281,7 +281,7 @@ static void drm_pci_agp_destroy(struct drm_device *dev) if (dev->agp) { arch_phys_wc_del(dev->agp->agp_mtrr); drm_agp_clear(dev); - drm_agp_destroy(dev->agp); + kfree(dev->agp); dev->agp = NULL; } } -- cgit v1.2.3 From 4efafebe709b8daa90b2f34a6e242eec7df98f25 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:38 +0100 Subject: drm: kill the ->agp_destroy callback Call drm_pci_agp_destroy directly, there's no point in the indirection. Long term we want to shuffle this into each driver's unload logic, but that needs cleared-up drm lifetime rules first. v2: Add a dummy function for !CONFIG_PCI, spotted my David Herrmann. v3: Fixup for the coding style police. Reviewed-by: David Herrmann Cc: David Herrmann Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index f710e3d9d847..6dfae6b3c0bc 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -276,7 +276,7 @@ static void drm_pci_agp_init(struct drm_device *dev) } } -static void drm_pci_agp_destroy(struct drm_device *dev) +void drm_pci_agp_destroy(struct drm_device *dev) { if (dev->agp) { arch_phys_wc_del(dev->agp->agp_mtrr); @@ -293,7 +293,6 @@ static struct drm_bus drm_pci_bus = { .set_busid = drm_pci_set_busid, .set_unique = drm_pci_set_unique, .irq_by_busid = drm_pci_irq_by_busid, - .agp_destroy = drm_pci_agp_destroy, }; /** @@ -457,6 +456,7 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return -1; } +void drm_pci_agp_destroy(struct drm_device *dev) {} #endif EXPORT_SYMBOL(drm_pci_init); -- cgit v1.2.3 From 8a5a80081a1cc8b4f7bdf777fde3d955a020481c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 11:34:39 +0100 Subject: drm: remove global_mutex locking around agp_init David Herrmann dutifully moved this locking along when moving the agp_init call out of the generic drm_dev_register into the pci specific load helpers. But afaict there's no need and the reason for that locking has been purely a historical accident - we need the lock around the driver dev node registration to paper over the midlayer init races, and the agp init simply ended up in there. The real fix for all this is of course to delay the dev (and sysfs/debugfs) interface registration until everything is fully set up. Until then stop the cargo-cult locking from spreading and remove the locking. Cc: David Herrmann Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_pci.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers/gpu/drm/drm_pci.c') diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 6dfae6b3c0bc..5736aaa7e86c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -330,9 +330,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, if (drm_core_check_feature(dev, DRIVER_MODESET)) pci_set_drvdata(pdev, dev); - mutex_lock(&drm_global_mutex); drm_pci_agp_init(dev); - mutex_unlock(&drm_global_mutex); ret = drm_dev_register(dev, ent->driver_data); if (ret) @@ -350,9 +348,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, return 0; err_agp: - mutex_lock(&drm_global_mutex); drm_pci_agp_destroy(dev); - mutex_unlock(&drm_global_mutex); pci_disable_device(pdev); err_free: drm_dev_free(dev); -- cgit v1.2.3