With this we can drop the final kfree from the release function.
The mock device in the selftests needed it's pci_device split
up from the drm_device. In the future we could simplify this again
by allocating the pci_device as a managed allocation too.
v2: I overlooked that i915_driver_destroy is also called in the
unwind code of the error path. There we need a drm_dev_put.
Similar for the mock object.
Now the problem with that is that the drm_driver->release callbacks
for both the real driver and the mock one assume everything has been
set up. Hence going through that path for a partially set up driver
will result in issues. Quickest fix is to disable the ->release() hook
until the driver is fully initialized, and keep the onion unwinding.
Long term would be cleanest to move everything over to drmm_ release
actions, but that's a lot of work for a big driver like i915. Plus
more core work needed first anyway.
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
Cc: Andi Shyti <andi.shyti@xxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
---
drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++-
drivers/gpu/drm/i915/i915_drv.h | 3 +++
.../gpu/drm/i915/selftests/mock_gem_device.c | 18 +++++++++++++++++-
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 050e4d7c6723..61d874b61cb3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -43,6 +43,7 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_ioctl.h>
#include <drm/drm_irq.h>
+#include <drm/drm_managed.h>
#include <drm/drm_probe_helper.h>
#include <drm/i915_drm.h>
@@ -1363,6 +1364,7 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
}
i915->drm.dev_private = i915;
+ drmm_add_final_kfree(&i915->drm, i915);
i915->drm.pdev = pdev;
pci_set_drvdata(pdev, i915);
@@ -1382,7 +1384,6 @@ static void i915_driver_destroy(struct drm_i915_private *i915)
struct pci_dev *pdev = i915->drm.pdev;
drm_dev_fini(&i915->drm);
- kfree(i915);
}
/**
@@ -1458,6 +1459,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
i915_welcome_messages(i915);
+ i915->do_release = true;
+
return 0;
out_cleanup_hw:
@@ -1474,6 +1477,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
out_fini:
i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
i915_driver_destroy(i915);
+ drm_dev_put(&i915->drm);
return ret;
}
@@ -1516,6 +1520,9 @@ static void i915_driver_release(struct drm_device *dev)
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
+ if (!dev_priv->do_release)
+ return;
+
disable_rpm_wakeref_asserts(rpm);
i915_gem_driver_release(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3330b538d379..6e984fd7ae77 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -884,6 +884,9 @@ struct i915_selftest_stash {
struct drm_i915_private {
struct drm_device drm;
+ /* FIXME: Device release actions should all be moved to drmm_ */
+ bool do_release;
+
const struct intel_device_info __info; /* Use INTEL_INFO() to access. */
struct intel_runtime_info __runtime; /* Use RUNTIME_INFO() to access. */
struct intel_driver_caps caps;
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 3b8986983afc..9b06d5ec889a 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -25,6 +25,8 @@
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
+#include <drm/drm_managed.h>
+
#include "gt/intel_gt.h"
#include "gt/intel_gt_requests.h"
#include "gt/mock_engine.h"
@@ -55,6 +57,9 @@ static void mock_device_release(struct drm_device *dev)
{
struct drm_i915_private *i915 = to_i915(dev);
+ if (!i915->do_release)
+ return;
+
mock_device_flush(i915);
intel_gt_driver_remove(&i915->gt);
@@ -114,9 +119,14 @@ struct drm_i915_private *mock_gem_device(void)
struct pci_dev *pdev;
int err;
- pdev = kzalloc(sizeof(*pdev) + sizeof(*i915), GFP_KERNEL);
+ pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
if (!pdev)
goto err;
+ i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
+ if (!i915) {
+ kfree(pdev);
+ goto err;
+ }
device_initialize(&pdev->dev);
pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
@@ -141,10 +151,13 @@ struct drm_i915_private *mock_gem_device(void)
err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
if (err) {
pr_err("Failed to initialise mock GEM device: err=%d\n", err);
+ kfree(i915);
+
goto put_device;
}
i915->drm.pdev = pdev;
i915->drm.dev_private = i915;
+ drmm_add_final_kfree(&i915->drm, i915);
intel_runtime_pm_init_early(&i915->runtime_pm);
@@ -189,6 +202,8 @@ struct drm_i915_private *mock_gem_device(void)
__clear_bit(I915_WEDGED, &i915->gt.reset.flags);
intel_engines_driver_register(i915);
+ i915->do_release = true;
+
return i915;
err_context:
@@ -200,6 +215,7 @@ struct drm_i915_private *mock_gem_device(void)
intel_memory_regions_driver_release(i915);
drm_mode_config_cleanup(&i915->drm);
drm_dev_fini(&i915->drm);
+ drm_dev_put(&i915->drm);
put_device:
put_device(&pdev->dev);
err:
--
2.24.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
