Re: [PATCH 12/38] drm/fence: add fence to drm_pending_event

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 1, 2016 at 6:06 PM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
>
> Now a drm_pending_event can either send a real drm_event or signal a
> fence, or both. It allow us to signal via fences when the buffer is
> displayed on the screen. Which in turn means that the previous buffer
> is not in use anymore and can be freed or sent back to another driver
> for processing.
>
> v2: Comments from Daniel Vetter
>         - call fence_signal in drm_send_event_locked()
>         - remove unneeded !e->event check
>
> v3: Remove drm_pending_event->destroy to fix a leak when e->file_priv
> is not set.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx> (v2)
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>


I suppose one could argue that the s/destroy/kfree/ change might
warrant a separate change, but the patch is very readable as-is, so:

Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>

> ---
>  drivers/gpu/drm/drm_atomic.c                | 19 +++++++++++++------
>  drivers/gpu/drm/drm_fops.c                  | 16 +++++++++-------
>  drivers/gpu/drm/nouveau/nouveau_usif.c      |  1 -
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  2 +-
>  include/drm/drmP.h                          |  3 ++-
>  5 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5093d81f60f7..5e4b820a977c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1413,7 +1413,8 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
>   */
>
>  static struct drm_pending_vblank_event *create_vblank_event(
> -               struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data)
> +               struct drm_device *dev, struct drm_file *file_priv,
> +               struct fence *fence, uint64_t user_data)
>  {
>         struct drm_pending_vblank_event *e = NULL;
>         int ret;
> @@ -1426,12 +1427,17 @@ static struct drm_pending_vblank_event *create_vblank_event(
>         e->event.base.length = sizeof(e->event);
>         e->event.user_data = user_data;
>
> -       ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
> -       if (ret) {
> -               kfree(e);
> -               return NULL;
> +       if (file_priv) {
> +               ret = drm_event_reserve_init(dev, file_priv, &e->base,
> +                                            &e->event.base);
> +               if (ret) {
> +                       kfree(e);
> +                       return NULL;
> +               }
>         }
>
> +       e->base.fence = fence;
> +
>         return e;
>  }
>
> @@ -1671,7 +1677,8 @@ retry:
>                 for_each_crtc_in_state(state, crtc, crtc_state, i) {
>                         struct drm_pending_vblank_event *e;
>
> -                       e = create_vblank_event(dev, file_priv, arg->user_data);
> +                       e = create_vblank_event(dev, file_priv, NULL,
> +                                               arg->user_data);
>                         if (!e) {
>                                 ret = -ENOMEM;
>                                 goto out;
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 868871068956..4c4b30f7a9f2 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -294,7 +294,7 @@ static void drm_events_release(struct drm_file *file_priv)
>         /* Remove unconsumed events */
>         list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
>                 list_del(&e->link);
> -               e->destroy(e);
> +               kfree(e);
>         }
>
>         spin_unlock_irqrestore(&dev->event_lock, flags);
> @@ -525,7 +525,7 @@ put_back_event:
>                         }
>
>                         ret += length;
> -                       e->destroy(e);
> +                       kfree(e);
>                 }
>         }
>         mutex_unlock(&file_priv->event_read_lock);
> @@ -602,9 +602,6 @@ int drm_event_reserve_init_locked(struct drm_device *dev,
>         list_add(&p->pending_link, &file_priv->pending_event_list);
>         p->file_priv = file_priv;
>
> -       /* we *could* pass this in as arg, but everyone uses kfree: */
> -       p->destroy = (void (*) (struct drm_pending_event *)) kfree;
> -
>         return 0;
>  }
>  EXPORT_SYMBOL(drm_event_reserve_init_locked);
> @@ -667,7 +664,7 @@ void drm_event_cancel_free(struct drm_device *dev,
>                 list_del(&p->pending_link);
>         }
>         spin_unlock_irqrestore(&dev->event_lock, flags);
> -       p->destroy(p);
> +       kfree(p);
>  }
>  EXPORT_SYMBOL(drm_event_cancel_free);
>
> @@ -689,8 +686,13 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e)
>  {
>         assert_spin_locked(&dev->event_lock);
>
> +       if (e->fence) {
> +               fence_signal(e->fence);
> +               fence_put(e->fence);
> +       }
> +
>         if (!e->file_priv) {
> -               e->destroy(e);
> +               kfree(e);
>                 return;
>         }
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_usif.c b/drivers/gpu/drm/nouveau/nouveau_usif.c
> index 675e9e077a95..08f9c6fa0f7f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_usif.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_usif.c
> @@ -212,7 +212,6 @@ usif_notify_get(struct drm_file *f, void *data, u32 size, void *argv, u32 argc)
>         ntfy->p->base.event = &ntfy->p->e.base;
>         ntfy->p->base.file_priv = f;
>         ntfy->p->base.pid = current->pid;
> -       ntfy->p->base.destroy =(void(*)(struct drm_pending_event *))kfree;
>         ntfy->p->e.base.type = DRM_NOUVEAU_EVENT_NVIF;
>         ntfy->p->e.base.length = sizeof(ntfy->p->e.base) + ntfy->reply;
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 1c4d5b5a70a2..5567fb43e674 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -889,7 +889,7 @@ static void vop_crtc_cancel_pending_vblank(struct drm_crtc *crtc,
>         if (e && e->base.file_priv == file_priv) {
>                 vop->event = NULL;
>
> -               e->base.destroy(&e->base);
> +               kfree(&e->base);
>                 file_priv->event_space += sizeof(e->event);
>         }
>         spin_unlock_irqrestore(&drm->event_lock, flags);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index f51fa4328494..51f751d1c8a4 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -57,6 +57,7 @@
>  #include <linux/types.h>
>  #include <linux/vmalloc.h>
>  #include <linux/workqueue.h>
> +#include <linux/fence.h>
>
>  #include <asm/mman.h>
>  #include <asm/pgalloc.h>
> @@ -283,12 +284,12 @@ struct drm_ioctl_desc {
>  /* Event queued up for userspace to read */
>  struct drm_pending_event {
>         struct drm_event *event;
> +       struct fence *fence;
>         struct list_head link;
>         struct list_head pending_link;
>         struct drm_file *file_priv;
>         pid_t pid; /* pid of requester, no guarantee it's valid by the time
>                       we deliver the event, for tracing only */
> -       void (*destroy)(struct drm_pending_event *event);
>  };
>
>  /* initial implementaton using a linked list - todo hashtab */
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux