diff mbox series

[08/24] drm/i915/selftests: align more to real device lifetimes

Message ID 20200904143941.110665-9-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series drm_managed, leftovers | expand

Commit Message

Daniel Vetter Sept. 4, 2020, 2:39 p.m. UTC
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.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 .../gpu/drm/i915/selftests/mock_gem_device.c  | 42 +++++++++++--------
 1 file changed, 25 insertions(+), 17 deletions(-)

Comments

Maarten Lankhorst Sept. 11, 2020, 8:59 a.m. UTC | #1
Op 04-09-2020 om 16:39 schreef Daniel Vetter:
> 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.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  .../gpu/drm/i915/selftests/mock_gem_device.c  | 42 +++++++++++--------
>  1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index fbb403edb7a0..164ad1746da9 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -128,12 +128,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;
> @@ -146,8 +140,29 @@ struct drm_i915_private *mock_gem_device(void)
>  	iommu.priv = (void *)-1;
>  	pdev->dev.iommu = &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 (err) {
> +		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);
> @@ -155,16 +170,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);
>  
> @@ -231,5 +236,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);
>  }

Looks sane,

For patch 7 and 8:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Matthew Auld Sept. 11, 2020, 9:08 a.m. UTC | #2
On Fri, 4 Sep 2020 at 15:40, 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.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  .../gpu/drm/i915/selftests/mock_gem_device.c  | 42 +++++++++++--------
>  1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index fbb403edb7a0..164ad1746da9 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -128,12 +128,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;
> @@ -146,8 +140,29 @@ struct drm_i915_private *mock_gem_device(void)
>         iommu.priv = (void *)-1;
>         pdev->dev.iommu = &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 (err) {

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);
> @@ -155,16 +170,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);
>
> @@ -231,5 +236,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
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index fbb403edb7a0..164ad1746da9 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -128,12 +128,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;
@@ -146,8 +140,29 @@  struct drm_i915_private *mock_gem_device(void)
 	iommu.priv = (void *)-1;
 	pdev->dev.iommu = &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 (err) {
+		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);
@@ -155,16 +170,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);
 
@@ -231,5 +236,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);
 }