On Wed, Nov 19, 2014 at 05:47:14PM -0200, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zanoni@intel.com
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.
Intuitively, (ii) seems like the natural choice to me. The pending userspace events really are fd-specific, so it seems natural to stick them in a drm_file list rather than a global one. You'd have to remove both the base class and the subclass from their respective lists when you're done with an event, but since all of this is done under the event_lock spinlock, it doesn't seem like it would be too complicated. Am I overlooking something that makes it more challenging?
Matt
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);
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;
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);} @@ -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);@@ -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;
-- 2.1.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx