Lets order things correctly: ->load() ->fistopen() ->open() ->close() ->lastclose() ->unload() This doesn't change much as only savage and radeon use ->firstopen() and they just do map-initialization. Therefore, the global drm mutex makes sure there cannot be any other f_op between ->open() and ->firstopen(). However, once we get rid of that lock, we really want ->firstopen() to initialize the device before ->open() is called. Furthermore, this fixes the clean-up path in drm_open(). We currently don't cleanup the drm_file object if ->firstopen() fails. Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> --- drivers/gpu/drm/drm_fops.c | 139 +++++++++++++++++++++------------------------ include/drm/drmP.h | 2 +- 2 files changed, 66 insertions(+), 75 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 021fe5d..8e73519 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -45,7 +45,7 @@ EXPORT_SYMBOL(drm_global_mutex); static int drm_open_helper(struct file *filp, struct drm_minor *minor); -static int drm_setup(struct drm_device * dev) +static int drm_firstopen(struct drm_device * dev) { int ret; @@ -66,6 +66,57 @@ static int drm_setup(struct drm_device * dev) } /** + * drm_legacy_dev_reinit + * + * Reinitializes a legacy/ums drm device in it's lastclose function. + */ +static void drm_legacy_dev_reinit(struct drm_device *dev) +{ + if (drm_core_check_feature(dev, DRIVER_MODESET)) + return; + + dev->sigdata.lock = NULL; + + dev->context_flag = 0; + dev->last_context = 0; + dev->if_version = 0; +} + +void drm_lastclose(struct drm_device * dev) +{ + struct drm_vma_entry *vma, *vma_temp; + + DRM_DEBUG("\n"); + + if (dev->driver->lastclose) + dev->driver->lastclose(dev); + DRM_DEBUG("driver lastclose completed\n"); + + if (dev->irq_enabled && !drm_core_check_feature(dev, DRIVER_MODESET)) + drm_irq_uninstall(dev); + + mutex_lock(&dev->struct_mutex); + + drm_agp_clear(dev); + + drm_legacy_sg_cleanup(dev); + + /* Clear vma list (only built for debugging) */ + list_for_each_entry_safe(vma, vma_temp, &dev->vmalist, head) { + list_del(&vma->head); + kfree(vma); + } + + drm_legacy_dma_takedown(dev); + + mutex_unlock(&dev->struct_mutex); + + drm_legacy_dev_reinit(dev); + + DRM_DEBUG("lastclose completed\n"); +} + +/** * Open file. * * \param inode device inode @@ -81,31 +132,33 @@ int drm_open(struct inode *inode, struct file *filp) struct drm_device *dev; struct drm_minor *minor; int retcode; - int need_setup = 0; minor = drm_minor_acquire(iminor(inode)); if (IS_ERR(minor)) return PTR_ERR(minor); dev = minor->dev; - if (!dev->open_count++) - need_setup = 1; /* share address_space across all char-devs of a single device */ filp->f_mapping = dev->anon_inode->i_mapping; + if (!dev->open_count) { + retcode = drm_firstopen(dev); + if (retcode) + goto err_minor; + } + ++dev->open_count; + retcode = drm_open_helper(filp, minor); if (retcode) goto err_undo; - if (need_setup) { - retcode = drm_setup(dev); - if (retcode) - goto err_undo; - } + return 0; err_undo: - dev->open_count--; + if (!--dev->open_count) + drm_lastclose(dev); +err_minor: drm_minor_release(minor); return retcode; } @@ -346,67 +399,6 @@ static void drm_events_release(struct drm_file *file_priv) } /** - * drm_legacy_dev_reinit - * - * Reinitializes a legacy/ums drm device in it's lastclose function. - */ -static void drm_legacy_dev_reinit(struct drm_device *dev) -{ - if (drm_core_check_feature(dev, DRIVER_MODESET)) - return; - - dev->sigdata.lock = NULL; - - dev->context_flag = 0; - dev->last_context = 0; - dev->if_version = 0; -} - -/** - * Take down the DRM device. - * - * \param dev DRM device structure. - * - * Frees every resource in \p dev. - * - * \sa drm_device - */ -int drm_lastclose(struct drm_device * dev) -{ - struct drm_vma_entry *vma, *vma_temp; - - DRM_DEBUG("\n"); - - if (dev->driver->lastclose) - dev->driver->lastclose(dev); - DRM_DEBUG("driver lastclose completed\n"); - - if (dev->irq_enabled && !drm_core_check_feature(dev, DRIVER_MODESET)) - drm_irq_uninstall(dev); - - mutex_lock(&dev->struct_mutex); - - drm_agp_clear(dev); - - drm_legacy_sg_cleanup(dev); - - /* Clear vma list (only built for debugging) */ - list_for_each_entry_safe(vma, vma_temp, &dev->vmalist, head) { - list_del(&vma->head); - kfree(vma); - } - - drm_legacy_dma_takedown(dev); - - mutex_unlock(&dev->struct_mutex); - - drm_legacy_dev_reinit(dev); - - DRM_DEBUG("lastclose completed\n"); - return 0; -} - -/** * Release file. * * \param inode device inode @@ -423,7 +415,6 @@ int drm_release(struct inode *inode, struct file *filp) struct drm_file *file_priv = filp->private_data; struct drm_minor *minor = file_priv->minor; struct drm_device *dev = minor->dev; - int retcode = 0; mutex_lock(&drm_global_mutex); @@ -541,7 +532,7 @@ int drm_release(struct inode *inode, struct file *filp) */ if (!--dev->open_count) { - retcode = drm_lastclose(dev); + drm_lastclose(dev); if (drm_device_is_unplugged(dev)) drm_put_dev(dev); } @@ -549,7 +540,7 @@ int drm_release(struct inode *inode, struct file *filp) drm_minor_release(minor); - return retcode; + return 0; } EXPORT_SYMBOL(drm_release); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index d3d9be6..d1730c5 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1181,7 +1181,7 @@ extern long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); extern long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); -extern int drm_lastclose(struct drm_device *dev); +extern void drm_lastclose(struct drm_device *dev); extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags); /* Device support (drm_fops.h) */ -- 2.0.2 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel