Hi,
On 2023/6/26 15:56, Thomas Zimmermann wrote:
Hi
Am 25.06.23 um 07:09 schrieb Sui Jingfeng:
As this function can be replaced with drm_dev_unregister() +
drm_dev_put(),
it is already marked as deprecated, so remove it. No functional change.
Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
---
drivers/gpu/drm/drm_drv.c | 28 ----------------------------
drivers/gpu/drm/drm_pci.c | 3 ++-
drivers/gpu/drm/radeon/radeon_drv.c | 3 ++-
include/drm/drm_drv.h | 1 -
4 files changed, 4 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 12687dd9e1ac..5057307fe22a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -406,34 +406,6 @@ void drm_minor_release(struct drm_minor *minor)
* possibly leaving the hardware enabled.
*/
-/**
- * drm_put_dev - Unregister and release a DRM device
- * @dev: DRM device
- *
- * Called at module unload time or when a PCI device is unplugged.
- *
- * Cleans up all DRM device, calling drm_lastclose().
- *
- * Note: Use of this function is deprecated. It will eventually go away
- * completely. Please use drm_dev_unregister() and drm_dev_put()
explicitly
- * instead to make sure that the device isn't userspace accessible
any more
- * while teardown is in progress, ensuring that userspace can't
access an
- * inconsistent state.
- */
-void drm_put_dev(struct drm_device *dev)
-{
- DRM_DEBUG("\n");
-
- if (!dev) {
- DRM_ERROR("cleanup called no dev\n");
- return;
- }
-
- drm_dev_unregister(dev);
- drm_dev_put(dev);
-}
-EXPORT_SYMBOL(drm_put_dev);
-
/**
* drm_dev_enter - Enter device critical section
* @dev: DRM device
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 39d35fc3a43b..b3a68a92eaa6 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -257,7 +257,8 @@ void drm_legacy_pci_exit(const struct drm_driver
*driver,
legacy_dev_list) {
if (dev->driver == driver) {
list_del(&dev->legacy_dev_list);
- drm_put_dev(dev);
+ drm_dev_unregister(dev);
+ drm_dev_put(dev);
}
}
mutex_unlock(&legacy_dev_list_lock);
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index e4374814f0ef..a4955ae10659 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -357,7 +357,8 @@ radeon_pci_remove(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
- drm_put_dev(dev);
Did you verify that dev cannot be NULL here? There was a check in
drm_put_dev() for !dev.
I have verified :
1)
If there is no radeon card(say R5-340) mounted in the system, I
modprobe the radeon.ko manually.
then both the radeon_pci_probe() and the radeon_pci_remove() function
won't get called.
There is no chance that the driver_probe_device() function getting called.
|- driver_register()
|-- bus_add_driver()
|--- driver_attach()
|---- bus_for_each_dev(drv->bus, NULL, drv, __driver_attach)
|----- __driver_attach()
|------ __device_attach_driver()
// There is no chance that the driver_probe_device() function get called.
|------- driver_probe_device(drv, dev)
```
2) normal case:
If there are radeon cards mounted in the system,
then as long as the pci_set_drvdata(pdev, dev) get called,
the 'driver_data' member of struct device will hold the pointer to the
'struct drm_device';
So, it will be fine as long as the radeon.ko get loaded normally.
I'm sure it will works as expected on normal case, with 100% confident.
3) Abnormal case
If there is a error happen before the 'pci_set_drvdata(pdev, dev)'
function get called.
It is also don't need to worry, if the ->probe() failed, then the
->remove will be get called.
I have verified that
if the ->probe() failed, then the ->remove will be get called.
I'm doing the test by add a line before the drm_dev_alloc()
function in the body of radeon_pci_probe() function.
See below:
```
return -ENODEV;
dev = drm_dev_alloc(&kms_driver, &pdev->dev);
if (IS_ERR(dev))
return PTR_ERR(dev);
ret = pci_enable_device(pdev);
if (ret)
goto err_free;
```
So, there is no problem, as far as I can see.
Best regards
Thomas
+ drm_dev_unregister(dev);
+ drm_dev_put(dev);
}
static void
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 89e2706cac56..289c97b12e82 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -511,7 +511,6 @@ void drm_dev_unregister(struct drm_device *dev);
void drm_dev_get(struct drm_device *dev);
void drm_dev_put(struct drm_device *dev);
-void drm_put_dev(struct drm_device *dev);
bool drm_dev_enter(struct drm_device *dev, int *idx);
void drm_dev_exit(int idx);
void drm_dev_unplug(struct drm_device *dev);
--
Jingfeng