Re: [PATCH 06/20] drm/i915: Remove obsolete engine cleanup

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx