[PATCH xf86-video-amdgpu 2.5/4] Add amdgpu_drm_handle_event wrapper for drmHandleEvent

Michel Dänzer michel at daenzer.net
Fri Aug 3 16:18:27 UTC 2018


From: Michel Dänzer <michel.daenzer at amd.com>

Instead of processing DRM events directly from drmHandleEvent's
callbacks, there are three phases:

1. drmHandleEvent is called, and signalled events are re-queued to
   _signalled lists from its callbacks.
2. Signalled page flip completion events are processed.
3. Signalled vblank events are processed.

This should make sure that we never call drmHandleEvent from one of its
callbacks, which would usually result in blocking forever.

Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
---
 src/amdgpu_drm_queue.c | 94 +++++++++++++++++++++++++++++++++++-------
 src/amdgpu_drm_queue.h |  1 +
 src/amdgpu_present.c   |  2 +-
 src/drmmode_display.c  |  4 +-
 4 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c
index 9c0166147..f8660828c 100644
--- a/src/amdgpu_drm_queue.c
+++ b/src/amdgpu_drm_queue.c
@@ -40,6 +40,7 @@
 
 struct amdgpu_drm_queue_entry {
 	struct xorg_list list;
+	uint64_t usec;
 	uint64_t id;
 	uintptr_t seq;
 	void *data;
@@ -47,38 +48,75 @@ struct amdgpu_drm_queue_entry {
 	xf86CrtcPtr crtc;
 	amdgpu_drm_handler_proc handler;
 	amdgpu_drm_abort_proc abort;
+	unsigned int frame;
 };
 
 static int amdgpu_drm_queue_refcnt;
 static struct xorg_list amdgpu_drm_queue;
+static struct xorg_list amdgpu_drm_flip_signalled;
+static struct xorg_list amdgpu_drm_vblank_signalled;
 static uintptr_t amdgpu_drm_queue_seq;
 
 
 /*
- * Handle a DRM event
+ * Process a DRM event
  */
 static void
-amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec,
-			 unsigned int usec, void *user_ptr)
+amdgpu_drm_queue_handle_one(struct amdgpu_drm_queue_entry *e)
+{
+	xorg_list_del(&e->list);
+	if (e->handler) {
+		e->handler(e->crtc, e->frame, e->usec, e->data);
+	} else
+		e->abort(e->crtc, e->data);
+	free(e);
+}
+
+static void
+amdgpu_drm_queue_handler(struct xorg_list *signalled, unsigned int frame,
+			 unsigned int sec, unsigned int usec, void *user_ptr)
 {
 	uintptr_t seq = (uintptr_t)user_ptr;
 	struct amdgpu_drm_queue_entry *e, *tmp;
 
 	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue, list) {
 		if (e->seq == seq) {
-			xorg_list_del(&e->list);
-			if (e->handler)
-				e->handler(e->crtc, frame,
-					   (uint64_t)sec * 1000000 + usec,
-					   e->data);
-			else
+			if (!e->handler) {
 				e->abort(e->crtc, e->data);
-			free(e);
+				break;
+			}
+
+			xorg_list_del(&e->list);
+			e->usec = (uint64_t)sec * 1000000 + usec;
+			e->frame = frame;
+			xorg_list_append(&e->list, signalled);
 			break;
 		}
 	}
 }
 
+/*
+ * Signal a DRM page flip event
+ */
+static void
+amdgpu_drm_page_flip_handler(int fd, unsigned int frame, unsigned int sec,
+			     unsigned int usec, void *user_ptr)
+{
+	amdgpu_drm_queue_handler(&amdgpu_drm_flip_signalled, frame, sec, usec,
+				 user_ptr);
+}
+
+/*
+ * Signal a DRM vblank event
+ */
+static void
+amdgpu_drm_vblank_handler(int fd, unsigned int frame, unsigned int sec,
+			  unsigned int usec, void *user_ptr)
+{
+	amdgpu_drm_queue_handler(&amdgpu_drm_vblank_signalled, frame, sec, usec,
+				 user_ptr);
+}
+
 /*
  * Enqueue a potential drm response; when the associated response
  * appears, we've got data to pass to the handler from here
@@ -177,6 +215,26 @@ amdgpu_drm_abort_id(uint64_t id)
 	}
 }
 
+/*
+ * drmHandleEvent wrapper
+ */
+int
+amdgpu_drm_handle_event(int fd, drmEventContext *event_context)
+{
+	struct amdgpu_drm_queue_entry *e, *tmp;
+	int r;
+
+	r = drmHandleEvent(fd, event_context);
+
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_flip_signalled, list)
+		amdgpu_drm_queue_handle_one(e);
+
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_vblank_signalled, list)
+		amdgpu_drm_queue_handle_one(e);
+
+	return r;
+}
+
 /*
  * Wait for pending page flip on given CRTC to complete
  */
@@ -184,10 +242,14 @@ void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc)
 {
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
-	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+	struct amdgpu_drm_queue_entry *e, *tmp;
+
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_flip_signalled, list)
+		amdgpu_drm_queue_handle_one(e);
 
-	while (drmmode_crtc->flip_pending &&
-	       drmHandleEvent(pAMDGPUEnt->fd, &drmmode->event_context) > 0);
+	while (drmmode_crtc->flip_pending
+	       && amdgpu_drm_handle_event(pAMDGPUEnt->fd,
+					  &drmmode_crtc->drmmode->event_context) > 0);
 }
 
 /*
@@ -200,13 +262,15 @@ amdgpu_drm_queue_init(ScrnInfoPtr scrn)
 	drmmode_ptr drmmode = &info->drmmode;
 
 	drmmode->event_context.version = 2;
-	drmmode->event_context.vblank_handler = amdgpu_drm_queue_handler;
-	drmmode->event_context.page_flip_handler = amdgpu_drm_queue_handler;
+	drmmode->event_context.vblank_handler = amdgpu_drm_vblank_handler;
+	drmmode->event_context.page_flip_handler = amdgpu_drm_page_flip_handler;
 
 	if (amdgpu_drm_queue_refcnt++)
 		return;
 
 	xorg_list_init(&amdgpu_drm_queue);
+	xorg_list_init(&amdgpu_drm_flip_signalled);
+	xorg_list_init(&amdgpu_drm_vblank_signalled);
 }
 
 /*
diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h
index cb6d5ff59..48ba9ab6e 100644
--- a/src/amdgpu_drm_queue.h
+++ b/src/amdgpu_drm_queue.h
@@ -49,6 +49,7 @@ uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client,
 void amdgpu_drm_abort_client(ClientPtr client);
 void amdgpu_drm_abort_entry(uintptr_t seq);
 void amdgpu_drm_abort_id(uint64_t id);
+int amdgpu_drm_handle_event(int fd, drmEventContext *event_context);
 void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc);
 void amdgpu_drm_queue_init(ScrnInfoPtr scrn);
 void amdgpu_drm_queue_close(ScrnInfoPtr scrn);
diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index 4e74bfac9..a6676afdf 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -110,7 +110,7 @@ amdgpu_present_flush_drm_events(ScreenPtr screen)
 	if (r <= 0)
 		return 0;
 
-	return drmHandleEvent(pAMDGPUEnt->fd, &drmmode->event_context) >= 0;
+	return amdgpu_drm_handle_event(pAMDGPUEnt->fd, &drmmode->event_context) >= 0;
 }
 
 /*
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 7f521b66d..167b30091 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -3096,7 +3096,7 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
 static void drmmode_notify_fd(int fd, int notify, void *data)
 {
 	drmmode_ptr drmmode = data;
-	drmHandleEvent(fd, &drmmode->event_context);
+	amdgpu_drm_handle_event(fd, &drmmode->event_context);
 }
 #else
 static void drm_wakeup_handler(pointer data, int err, pointer p)
@@ -3106,7 +3106,7 @@ static void drm_wakeup_handler(pointer data, int err, pointer p)
 	fd_set *read_mask = p;
 
 	if (err >= 0 && FD_ISSET(pAMDGPUEnt->fd, read_mask)) {
-		drmHandleEvent(pAMDGPUEnt->fd, &drmmode->event_context);
+		amdgpu_drm_handle_event(pAMDGPUEnt->fd, &drmmode->event_context);
 	}
 }
 #endif
-- 
2.18.0



More information about the amd-gfx mailing list