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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> Acked-by: Thierry Reding <treding@xxxxxxxxxx>
> Cc: Jerome Glisse <jglisse@xxxxxxxxxx>
> Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
