[Intel-gfx] [PATCH 2/2] drm/vblank: Unexport drm_vblank_cleanup

[Daniel Vetter]

On Mon, Jun 26, 2017 at 06:19:49PM +0200, Daniel Vetter wrote:
> There's no reason for drivers to call this, and all the ones I've
> removed looked very fishy:
> - Proper quiescenting of the vblank machinery should be done by
>   calling drm_crtc_vblank_off(), which is best done by shutting down
>   the entire display engine with drm_atomic_helper_shutdown.
> 
> - Releasing of allocated memory is done by the core already, it calls
>   drm_vblank_cleanup as a fallback.
> 
> - drm_vblank_cleanup also has checks for drivers which forget to clean
>   up vblank interrupts.
> 
> This essentially reverts
> 
> commit e77cef9c2d87db835ad9d70cde4a9b00b0ca2262
> Author: Jerome Glisse <jglisse at redhat.com>
> Date:   Thu Jan 7 15:39:13 2010 +0100
> 
>     drm: Avoid calling vblank function is vblank wasn't initialized
> 
> which was done to fix a bug in radeon code with msi interrupts:
> 
> commit 003e69f9862bcda89a75c27750efdbc17ac02945
> Author: Jerome Glisse <jglisse at redhat.com>
> Date:   Thu Jan 7 15:39:14 2010 +0100
> 
>     drm/radeon/kms: Don't try to enable IRQ if we have no handler installed
> 
> Afaict from digging around in old code, this was needed to avoid
> blowing up in the ums fallback, and has stopped serving it's purpose
> long ago - if irq init fails, the driver fails to load, and there's
> really no way to blow up anymore.
> 
> Long story short, this was most likely a small ums compat/fallback
> hack that became a thing of it's own and got cargo-cult duplicated all
> over the drm codebase for essentially no gain at all.
> 
> v2: Mention that for drivers with a ->release callback cleanup is
> handled by drm_dev_fini() (Thierry).
> 
> Cc: Thierry Reding <treding at nvidia.com>
> Acked-by: Thierry Reding <treding at nvidia.com>
> Cc: Jerome Glisse <jglisse at redhat.com>
> Reviewed-by: Sean Paul <seanpaul at chromium.org>
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>

Vacuumed up all the remaining patches here, thx to everyon who helped with
the reviews.
-Daniel

> ---
>  drivers/gpu/drm/drm_internal.h |  1 +
>  drivers/gpu/drm/drm_vblank.c   | 19 ++-----------------
>  include/drm/drm_vblank.h       |  1 -
>  3 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index f89371e920e6..068b685608cf 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -57,6 +57,7 @@ int drm_gem_name_info(struct seq_file *m, void *data);
>  /* drm_vblank.c */
>  extern unsigned int drm_timestamp_monotonic;
>  void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe);
> +void drm_vblank_cleanup(struct drm_device *dev);
>  
>  /* IOCTLS */
>  int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 7e3f59182571..05d043e9219f 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -394,19 +394,6 @@ static void vblank_disable_fn(unsigned long arg)
>  	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
>  }
>  
> -/**
> - * drm_vblank_cleanup - cleanup vblank support
> - * @dev: DRM device
> - *
> - * This function cleans up any resources allocated in drm_vblank_init(). It is
> - * called by the DRM core when @dev is finalized.
> - *
> - * Drivers can call drm_vblank_cleanup() if they need to quiescent the vblank
> - * interrupt in their unload code. But in general this should be handled by
> - * disabling all active &drm_crtc through e.g. drm_atomic_helper_shutdown, which
> - * should end up calling drm_crtc_vblank_off().
> - *
> - */
>  void drm_vblank_cleanup(struct drm_device *dev)
>  {
>  	unsigned int pipe;
> @@ -428,7 +415,6 @@ void drm_vblank_cleanup(struct drm_device *dev)
>  
>  	dev->num_crtcs = 0;
>  }
> -EXPORT_SYMBOL(drm_vblank_cleanup);
>  
>  /**
>   * drm_vblank_init - initialize vblank support
> @@ -436,9 +422,8 @@ EXPORT_SYMBOL(drm_vblank_cleanup);
>   * @num_crtcs: number of CRTCs supported by @dev
>   *
>   * This function initializes vblank support for @num_crtcs display pipelines.
> - * Drivers do not need to call drm_vblank_cleanup(), cleanup is already handled
> - * by the DRM core, or through calling drm_dev_fini() for drivers with a
> - * &drm_driver.release callback.
> + * Cleanup is handled by the DRM core, or through calling drm_dev_fini() for
> + * drivers with a &drm_driver.release callback.
>   *
>   * Returns:
>   * Zero on success or a negative error code on failure.
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 4ceef128582f..7fba9efe4951 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -168,7 +168,6 @@ void drm_crtc_wait_one_vblank(struct drm_crtc *crtc);
>  void drm_crtc_vblank_off(struct drm_crtc *crtc);
>  void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>  void drm_crtc_vblank_on(struct drm_crtc *crtc);
> -void drm_vblank_cleanup(struct drm_device *dev);
>  u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
>  
>  bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch