Re: [PATCH 02/22] drm: Add functions to setup/tear down drm_events.

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@gmail.com Acked-by: Daniel Stone daniels@collabora.com Reviewed-by: Alex Deucher alexander.deucher@amd.com Cc: Rob Clark robdclark@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com

Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com

---

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,