Hi Chris,
Thank you for the patch.
On Thursday 08 Dec 2016 08:18:40 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@xxxxxxxxxxxxxxxxxx>
> ---
> 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);
For drivers embedding the drm_device structure, you should only call
.release() at the very end of this function, as the callback will free memory,
including the embedded struct drm_device. Similarly, the kfree() at the end
should be made conditional. Or, better, you could implement a release function
that just wraps kfree(), and set it as the release handler in drm_dev_alloc().
Drivers using drm_dev_init() would need to provide their own release handler.
> 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 *);
> +
> 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);
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
