[RFC 6/7] drm: make vblank_event_list handle drm_vblank_wait_item types

Paulo Zanoni
2014-11-19T19:47:14Z

Which means the list doesn't really need to know if the event is from
user space or kernel space.

The only place here where we have to break the abstraction is at
drm_fops, when we're releasing all the events associated with a
file_priv. Here we introduced item.from_user_space, that needs to be
checked before we transform the item pointer into the appropriate
drm_pending_vblank_event pointer. Other strategies to solve this
problem - instead of adding item->from_user_space - would be to: (i)
store a copy of the file_priv pointer in the drm_vblank_wait_item
structure, but then we'd also need a custom destroy() function; or
(ii) add file_priv->pending_event_list, but then we'd have to keep all
the user space items in sync with dev->vblank_event_list, which could
lead to complicated code.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/drm_fops.c | 15 +++++++++++----
drivers/gpu/drm/drm_irq.c | 33 +++++++++++++++++----------------
include/drm/drmP.h | 4 +++-
3 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 47c5e58..fbdbde2 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -280,18 +280,25 @@ static void drm_events_release(struct drm_file *file_priv)
{
struct drm_device *dev = file_priv->minor->dev;
struct drm_pending_event *e, *et;
- struct drm_pending_vblank_event *v, *vt;
+ struct drm_vblank_wait_item *i, *it;
unsigned long flags;

spin_lock_irqsave(&dev->event_lock, flags);

/* Remove pending flips */
- list_for_each_entry_safe(v, vt, &dev->vblank_event_list, base.link)
+ list_for_each_entry_safe(i, it, &dev->vblank_event_list, link) {
+ struct drm_pending_vblank_event *v;
+
+ if (!i->from_user_space)
+ continue;
+
+ v = container_of(i, struct drm_pending_vblank_event, item);
if (v->base.file_priv == file_priv) {
- list_del(&v->base.link);
- drm_vblank_put(dev, v->item.pipe);
+ list_del(&i->link);
+ drm_vblank_put(dev, i->pipe);
v->base.destroy(&v->base);
}
+ }

/* Remove unconsumed events */
list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 4c03240..dd091c3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1144,7 +1144,7 @@ static void drm_wait_vblank_callback(struct drm_device *dev,
void drm_vblank_off(struct drm_device *dev, int crtc)
{
struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
- struct drm_pending_vblank_event *e, *t;
+ struct drm_vblank_wait_item *i, *t;
struct timeval now;
unsigned long irqflags;
unsigned int seq;
@@ -1171,15 +1171,15 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
/* Send any queued vblank events, lest the natives grow disquiet */
seq = drm_vblank_count_and_time(dev, crtc, &now);

- list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
- if (e->item.pipe != crtc)
+ list_for_each_entry_safe(i, t, &dev->vblank_event_list, link) {
+ if (i->pipe != crtc)
continue;
DRM_DEBUG("Sending premature vblank event on disable: \
wanted %d, current %d\n",
- e->item.wanted_seq, seq);
- list_del(&e->base.link);
- drm_vblank_put(dev, e->item.pipe);
- drm_wait_vblank_callback(dev, &e->item, seq, &now, true);
+ i->wanted_seq, seq);
+ list_del(&i->link);
+ drm_vblank_put(dev, i->pipe);
+ drm_wait_vblank_callback(dev, i, seq, &now, true);
}
spin_unlock_irqrestore(&dev->event_lock, irqflags);
}
@@ -1427,6 +1427,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
e->base.event = &e->event.base;
e->base.file_priv = file_priv;
e->base.destroy = (void (*) (struct drm_pending_event *)) kfree;
+ e->item.from_user_space = true;
e->item.callback = callback;
e->item.callback_from_work = callback_from_work;
if (callback_from_work)
@@ -1476,7 +1477,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,
vblwait->reply.sequence = seq;
} else {
/* drm_handle_vblank_events will call drm_vblank_put */
- list_add_tail(&e->base.link, &dev->vblank_event_list);
+ list_add_tail(&e->item.link, &dev->vblank_event_list);
vblwait->reply.sequence = vblwait->request.sequence;
}

@@ -1635,7 +1636,7 @@ EXPORT_SYMBOL(drm_wait_vblank_kernel);

static void drm_handle_vblank_events(struct drm_device *dev, int crtc)
{
- struct drm_pending_vblank_event *e, *t;
+ struct drm_vblank_wait_item *i, *t;
struct timeval now;
unsigned int seq;

@@ -1643,18 +1644,18 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc)

seq = drm_vblank_count_and_time(dev, crtc, &now);

- list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
- if (e->item.pipe != crtc)
+ list_for_each_entry_safe(i, t, &dev->vblank_event_list, link) {
+ if (i->pipe != crtc)
continue;
- if ((seq - e->item.wanted_seq) > (1<<23))
+ if ((seq - i->wanted_seq) > (1<<23))
continue;

DRM_DEBUG("vblank event on %d, current %d\n",
- e->item.wanted_seq, seq);
+ i->wanted_seq, seq);

- list_del(&e->base.link);
- drm_vblank_put(dev, e->item.pipe);
- drm_wait_vblank_callback(dev, &e->item, seq, &now, false);
+ list_del(&i->link);
+ drm_vblank_put(dev, i->pipe);
+ drm_wait_vblank_callback(dev, i, seq, &now, false);
}

trace_drm_vblank_event(crtc, seq);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 474c892..46724d9 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -258,7 +258,7 @@ struct drm_ioctl_desc {
/* Event queued up for userspace to read */
struct drm_pending_event {
struct drm_event *event;
- struct list_head link;
+ struct list_head link; /* file_priv->event_list */
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 */
@@ -668,8 +668,10 @@ typedef void (*drm_vblank_callback_t)(struct drm_device *dev,
bool premature);

struct drm_vblank_wait_item {
+ struct list_head link; /* dev->vblank_event_list */
int pipe;
unsigned int wanted_seq;
+ bool from_user_space;

drm_vblank_callback_t callback;
bool callback_from_work;