On Thu, Sep 01, 2016 at 02:48:35PM +0200, David Herrmann wrote:
> Rather than doing drm_file allocation/destruction right in the fops, lets
> provide separate helpers. This decouples drm_file management from the
> still-mandatory drm-fops. It prepares for use of drm_file without the
> fops, both by possible separate fops implementations and APIs (not that I
> am aware of any such plans), and more importantly from in-kernel use where
> no real file is available.
>
> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_drv.c | 135 +++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_fops.c | 132 +++-------------------------------------
> drivers/gpu/drm/drm_internal.h | 4 ++
> 3 files changed, 147 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 57ce973..9ab0016 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -95,6 +95,141 @@ void drm_ut_debug_printk(const char *function_name, const char *format, ...)
> }
> EXPORT_SYMBOL(drm_ut_debug_printk);
>
> +/**
> + * drm_file_alloc - allocate file context
> + * @minor: minor to allocate on
> + *
> + * This allocates a new DRM file context. It is not linked into any context and
> + * can be used by the caller freely. Note that the context keeps a pointer to
> + * @minor, so it must be freed before @minor is.
> + *
> + * The legacy paths might require the drm_global_mutex to be held.
> + *
> + * RETURNS:
> + * Pointer to newly allocated context, ERR_PTR on failure.
> + */
Hm, in drm core we only type kerneldoc for exported stuff, not internal
interfaces. The idea being that the target audience for those docs is
(mostly) driver authors.
And since you're touching this, drm_file.[hc] would look pretty I think,
with kerneldoc for the structures ...
-Daniel
> +struct drm_file *drm_file_alloc(struct drm_minor *minor)
> +{
> + struct drm_device *dev = minor->dev;
> + struct drm_file *file;
> + int ret;
> +
> + file = kzalloc(sizeof(*file), GFP_KERNEL);
> + if (!file)
> + return ERR_PTR(-ENOMEM);
> +
> + file->pid = get_pid(task_pid(current));
> + file->minor = minor;
> + file->authenticated = capable(CAP_SYS_ADMIN); /* legacy compat */
> + INIT_LIST_HEAD(&file->lhead);
> + INIT_LIST_HEAD(&file->fbs);
> + mutex_init(&file->fbs_lock);
> + INIT_LIST_HEAD(&file->blobs);
> + INIT_LIST_HEAD(&file->pending_event_list);
> + INIT_LIST_HEAD(&file->event_list);
> + init_waitqueue_head(&file->event_wait);
> + file->event_space = 4096; /* set aside 4k for event buffer */
> + mutex_init(&file->event_read_lock);
> +
> + if (drm_core_check_feature(dev, DRIVER_GEM))
> + drm_gem_open(dev, file);
> + if (drm_core_check_feature(dev, DRIVER_PRIME))
> + drm_prime_init_file_private(&file->prime);
> +
> + if (dev->driver->open) {
> + ret = dev->driver->open(dev, file);
> + if (ret < 0)
> + goto out_prime_destroy;
> + }
> +
> + if (drm_is_primary_client(file)) {
> + ret = drm_master_open(file);
> + if (ret)
> + goto out_close;
> + }
> +
> + return file;
> +
> +out_close:
> + if (dev->driver->postclose)
> + dev->driver->postclose(dev, file);
> +out_prime_destroy:
> + if (drm_core_check_feature(dev, DRIVER_PRIME))
> + drm_prime_destroy_file_private(&file->prime);
> + if (drm_core_check_feature(dev, DRIVER_GEM))
> + drm_gem_release(dev, file);
> + put_pid(file->pid);
> + kfree(file);
> + return ERR_PTR(ret);
> +}
> +
> +/**
> + * drm_file_free - free file context
> + * @file: context to free, or NULL
> + *
> + * This destroys and deallocates a DRM file context previously allocated via
> + * drm_file_alloc(). The caller must make sure to unlink it from any contexts
> + * before calling this.
> + *
> + * The legacy paths might require the drm_global_mutex to be held.
> + *
> + * If NULL is passed, this is a no-op.
> + *
> + * RETURNS:
> + * 0 on success, or error code on failure.
> + */
> +void drm_file_free(struct drm_file *file)
> +{
> + struct drm_pending_event *e;
> + struct drm_device *dev;
> +
> + if (!file)
> + return;
> +
> + dev = file->minor->dev;
> +
> + if (dev->driver->preclose)
> + dev->driver->preclose(dev, file);
> + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> + drm_legacy_lock_release(dev, file->legacy_filp);
> + if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
> + drm_legacy_reclaim_buffers(dev, file);
> +
> + spin_lock_irq(&dev->event_lock);
> + while ((e = list_first_entry_or_null(&file->pending_event_list,
> + struct drm_pending_event,
> + pending_link))) {
> + list_del(&e->pending_link);
> + e->file_priv = NULL;
> + }
> + while ((e = list_first_entry_or_null(&file->event_list,
> + struct drm_pending_event, link))) {
> + list_del(&e->link);
> + kfree(e);
> + }
> + spin_unlock_irq(&dev->event_lock);
> +
> + drm_legacy_ctxbitmap_flush(dev, file);
> +
> + if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> + drm_fb_release(file);
> + drm_property_destroy_user_blobs(dev, file);
> + }
> +
> + if (drm_core_check_feature(dev, DRIVER_GEM))
> + drm_gem_release(dev, file);
> + if (drm_is_primary_client(file))
> + drm_master_release(file);
> + if (dev->driver->postclose)
> + dev->driver->postclose(dev, file);
> + if (drm_core_check_feature(dev, DRIVER_PRIME))
> + drm_prime_destroy_file_private(&file->prime);
> +
> + WARN_ON(!list_empty(&file->event_list));
> + put_pid(file->pid);
> + kfree(file);
> +}
> +
> /*
> * DRM Minors
> * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 69ef23c..359c636 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -182,7 +182,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
> {
> struct drm_device *dev = minor->dev;
> struct drm_file *priv;
> - int ret;
>
> if (filp->f_flags & O_EXCL)
> return -EBUSY; /* No exclusive opens */
> @@ -193,47 +192,12 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
>
> DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor->index);
>
> - priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> + priv = drm_file_alloc(minor);
> + if (IS_ERR(priv))
> + return PTR_ERR(priv);
>
> - filp->private_data = priv;
> priv->legacy_filp = filp;
> - priv->pid = get_pid(task_pid(current));
> - priv->minor = minor;
> -
> - /* for compatibility root is always authenticated */
> - priv->authenticated = capable(CAP_SYS_ADMIN);
> - priv->lock_count = 0;
> -
> - INIT_LIST_HEAD(&priv->lhead);
> - INIT_LIST_HEAD(&priv->fbs);
> - mutex_init(&priv->fbs_lock);
> - INIT_LIST_HEAD(&priv->blobs);
> - INIT_LIST_HEAD(&priv->pending_event_list);
> - INIT_LIST_HEAD(&priv->event_list);
> - init_waitqueue_head(&priv->event_wait);
> - priv->event_space = 4096; /* set aside 4k for event buffer */
> -
> - mutex_init(&priv->event_read_lock);
> -
> - if (drm_core_check_feature(dev, DRIVER_GEM))
> - drm_gem_open(dev, priv);
> -
> - if (drm_core_check_feature(dev, DRIVER_PRIME))
> - drm_prime_init_file_private(&priv->prime);
> -
> - if (dev->driver->open) {
> - ret = dev->driver->open(dev, priv);
> - if (ret < 0)
> - goto out_prime_destroy;
> - }
> -
> - if (drm_is_primary_client(priv)) {
> - ret = drm_master_open(priv);
> - if (ret)
> - goto out_close;
> - }
> + filp->private_data = priv;
>
> mutex_lock(&dev->filelist_mutex);
> list_add(&priv->lhead, &dev->filelist);
> @@ -260,43 +224,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
> #endif
>
> return 0;
> -
> -out_close:
> - if (dev->driver->postclose)
> - dev->driver->postclose(dev, priv);
> -out_prime_destroy:
> - if (drm_core_check_feature(dev, DRIVER_PRIME))
> - drm_prime_destroy_file_private(&priv->prime);
> - if (drm_core_check_feature(dev, DRIVER_GEM))
> - drm_gem_release(dev, priv);
> - put_pid(priv->pid);
> - kfree(priv);
> - filp->private_data = NULL;
> - return ret;
> -}
> -
> -static void drm_events_release(struct drm_file *file_priv)
> -{
> - struct drm_device *dev = file_priv->minor->dev;
> - struct drm_pending_event *e, *et;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&dev->event_lock, flags);
> -
> - /* Unlink pending events */
> - list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
> - pending_link) {
> - list_del(&e->pending_link);
> - e->file_priv = NULL;
> - }
> -
> - /* Remove unconsumed events */
> - list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
> - list_del(&e->link);
> - kfree(e);
> - }
> -
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> }
>
> /*
> @@ -370,59 +297,16 @@ int drm_release(struct inode *inode, struct file *filp)
>
> mutex_lock(&drm_global_mutex);
>
> - DRM_DEBUG("open_count = %d\n", dev->open_count);
> -
> - mutex_lock(&dev->filelist_mutex);
> - list_del(&file_priv->lhead);
> - mutex_unlock(&dev->filelist_mutex);
> -
> - if (dev->driver->preclose)
> - dev->driver->preclose(dev, file_priv);
> -
> - /* ========================================================
> - * Begin inline drm_release
> - */
> -
> DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n",
> task_pid_nr(current),
> (long)old_encode_dev(file_priv->minor->kdev->devt),
> dev->open_count);
>
> - if (!drm_core_check_feature(dev, DRIVER_MODESET))
> - drm_legacy_lock_release(dev, filp);
> -
> - if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
> - drm_legacy_reclaim_buffers(dev, file_priv);
> -
> - drm_events_release(file_priv);
> -
> - if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> - drm_fb_release(file_priv);
> - drm_property_destroy_user_blobs(dev, file_priv);
> - }
> -
> - if (drm_core_check_feature(dev, DRIVER_GEM))
> - drm_gem_release(dev, file_priv);
> -
> - drm_legacy_ctxbitmap_flush(dev, file_priv);
> -
> - if (drm_is_primary_client(file_priv))
> - drm_master_release(file_priv);
> -
> - if (dev->driver->postclose)
> - dev->driver->postclose(dev, file_priv);
> -
> - if (drm_core_check_feature(dev, DRIVER_PRIME))
> - drm_prime_destroy_file_private(&file_priv->prime);
> -
> - WARN_ON(!list_empty(&file_priv->event_list));
> -
> - put_pid(file_priv->pid);
> - kfree(file_priv);
> + mutex_lock(&dev->filelist_mutex);
> + list_del(&file_priv->lhead);
> + mutex_unlock(&dev->filelist_mutex);
>
> - /* ========================================================
> - * End inline drm_release
> - */
> + drm_file_free(file_priv);
>
> if (!--dev->open_count) {
> drm_lastclose(dev);
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index b86dc9b..9b66cc4 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -28,6 +28,10 @@ extern unsigned int drm_timestamp_monotonic;
> extern struct mutex drm_global_mutex;
> void drm_lastclose(struct drm_device *dev);
>
> +/* drm_drv.c */
> +struct drm_file *drm_file_alloc(struct drm_minor *minor);
> +void drm_file_free(struct drm_file *file);
> +
> /* drm_pci.c */
> int drm_irq_by_busid(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> --
> 2.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
