Hi Daniel,
Thank you for the patch.
On Monday 11 January 2016 22:40:56 Daniel Vetter wrote:
> An attempt at not spreading out the file_priv->event_space stuff out
> quite so far and wide. And I think fixes something in ipp_get_event()
> that is broken (or if they are doing something more weird/subtle, then
> breaks it in a fun way).
>
> Based upon a patch from Rob Clark, rebased and polished.
>
> v2: Spelling fixes (Alex).
>
> Cc: Alex Deucher <alexdeucher@xxxxxxxxx>
> Acked-by: Daniel Stone <daniels@xxxxxxxxxxxxx>
> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Rob Clark <robdclark@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic.c | 44 ++++++++---------------------
> drivers/gpu/drm/drm_crtc.c | 36 +++++++-----------------
> drivers/gpu/drm/drm_fops.c | 67 +++++++++++++++++++++++++++++++++++++++++
> include/drm/drmP.h | 7 ++++-
> 4 files changed, 94 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3f74193885f1..8fb469c4e4b8 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1347,44 +1347,23 @@ static struct drm_pending_vblank_event
> *create_vblank_event( struct drm_device *dev, struct drm_file *file_priv,
> uint64_t user_data) {
> struct drm_pending_vblank_event *e = NULL;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&dev->event_lock, flags);
> - if (file_priv->event_space < sizeof e->event) {
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - goto out;
> - }
> - file_priv->event_space -= sizeof e->event;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> + int ret;
>
> e = kzalloc(sizeof *e, GFP_KERNEL);
> - if (e == NULL) {
> - spin_lock_irqsave(&dev->event_lock, flags);
> - file_priv->event_space += sizeof e->event;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - goto out;
> - }
> + if (!e)
> + return NULL;
>
> e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
> - e->event.base.length = sizeof e->event;
> + e->event.base.length = sizeof(e->event);
> e->event.user_data = user_data;
> - e->base.event = &e->event.base;
> - e->base.file_priv = file_priv;
> - e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
> -
> -out:
> - return e;
> -}
>
> -static void destroy_vblank_event(struct drm_device *dev,
> - struct drm_file *file_priv, struct drm_pending_vblank_event *e)
> -{
> - unsigned long flags;
> + ret = drm_event_reserve_init(dev, file_priv, &e->base, &e->event.base);
> + if (ret) {
> + kfree(e);
> + return NULL;
> + }
>
> - spin_lock_irqsave(&dev->event_lock, flags);
> - file_priv->event_space += sizeof e->event;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - kfree(e);
> + return e;
> }
>
> static int atomic_set_prop(struct drm_atomic_state *state,
> @@ -1646,8 +1625,7 @@ out:
> if (!crtc_state->event)
> continue;
>
> - destroy_vblank_event(dev, file_priv,
> - crtc_state->event);
> + drm_event_cancel_free(dev, &crtc_state->event->base);
> }
> }
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 1e75a145834a..60a4184d41b7 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5264,7 +5264,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> struct drm_crtc *crtc;
> struct drm_framebuffer *fb = NULL;
> struct drm_pending_vblank_event *e = NULL;
> - unsigned long flags;
> int ret = -EINVAL;
>
> if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
> @@ -5315,41 +5314,26 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> }
>
> if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> - ret = -ENOMEM;
> - spin_lock_irqsave(&dev->event_lock, flags);
> - if (file_priv->event_space < sizeof(e->event)) {
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - goto out;
> - }
> - file_priv->event_space -= sizeof(e->event);
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
> - e = kzalloc(sizeof(*e), GFP_KERNEL);
> - if (e == NULL) {
> - spin_lock_irqsave(&dev->event_lock, flags);
> - file_priv->event_space += sizeof(e->event);
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> + e = kzalloc(sizeof *e, GFP_KERNEL);
> + if (!e) {
> + ret = -ENOMEM;
> goto out;
> }
> -
> e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
> e->event.base.length = sizeof(e->event);
> e->event.user_data = page_flip->user_data;
> - e->base.event = &e->event.base;
> - e->base.file_priv = file_priv;
> - e->base.destroy =
> - (void (*) (struct drm_pending_event *)) kfree;
> + ret = drm_event_reserve_init(dev, file_priv, &e->base, &e-
>event.base);
> + if (ret) {
> + kfree(e);
> + goto out;
> + }
> }
>
> crtc->primary->old_fb = crtc->primary->fb;
> ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
> if (ret) {
> - if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> - spin_lock_irqsave(&dev->event_lock, flags);
> - file_priv->event_space += sizeof(e->event);
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - kfree(e);
> - }
> + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT)
> + drm_event_cancel_free(dev, &e->base);
> /* Keep the old fb, don't unref it. */
> crtc->primary->old_fb = NULL;
> } else {
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 73075a1fa380..476408b638e3 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -672,3 +672,70 @@ unsigned int drm_poll(struct file *filp, struct
> poll_table_struct *wait) return mask;
> }
> EXPORT_SYMBOL(drm_poll);
> +
> +/**
> + * drm_event_reserve_init - init a DRM event and reserve space for it
> + * @dev: DRM device
> + * @file_priv: DRM file private data
> + * @p: tracking structure for the pending event
> + * @e: actual event data to deliver to userspace
> + *
> + * This function prepares the passed in event for eventual delivery. If the
> event + * doesn't get delivered (because the IOCTL fails later on, before
> queuing up + * anything) then the even must be cancelled and freed using
> + * drm_event_cancel_free().
> + *
> + * If callers embedded @p into a larger structure it must be allocated with
> + * kmalloc and @p must be the first member element.
> + *
> + * RETURNS:
> + *
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_event_reserve_init(struct drm_device *dev,
> + struct drm_file *file_priv,
> + struct drm_pending_event *p,
> + struct drm_event *e)
> +{
> + unsigned long flags;
> + int ret = 0;
> +
> + spin_lock_irqsave(&dev->event_lock, flags);
> +
> + if (file_priv->event_space < e->length) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + file_priv->event_space -= e->length;
> +
> + p->event = e;
> + p->file_priv = file_priv;
> +
> + /* we *could* pass this in as arg, but everyone uses kfree: */
> + p->destroy = (void (*) (struct drm_pending_event *)) kfree;
> +
> +out:
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_event_reserve_init);
> +
> +/**
> + * drm_event_cancel_free - free a DRM event and release it's space
> + * @dev: DRM device
> + * @p: tracking structure for the pending event
> + *
> + * This function frees the event @p initialized with
> drm_event_reserve_init() + * and releases any allocated space.
> + */
> +void drm_event_cancel_free(struct drm_device *dev,
> + struct drm_pending_event *p)
> +{
> + unsigned long flags;
> + spin_lock_irqsave(&dev->event_lock, flags);
> + p->file_priv->event_space += p->event->length;
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + p->destroy(p);
> +}
> +EXPORT_SYMBOL(drm_event_cancel_free);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index fc9b9ad5b089..ad4d0a31294d 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -925,8 +925,13 @@ ssize_t drm_read(struct file *filp, char __user
> *buffer, size_t count, loff_t *offset);
> int drm_release(struct inode *inode, struct file *filp);
> int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv);
> -
> unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
> +int drm_event_reserve_init(struct drm_device *dev,
> + struct drm_file *file_priv,
> + struct drm_pending_event *p,
> + struct drm_event *e);
> +void drm_event_cancel_free(struct drm_device *dev,
> + struct drm_pending_event *p);
>
> /* Misc. IOCTL support (drm_ioctl.c) */
> int drm_noop(struct drm_device *dev, void *data,
--
Regards,
Laurent Pinchart
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
