amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] managed drm_device, absolute final leftover bits
@ 2020-09-18 13:25 Daniel Vetter
  2020-09-18 13:25 ` [PATCH 1/4] drm/i915/selftest: Create mock_destroy_device Daniel Vetter
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 13:25 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, amd-gfx

Hi all,

These are the leftovers of the leftovers of my initial drmm series to
manage drm_device.

Changes:
- bugfixed i915 selftests
- patch from Luben to finalize the admgpu conversion

Alex & i915 maintainers, pls ack for merging this all through
drm-misc-next since otherwise the final patch (and the resulting confusion
with outdated docs) is held up another round.

Cheers, Daniel

Daniel Vetter (3):
  drm/i915/selftest: Create mock_destroy_device
  drm/i915/selftests: align more to real device lifetimes
  drm/dev: Remove drm_dev_init

Luben Tuikov (1):
  drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2)

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 16 ++----
 drivers/gpu/drm/drm_drv.c                     | 41 ++--------------
 drivers/gpu/drm/drm_internal.h                |  1 +
 drivers/gpu/drm/drm_managed.c                 | 13 -----
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
 .../drm/i915/gem/selftests/i915_gem_context.c |  2 +-
 .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  2 +-
 .../drm/i915/gem/selftests/i915_gem_object.c  |  2 +-
 .../drm/i915/gem/selftests/i915_gem_phys.c    |  2 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
 .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c     |  2 +-
 .../drm/i915/selftests/intel_memory_region.c  |  2 +-
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 49 ++++++++++++-------
 .../gpu/drm/i915/selftests/mock_gem_device.h  |  2 +
 include/drm/drm_drv.h                         |  4 --
 18 files changed, 51 insertions(+), 97 deletions(-)

-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4] drm/i915/selftest: Create mock_destroy_device
  2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
@ 2020-09-18 13:25 ` Daniel Vetter
  2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 13:25 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx, Daniel Vetter

Just some prep work before we rework the lifetime handling, which
requires replacing all the drm_dev_put in selftests by something else.

v2: Don't go with a static inline, upsets the header tests and
separation.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/gem/selftests/huge_pages.c       | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c  | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c  | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c    | 2 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c           | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_evict.c       | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c         | 2 +-
 drivers/gpu/drm/i915/selftests/i915_request.c         | 2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c             | 2 +-
 drivers/gpu/drm/i915/selftests/intel_memory_region.c  | 2 +-
 drivers/gpu/drm/i915/selftests/mock_gem_device.c      | 7 ++++++-
 drivers/gpu/drm/i915/selftests/mock_gem_device.h      | 2 ++
 13 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 5daf4a2be422..1f35e71429b4 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1617,7 +1617,7 @@ int i915_gem_huge_page_mock_selftests(void)
 out_put:
 	i915_vm_put(&ppgtt->vm);
 out_unlock:
-	drm_dev_put(&dev_priv->drm);
+	mock_destroy_device(dev_priv);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 99becb86abd3..d3f87dc4eda3 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1997,7 +1997,7 @@ int i915_gem_context_mock_selftests(void)
 
 	err = i915_subtests(tests, i915);
 
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index 2a52b92586b9..0845ce1ae37c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -272,7 +272,7 @@ int i915_gem_dmabuf_mock_selftests(void)
 
 	err = i915_subtests(tests, i915);
 
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
index faa5b6d91795..bf853c40ec65 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object.c
@@ -85,7 +85,7 @@ int i915_gem_object_mock_selftests(void)
 
 	err = i915_subtests(tests, i915);
 
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
index a94243dc4c5c..8cee68c6a6dc 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_phys.c
@@ -73,6 +73,6 @@ int i915_gem_phys_mock_selftests(void)
 
 	err = i915_subtests(tests, i915);
 
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index 96d164a3841d..19c2cb166e7c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -158,7 +158,7 @@ static int mock_hwsp_freelist(void *arg)
 		__mock_hwsp_record(&state, na, NULL);
 	kfree(state.history);
 err_put:
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 028baae9631f..f88473d396f4 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -536,7 +536,7 @@ int i915_gem_evict_mock_selftests(void)
 	with_intel_runtime_pm(&i915->runtime_pm, wakeref)
 		err = i915_subtests(tests, &i915->gt);
 
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index af8205a2bd8f..c53a222e3dec 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1727,7 +1727,7 @@ int i915_gem_gtt_mock_selftests(void)
 	mock_fini_ggtt(ggtt);
 	kfree(ggtt);
 out_put:
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
index 3092ca763789..64bbb8288249 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -527,7 +527,7 @@ int i915_request_mock_selftests(void)
 	with_intel_runtime_pm(&i915->runtime_pm, wakeref)
 		err = i915_subtests(tests, i915);
 
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 
 	return err;
 }
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
index 88c5e9acb84c..1b6125e4c1ac 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -841,7 +841,7 @@ int i915_vma_mock_selftests(void)
 	mock_fini_ggtt(ggtt);
 	kfree(ggtt);
 out_put:
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index 93a38a323584..334b0648e253 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -791,7 +791,7 @@ int intel_memory_region_mock_selftests(void)
 
 	intel_memory_region_put(mem);
 out_unref:
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 397c313a8b69..ac600d395c8f 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -222,7 +222,12 @@ struct drm_i915_private *mock_gem_device(void)
 	intel_gt_driver_late_release(&i915->gt);
 	intel_memory_regions_driver_release(i915);
 	drm_mode_config_cleanup(&i915->drm);
-	drm_dev_put(&i915->drm);
+	mock_destroy_device(i915);
 
 	return NULL;
 }
+
+void mock_destroy_device(struct drm_i915_private *i915)
+{
+	drm_dev_put(&i915->drm);
+}
diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.h b/drivers/gpu/drm/i915/selftests/mock_gem_device.h
index b5dc4e394555..953cfe4fab34 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.h
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.h
@@ -7,4 +7,6 @@ struct drm_i915_private;
 struct drm_i915_private *mock_gem_device(void);
 void mock_device_flush(struct drm_i915_private *i915);
 
+void mock_destroy_device(struct drm_i915_private *i915);
+
 #endif /* !__MOCK_GEM_DEVICE_H__ */
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
  2020-09-18 13:25 ` [PATCH 1/4] drm/i915/selftest: Create mock_destroy_device Daniel Vetter
@ 2020-09-18 13:25 ` Daniel Vetter
  2020-09-18 17:50   ` Matthew Auld
                     ` (3 more replies)
  2020-09-18 13:25 ` [PATCH 3/4] drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2) Daniel Vetter
                   ` (2 subsequent siblings)
  4 siblings, 4 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 13:25 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx, Daniel Vetter, Matthew Auld

The big change is device_add so that device_del can auto-cleanup
devres resources. This allows us to use devm_drm_dev_alloc, which
removes the last user of drm_dev_init.

v2: Rebased

v3: use devres_open/release_group so we can use devm without real
hacks in the driver core or having to create an entire fake bus for
testing drivers. Might want to extract this into helpers eventually,
maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.

v4:
- Fix IS_ERR handling (Matt)
- Delete surplus put_device() in mock_device_release (intel-gfx-ci)

Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 44 +++++++++++--------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index ac600d395c8f..816f9af15fb3 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
 
 out:
 	i915_params_free(&i915->params);
-	put_device(&i915->drm.pdev->dev);
-	i915->drm.pdev = NULL;
 }
 
 static struct drm_driver mock_driver = {
@@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
 	pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
 	if (!pdev)
 		return NULL;
-	i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-	if (!i915) {
-		kfree(pdev);
-		return NULL;
-	}
-
 	device_initialize(&pdev->dev);
 	pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
 	pdev->dev.release = release_dev;
@@ -144,8 +136,29 @@ struct drm_i915_private *mock_gem_device(void)
 	/* HACK to disable iommu for the fake device; force identity mapping */
 	pdev->dev.iommu = &fake_iommu;
 #endif
+	err = device_add(&pdev->dev);
+	if (err) {
+		kfree(pdev);
+		return NULL;
+	}
+
+	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
+		device_del(&pdev->dev);
+		return NULL;
+	}
+
+	i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
+				  struct drm_i915_private, drm);
+	if (IS_ERR(i915)) {
+		pr_err("Failed to allocate mock GEM device: err=%d\n", err);
+		devres_release_group(&pdev->dev, NULL);
+		device_del(&pdev->dev);
+
+		return NULL;
+	}
 
 	pci_set_drvdata(pdev, i915);
+	i915->drm.pdev = pdev;
 
 	dev_pm_domain_set(&pdev->dev, &pm_domain);
 	pm_runtime_enable(&pdev->dev);
@@ -153,16 +166,6 @@ struct drm_i915_private *mock_gem_device(void)
 	if (pm_runtime_enabled(&pdev->dev))
 		WARN_ON(pm_runtime_get_sync(&pdev->dev));
 
-	err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
-	if (err) {
-		pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-		put_device(&pdev->dev);
-		kfree(i915);
-
-		return NULL;
-	}
-	i915->drm.pdev = pdev;
-	drmm_add_final_kfree(&i915->drm, i915);
 
 	i915_params_copy(&i915->params, &i915_modparams);
 
@@ -229,5 +232,8 @@ struct drm_i915_private *mock_gem_device(void)
 
 void mock_destroy_device(struct drm_i915_private *i915)
 {
-	drm_dev_put(&i915->drm);
+	struct device *dev = i915->drm.dev;
+
+	devres_release_group(dev, NULL);
+	device_del(dev);
 }
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2)
  2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
  2020-09-18 13:25 ` [PATCH 1/4] drm/i915/selftest: Create mock_destroy_device Daniel Vetter
  2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
@ 2020-09-18 13:25 ` Daniel Vetter
  2020-09-18 13:25 ` [PATCH 4/4] drm/dev: Remove drm_dev_init Daniel Vetter
  2020-09-18 15:03 ` [PATCH 0/4] managed drm_device, absolute final leftover bits Alex Deucher
  4 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 13:25 UTC (permalink / raw)
  To: DRI Development
  Cc: Alex Deucher, Daniel Vetter, Intel Graphics Development,
	Luben Tuikov, amd-gfx

From: Luben Tuikov <luben.tuikov@amd.com>

Convert to using devm_drm_dev_alloc(),
as drm_dev_init() is going away.

v2: Remove drm_dev_put() since
    a) devres doesn't do refcounting, see
    Documentation/driver-api/driver-model/devres.rst,
    Section 4, paragraph 1; and since
    b) devres acts as garbage collector when
    the DRM device's parent's devres "action" callback
    is called to free the container device (amdgpu_device),
    which embeds the DRM dev.

Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6edde2b9e402..3ded6f43f982 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1142,25 +1142,20 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
-	if (!adev)
-		return -ENOMEM;
+	adev = devm_drm_dev_alloc(&pdev->dev, &kms_driver, typeof(*adev), ddev);
+	if (IS_ERR(adev))
+		return PTR_ERR(adev);
 
 	adev->dev  = &pdev->dev;
 	adev->pdev = pdev;
 	ddev = adev_to_drm(adev);
-	ret = drm_dev_init(ddev, &kms_driver, &pdev->dev);
-	if (ret)
-		goto err_free;
-
-	drmm_add_final_kfree(ddev, adev);
 
 	if (!supports_atomic)
 		ddev->driver_features &= ~DRIVER_ATOMIC;
 
 	ret = pci_enable_device(pdev);
 	if (ret)
-		goto err_free;
+		return ret;
 
 	ddev->pdev = pdev;
 	pci_set_drvdata(pdev, ddev);
@@ -1188,8 +1183,6 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 
 err_pci:
 	pci_disable_device(pdev);
-err_free:
-	drm_dev_put(ddev);
 	return ret;
 }
 
@@ -1206,7 +1199,6 @@ amdgpu_pci_remove(struct pci_dev *pdev)
 	amdgpu_driver_unload_kms(dev);
 	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
-	drm_dev_put(dev);
 }
 
 static void
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] drm/dev: Remove drm_dev_init
  2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
                   ` (2 preceding siblings ...)
  2020-09-18 13:25 ` [PATCH 3/4] drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2) Daniel Vetter
@ 2020-09-18 13:25 ` Daniel Vetter
  2020-09-18 14:26   ` Thomas Zimmermann
  2020-09-18 15:03 ` [PATCH 0/4] managed drm_device, absolute final leftover bits Alex Deucher
  4 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 13:25 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, amd-gfx

We can now also delete drm_dev_init, now that vkms, vgem and i915
selftests are resolved.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_drv.c      | 41 +++-------------------------------
 drivers/gpu/drm/drm_internal.h |  1 +
 drivers/gpu/drm/drm_managed.c  | 13 -----------
 include/drm/drm_drv.h          |  4 ----
 4 files changed, 4 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7c1689842ec0..457ac0f82be2 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -573,43 +573,9 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
 	drm_legacy_destroy_members(dev);
 }
 
-/**
- * drm_dev_init - Initialise new DRM device
- * @dev: DRM device
- * @driver: DRM driver
- * @parent: Parent device object
- *
- * Initialize a new DRM device. No device registration is done.
- * Call drm_dev_register() to advertice the device to user space and register it
- * with other core subsystems. This should be done last in the device
- * initialization sequence to make sure userspace can't access an inconsistent
- * state.
- *
- * The initial ref-count of the object is 1. Use drm_dev_get() and
- * drm_dev_put() to take and drop further ref-counts.
- *
- * It is recommended that drivers embed &struct drm_device into their own device
- * structure.
- *
- * Drivers that do not want to allocate their own device struct
- * embedding &struct drm_device can call drm_dev_alloc() instead. For drivers
- * that do embed &struct drm_device it must be placed first in the overall
- * structure, and the overall structure must be allocated using kmalloc(): The
- * drm core's release function unconditionally calls kfree() on the @dev pointer
- * when the final reference is released. To override this behaviour, and so
- * allow embedding of the drm_device inside the driver's device struct at an
- * arbitrary offset, you must supply a &drm_driver.release callback and control
- * the finalization explicitly.
- *
- * Note that drivers must call drmm_add_final_kfree() after this function has
- * completed successfully.
- *
- * RETURNS:
- * 0 on success, or error code on failure.
- */
-int drm_dev_init(struct drm_device *dev,
-		 struct drm_driver *driver,
-		 struct device *parent)
+static int drm_dev_init(struct drm_device *dev,
+			struct drm_driver *driver,
+			struct device *parent)
 {
 	int ret;
 
@@ -689,7 +655,6 @@ int drm_dev_init(struct drm_device *dev,
 
 	return ret;
 }
-EXPORT_SYMBOL(drm_dev_init);
 
 static void devm_drm_dev_init_release(void *data)
 {
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 8e01caaf95cc..b65865c630b0 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -95,6 +95,7 @@ void drm_minor_release(struct drm_minor *minor);
 
 /* drm_managed.c */
 void drm_managed_release(struct drm_device *dev);
+void drmm_add_final_kfree(struct drm_device *dev, void *container);
 
 /* drm_vblank.c */
 static inline bool drm_vblank_passed(u64 seq, u64 ref)
diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
index c36e3d98fd71..37d7db6223be 100644
--- a/drivers/gpu/drm/drm_managed.c
+++ b/drivers/gpu/drm/drm_managed.c
@@ -125,18 +125,6 @@ static void add_dr(struct drm_device *dev, struct drmres *dr)
 		       dr, dr->node.name, (unsigned long) dr->node.size);
 }
 
-/**
- * drmm_add_final_kfree - add release action for the final kfree()
- * @dev: DRM device
- * @container: pointer to the kmalloc allocation containing @dev
- *
- * Since the allocation containing the struct &drm_device must be allocated
- * before it can be initialized with drm_dev_init() there's no way to allocate
- * that memory with drmm_kmalloc(). To side-step this chicken-egg problem the
- * pointer for this final kfree() must be specified by calling this function. It
- * will be released in the final drm_dev_put() for @dev, after all other release
- * actions installed through drmm_add_action() have been processed.
- */
 void drmm_add_final_kfree(struct drm_device *dev, void *container)
 {
 	WARN_ON(dev->managed.final_kfree);
@@ -144,7 +132,6 @@ void drmm_add_final_kfree(struct drm_device *dev, void *container)
 	WARN_ON(dev + 1 > (struct drm_device *) (container + ksize(container)));
 	dev->managed.final_kfree = container;
 }
-EXPORT_SYMBOL(drmm_add_final_kfree);
 
 int __drmm_add_action(struct drm_device *dev,
 		      drmres_release_t action,
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 533c6e1a5a95..b8ce9147c9a6 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -591,10 +591,6 @@ struct drm_driver {
 	int dev_priv_size;
 };
 
-int drm_dev_init(struct drm_device *dev,
-		 struct drm_driver *driver,
-		 struct device *parent);
-
 void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver,
 			   size_t size, size_t offset);
 
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/4] drm/dev: Remove drm_dev_init
  2020-09-18 13:25 ` [PATCH 4/4] drm/dev: Remove drm_dev_init Daniel Vetter
@ 2020-09-18 14:26   ` Thomas Zimmermann
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Zimmermann @ 2020-09-18 14:26 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development; +Cc: Intel Graphics Development, amd-gfx


[-- Attachment #1.1.1: Type: text/plain, Size: 5618 bytes --]



Am 18.09.20 um 15:25 schrieb Daniel Vetter:
> We can now also delete drm_dev_init, now that vkms, vgem and i915
> selftests are resolved.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>  drivers/gpu/drm/drm_drv.c      | 41 +++-------------------------------
>  drivers/gpu/drm/drm_internal.h |  1 +
>  drivers/gpu/drm/drm_managed.c  | 13 -----------
>  include/drm/drm_drv.h          |  4 ----
>  4 files changed, 4 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7c1689842ec0..457ac0f82be2 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -573,43 +573,9 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
>  	drm_legacy_destroy_members(dev);
>  }
>  
> -/**
> - * drm_dev_init - Initialise new DRM device
> - * @dev: DRM device
> - * @driver: DRM driver
> - * @parent: Parent device object
> - *
> - * Initialize a new DRM device. No device registration is done.
> - * Call drm_dev_register() to advertice the device to user space and register it
> - * with other core subsystems. This should be done last in the device
> - * initialization sequence to make sure userspace can't access an inconsistent
> - * state.
> - *
> - * The initial ref-count of the object is 1. Use drm_dev_get() and
> - * drm_dev_put() to take and drop further ref-counts.
> - *
> - * It is recommended that drivers embed &struct drm_device into their own device
> - * structure.
> - *
> - * Drivers that do not want to allocate their own device struct
> - * embedding &struct drm_device can call drm_dev_alloc() instead. For drivers
> - * that do embed &struct drm_device it must be placed first in the overall
> - * structure, and the overall structure must be allocated using kmalloc(): The
> - * drm core's release function unconditionally calls kfree() on the @dev pointer
> - * when the final reference is released. To override this behaviour, and so
> - * allow embedding of the drm_device inside the driver's device struct at an
> - * arbitrary offset, you must supply a &drm_driver.release callback and control
> - * the finalization explicitly.
> - *
> - * Note that drivers must call drmm_add_final_kfree() after this function has
> - * completed successfully.
> - *
> - * RETURNS:
> - * 0 on success, or error code on failure.
> - */
> -int drm_dev_init(struct drm_device *dev,
> -		 struct drm_driver *driver,
> -		 struct device *parent)
> +static int drm_dev_init(struct drm_device *dev,
> +			struct drm_driver *driver,
> +			struct device *parent)
>  {
>  	int ret;
>  
> @@ -689,7 +655,6 @@ int drm_dev_init(struct drm_device *dev,
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(drm_dev_init);
>  
>  static void devm_drm_dev_init_release(void *data)
>  {
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 8e01caaf95cc..b65865c630b0 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -95,6 +95,7 @@ void drm_minor_release(struct drm_minor *minor);
>  
>  /* drm_managed.c */
>  void drm_managed_release(struct drm_device *dev);
> +void drmm_add_final_kfree(struct drm_device *dev, void *container);
>  
>  /* drm_vblank.c */
>  static inline bool drm_vblank_passed(u64 seq, u64 ref)
> diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c
> index c36e3d98fd71..37d7db6223be 100644
> --- a/drivers/gpu/drm/drm_managed.c
> +++ b/drivers/gpu/drm/drm_managed.c
> @@ -125,18 +125,6 @@ static void add_dr(struct drm_device *dev, struct drmres *dr)
>  		       dr, dr->node.name, (unsigned long) dr->node.size);
>  }
>  
> -/**
> - * drmm_add_final_kfree - add release action for the final kfree()
> - * @dev: DRM device
> - * @container: pointer to the kmalloc allocation containing @dev
> - *
> - * Since the allocation containing the struct &drm_device must be allocated
> - * before it can be initialized with drm_dev_init() there's no way to allocate
> - * that memory with drmm_kmalloc(). To side-step this chicken-egg problem the
> - * pointer for this final kfree() must be specified by calling this function. It
> - * will be released in the final drm_dev_put() for @dev, after all other release
> - * actions installed through drmm_add_action() have been processed.
> - */
>  void drmm_add_final_kfree(struct drm_device *dev, void *container)
>  {
>  	WARN_ON(dev->managed.final_kfree);
> @@ -144,7 +132,6 @@ void drmm_add_final_kfree(struct drm_device *dev, void *container)
>  	WARN_ON(dev + 1 > (struct drm_device *) (container + ksize(container)));
>  	dev->managed.final_kfree = container;
>  }
> -EXPORT_SYMBOL(drmm_add_final_kfree);
>  
>  int __drmm_add_action(struct drm_device *dev,
>  		      drmres_release_t action,
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 533c6e1a5a95..b8ce9147c9a6 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -591,10 +591,6 @@ struct drm_driver {
>  	int dev_priv_size;
>  };
>  
> -int drm_dev_init(struct drm_device *dev,
> -		 struct drm_driver *driver,
> -		 struct device *parent);
> -
>  void *__devm_drm_dev_alloc(struct device *parent, struct drm_driver *driver,
>  			   size_t size, size_t offset);
>  
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] managed drm_device, absolute final leftover bits
  2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
                   ` (3 preceding siblings ...)
  2020-09-18 13:25 ` [PATCH 4/4] drm/dev: Remove drm_dev_init Daniel Vetter
@ 2020-09-18 15:03 ` Alex Deucher
  2020-09-18 17:12   ` [Intel-gfx] " Rodrigo Vivi
  4 siblings, 1 reply; 16+ messages in thread
From: Alex Deucher @ 2020-09-18 15:03 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development, amd-gfx list, DRI Development

On Fri, Sep 18, 2020 at 9:25 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> Hi all,
>
> These are the leftovers of the leftovers of my initial drmm series to
> manage drm_device.
>
> Changes:
> - bugfixed i915 selftests
> - patch from Luben to finalize the admgpu conversion
>
> Alex & i915 maintainers, pls ack for merging this all through
> drm-misc-next since otherwise the final patch (and the resulting confusion
> with outdated docs) is held up another round.

Acked-by: Alex Deucher <alexander.deucher@amd.com>

>
> Cheers, Daniel
>
> Daniel Vetter (3):
>   drm/i915/selftest: Create mock_destroy_device
>   drm/i915/selftests: align more to real device lifetimes
>   drm/dev: Remove drm_dev_init
>
> Luben Tuikov (1):
>   drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2)
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 16 ++----
>  drivers/gpu/drm/drm_drv.c                     | 41 ++--------------
>  drivers/gpu/drm/drm_internal.h                |  1 +
>  drivers/gpu/drm/drm_managed.c                 | 13 -----
>  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
>  .../drm/i915/gem/selftests/i915_gem_context.c |  2 +-
>  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  2 +-
>  .../drm/i915/gem/selftests/i915_gem_object.c  |  2 +-
>  .../drm/i915/gem/selftests/i915_gem_phys.c    |  2 +-
>  drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
>  .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_vma.c     |  2 +-
>  .../drm/i915/selftests/intel_memory_region.c  |  2 +-
>  .../gpu/drm/i915/selftests/mock_gem_device.c  | 49 ++++++++++++-------
>  .../gpu/drm/i915/selftests/mock_gem_device.h  |  2 +
>  include/drm/drm_drv.h                         |  4 --
>  18 files changed, 51 insertions(+), 97 deletions(-)
>
> --
> 2.28.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Intel-gfx] [PATCH 0/4] managed drm_device, absolute final leftover bits
  2020-09-18 15:03 ` [PATCH 0/4] managed drm_device, absolute final leftover bits Alex Deucher
@ 2020-09-18 17:12   ` Rodrigo Vivi
  2020-09-21  9:09     ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2020-09-18 17:12 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development, amd-gfx list

On Fri, Sep 18, 2020 at 11:03:12AM -0400, Alex Deucher wrote:
> On Fri, Sep 18, 2020 at 9:25 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > Hi all,
> >
> > These are the leftovers of the leftovers of my initial drmm series to
> > manage drm_device.
> >
> > Changes:
> > - bugfixed i915 selftests
> > - patch from Luben to finalize the admgpu conversion
> >
> > Alex & i915 maintainers, pls ack for merging this all through
> > drm-misc-next since otherwise the final patch (and the resulting confusion
> > with outdated docs) is held up another round.
> 
> Acked-by: Alex Deucher <alexander.deucher@amd.com>


Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> >
> > Cheers, Daniel
> >
> > Daniel Vetter (3):
> >   drm/i915/selftest: Create mock_destroy_device
> >   drm/i915/selftests: align more to real device lifetimes
> >   drm/dev: Remove drm_dev_init
> >
> > Luben Tuikov (1):
> >   drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2)
> >
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 16 ++----
> >  drivers/gpu/drm/drm_drv.c                     | 41 ++--------------
> >  drivers/gpu/drm/drm_internal.h                |  1 +
> >  drivers/gpu/drm/drm_managed.c                 | 13 -----
> >  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
> >  .../drm/i915/gem/selftests/i915_gem_context.c |  2 +-
> >  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  2 +-
> >  .../drm/i915/gem/selftests/i915_gem_object.c  |  2 +-
> >  .../drm/i915/gem/selftests/i915_gem_phys.c    |  2 +-
> >  drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
> >  .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
> >  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  2 +-
> >  drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
> >  drivers/gpu/drm/i915/selftests/i915_vma.c     |  2 +-
> >  .../drm/i915/selftests/intel_memory_region.c  |  2 +-
> >  .../gpu/drm/i915/selftests/mock_gem_device.c  | 49 ++++++++++++-------
> >  .../gpu/drm/i915/selftests/mock_gem_device.h  |  2 +
> >  include/drm/drm_drv.h                         |  4 --
> >  18 files changed, 51 insertions(+), 97 deletions(-)
> >
> > --
> > 2.28.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
@ 2020-09-18 17:50   ` Matthew Auld
  2020-09-18 18:22     ` Daniel Vetter
  2020-09-18 20:00   ` [PATCH] " Daniel Vetter
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Matthew Auld @ 2020-09-18 17:50 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx, DRI Development

On Fri, 18 Sep 2020 at 14:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> The big change is device_add so that device_del can auto-cleanup
> devres resources. This allows us to use devm_drm_dev_alloc, which
> removes the last user of drm_dev_init.
>
> v2: Rebased
>
> v3: use devres_open/release_group so we can use devm without real
> hacks in the driver core or having to create an entire fake bus for
> testing drivers. Might want to extract this into helpers eventually,
> maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
>
> v4:
> - Fix IS_ERR handling (Matt)
> - Delete surplus put_device() in mock_device_release (intel-gfx-ci)
>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  .../gpu/drm/i915/selftests/mock_gem_device.c  | 44 +++++++++++--------
>  1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index ac600d395c8f..816f9af15fb3 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
>
>  out:
>         i915_params_free(&i915->params);
> -       put_device(&i915->drm.pdev->dev);
> -       i915->drm.pdev = NULL;
>  }
>
>  static struct drm_driver mock_driver = {
> @@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
>         pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
>         if (!pdev)
>                 return NULL;
> -       i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
> -       if (!i915) {
> -               kfree(pdev);
> -               return NULL;
> -       }
> -
>         device_initialize(&pdev->dev);
>         pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
>         pdev->dev.release = release_dev;
> @@ -144,8 +136,29 @@ struct drm_i915_private *mock_gem_device(void)
>         /* HACK to disable iommu for the fake device; force identity mapping */
>         pdev->dev.iommu = &fake_iommu;
>  #endif
> +       err = device_add(&pdev->dev);
> +       if (err) {
> +               kfree(pdev);
> +               return NULL;
> +       }
> +
> +       if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
> +               device_del(&pdev->dev);
> +               return NULL;
> +       }
> +
> +       i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
> +                                 struct drm_i915_private, drm);
> +       if (IS_ERR(i915)) {
> +               pr_err("Failed to allocate mock GEM device: err=%d\n", err);

err = PTR_ERR(i915)

Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 17:50   ` Matthew Auld
@ 2020-09-18 18:22     ` Daniel Vetter
  2020-09-18 18:31       ` Matthew Auld
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 18:22 UTC (permalink / raw)
  To: Matthew Auld
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx list, DRI Development

On Fri, Sep 18, 2020 at 7:50 PM Matthew Auld
<matthew.william.auld@gmail.com> wrote:
>
> On Fri, 18 Sep 2020 at 14:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > The big change is device_add so that device_del can auto-cleanup
> > devres resources. This allows us to use devm_drm_dev_alloc, which
> > removes the last user of drm_dev_init.
> >
> > v2: Rebased
> >
> > v3: use devres_open/release_group so we can use devm without real
> > hacks in the driver core or having to create an entire fake bus for
> > testing drivers. Might want to extract this into helpers eventually,
> > maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
> >
> > v4:
> > - Fix IS_ERR handling (Matt)
> > - Delete surplus put_device() in mock_device_release (intel-gfx-ci)
> >
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  .../gpu/drm/i915/selftests/mock_gem_device.c  | 44 +++++++++++--------
> >  1 file changed, 25 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > index ac600d395c8f..816f9af15fb3 100644
> > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > @@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
> >
> >  out:
> >         i915_params_free(&i915->params);
> > -       put_device(&i915->drm.pdev->dev);
> > -       i915->drm.pdev = NULL;
> >  }
> >
> >  static struct drm_driver mock_driver = {
> > @@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
> >         pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> >         if (!pdev)
> >                 return NULL;
> > -       i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
> > -       if (!i915) {
> > -               kfree(pdev);
> > -               return NULL;
> > -       }
> > -
> >         device_initialize(&pdev->dev);
> >         pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
> >         pdev->dev.release = release_dev;
> > @@ -144,8 +136,29 @@ struct drm_i915_private *mock_gem_device(void)
> >         /* HACK to disable iommu for the fake device; force identity mapping */
> >         pdev->dev.iommu = &fake_iommu;
> >  #endif
> > +       err = device_add(&pdev->dev);
> > +       if (err) {
> > +               kfree(pdev);
> > +               return NULL;
> > +       }
> > +
> > +       if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
> > +               device_del(&pdev->dev);
> > +               return NULL;
> > +       }
> > +
> > +       i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
> > +                                 struct drm_i915_private, drm);
> > +       if (IS_ERR(i915)) {
> > +               pr_err("Failed to allocate mock GEM device: err=%d\n", err);
>
> err = PTR_ERR(i915)

Are you sure? We return a pointer here, and callers just expect NULL
when stuff fails (so neither errno nor ptr-encoded errno).
-Daniel

> Reviewed-by: Matthew Auld <matthew.auld@intel.com>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 18:22     ` Daniel Vetter
@ 2020-09-18 18:31       ` Matthew Auld
  2020-09-18 18:46         ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Matthew Auld @ 2020-09-18 18:31 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx list, DRI Development

On Fri, 18 Sep 2020 at 19:22, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Fri, Sep 18, 2020 at 7:50 PM Matthew Auld
> <matthew.william.auld@gmail.com> wrote:
> >
> > On Fri, 18 Sep 2020 at 14:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > The big change is device_add so that device_del can auto-cleanup
> > > devres resources. This allows us to use devm_drm_dev_alloc, which
> > > removes the last user of drm_dev_init.
> > >
> > > v2: Rebased
> > >
> > > v3: use devres_open/release_group so we can use devm without real
> > > hacks in the driver core or having to create an entire fake bus for
> > > testing drivers. Might want to extract this into helpers eventually,
> > > maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
> > >
> > > v4:
> > > - Fix IS_ERR handling (Matt)
> > > - Delete surplus put_device() in mock_device_release (intel-gfx-ci)
> > >
> > > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  .../gpu/drm/i915/selftests/mock_gem_device.c  | 44 +++++++++++--------
> > >  1 file changed, 25 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > index ac600d395c8f..816f9af15fb3 100644
> > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > @@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
> > >
> > >  out:
> > >         i915_params_free(&i915->params);
> > > -       put_device(&i915->drm.pdev->dev);
> > > -       i915->drm.pdev = NULL;
> > >  }
> > >
> > >  static struct drm_driver mock_driver = {
> > > @@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
> > >         pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> > >         if (!pdev)
> > >                 return NULL;
> > > -       i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
> > > -       if (!i915) {
> > > -               kfree(pdev);
> > > -               return NULL;
> > > -       }
> > > -
> > >         device_initialize(&pdev->dev);
> > >         pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
> > >         pdev->dev.release = release_dev;
> > > @@ -144,8 +136,29 @@ struct drm_i915_private *mock_gem_device(void)
> > >         /* HACK to disable iommu for the fake device; force identity mapping */
> > >         pdev->dev.iommu = &fake_iommu;
> > >  #endif
> > > +       err = device_add(&pdev->dev);
> > > +       if (err) {
> > > +               kfree(pdev);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
> > > +               device_del(&pdev->dev);
> > > +               return NULL;
> > > +       }
> > > +
> > > +       i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
> > > +                                 struct drm_i915_private, drm);
> > > +       if (IS_ERR(i915)) {
> > > +               pr_err("Failed to allocate mock GEM device: err=%d\n", err);
> >
> > err = PTR_ERR(i915)
>
> Are you sure? We return a pointer here, and callers just expect NULL
> when stuff fails (so neither errno nor ptr-encoded errno).

I just meant for the pr_err() which is printing the err(from the
copy-paste), but it will always be zero without the above.

> -Daniel
>
> > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 18:31       ` Matthew Auld
@ 2020-09-18 18:46         ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 18:46 UTC (permalink / raw)
  To: Matthew Auld
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx list, DRI Development

On Fri, Sep 18, 2020 at 8:31 PM Matthew Auld
<matthew.william.auld@gmail.com> wrote:
>
> On Fri, 18 Sep 2020 at 19:22, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > On Fri, Sep 18, 2020 at 7:50 PM Matthew Auld
> > <matthew.william.auld@gmail.com> wrote:
> > >
> > > On Fri, 18 Sep 2020 at 14:25, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > >
> > > > The big change is device_add so that device_del can auto-cleanup
> > > > devres resources. This allows us to use devm_drm_dev_alloc, which
> > > > removes the last user of drm_dev_init.
> > > >
> > > > v2: Rebased
> > > >
> > > > v3: use devres_open/release_group so we can use devm without real
> > > > hacks in the driver core or having to create an entire fake bus for
> > > > testing drivers. Might want to extract this into helpers eventually,
> > > > maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.
> > > >
> > > > v4:
> > > > - Fix IS_ERR handling (Matt)
> > > > - Delete surplus put_device() in mock_device_release (intel-gfx-ci)
> > > >
> > > > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > >  .../gpu/drm/i915/selftests/mock_gem_device.c  | 44 +++++++++++--------
> > > >  1 file changed, 25 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > index ac600d395c8f..816f9af15fb3 100644
> > > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> > > > @@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
> > > >
> > > >  out:
> > > >         i915_params_free(&i915->params);
> > > > -       put_device(&i915->drm.pdev->dev);
> > > > -       i915->drm.pdev = NULL;
> > > >  }
> > > >
> > > >  static struct drm_driver mock_driver = {
> > > > @@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
> > > >         pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
> > > >         if (!pdev)
> > > >                 return NULL;
> > > > -       i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
> > > > -       if (!i915) {
> > > > -               kfree(pdev);
> > > > -               return NULL;
> > > > -       }
> > > > -
> > > >         device_initialize(&pdev->dev);
> > > >         pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
> > > >         pdev->dev.release = release_dev;
> > > > @@ -144,8 +136,29 @@ struct drm_i915_private *mock_gem_device(void)
> > > >         /* HACK to disable iommu for the fake device; force identity mapping */
> > > >         pdev->dev.iommu = &fake_iommu;
> > > >  #endif
> > > > +       err = device_add(&pdev->dev);
> > > > +       if (err) {
> > > > +               kfree(pdev);
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
> > > > +               device_del(&pdev->dev);
> > > > +               return NULL;
> > > > +       }
> > > > +
> > > > +       i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
> > > > +                                 struct drm_i915_private, drm);
> > > > +       if (IS_ERR(i915)) {
> > > > +               pr_err("Failed to allocate mock GEM device: err=%d\n", err);
> > >
> > > err = PTR_ERR(i915)
> >
> > Are you sure? We return a pointer here, and callers just expect NULL
> > when stuff fails (so neither errno nor ptr-encoded errno).
>
> I just meant for the pr_err() which is printing the err(from the
> copy-paste), but it will always be zero without the above.

Ah right, I missed that when applying your previous comment, thanks
for pointing it out.
-Daniel

>
> > -Daniel
> >
> > > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
  2020-09-18 17:50   ` Matthew Auld
@ 2020-09-18 20:00   ` Daniel Vetter
  2020-09-19 13:38   ` Daniel Vetter
  2020-09-19 13:40   ` Daniel Vetter
  3 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-18 20:00 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx, Daniel Vetter, Matthew Auld

To avoid having to create all the device and driver scaffolding we
just manually create and destroy a devres_group.

v2: Rebased

v3: use devres_open/release_group so we can use devm without real
hacks in the driver core or having to create an entire fake bus for
testing drivers. Might want to extract this into helpers eventually,
maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.

v4:
- Fix IS_ERR handling (Matt)
- Delete surplus put_device() in mock_device_release (intel-gfx-ci)

v5:
- do not switch to device_add - it breaks runtime pm in the tests and
  with the devres_group_add/release no longer needed for automatic
  cleanup (CI). Update commit message to match.
- print correct error in pr_err (Matt)

Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> (v4)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 38 +++++++++----------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index ac600d395c8f..3b574597cd7f 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
 
 out:
 	i915_params_free(&i915->params);
-	put_device(&i915->drm.pdev->dev);
-	i915->drm.pdev = NULL;
 }
 
 static struct drm_driver mock_driver = {
@@ -128,12 +126,6 @@ struct drm_i915_private *mock_gem_device(void)
 	pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
 	if (!pdev)
 		return NULL;
-	i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-	if (!i915) {
-		kfree(pdev);
-		return NULL;
-	}
-
 	device_initialize(&pdev->dev);
 	pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
 	pdev->dev.release = release_dev;
@@ -144,8 +136,23 @@ struct drm_i915_private *mock_gem_device(void)
 	/* HACK to disable iommu for the fake device; force identity mapping */
 	pdev->dev.iommu = &fake_iommu;
 #endif
+	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
+		put_device(&pdev->dev);
+		return NULL;
+	}
+
+	i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
+				  struct drm_i915_private, drm);
+	if (IS_ERR(i915)) {
+		pr_err("Failed to allocate mock GEM device: err=%d\n", PTR_ERR(i915));
+		devres_release_group(&pdev->dev, NULL);
+		put_device(&pdev->dev);
+
+		return NULL;
+	}
 
 	pci_set_drvdata(pdev, i915);
+	i915->drm.pdev = pdev;
 
 	dev_pm_domain_set(&pdev->dev, &pm_domain);
 	pm_runtime_enable(&pdev->dev);
@@ -153,16 +160,6 @@ struct drm_i915_private *mock_gem_device(void)
 	if (pm_runtime_enabled(&pdev->dev))
 		WARN_ON(pm_runtime_get_sync(&pdev->dev));
 
-	err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
-	if (err) {
-		pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-		put_device(&pdev->dev);
-		kfree(i915);
-
-		return NULL;
-	}
-	i915->drm.pdev = pdev;
-	drmm_add_final_kfree(&i915->drm, i915);
 
 	i915_params_copy(&i915->params, &i915_modparams);
 
@@ -229,5 +226,8 @@ struct drm_i915_private *mock_gem_device(void)
 
 void mock_destroy_device(struct drm_i915_private *i915)
 {
-	drm_dev_put(&i915->drm);
+	struct device *dev = i915->drm.dev;
+
+	devres_release_group(dev, NULL);
+	put_device(dev);
 }
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
  2020-09-18 17:50   ` Matthew Auld
  2020-09-18 20:00   ` [PATCH] " Daniel Vetter
@ 2020-09-19 13:38   ` Daniel Vetter
  2020-09-19 13:40   ` Daniel Vetter
  3 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-19 13:38 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx, Daniel Vetter, Matthew Auld

To avoid having to create all the device and driver scaffolding we
just manually create and destroy a devres_group.

v2: Rebased

v3: use devres_open/release_group so we can use devm without real
hacks in the driver core or having to create an entire fake bus for
testing drivers. Might want to extract this into helpers eventually,
maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.

v4:
- Fix IS_ERR handling (Matt)
- Delete surplus put_device() in mock_device_release (intel-gfx-ci)

v5:
- do not switch to device_add - it breaks runtime pm in the tests and
  with the devres_group_add/release no longer needed for automatic
  cleanup (CI). Update commit message to match.
- print correct error in pr_err (Matt)

v6: Remove now unused err variable (CI).

Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> (v4)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 39 +++++++++----------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index ac600d395c8f..b7db3ec346ba 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
 
 out:
 	i915_params_free(&i915->params);
-	put_device(&i915->drm.pdev->dev);
-	i915->drm.pdev = NULL;
 }
 
 static struct drm_driver mock_driver = {
@@ -123,17 +121,10 @@ struct drm_i915_private *mock_gem_device(void)
 #endif
 	struct drm_i915_private *i915;
 	struct pci_dev *pdev;
-	int err;
 
 	pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
 	if (!pdev)
 		return NULL;
-	i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-	if (!i915) {
-		kfree(pdev);
-		return NULL;
-	}
-
 	device_initialize(&pdev->dev);
 	pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
 	pdev->dev.release = release_dev;
@@ -144,8 +135,23 @@ struct drm_i915_private *mock_gem_device(void)
 	/* HACK to disable iommu for the fake device; force identity mapping */
 	pdev->dev.iommu = &fake_iommu;
 #endif
+	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
+		put_device(&pdev->dev);
+		return NULL;
+	}
+
+	i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
+				  struct drm_i915_private, drm);
+	if (IS_ERR(i915)) {
+		pr_err("Failed to allocate mock GEM device: err=%d\n", PTR_ERR(i915));
+		devres_release_group(&pdev->dev, NULL);
+		put_device(&pdev->dev);
+
+		return NULL;
+	}
 
 	pci_set_drvdata(pdev, i915);
+	i915->drm.pdev = pdev;
 
 	dev_pm_domain_set(&pdev->dev, &pm_domain);
 	pm_runtime_enable(&pdev->dev);
@@ -153,16 +159,6 @@ struct drm_i915_private *mock_gem_device(void)
 	if (pm_runtime_enabled(&pdev->dev))
 		WARN_ON(pm_runtime_get_sync(&pdev->dev));
 
-	err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
-	if (err) {
-		pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-		put_device(&pdev->dev);
-		kfree(i915);
-
-		return NULL;
-	}
-	i915->drm.pdev = pdev;
-	drmm_add_final_kfree(&i915->drm, i915);
 
 	i915_params_copy(&i915->params, &i915_modparams);
 
@@ -229,5 +225,8 @@ struct drm_i915_private *mock_gem_device(void)
 
 void mock_destroy_device(struct drm_i915_private *i915)
 {
-	drm_dev_put(&i915->drm);
+	struct device *dev = i915->drm.dev;
+
+	devres_release_group(dev, NULL);
+	put_device(dev);
 }
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] drm/i915/selftests: align more to real device lifetimes
  2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
                     ` (2 preceding siblings ...)
  2020-09-19 13:38   ` Daniel Vetter
@ 2020-09-19 13:40   ` Daniel Vetter
  3 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-19 13:40 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Maarten Lankhorst,
	amd-gfx, Daniel Vetter, Matthew Auld

To avoid having to create all the device and driver scaffolding we
just manually create and destroy a devres_group.

v2: Rebased

v3: use devres_open/release_group so we can use devm without real
hacks in the driver core or having to create an entire fake bus for
testing drivers. Might want to extract this into helpers eventually,
maybe as a mock_drm_dev_alloc or test_drm_dev_alloc.

v4:
- Fix IS_ERR handling (Matt)
- Delete surplus put_device() in mock_device_release (intel-gfx-ci)

v5:
- do not switch to device_add - it breaks runtime pm in the tests and
  with the devres_group_add/release no longer needed for automatic
  cleanup (CI). Update commit message to match.
- print correct error in pr_err (Matt)

v6: Remove now unused err variable (CI).

v7: More warning fixes ...

Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v3)
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> (v4)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 39 +++++++++----------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index ac600d395c8f..b6c42fd872ad 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -79,8 +79,6 @@ static void mock_device_release(struct drm_device *dev)
 
 out:
 	i915_params_free(&i915->params);
-	put_device(&i915->drm.pdev->dev);
-	i915->drm.pdev = NULL;
 }
 
 static struct drm_driver mock_driver = {
@@ -123,17 +121,10 @@ struct drm_i915_private *mock_gem_device(void)
 #endif
 	struct drm_i915_private *i915;
 	struct pci_dev *pdev;
-	int err;
 
 	pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
 	if (!pdev)
 		return NULL;
-	i915 = kzalloc(sizeof(*i915), GFP_KERNEL);
-	if (!i915) {
-		kfree(pdev);
-		return NULL;
-	}
-
 	device_initialize(&pdev->dev);
 	pdev->class = PCI_BASE_CLASS_DISPLAY << 16;
 	pdev->dev.release = release_dev;
@@ -144,8 +135,23 @@ struct drm_i915_private *mock_gem_device(void)
 	/* HACK to disable iommu for the fake device; force identity mapping */
 	pdev->dev.iommu = &fake_iommu;
 #endif
+	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL)) {
+		put_device(&pdev->dev);
+		return NULL;
+	}
+
+	i915 = devm_drm_dev_alloc(&pdev->dev, &mock_driver,
+				  struct drm_i915_private, drm);
+	if (IS_ERR(i915)) {
+		pr_err("Failed to allocate mock GEM device: err=%ld\n", PTR_ERR(i915));
+		devres_release_group(&pdev->dev, NULL);
+		put_device(&pdev->dev);
+
+		return NULL;
+	}
 
 	pci_set_drvdata(pdev, i915);
+	i915->drm.pdev = pdev;
 
 	dev_pm_domain_set(&pdev->dev, &pm_domain);
 	pm_runtime_enable(&pdev->dev);
@@ -153,16 +159,6 @@ struct drm_i915_private *mock_gem_device(void)
 	if (pm_runtime_enabled(&pdev->dev))
 		WARN_ON(pm_runtime_get_sync(&pdev->dev));
 
-	err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
-	if (err) {
-		pr_err("Failed to initialise mock GEM device: err=%d\n", err);
-		put_device(&pdev->dev);
-		kfree(i915);
-
-		return NULL;
-	}
-	i915->drm.pdev = pdev;
-	drmm_add_final_kfree(&i915->drm, i915);
 
 	i915_params_copy(&i915->params, &i915_modparams);
 
@@ -229,5 +225,8 @@ struct drm_i915_private *mock_gem_device(void)
 
 void mock_destroy_device(struct drm_i915_private *i915)
 {
-	drm_dev_put(&i915->drm);
+	struct device *dev = i915->drm.dev;
+
+	devres_release_group(dev, NULL);
+	put_device(dev);
 }
-- 
2.28.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Intel-gfx] [PATCH 0/4] managed drm_device, absolute final leftover bits
  2020-09-18 17:12   ` [Intel-gfx] " Rodrigo Vivi
@ 2020-09-21  9:09     ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-09-21  9:09 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: Alex Deucher, Daniel Vetter, Intel Graphics Development,
	DRI Development, amd-gfx list

On Fri, Sep 18, 2020 at 01:12:21PM -0400, Rodrigo Vivi wrote:
> On Fri, Sep 18, 2020 at 11:03:12AM -0400, Alex Deucher wrote:
> > On Fri, Sep 18, 2020 at 9:25 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > Hi all,
> > >
> > > These are the leftovers of the leftovers of my initial drmm series to
> > > manage drm_device.
> > >
> > > Changes:
> > > - bugfixed i915 selftests
> > > - patch from Luben to finalize the admgpu conversion
> > >
> > > Alex & i915 maintainers, pls ack for merging this all through
> > > drm-misc-next since otherwise the final patch (and the resulting confusion
> > > with outdated docs) is held up another round.
> > 
> > Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 
> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Entire series merged into drm-misc-next.
-Daniel

> 
> > 
> > >
> > > Cheers, Daniel
> > >
> > > Daniel Vetter (3):
> > >   drm/i915/selftest: Create mock_destroy_device
> > >   drm/i915/selftests: align more to real device lifetimes
> > >   drm/dev: Remove drm_dev_init
> > >
> > > Luben Tuikov (1):
> > >   drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2)
> > >
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 16 ++----
> > >  drivers/gpu/drm/drm_drv.c                     | 41 ++--------------
> > >  drivers/gpu/drm/drm_internal.h                |  1 +
> > >  drivers/gpu/drm/drm_managed.c                 | 13 -----
> > >  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
> > >  .../drm/i915/gem/selftests/i915_gem_context.c |  2 +-
> > >  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  2 +-
> > >  .../drm/i915/gem/selftests/i915_gem_object.c  |  2 +-
> > >  .../drm/i915/gem/selftests/i915_gem_phys.c    |  2 +-
> > >  drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 +-
> > >  .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
> > >  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  2 +-
> > >  drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
> > >  drivers/gpu/drm/i915/selftests/i915_vma.c     |  2 +-
> > >  .../drm/i915/selftests/intel_memory_region.c  |  2 +-
> > >  .../gpu/drm/i915/selftests/mock_gem_device.c  | 49 ++++++++++++-------
> > >  .../gpu/drm/i915/selftests/mock_gem_device.h  |  2 +
> > >  include/drm/drm_drv.h                         |  4 --
> > >  18 files changed, 51 insertions(+), 97 deletions(-)
> > >
> > > --
> > > 2.28.0
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-09-21  9:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-18 13:25 [PATCH 0/4] managed drm_device, absolute final leftover bits Daniel Vetter
2020-09-18 13:25 ` [PATCH 1/4] drm/i915/selftest: Create mock_destroy_device Daniel Vetter
2020-09-18 13:25 ` [PATCH 2/4] drm/i915/selftests: align more to real device lifetimes Daniel Vetter
2020-09-18 17:50   ` Matthew Auld
2020-09-18 18:22     ` Daniel Vetter
2020-09-18 18:31       ` Matthew Auld
2020-09-18 18:46         ` Daniel Vetter
2020-09-18 20:00   ` [PATCH] " Daniel Vetter
2020-09-19 13:38   ` Daniel Vetter
2020-09-19 13:40   ` Daniel Vetter
2020-09-18 13:25 ` [PATCH 3/4] drm/amdgpu: Convert to using devm_drm_dev_alloc() (v2) Daniel Vetter
2020-09-18 13:25 ` [PATCH 4/4] drm/dev: Remove drm_dev_init Daniel Vetter
2020-09-18 14:26   ` Thomas Zimmermann
2020-09-18 15:03 ` [PATCH 0/4] managed drm_device, absolute final leftover bits Alex Deucher
2020-09-18 17:12   ` [Intel-gfx] " Rodrigo Vivi
2020-09-21  9:09     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).