Commit 7fb81e9d authored by Daniel Vetter's avatar Daniel Vetter

drm/i915: Use drmm_add_final_kfree

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.

v3: Fix i915_drm pointer wrangling in mock_gem_device. Also switch
over to start using drm_dev_put() to clean up even on the error path.
Aside I think the current error path is leaking the allocation.

v4: more fixes for intel-gfx-ci, some if it damage from v3 :-/
Reviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Andi Shyti <andi.shyti@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Link: https://patchwork.freedesktop.org/patch/msgid/20200323144950.3018436-9-daniel.vetter@ffwll.ch
parent c1b164a5
...@@ -43,6 +43,7 @@ ...@@ -43,6 +43,7 @@
#include <drm/drm_atomic_helper.h> #include <drm/drm_atomic_helper.h>
#include <drm/drm_ioctl.h> #include <drm/drm_ioctl.h>
#include <drm/drm_irq.h> #include <drm/drm_irq.h>
#include <drm/drm_managed.h>
#include <drm/drm_probe_helper.h> #include <drm/drm_probe_helper.h>
#include "display/intel_acpi.h" #include "display/intel_acpi.h"
...@@ -890,6 +891,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) ...@@ -890,6 +891,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
return ERR_PTR(err); return ERR_PTR(err);
} }
drmm_add_final_kfree(&i915->drm, i915);
i915->drm.pdev = pdev; i915->drm.pdev = pdev;
pci_set_drvdata(pdev, i915); pci_set_drvdata(pdev, i915);
...@@ -906,7 +909,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) ...@@ -906,7 +909,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
static void i915_driver_destroy(struct drm_i915_private *i915) static void i915_driver_destroy(struct drm_i915_private *i915)
{ {
drm_dev_fini(&i915->drm); drm_dev_fini(&i915->drm);
kfree(i915);
} }
/** /**
...@@ -990,6 +992,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) ...@@ -990,6 +992,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
i915_welcome_messages(i915); i915_welcome_messages(i915);
i915->do_release = true;
return 0; return 0;
out_cleanup_irq: out_cleanup_irq:
...@@ -1010,6 +1014,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) ...@@ -1010,6 +1014,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
out_fini: out_fini:
i915_probe_error(i915, "Device initialization failed (%d)\n", ret); i915_probe_error(i915, "Device initialization failed (%d)\n", ret);
i915_driver_destroy(i915); i915_driver_destroy(i915);
drm_dev_put(&i915->drm);
return ret; return ret;
} }
...@@ -1049,6 +1054,9 @@ static void i915_driver_release(struct drm_device *dev) ...@@ -1049,6 +1054,9 @@ static void i915_driver_release(struct drm_device *dev)
struct drm_i915_private *dev_priv = to_i915(dev); struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_runtime_pm *rpm = &dev_priv->runtime_pm; struct intel_runtime_pm *rpm = &dev_priv->runtime_pm;
if (!dev_priv->do_release)
return;
disable_rpm_wakeref_asserts(rpm); disable_rpm_wakeref_asserts(rpm);
i915_gem_driver_release(dev_priv); i915_gem_driver_release(dev_priv);
......
...@@ -823,6 +823,9 @@ struct i915_selftest_stash { ...@@ -823,6 +823,9 @@ struct i915_selftest_stash {
struct drm_i915_private { struct drm_i915_private {
struct drm_device drm; 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. */ const struct intel_device_info __info; /* Use INTEL_INFO() to access. */
struct intel_runtime_info __runtime; /* Use RUNTIME_INFO() to access. */ struct intel_runtime_info __runtime; /* Use RUNTIME_INFO() to access. */
struct intel_driver_caps caps; struct intel_driver_caps caps;
......
...@@ -25,6 +25,8 @@ ...@@ -25,6 +25,8 @@
#include <linux/pm_domain.h> #include <linux/pm_domain.h>
#include <linux/pm_runtime.h> #include <linux/pm_runtime.h>
#include <drm/drm_managed.h>
#include "gt/intel_gt.h" #include "gt/intel_gt.h"
#include "gt/intel_gt_requests.h" #include "gt/intel_gt_requests.h"
#include "gt/mock_engine.h" #include "gt/mock_engine.h"
...@@ -55,6 +57,9 @@ static void mock_device_release(struct drm_device *dev) ...@@ -55,6 +57,9 @@ static void mock_device_release(struct drm_device *dev)
{ {
struct drm_i915_private *i915 = to_i915(dev); struct drm_i915_private *i915 = to_i915(dev);
if (!i915->do_release)
goto out;
mock_device_flush(i915); mock_device_flush(i915);
intel_gt_driver_remove(&i915->gt); intel_gt_driver_remove(&i915->gt);
...@@ -72,7 +77,9 @@ static void mock_device_release(struct drm_device *dev) ...@@ -72,7 +77,9 @@ static void mock_device_release(struct drm_device *dev)
drm_mode_config_cleanup(&i915->drm); drm_mode_config_cleanup(&i915->drm);
drm_dev_fini(&i915->drm); drm_dev_fini(&i915->drm);
out:
put_device(&i915->drm.pdev->dev); put_device(&i915->drm.pdev->dev);
i915->drm.pdev = NULL;
} }
static struct drm_driver mock_driver = { static struct drm_driver mock_driver = {
...@@ -114,9 +121,14 @@ struct drm_i915_private *mock_gem_device(void) ...@@ -114,9 +121,14 @@ struct drm_i915_private *mock_gem_device(void)
struct pci_dev *pdev; struct pci_dev *pdev;
int err; int err;
pdev = kzalloc(sizeof(*pdev) + sizeof(*i915), GFP_KERNEL); pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
if (!pdev) if (!pdev)
goto err; return NULL;
i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
if (!i915) {
kfree(pdev);
return NULL;
}
device_initialize(&pdev->dev); device_initialize(&pdev->dev);
pdev->class = PCI_BASE_CLASS_DISPLAY << 16; pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
...@@ -129,7 +141,6 @@ struct drm_i915_private *mock_gem_device(void) ...@@ -129,7 +141,6 @@ struct drm_i915_private *mock_gem_device(void)
pdev->dev.archdata.iommu = (void *)-1; pdev->dev.archdata.iommu = (void *)-1;
#endif #endif
i915 = (struct drm_i915_private *)(pdev + 1);
pci_set_drvdata(pdev, i915); pci_set_drvdata(pdev, i915);
dev_pm_domain_set(&pdev->dev, &pm_domain); dev_pm_domain_set(&pdev->dev, &pm_domain);
...@@ -141,9 +152,13 @@ struct drm_i915_private *mock_gem_device(void) ...@@ -141,9 +152,13 @@ struct drm_i915_private *mock_gem_device(void)
err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev); err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
if (err) { if (err) {
pr_err("Failed to initialise mock GEM device: err=%d\n", err); pr_err("Failed to initialise mock GEM device: err=%d\n", err);
goto put_device; put_device(&pdev->dev);
kfree(i915);
return NULL;
} }
i915->drm.pdev = pdev; i915->drm.pdev = pdev;
drmm_add_final_kfree(&i915->drm, i915);
intel_runtime_pm_init_early(&i915->runtime_pm); intel_runtime_pm_init_early(&i915->runtime_pm);
...@@ -188,6 +203,8 @@ struct drm_i915_private *mock_gem_device(void) ...@@ -188,6 +203,8 @@ struct drm_i915_private *mock_gem_device(void)
__clear_bit(I915_WEDGED, &i915->gt.reset.flags); __clear_bit(I915_WEDGED, &i915->gt.reset.flags);
intel_engines_driver_register(i915); intel_engines_driver_register(i915);
i915->do_release = true;
return i915; return i915;
err_context: err_context:
...@@ -199,8 +216,7 @@ struct drm_i915_private *mock_gem_device(void) ...@@ -199,8 +216,7 @@ struct drm_i915_private *mock_gem_device(void)
intel_memory_regions_driver_release(i915); intel_memory_regions_driver_release(i915);
drm_mode_config_cleanup(&i915->drm); drm_mode_config_cleanup(&i915->drm);
drm_dev_fini(&i915->drm); drm_dev_fini(&i915->drm);
put_device: drm_dev_put(&i915->drm);
put_device(&pdev->dev);
err:
return NULL; return NULL;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment