[PATCH xf86-video-amdgpu 1/4] Move DRM event queue related initialization to amdgpu_drm_queue_init

[PATCH xf86-video-amdgpu 1/4] Move DRM event queue related initialization to amdgpu_drm_queue_init

[Michel Dänzer]

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

And make amdgpu_drm_queue_handler not directly accessible outside of
amdgpu_drm_queue.c.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/amdgpu_dri2.c      | 14 ++++++++------
 src/amdgpu_drm_queue.c | 11 +++++++++--
 src/amdgpu_drm_queue.h |  5 +----
 src/amdgpu_kms.c       |  2 +-
 src/drmmode_display.c  |  4 ----
 5 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c
index a9238e536..96b2d1760 100644
--- a/src/amdgpu_dri2.c
+++ b/src/amdgpu_dri2.c
@@ -873,13 +873,15 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data)
 
 	scrn = crtc->scrn;
 	pAMDGPUEnt = AMDGPUEntPriv(scrn);
+	drmmode_crtc = event_info->crtc->driver_private;
 	ret = drmmode_get_current_ust(pAMDGPUEnt->fd, &drm_now);
 	if (ret) {
 		xf86DrvMsg(scrn->scrnIndex, X_ERROR,
 			   "%s cannot get current time\n", __func__);
 		if (event_info->drm_queue_seq)
-			amdgpu_drm_queue_handler(pAMDGPUEnt->fd, 0, 0, 0,
-						 (void*)event_info->drm_queue_seq);
+			drmmode_crtc->drmmode->event_context.
+				vblank_handler(pAMDGPUEnt->fd, 0, 0, 0,
+					       (void*)event_info->drm_queue_seq);
 		else
 			amdgpu_dri2_frame_event_handler(crtc, 0, 0, data);
 		return 0;
@@ -888,15 +890,15 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data)
 	 * calculate the frame number from current time
 	 * that would come from CRTC if it were running
 	 */
-	drmmode_crtc = event_info->crtc->driver_private;
 	delta_t = drm_now - (CARD64) drmmode_crtc->dpms_last_ust;
 	delta_seq = delta_t * drmmode_crtc->dpms_last_fps;
 	delta_seq /= 1000000;
 	frame = (CARD64) drmmode_crtc->dpms_last_seq + delta_seq;
 	if (event_info->drm_queue_seq)
-		amdgpu_drm_queue_handler(pAMDGPUEnt->fd, frame, drm_now / 1000000,
-					 drm_now % 1000000,
-					 (void*)event_info->drm_queue_seq);
+		drmmode_crtc->drmmode->event_context.
+			vblank_handler(pAMDGPUEnt->fd, frame, drm_now / 1000000,
+				       drm_now % 1000000,
+				       (void*)event_info->drm_queue_seq);
 	else
 		amdgpu_dri2_frame_event_handler(crtc, frame, drm_now, data);
 	return 0;
diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c
index d1456ca84..dfe0148c8 100644
--- a/src/amdgpu_drm_queue.c
+++ b/src/amdgpu_drm_queue.c
@@ -57,7 +57,7 @@ static uintptr_t amdgpu_drm_queue_seq;
 /*
  * Handle a DRM event
  */
-void
+static void
 amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec,
 			 unsigned int usec, void *user_ptr)
 {
@@ -181,8 +181,15 @@ amdgpu_drm_abort_id(uint64_t id)
  * Initialize the DRM event queue
  */
 void
-amdgpu_drm_queue_init()
+amdgpu_drm_queue_init(ScrnInfoPtr scrn)
 {
+	AMDGPUInfoPtr info = AMDGPUPTR(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;
+
 	if (amdgpu_drm_queue_refcnt++)
 		return;
 
diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h
index 36ee900aa..328b5e6b1 100644
--- a/src/amdgpu_drm_queue.h
+++ b/src/amdgpu_drm_queue.h
@@ -42,9 +42,6 @@ typedef void (*amdgpu_drm_handler_proc)(xf86CrtcPtr crtc, uint32_t seq,
 					uint64_t usec, void *data);
 typedef void (*amdgpu_drm_abort_proc)(xf86CrtcPtr crtc, void *data);
 
-void amdgpu_drm_queue_handler(int fd, unsigned int frame,
-			      unsigned int tv_sec, unsigned int tv_usec,
-			      void *user_ptr);
 uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client,
 				 uint64_t id, void *data,
 				 amdgpu_drm_handler_proc handler,
@@ -52,7 +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);
-void amdgpu_drm_queue_init();
+void amdgpu_drm_queue_init(ScrnInfoPtr scrn);
 void amdgpu_drm_queue_close(ScrnInfoPtr scrn);
 
 #endif /* _AMDGPU_DRM_QUEUE_H_ */
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 7b13d777e..e2925cc83 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -1407,7 +1407,7 @@ Bool AMDGPUPreInit_KMS(ScrnInfoPtr pScrn, int flags)
 	if (!AMDGPUPreInitAccel_KMS(pScrn))
 		return FALSE;
 
-	amdgpu_drm_queue_init();
+	amdgpu_drm_queue_init(pScrn);
 
 	/* don't enable tiling if accel is not enabled */
 	if (info->use_glamor) {
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 92f58c157..c45b79d21 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -3315,10 +3315,6 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
 
 	xf86InitialConfiguration(pScrn, TRUE);
 
-	drmmode->event_context.version = 2;
-	drmmode->event_context.vblank_handler = amdgpu_drm_queue_handler;
-	drmmode->event_context.page_flip_handler = amdgpu_drm_queue_handler;
-
 	pAMDGPUEnt->has_page_flip_target = drmmode_probe_page_flip_target(pAMDGPUEnt);
 
 	drmModeFreeResources(mode_res);
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 2/4] Add amdgpu_drm_wait_pending_flip function

[Michel Dänzer]

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

Replacing the drmmode_crtc_wait_pending_event macro.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/amdgpu_drm_queue.c | 13 +++++++++++++
 src/amdgpu_drm_queue.h |  1 +
 src/drmmode_display.c  | 22 ++++------------------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c
index dfe0148c8..9c0166147 100644
--- a/src/amdgpu_drm_queue.c
+++ b/src/amdgpu_drm_queue.c
@@ -177,6 +177,19 @@ amdgpu_drm_abort_id(uint64_t id)
 	}
 }
 
+/*
+ * Wait for pending page flip on given CRTC to complete
+ */
+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;
+
+	while (drmmode_crtc->flip_pending &&
+	       drmHandleEvent(pAMDGPUEnt->fd, &drmmode->event_context) > 0);
+}
+
 /*
  * Initialize the DRM event queue
  */
diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h
index 328b5e6b1..cb6d5ff59 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);
+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/drmmode_display.c b/src/drmmode_display.c
index c45b79d21..615d3be8f 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -97,13 +97,6 @@ AMDGPUZaphodStringMatches(ScrnInfoPtr pScrn, const char *s, char *output_name)
 }
 
 
-/* Wait for the boolean condition to be FALSE */
-#define drmmode_crtc_wait_pending_event(drmmode_crtc, fd, condition) \
-	do {} while ((condition) && \
-		     drmHandleEvent(fd, &drmmode_crtc->drmmode->event_context) \
-		     > 0);
-
-
 static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn,
 					  int width, int height,
 					  int depth, int bpp,
@@ -287,8 +280,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 	if (drmmode_crtc->dpms_mode == DPMSModeOn && mode != DPMSModeOn) {
 		uint32_t seq;
 
-		drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd,
-						drmmode_crtc->flip_pending);
+		amdgpu_drm_wait_pending_flip(crtc);
 
 		/*
 		 * On->Off transition: record the last vblank time,
@@ -1361,8 +1353,7 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
 			goto done;
 		}
 
-		drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd,
-						drmmode_crtc->flip_pending);
+		amdgpu_drm_wait_pending_flip(crtc);
 
 		if (!drmmode_set_mode(crtc, fb, mode, x, y))
 			goto done;
@@ -2329,15 +2320,11 @@ drmmode_output_set_tear_free(AMDGPUEntPtr pAMDGPUEnt,
 	drmmode_output->tear_free = tear_free;
 
 	if (crtc) {
-		drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
-
 		/* Wait for pending flips before drmmode_set_mode_major calls
 		 * drmmode_crtc_update_tear_free, to prevent a nested
 		 * drmHandleEvent call, which would hang
 		 */
-		drmmode_crtc_wait_pending_event(drmmode_crtc,
-						pAMDGPUEnt->fd,
-						drmmode_crtc->flip_pending);
+		amdgpu_drm_wait_pending_flip(crtc);
 		drmmode_set_mode_major(crtc, &crtc->mode, crtc->rotation,
 				       crtc->x, crtc->y);
 	}
@@ -3954,8 +3941,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 			amdgpu_glamor_flush(crtc->scrn);
 
 			if (drmmode_crtc->scanout_update_pending) {
-				drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd,
-								drmmode_crtc->flip_pending);
+				amdgpu_drm_wait_pending_flip(crtc);
 				amdgpu_drm_abort_entry(drmmode_crtc->scanout_update_pending);
 				drmmode_crtc->scanout_update_pending = 0;
 			}
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 3/4] Defer vblank event handling while waiting for a pending flip

[Michel Dänzer]

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

This is a more robust way to prevent hangs due to nested calls to
drmHandleEvent.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/amdgpu_drm_queue.c | 124 +++++++++++++++++++++++++++++++++++++----
 src/amdgpu_drm_queue.h |   1 +
 src/drmmode_display.c  |  18 ++++--
 src/drmmode_display.h  |   4 ++
 4 files changed, 131 insertions(+), 16 deletions(-)

diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c
index 9c0166147..d4353e703 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,13 +48,27 @@ 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_queue_deferred;
 static uintptr_t amdgpu_drm_queue_seq;
 
 
+static void
+amdgpu_drm_queue_handle_one(struct amdgpu_drm_queue_entry *e, unsigned int frame,
+			    uint64_t usec)
+{
+	xorg_list_del(&e->list);
+	if (e->handler) {
+		e->handler(e->crtc, frame, usec, e->data);
+	} else
+		e->abort(e->crtc, e->data);
+	free(e);
+}
+
 /*
  * Handle a DRM event
  */
@@ -66,19 +81,80 @@ amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec,
 
 	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
-				e->abort(e->crtc, e->data);
-			free(e);
+			amdgpu_drm_queue_handle_one(e, frame, (uint64_t)sec *
+						    1000000 + usec);
 			break;
 		}
 	}
 }
 
+/*
+ * Handle a DRM vblank event
+ */
+static void
+amdgpu_drm_queue_vblank_handler(int fd, 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) {
+			AMDGPUInfoPtr info = AMDGPUPTR(e->crtc->scrn);
+			drmmode_ptr drmmode = &info->drmmode;
+
+			e->usec = (uint64_t)sec * 1000000 + usec;
+
+			if (drmmode->event_context.vblank_handler ==
+			    amdgpu_drm_queue_vblank_handler) {
+				amdgpu_drm_queue_handle_one(e, frame, e->usec);
+			} else {
+				xorg_list_del(&e->list);
+				e->frame = frame;
+				xorg_list_append(&e->list,
+						 &amdgpu_drm_queue_deferred);
+			}
+
+			break;
+		}
+	}
+}
+
+/*
+ * Re-queue a DRM vblank event for deferred handling
+ */
+static void
+amdgpu_drm_queue_vblank_defer(int fd, unsigned int frame, unsigned int sec,
+			      unsigned int usec, void *user_ptr)
+{
+	amdgpu_drm_queue_vblank_handler(fd, frame, sec, usec, user_ptr);
+}
+
+/*
+ * Handle deferred DRM vblank events
+ *
+ * This function must be called after amdgpu_drm_wait_pending_flip, once
+ * it's safe to attempt queueing a flip again
+ */
+void
+amdgpu_drm_queue_handle_deferred(xf86CrtcPtr crtc)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
+	struct amdgpu_drm_queue_entry *e, *tmp;
+
+	if (drmmode_crtc->wait_flip_nesting_level == 0 ||
+	    --drmmode_crtc->wait_flip_nesting_level > 0)
+		return;
+
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue_deferred, list) {
+		if (e->crtc == crtc)
+			amdgpu_drm_queue_handle_one(e, e->frame, e->usec);
+	}
+
+	drmmode->event_context.vblank_handler = amdgpu_drm_queue_vblank_handler;
+}
+
 /*
  * Enqueue a potential drm response; when the associated response
  * appears, we've got data to pass to the handler from here
@@ -134,12 +210,17 @@ amdgpu_drm_abort_one(struct amdgpu_drm_queue_entry *e)
 void
 amdgpu_drm_abort_client(ClientPtr client)
 {
-	struct amdgpu_drm_queue_entry *e;
+	struct amdgpu_drm_queue_entry *e, *tmp;
 
 	xorg_list_for_each_entry(e, &amdgpu_drm_queue, list) {
 		if (e->client == client)
 			e->handler = NULL;
 	}
+
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue_deferred, list) {
+		if (e->client == client)
+			amdgpu_drm_abort_one(e);
+	}
 }
 
 /*
@@ -153,6 +234,13 @@ amdgpu_drm_abort_entry(uintptr_t seq)
 	if (seq == AMDGPU_DRM_QUEUE_ERROR)
 		return;
 
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue_deferred, list) {
+		if (e->seq == seq) {
+			amdgpu_drm_abort_one(e);
+			return;
+		}
+	}
+
 	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue, list) {
 		if (e->seq == seq) {
 			amdgpu_drm_abort_one(e);
@@ -169,6 +257,13 @@ amdgpu_drm_abort_id(uint64_t id)
 {
 	struct amdgpu_drm_queue_entry *e, *tmp;
 
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue_deferred, list) {
+		if (e->id == id) {
+			amdgpu_drm_abort_one(e);
+			break;
+		}
+	}
+
 	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue, list) {
 		if (e->id == id) {
 			amdgpu_drm_abort_one(e);
@@ -186,6 +281,9 @@ void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc)
 	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
 	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 
+	drmmode_crtc->wait_flip_nesting_level++;
+	drmmode->event_context.vblank_handler = amdgpu_drm_queue_vblank_defer;
+
 	while (drmmode_crtc->flip_pending &&
 	       drmHandleEvent(pAMDGPUEnt->fd, &drmmode->event_context) > 0);
 }
@@ -200,13 +298,14 @@ 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.vblank_handler = amdgpu_drm_queue_vblank_handler;
 	drmmode->event_context.page_flip_handler = amdgpu_drm_queue_handler;
 
 	if (amdgpu_drm_queue_refcnt++)
 		return;
 
 	xorg_list_init(&amdgpu_drm_queue);
+	xorg_list_init(&amdgpu_drm_queue_deferred);
 }
 
 /*
@@ -222,5 +321,10 @@ amdgpu_drm_queue_close(ScrnInfoPtr scrn)
 			amdgpu_drm_abort_one(e);
 	}
 
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue_deferred, list) {
+		if (e->crtc->scrn == scrn)
+			amdgpu_drm_abort_one(e);
+	}
+
 	amdgpu_drm_queue_refcnt--;
 }
diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h
index cb6d5ff59..75609e39b 100644
--- a/src/amdgpu_drm_queue.h
+++ b/src/amdgpu_drm_queue.h
@@ -42,6 +42,7 @@ typedef void (*amdgpu_drm_handler_proc)(xf86CrtcPtr crtc, uint32_t seq,
 					uint64_t usec, void *data);
 typedef void (*amdgpu_drm_abort_proc)(xf86CrtcPtr crtc, void *data);
 
+void amdgpu_drm_queue_handle_deferred(xf86CrtcPtr crtc);
 uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client,
 				 uint64_t id, void *data,
 				 amdgpu_drm_handler_proc handler,
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 615d3be8f..3b2de6839 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -305,6 +305,9 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 				nominal_frame_rate /= pix_in_frame;
 			drmmode_crtc->dpms_last_fps = nominal_frame_rate;
 		}
+
+		drmmode_crtc->dpms_mode = mode;
+		amdgpu_drm_queue_handle_deferred(crtc);
 	} else if (drmmode_crtc->dpms_mode != DPMSModeOn && mode == DPMSModeOn) {
 		/*
 		 * Off->On transition: calculate and accumulate the
@@ -322,8 +325,9 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 			drmmode_crtc->interpolated_vblanks += delta_seq;
 
 		}
+
+		drmmode_crtc->dpms_mode = DPMSModeOn;
 	}
-	drmmode_crtc->dpms_mode = mode;
 }
 
 static void
@@ -1415,6 +1419,7 @@ done:
 		}
 	}
 
+	amdgpu_drm_queue_handle_deferred(crtc);
 	return ret;
 }
 
@@ -2320,11 +2325,6 @@ drmmode_output_set_tear_free(AMDGPUEntPtr pAMDGPUEnt,
 	drmmode_output->tear_free = tear_free;
 
 	if (crtc) {
-		/* Wait for pending flips before drmmode_set_mode_major calls
-		 * drmmode_crtc_update_tear_free, to prevent a nested
-		 * drmHandleEvent call, which would hang
-		 */
-		amdgpu_drm_wait_pending_flip(crtc);
 		drmmode_set_mode_major(crtc, &crtc->mode, crtc->rotation,
 				       crtc->x, crtc->y);
 	}
@@ -3861,6 +3861,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 	int i;
 	uint32_t flip_flags = flip_sync == FLIP_ASYNC ? DRM_MODE_PAGE_FLIP_ASYNC : 0;
 	drmmode_flipdata_ptr flipdata;
+	Bool handle_deferred = FALSE;
 	uintptr_t drm_queue_seq = 0;
 
 	flipdata = calloc(1, sizeof(drmmode_flipdata_rec));
@@ -3942,6 +3943,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 
 			if (drmmode_crtc->scanout_update_pending) {
 				amdgpu_drm_wait_pending_flip(crtc);
+				handle_deferred = TRUE;
 				amdgpu_drm_abort_entry(drmmode_crtc->scanout_update_pending);
 				drmmode_crtc->scanout_update_pending = 0;
 			}
@@ -3975,6 +3977,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 		drm_queue_seq = 0;
 	}
 
+	if (handle_deferred)
+		amdgpu_drm_queue_handle_deferred(ref_crtc);
 	if (flipdata->flip_count > 0)
 		return TRUE;
 
@@ -3995,5 +3999,7 @@ error:
 
 	xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Page flip failed: %s\n",
 		   strerror(errno));
+	if (handle_deferred)
+		amdgpu_drm_queue_handle_deferred(ref_crtc);
 	return FALSE;
 }
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 8b949f79d..28d15e282 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -119,6 +119,10 @@ typedef struct {
 
 	/* Modeset needed for DPMS on */
 	Bool need_modeset;
+	/* For keeping track of nested calls to drm_wait_pending_flip /
+	 * drm_queue_handle_deferred
+	 */
+	int wait_flip_nesting_level;
 	/* A flip to this FB is pending for this CRTC */
 	struct drmmode_fb *flip_pending;
 	/* The FB currently being scanned out by this CRTC, if any */
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 4/4] Remove drmmode_crtc_private_rec::present_vblank_* related code

[Michel Dänzer]

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

Not needed anymore with the more robust mechanism introduced in the
previous change.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/amdgpu_kms.c      |  9 ---------
 src/amdgpu_present.c  | 34 +++-------------------------------
 src/drmmode_display.h |  7 -------
 3 files changed, 3 insertions(+), 47 deletions(-)

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index e2925cc83..80e4c947c 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -436,15 +436,6 @@ amdgpu_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t usec,
 	drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
 			     drmmode_crtc->flip_pending);
 	amdgpu_scanout_flip_abort(crtc, event_data);
-
-#ifdef HAVE_PRESENT_H
-	if (drmmode_crtc->present_vblank_event_id) {
-		present_event_notify(drmmode_crtc->present_vblank_event_id,
-				     drmmode_crtc->present_vblank_usec,
-				     drmmode_crtc->present_vblank_msc);
-		drmmode_crtc->present_vblank_event_id = 0;
-	}
-#endif
 }
 
 
diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index 4e74bfac9..2f42fae69 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -52,7 +52,6 @@ static present_screen_info_rec amdgpu_present_screen_info;
 
 struct amdgpu_present_vblank_event {
 	uint64_t event_id;
-	Bool vblank_for_flip;
 	Bool unflip;
 };
 
@@ -120,26 +119,9 @@ static void
 amdgpu_present_vblank_handler(xf86CrtcPtr crtc, unsigned int msc,
 			      uint64_t usec, void *data)
 {
-	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
 	struct amdgpu_present_vblank_event *event = data;
 
-	if (event->vblank_for_flip &&
-	    drmmode_crtc->tear_free &&
-	    drmmode_crtc->scanout_update_pending) {
-		if (drmmode_crtc->present_vblank_event_id != 0) {
-			xf86DrvMsg(crtc->scrn->scrnIndex, X_WARNING,
-				   "Need to handle previously deferred vblank event\n");
-			present_event_notify(drmmode_crtc->present_vblank_event_id,
-					     drmmode_crtc->present_vblank_usec,
-					     drmmode_crtc->present_vblank_msc);
-		}
-
-		drmmode_crtc->present_vblank_event_id = event->event_id;
-		drmmode_crtc->present_vblank_msc = msc;
-		drmmode_crtc->present_vblank_usec = usec;
-	} else
-		present_event_notify(event->event_id, usec, msc);
-
+	present_event_notify(event->event_id, usec, msc);
 	free(event);
 }
 
@@ -162,7 +144,6 @@ static int
 amdgpu_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
 {
 	xf86CrtcPtr xf86_crtc = crtc->devPrivate;
-	drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
 	ScreenPtr screen = crtc->pScreen;
 	struct amdgpu_present_vblank_event *event;
 	uintptr_t drm_queue_seq;
@@ -171,8 +152,6 @@ amdgpu_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
 	if (!event)
 		return BadAlloc;
 	event->event_id = event_id;
-	event->vblank_for_flip = drmmode_crtc->present_flip_expected;
-	drmmode_crtc->present_flip_expected = FALSE;
 
 	drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc,
 					       AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
@@ -257,7 +236,6 @@ amdgpu_present_check_flip(RRCrtcPtr crtc, WindowPtr window, PixmapPtr pixmap,
 			  Bool sync_flip)
 {
 	xf86CrtcPtr xf86_crtc = crtc->devPrivate;
-	drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
 	ScreenPtr screen = window->drawable.pScreen;
 	ScrnInfoPtr scrn = xf86_crtc->scrn;
 	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -265,8 +243,6 @@ amdgpu_present_check_flip(RRCrtcPtr crtc, WindowPtr window, PixmapPtr pixmap,
 	int num_crtcs_on;
 	int i;
 
-	drmmode_crtc->present_flip_expected = FALSE;
-
 	if (!scrn->vtSema)
 		return FALSE;
 
@@ -296,7 +272,6 @@ amdgpu_present_check_flip(RRCrtcPtr crtc, WindowPtr window, PixmapPtr pixmap,
 	if (num_crtcs_on == 0)
 		return FALSE;
 
-	drmmode_crtc->present_flip_expected = TRUE;
 	return TRUE;
 }
 
@@ -337,7 +312,6 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
                    PixmapPtr pixmap, Bool sync_flip)
 {
 	xf86CrtcPtr xf86_crtc = crtc->devPrivate;
-	drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
 	ScreenPtr screen = crtc->pScreen;
 	ScrnInfoPtr scrn = xf86_crtc->scrn;
 	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
@@ -345,11 +319,11 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
 	Bool ret = FALSE;
 
 	if (!amdgpu_present_check_flip(crtc, screen->root, pixmap, sync_flip))
-		goto out;
+		return ret;
 
 	event = calloc(1, sizeof(struct amdgpu_present_vblank_event));
 	if (!event)
-		goto out;
+		return ret;
 
 	event->event_id = event_id;
 
@@ -366,8 +340,6 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
 	else
 		info->drmmode.present_flipping = TRUE;
 
- out:
-	drmmode_crtc->present_flip_expected = FALSE;
 	return ret;
 }
 
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 28d15e282..88f30748e 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -128,13 +128,6 @@ typedef struct {
 	/* The FB currently being scanned out by this CRTC, if any */
 	struct drmmode_fb *fb;
 
-#ifdef HAVE_PRESENT_H
-	/* Deferred processing of Present vblank event */
-	uint64_t present_vblank_event_id;
-	uint64_t present_vblank_usec;
-	unsigned present_vblank_msc;
-	Bool present_flip_expected;
-#endif
 	struct drm_color_lut *degamma_lut;
 	struct drm_color_ctm *ctm;
 	struct drm_color_lut *gamma_lut;
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 4/4] Remove drmmode_crtc_private_rec::present_vblank_* related code

[Alex Deucher]

On Tue, Jul 24, 2018 at 1:16 PM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> Not needed anymore with the more robust mechanism introduced in the
> previous change.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Series is:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  src/amdgpu_kms.c      |  9 ---------
>  src/amdgpu_present.c  | 34 +++-------------------------------
>  src/drmmode_display.h |  7 -------
>  3 files changed, 3 insertions(+), 47 deletions(-)
>
> diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
> index e2925cc83..80e4c947c 100644
> --- a/src/amdgpu_kms.c
> +++ b/src/amdgpu_kms.c
> @@ -436,15 +436,6 @@ amdgpu_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t usec,
>         drmmode_fb_reference(pAMDGPUEnt->fd, &drmmode_crtc->fb,
>                              drmmode_crtc->flip_pending);
>         amdgpu_scanout_flip_abort(crtc, event_data);
> -
> -#ifdef HAVE_PRESENT_H
> -       if (drmmode_crtc->present_vblank_event_id) {
> -               present_event_notify(drmmode_crtc->present_vblank_event_id,
> -                                    drmmode_crtc->present_vblank_usec,
> -                                    drmmode_crtc->present_vblank_msc);
> -               drmmode_crtc->present_vblank_event_id = 0;
> -       }
> -#endif
>  }
>
>
> diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
> index 4e74bfac9..2f42fae69 100644
> --- a/src/amdgpu_present.c
> +++ b/src/amdgpu_present.c
> @@ -52,7 +52,6 @@ static present_screen_info_rec amdgpu_present_screen_info;
>
>  struct amdgpu_present_vblank_event {
>         uint64_t event_id;
> -       Bool vblank_for_flip;
>         Bool unflip;
>  };
>
> @@ -120,26 +119,9 @@ static void
>  amdgpu_present_vblank_handler(xf86CrtcPtr crtc, unsigned int msc,
>                               uint64_t usec, void *data)
>  {
> -       drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
>         struct amdgpu_present_vblank_event *event = data;
>
> -       if (event->vblank_for_flip &&
> -           drmmode_crtc->tear_free &&
> -           drmmode_crtc->scanout_update_pending) {
> -               if (drmmode_crtc->present_vblank_event_id != 0) {
> -                       xf86DrvMsg(crtc->scrn->scrnIndex, X_WARNING,
> -                                  "Need to handle previously deferred vblank event\n");
> -                       present_event_notify(drmmode_crtc->present_vblank_event_id,
> -                                            drmmode_crtc->present_vblank_usec,
> -                                            drmmode_crtc->present_vblank_msc);
> -               }
> -
> -               drmmode_crtc->present_vblank_event_id = event->event_id;
> -               drmmode_crtc->present_vblank_msc = msc;
> -               drmmode_crtc->present_vblank_usec = usec;
> -       } else
> -               present_event_notify(event->event_id, usec, msc);
> -
> +       present_event_notify(event->event_id, usec, msc);
>         free(event);
>  }
>
> @@ -162,7 +144,6 @@ static int
>  amdgpu_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
>  {
>         xf86CrtcPtr xf86_crtc = crtc->devPrivate;
> -       drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
>         ScreenPtr screen = crtc->pScreen;
>         struct amdgpu_present_vblank_event *event;
>         uintptr_t drm_queue_seq;
> @@ -171,8 +152,6 @@ amdgpu_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
>         if (!event)
>                 return BadAlloc;
>         event->event_id = event_id;
> -       event->vblank_for_flip = drmmode_crtc->present_flip_expected;
> -       drmmode_crtc->present_flip_expected = FALSE;
>
>         drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc,
>                                                AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
> @@ -257,7 +236,6 @@ amdgpu_present_check_flip(RRCrtcPtr crtc, WindowPtr window, PixmapPtr pixmap,
>                           Bool sync_flip)
>  {
>         xf86CrtcPtr xf86_crtc = crtc->devPrivate;
> -       drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
>         ScreenPtr screen = window->drawable.pScreen;
>         ScrnInfoPtr scrn = xf86_crtc->scrn;
>         xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
> @@ -265,8 +243,6 @@ amdgpu_present_check_flip(RRCrtcPtr crtc, WindowPtr window, PixmapPtr pixmap,
>         int num_crtcs_on;
>         int i;
>
> -       drmmode_crtc->present_flip_expected = FALSE;
> -
>         if (!scrn->vtSema)
>                 return FALSE;
>
> @@ -296,7 +272,6 @@ amdgpu_present_check_flip(RRCrtcPtr crtc, WindowPtr window, PixmapPtr pixmap,
>         if (num_crtcs_on == 0)
>                 return FALSE;
>
> -       drmmode_crtc->present_flip_expected = TRUE;
>         return TRUE;
>  }
>
> @@ -337,7 +312,6 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
>                     PixmapPtr pixmap, Bool sync_flip)
>  {
>         xf86CrtcPtr xf86_crtc = crtc->devPrivate;
> -       drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
>         ScreenPtr screen = crtc->pScreen;
>         ScrnInfoPtr scrn = xf86_crtc->scrn;
>         AMDGPUInfoPtr info = AMDGPUPTR(scrn);
> @@ -345,11 +319,11 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
>         Bool ret = FALSE;
>
>         if (!amdgpu_present_check_flip(crtc, screen->root, pixmap, sync_flip))
> -               goto out;
> +               return ret;
>
>         event = calloc(1, sizeof(struct amdgpu_present_vblank_event));
>         if (!event)
> -               goto out;
> +               return ret;
>
>         event->event_id = event_id;
>
> @@ -366,8 +340,6 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
>         else
>                 info->drmmode.present_flipping = TRUE;
>
> - out:
> -       drmmode_crtc->present_flip_expected = FALSE;
>         return ret;
>  }
>
> diff --git a/src/drmmode_display.h b/src/drmmode_display.h
> index 28d15e282..88f30748e 100644
> --- a/src/drmmode_display.h
> +++ b/src/drmmode_display.h
> @@ -128,13 +128,6 @@ typedef struct {
>         /* The FB currently being scanned out by this CRTC, if any */
>         struct drmmode_fb *fb;
>
> -#ifdef HAVE_PRESENT_H
> -       /* Deferred processing of Present vblank event */
> -       uint64_t present_vblank_event_id;
> -       uint64_t present_vblank_usec;
> -       unsigned present_vblank_msc;
> -       Bool present_flip_expected;
> -#endif
>         struct drm_color_lut *degamma_lut;
>         struct drm_color_ctm *ctm;
>         struct drm_color_lut *gamma_lut;
> --
> 2.18.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@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@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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 xf86-video-amdgpu 3/4] Defer vblank event handling while waiting for a pending flip

[Michel Dänzer]

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

This is to avoid submitting more flips while we are waiting for pending
ones to complete.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---

v2:
* Rebased on top of new patch 2.5

 src/amdgpu_drm_queue.c | 41 +++++++++++++++++++++++++++++++++++++++--
 src/amdgpu_drm_queue.h |  1 +
 src/drmmode_display.c  | 18 ++++++++++++------
 src/drmmode_display.h  |  4 ++++
 4 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c
index f8660828c..b13d28014 100644
--- a/src/amdgpu_drm_queue.c
+++ b/src/amdgpu_drm_queue.c
@@ -117,6 +117,30 @@ amdgpu_drm_vblank_handler(int fd, unsigned int frame, unsigned int sec,
 				 user_ptr);
 }
 
+/*
+ * Handle deferred DRM vblank events
+ *
+ * This function must be called after amdgpu_drm_wait_pending_flip, once
+ * it's safe to attempt queueing a flip again
+ */
+void
+amdgpu_drm_queue_handle_deferred(xf86CrtcPtr crtc)
+{
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	struct amdgpu_drm_queue_entry *e, *tmp;
+
+	if (drmmode_crtc->wait_flip_nesting_level == 0 ||
+	    --drmmode_crtc->wait_flip_nesting_level > 0)
+		return;
+
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_vblank_signalled, list) {
+		drmmode_crtc_private_ptr drmmode_crtc = e->crtc->driver_private;
+
+		if (drmmode_crtc->wait_flip_nesting_level == 0)
+			amdgpu_drm_queue_handle_one(e);
+	}
+}
+
 /*
  * Enqueue a potential drm response; when the associated response
  * appears, we've got data to pass to the handler from here
@@ -191,6 +215,13 @@ amdgpu_drm_abort_entry(uintptr_t seq)
 	if (seq == AMDGPU_DRM_QUEUE_ERROR)
 		return;
 
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_vblank_signalled, list) {
+		if (e->seq == seq) {
+			amdgpu_drm_abort_one(e);
+			return;
+		}
+	}
+
 	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_queue, list) {
 		if (e->seq == seq) {
 			amdgpu_drm_abort_one(e);
@@ -229,8 +260,12 @@ amdgpu_drm_handle_event(int fd, drmEventContext *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);
+	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_vblank_signalled, list) {
+		drmmode_crtc_private_ptr drmmode_crtc = e->crtc->driver_private;
+
+		if (drmmode_crtc->wait_flip_nesting_level == 0)
+			amdgpu_drm_queue_handle_one(e);
+	}
 
 	return r;
 }
@@ -244,6 +279,8 @@ void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc)
 	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
 	struct amdgpu_drm_queue_entry *e, *tmp;
 
+	drmmode_crtc->wait_flip_nesting_level++;
+
 	xorg_list_for_each_entry_safe(e, tmp, &amdgpu_drm_flip_signalled, list)
 		amdgpu_drm_queue_handle_one(e);
 
diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h
index 48ba9ab6e..4e7c8f4c4 100644
--- a/src/amdgpu_drm_queue.h
+++ b/src/amdgpu_drm_queue.h
@@ -42,6 +42,7 @@ typedef void (*amdgpu_drm_handler_proc)(xf86CrtcPtr crtc, uint32_t seq,
 					uint64_t usec, void *data);
 typedef void (*amdgpu_drm_abort_proc)(xf86CrtcPtr crtc, void *data);
 
+void amdgpu_drm_queue_handle_deferred(xf86CrtcPtr crtc);
 uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client,
 				 uint64_t id, void *data,
 				 amdgpu_drm_handler_proc handler,
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 167b30091..9919c0923 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -305,6 +305,9 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 				nominal_frame_rate /= pix_in_frame;
 			drmmode_crtc->dpms_last_fps = nominal_frame_rate;
 		}
+
+		drmmode_crtc->dpms_mode = mode;
+		amdgpu_drm_queue_handle_deferred(crtc);
 	} else if (drmmode_crtc->dpms_mode != DPMSModeOn && mode == DPMSModeOn) {
 		/*
 		 * Off->On transition: calculate and accumulate the
@@ -322,8 +325,9 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 			drmmode_crtc->interpolated_vblanks += delta_seq;
 
 		}
+
+		drmmode_crtc->dpms_mode = DPMSModeOn;
 	}
-	drmmode_crtc->dpms_mode = mode;
 }
 
 static void
@@ -1415,6 +1419,7 @@ done:
 		}
 	}
 
+	amdgpu_drm_queue_handle_deferred(crtc);
 	return ret;
 }
 
@@ -2320,11 +2325,6 @@ drmmode_output_set_tear_free(AMDGPUEntPtr pAMDGPUEnt,
 	drmmode_output->tear_free = tear_free;
 
 	if (crtc) {
-		/* Wait for pending flips before drmmode_set_mode_major calls
-		 * drmmode_crtc_update_tear_free, to prevent a nested
-		 * drmHandleEvent call, which would hang
-		 */
-		amdgpu_drm_wait_pending_flip(crtc);
 		drmmode_set_mode_major(crtc, &crtc->mode, crtc->rotation,
 				       crtc->x, crtc->y);
 	}
@@ -3864,6 +3864,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 	drmmode_crtc_private_ptr drmmode_crtc = config->crtc[0]->driver_private;
 	uint32_t flip_flags = flip_sync == FLIP_ASYNC ? DRM_MODE_PAGE_FLIP_ASYNC : 0;
 	drmmode_flipdata_ptr flipdata;
+	Bool handle_deferred = FALSE;
 	uintptr_t drm_queue_seq = 0;
 	struct drmmode_fb *fb;
 	int i = 0;
@@ -3946,6 +3947,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 
 			if (drmmode_crtc->scanout_update_pending) {
 				amdgpu_drm_wait_pending_flip(crtc);
+				handle_deferred = TRUE;
 				amdgpu_drm_abort_entry(drmmode_crtc->scanout_update_pending);
 				drmmode_crtc->scanout_update_pending = 0;
 			}
@@ -3981,6 +3983,8 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 		drm_queue_seq = 0;
 	}
 
+	if (handle_deferred)
+		amdgpu_drm_queue_handle_deferred(ref_crtc);
 	if (flipdata->flip_count > 0)
 		return TRUE;
 
@@ -4000,5 +4004,7 @@ error:
 
 	xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Page flip failed: %s\n",
 		   strerror(errno));
+	if (handle_deferred)
+		amdgpu_drm_queue_handle_deferred(ref_crtc);
 	return FALSE;
 }
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index 5618c6b40..27610d537 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -119,6 +119,10 @@ typedef struct {
 
 	/* Modeset needed for DPMS on */
 	Bool need_modeset;
+	/* For keeping track of nested calls to drm_wait_pending_flip /
+	 * drm_queue_handle_deferred
+	 */
+	int wait_flip_nesting_level;
 	/* A flip to this FB is pending for this CRTC */
 	struct drmmode_fb *flip_pending;
 	/* The FB currently being scanned out by this CRTC, if any */
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx