[PATCH 16/37] drm/hdlcd|mali: Drop drm_vblank_cleanup

[Liviu Dudau]

Hi Daniel,

On Wed, May 24, 2017 at 04:51:51PM +0200, Daniel Vetter wrote:
> IRQs are properly shut down, so it almost works as race-free shutdown.
> Except the irq is stopped after the vblank stuff, so boom anyway.
> Proper way would be to call drm_atomic_helper_shutdown before any of
> the kms things gets stopped. So no harm in removing the
> drm_vblank_cleanup here really.

Slightly confused on the implied message in the commit text: is "Proper way
would be to call drm_atomic_helper_shutdown" a hint? A promise of a future
patch? A message to the future us on how to fix things if they blow up?

If calling drm_atomic_helper_shutdown() is the proper thing to do, why does the
patch (and the series) avoids doing that? Lack of understanding of the driver's
internal workings? Then I want to help, if I can understand the new direction.

Best regards,
Liviu

> 
> Same story for both hdlcd and mali.
> 
> v2: Move misplaced malidp hunk to this patch (Liviu).
> 
> Cc: Liviu Dudau <liviu.dudau at arm.com>
> Cc: Brian Starkey <brian.starkey at arm.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c  | 2 --
>  drivers/gpu/drm/arm/malidp_drv.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 0f49c4b12772..345c8357b273 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -340,7 +340,6 @@ static int hdlcd_drm_bind(struct device *dev)
>  	}
>  err_fbdev:
>  	drm_kms_helper_poll_fini(drm);
> -	drm_vblank_cleanup(drm);
>  err_vblank:
>  	pm_runtime_disable(drm->dev);
>  err_pm_active:
> @@ -368,7 +367,6 @@ static void hdlcd_drm_unbind(struct device *dev)
>  	}
>  	drm_kms_helper_poll_fini(drm);
>  	component_unbind_all(dev, drm);
> -	drm_vblank_cleanup(drm);
>  	pm_runtime_get_sync(drm->dev);
>  	drm_irq_uninstall(drm);
>  	pm_runtime_put_sync(drm->dev);
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 0d3eb537d08b..01b13d219917 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -652,7 +652,6 @@ static int malidp_bind(struct device *dev)
>  	drm_kms_helper_poll_fini(drm);
>  fbdev_fail:
>  	pm_runtime_get_sync(dev);
> -	drm_vblank_cleanup(drm);
>  vblank_fail:
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
> @@ -692,7 +691,6 @@ static void malidp_unbind(struct device *dev)
>  	}
>  	drm_kms_helper_poll_fini(drm);
>  	pm_runtime_get_sync(dev);
> -	drm_vblank_cleanup(drm);
>  	malidp_se_irq_fini(drm);
>  	malidp_de_irq_fini(drm);
>  	component_unbind_all(dev, drm);
> -- 
> 2.11.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯