On Wed, Dec 14, 2016 at 12:04:38AM +0200, Laurent Pinchart wrote:
> 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.
Not quite. The layering is wrong as the driver needs to release its
state prior to e.g. drm_gem_destroy. It is not for freeing the memory.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
