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
