[06/20] drm/i915: Remove obsolete engine cleanup
diff mbox series

Message ID 20190718070024.21781-6-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [01/20] drm/i915: Move aliasing_ppgtt underneath its i915_ggtt
Related show

Commit Message

Chris Wilson July 18, 2019, 7 a.m. UTC
Remove the outer layer cleanup of engine stubs; as i915_drv itself no
longer tries to preallocate and so is not responsible for either the
allocation or free. By the time we call the cleanup function, we already
have cleaned up the engines.

v2: Lack of symmetry between mmio_probe and mmio_release for handling
the error cleanup. engine->destroy() is a compound function that is
called earlier in the normal release as it ties together other bits of
state.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

Comments

Tvrtko Ursulin July 22, 2019, 12:46 p.m. UTC | #1
On 18/07/2019 08:00, Chris Wilson wrote:
> Remove the outer layer cleanup of engine stubs; as i915_drv itself no
> longer tries to preallocate and so is not responsible for either the
> allocation or free. By the time we call the cleanup function, we already
> have cleaned up the engines.
> 
> v2: Lack of symmetry between mmio_probe and mmio_release for handling
> the error cleanup. engine->destroy() is a compound function that is
> called earlier in the normal release as it ties together other bits of
> state.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.c | 15 ++-------------
>   1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 7c209743e478..d1c3499c8e03 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -848,15 +848,6 @@ static int i915_workqueues_init(struct drm_i915_private *dev_priv)
>   	return -ENOMEM;
>   }
>   
> -static void i915_engines_cleanup(struct drm_i915_private *i915)
> -{
> -	struct intel_engine_cs *engine;
> -	enum intel_engine_id id;
> -
> -	for_each_engine(engine, i915, id)
> -		kfree(engine);
> -}
> -
>   static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
>   {
>   	destroy_workqueue(dev_priv->hotplug.dp_wq);
> @@ -928,7 +919,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>   
>   	ret = i915_workqueues_init(dev_priv);
>   	if (ret < 0)
> -		goto err_engines;
> +		return ret;
>   
>   	intel_gt_init_early(&dev_priv->gt, dev_priv);
>   
> @@ -961,8 +952,6 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
>   	i915_gem_cleanup_early(dev_priv);
>   err_workqueues:
>   	i915_workqueues_cleanup(dev_priv);
> -err_engines:
> -	i915_engines_cleanup(dev_priv);
>   	return ret;
>   }
>   
> @@ -978,7 +967,6 @@ static void i915_driver_late_release(struct drm_i915_private *dev_priv)
>   	intel_uc_cleanup_early(&dev_priv->gt.uc);
>   	i915_gem_cleanup_early(dev_priv);
>   	i915_workqueues_cleanup(dev_priv);
> -	i915_engines_cleanup(dev_priv);
>   
>   	pm_qos_remove_request(&dev_priv->sb_qos);
>   	mutex_destroy(&dev_priv->sb_lock);
> @@ -1039,6 +1027,7 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv)
>    */
>   static void i915_driver_mmio_release(struct drm_i915_private *dev_priv)
>   {
> +	intel_engines_cleanup(dev_priv);
>   	intel_teardown_mchbar(dev_priv);
>   	intel_uncore_fini_mmio(&dev_priv->uncore);
>   	pci_dev_put(dev_priv->bridge_dev);
> 

Okay, looks okay. But do I dare to say that after all the efforts to 
make things more logical in the init/release paths we are still not 
there? :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
Chris Wilson July 22, 2019, 4:29 p.m. UTC | #2
Quoting Tvrtko Ursulin (2019-07-22 13:46:06)
> 
> On 18/07/2019 08:00, Chris Wilson wrote:
> >   static void i915_driver_mmio_release(struct drm_i915_private *dev_priv)
> >   {
> > +     intel_engines_cleanup(dev_priv);
> >       intel_teardown_mchbar(dev_priv);
> >       intel_uncore_fini_mmio(&dev_priv->uncore);
> >       pci_dev_put(dev_priv->bridge_dev);
> > 
> 
> Okay, looks okay. But do I dare to say that after all the efforts to 
> make things more logical in the init/release paths we are still not 
> there? :)

intel_engines_* are all placeholders for intel_gt_foo_bar :)
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7c209743e478..d1c3499c8e03 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -848,15 +848,6 @@  static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 	return -ENOMEM;
 }
 
-static void i915_engines_cleanup(struct drm_i915_private *i915)
-{
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-
-	for_each_engine(engine, i915, id)
-		kfree(engine);
-}
-
 static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
 {
 	destroy_workqueue(dev_priv->hotplug.dp_wq);
@@ -928,7 +919,7 @@  static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 
 	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
-		goto err_engines;
+		return ret;
 
 	intel_gt_init_early(&dev_priv->gt, dev_priv);
 
@@ -961,8 +952,6 @@  static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	i915_gem_cleanup_early(dev_priv);
 err_workqueues:
 	i915_workqueues_cleanup(dev_priv);
-err_engines:
-	i915_engines_cleanup(dev_priv);
 	return ret;
 }
 
@@ -978,7 +967,6 @@  static void i915_driver_late_release(struct drm_i915_private *dev_priv)
 	intel_uc_cleanup_early(&dev_priv->gt.uc);
 	i915_gem_cleanup_early(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
-	i915_engines_cleanup(dev_priv);
 
 	pm_qos_remove_request(&dev_priv->sb_qos);
 	mutex_destroy(&dev_priv->sb_lock);
@@ -1039,6 +1027,7 @@  static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv)
  */
 static void i915_driver_mmio_release(struct drm_i915_private *dev_priv)
 {
+	intel_engines_cleanup(dev_priv);
 	intel_teardown_mchbar(dev_priv);
 	intel_uncore_fini_mmio(&dev_priv->uncore);
 	pci_dev_put(dev_priv->bridge_dev);