On Mon, Mar 13, 2017 at 03:29:20PM -0400, Sean Paul wrote: > On Wed, Mar 08, 2017 at 03:12:55PM +0100, Daniel Vetter wrote: > > With all drivers converted there's only legacy dri1 drivers using it. > > Not going to touch those, instead just hide it like we've done with > > other dri1 driver hooks like firstopen. > > > > In all this I didn't find any real reason why we'd needed 2 hooks, and > > having symmetry between open and close just appeases my OCD better. > > Yeah, someone else could do an s/postclose/close/, but that's for > > someone who understands cocci. And maybe after this series is reviewed > > and landed, to avoid patch-regen churn. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > > --- > > drivers/gpu/drm/drm_file.c | 8 ++++---- > > include/drm/drm_drv.h | 23 ++--------------------- > > 2 files changed, 6 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > > index a8813a1115dc..f8483fc6d3d7 100644 > > --- a/drivers/gpu/drm/drm_file.c > > +++ b/drivers/gpu/drm/drm_file.c > > @@ -350,9 +350,8 @@ void drm_lastclose(struct drm_device * dev) > > * > > * This function must be used by drivers as their &file_operations.release > > * method. It frees any resources associated with the open file, and calls the > > - * &drm_driver.preclose and &drm_driver.lastclose driver callbacks. If this is > > - * the last open file for the DRM device also proceeds to call the > > - * &drm_driver.lastclose driver callback. > > + * &drm_driver.lastclose driver callback. If this is the last open file for the > > s/lastclose/postclose/ > > Once that's fixed, > > Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> Fixed locally and will include it when I'm resending the entire pile with the stragglers filled out. -Daniel > > > + * DRM device also proceeds to call the &drm_driver.lastclose driver callback. > > * > > * RETURNS: > > * > > @@ -372,7 +371,8 @@ int drm_release(struct inode *inode, struct file *filp) > > list_del(&file_priv->lhead); > > mutex_unlock(&dev->filelist_mutex); > > > > - if (dev->driver->preclose) > > + if (drm_core_check_feature(dev, DRIVER_LEGACY) && > > + dev->driver->preclose) > > dev->driver->preclose(dev, file_priv); > > > > /* ======================================================== > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > > index 8f900fb30275..fde343e0d581 100644 > > --- a/include/drm/drm_drv.h > > +++ b/include/drm/drm_drv.h > > @@ -104,23 +104,6 @@ struct drm_driver { > > int (*open) (struct drm_device *, struct drm_file *); > > > > /** > > - * @preclose: > > - * > > - * One of the driver callbacks when a new &struct drm_file is closed. > > - * Useful for tearing down driver-private data structures allocated in > > - * @open like buffer allocators, execution contexts or similar things. > > - * > > - * Since the display/modeset side of DRM can only be owned by exactly > > - * one &struct drm_file (see &drm_file.is_master and &drm_device.master) > > - * there should never be a need to tear down any modeset related > > - * resources in this callback. Doing so would be a driver design bug. > > - * > > - * FIXME: It is not really clear why there's both @preclose and > > - * @postclose. Without a really good reason, use @postclose only. > > - */ > > - void (*preclose) (struct drm_device *, struct drm_file *file_priv); > > - > > - /** > > * @postclose: > > * > > * One of the driver callbacks when a new &struct drm_file is closed. > > @@ -131,9 +114,6 @@ struct drm_driver { > > * one &struct drm_file (see &drm_file.is_master and &drm_device.master) > > * there should never be a need to tear down any modeset related > > * resources in this callback. Doing so would be a driver design bug. > > - * > > - * FIXME: It is not really clear why there's both @preclose and > > - * @postclose. Without a really good reason, use @postclose only. > > */ > > void (*postclose) (struct drm_device *, struct drm_file *); > > > > @@ -150,7 +130,7 @@ struct drm_driver { > > * state changes, e.g. in conjunction with the :ref:`vga_switcheroo` > > * infrastructure. > > * > > - * This is called after @preclose and @postclose have been called. > > + * This is called after @postclose hook has been called. > > * > > * NOTE: > > * > > @@ -516,6 +496,7 @@ struct drm_driver { > > /* List of devices hanging off this driver with stealth attach. */ > > struct list_head legacy_dev_list; > > int (*firstopen) (struct drm_device *); > > + void (*preclose) (struct drm_device *, struct drm_file *file_priv); > > 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@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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