[Intel-gfx] [PATCH] drm: Provide a driver hook for drm_dev_release()

[Daniel Vetter]

On Thu, Dec 08, 2016 at 08:18:40AM +0000, Chris Wilson wrote:
> Some state is coupled into the device lifetime outside of the
> load/unload timeframe and requires teardown during final unreference
> from drm_dev_release(). For example, dmabufs hold both a device and
> module reference and may live longer than expected (i.e. the current
> pattern of the driver tearing down its state and then releasing a
> reference to the drm device) and yet touch driver private state when
> destroyed.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_drv.c | 3 +++
>  include/drm/drm_drv.h     | 8 ++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index f74b7d06ec01..f945bbcc8eb3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -595,6 +595,9 @@ static void drm_dev_release(struct kref *ref)
>  {
>  	struct drm_device *dev = container_of(ref, struct drm_device, ref);
>  
> +	if (dev->driver->release)
> +		dev->driver->release(dev);
> +
>  	if (drm_core_check_feature(dev, DRIVER_GEM))
>  		drm_gem_destroy(dev);
>  
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index c4fc49583dc0..554104ccb939 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -71,6 +71,14 @@ struct drm_driver {
>  	void (*postclose) (struct drm_device *, struct drm_file *);
>  	void (*lastclose) (struct drm_device *);
>  	int (*unload) (struct drm_device *);
> +	/**
> +	 * @release:
> +	 *
> +	 * Optional callback for destroying device state after the final
> +	 * reference is released, i.e. the device is being destroyed.
> +	 */
> +	void (*release) (struct drm_device *);

I think sprinkling a reference to this hook into the documentation for
drm_dev_put would be real good. There's also a pile of text citing
drm_dev_unref(), especially in the overview section. I think that should
be udated to explain that release memory should only happen in ->release.
And I think with this change we can remove the cautious note that drm_dev_ref/unref are busted, too.

And while reviewing this entire mess I've noticed that we probably want to
move the call for drm_vblank_cleanup() from _unregister to
drm_dev_release?
-Daniel
> +
>  	int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv);
>  	int (*dma_quiescent) (struct drm_device *);
>  	int (*context_dtor) (struct drm_device *dev, int context);
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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