xf86-video-ati: Branch 'master' - 7 commits
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon Feb 11 11:14:41 UTC 2019
src/drmmode_display.c | 15 ++++++-----
src/radeon_dri2.c | 6 ++++
src/radeon_dri3.c | 31 +++++++++++++++++++++--
src/radeon_drm_queue.c | 19 ++++++++++++--
src/radeon_glamor.c | 2 -
src/radeon_kms.c | 66 +++++++++++++++++++++++++------------------------
6 files changed, 96 insertions(+), 43 deletions(-)
New commits:
commit 15697ee242c30b9ea6775624e8282e0171a113a7
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:27:10 2019 +0100
Keep waiting for a pending flip if drm_handle_event returns 0
drm_wait_pending_flip stopped waiting if drm_handle_event returned 0,
but that might have processed only some unrelated DRM events. As long as
the flip is pending, we have to keep waiting for its completion event.
Noticed while working on the previous fix.
(Ported from amdgpu commit 9045fb310f88780e250e60b80431ca153330e61b)
diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c
index 2e2b8404..fc043605 100644
--- a/src/radeon_drm_queue.c
+++ b/src/radeon_drm_queue.c
@@ -325,7 +325,7 @@ void radeon_drm_wait_pending_flip(xf86CrtcPtr crtc)
while (drmmode_crtc->flip_pending
&& radeon_drm_handle_event(pRADEONEnt->fd,
- &drmmode_crtc->drmmode->event_context) > 0);
+ &drmmode_crtc->drmmode->event_context) >= 0);
}
/*
commit 227123de3d862e691131708b7f55260bee17f2b7
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:24:41 2019 +0100
Call drmHandleEvent again if it was interrupted by a signal
drmHandleEvent can be interrupted by a signal in read(), in which case
it doesn't process any events but returns -1, which
drm_handle_event propagated to its callers. This could cause the
following failure cascade:
1. drm_wait_pending_flip stopped waiting for a pending flip.
2. Its caller cleared drmmode_crtc->flip_pending before the flip
completed.
3. Another flip was attempted but got an unexpected EBUSY error because
the previous flip was still pending.
4. TearFree was disabled due to the error.
The solution is to call drmHandleEvent if it was interrupted by a
signal. We can do that in drm_handle_event, because when that is called,
either it is known that there are events ready to be processed, or the
caller has to wait for events to arrive anyway.
Bugzilla: https://bugs.freedesktop.org/109364
(Ported from amdgpu commit 3ff2cc225f6bc08364ee007fa54e9d0150adaf11)
diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c
index d8a8243c..2e2b8404 100644
--- a/src/radeon_drm_queue.c
+++ b/src/radeon_drm_queue.c
@@ -30,6 +30,8 @@
#include "config.h"
#endif
+#include <errno.h>
+
#include <xorg-server.h>
#include <X11/Xdefs.h>
#include <list.h>
@@ -277,7 +279,20 @@ radeon_drm_handle_event(int fd, drmEventContext *event_context)
struct radeon_drm_queue_entry *e;
int r;
- r = drmHandleEvent(fd, event_context);
+ /* Retry drmHandleEvent if it was interrupted by a signal in read() */
+ do {
+ r = drmHandleEvent(fd, event_context);
+ } while (r < 0 && (errno == EINTR || errno == EAGAIN));
+
+ if (r < 0) {
+ static Bool printed;
+
+ if (!printed) {
+ ErrorF("%s: drmHandleEvent returned %d, errno=%d (%s)\n",
+ __func__, r, errno, strerror(errno));
+ printed = TRUE;
+ }
+ }
while (!xorg_list_is_empty(&radeon_drm_flip_signalled)) {
e = xorg_list_first_entry(&radeon_drm_flip_signalled,
commit 1bfdccf7639ee2f655dc659cafa63830ba28be85
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:11:10 2019 +0100
Only update drmmode_crtc->flip_pending after actually submitting a flip
And only clear it if it matches the framebuffer of the completed flip
being processed.
Fixes
(WW) RADEON(0): flip queue failed: Device or resource busy
(WW) RADEON(0): Page flip failed: Device or resource busy
(EE) RADEON(0): present flip failed
due to clobbering drmmode_crtc->flip_pending.
Reproducer: Enable TearFree, run warzone2100 fullscreen, toggle
Vertical sync on/off under Video Options. Discovered while investigating
https://bugs.freedesktop.org/109364 .
(Ported from amdgpu commit e72a02ba1d35743fefd939458b9d8cddce86e7f5)
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 71f3539f..c5fccd2a 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -2651,12 +2651,14 @@ drmmode_flip_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, void *even
flipdata->fe_usec = usec;
}
- if (drmmode_crtc->flip_pending == *fb) {
- drmmode_fb_reference(pRADEONEnt->fd,
- &drmmode_crtc->flip_pending, NULL);
+ if (*fb) {
+ if (drmmode_crtc->flip_pending == *fb) {
+ drmmode_fb_reference(pRADEONEnt->fd,
+ &drmmode_crtc->flip_pending, NULL);
+ }
+ drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->fb, *fb);
+ drmmode_fb_reference(pRADEONEnt->fd, fb, NULL);
}
- drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->fb, *fb);
- drmmode_fb_reference(pRADEONEnt->fd, fb, NULL);
if (--flipdata->flip_count == 0) {
/* Deliver MSC & UST from reference/current CRTC to flip event
@@ -3492,9 +3494,10 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
drmmode_crtc->ignore_damage = TRUE;
}
- next:
drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending,
flipdata->fb[crtc_id]);
+
+ next:
drm_queue_seq = 0;
}
diff --git a/src/radeon_kms.c b/src/radeon_kms.c
index 8b965622..ff4f8dcf 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -526,10 +526,14 @@ radeon_scanout_flip_abort(xf86CrtcPtr crtc, void *event_data)
{
RADEONEntPtr pRADEONEnt = RADEONEntPriv(crtc->scrn);
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ struct drmmode_fb *fb = event_data;
drmmode_crtc->scanout_update_pending = 0;
- drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending,
- NULL);
+
+ if (drmmode_crtc->flip_pending == fb) {
+ drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending,
+ NULL);
+ }
}
static void
@@ -538,9 +542,9 @@ radeon_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t usec,
{
RADEONEntPtr pRADEONEnt = RADEONEntPriv(crtc->scrn);
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ struct drmmode_fb *fb = event_data;
- drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->fb,
- drmmode_crtc->flip_pending);
+ drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->fb, fb);
radeon_scanout_flip_abort(crtc, event_data);
}
@@ -821,24 +825,31 @@ radeon_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
drmmode_crtc_private_ptr drmmode_crtc;
uintptr_t drm_queue_seq;
unsigned scanout_id;
+ struct drmmode_fb *fb;
if (!crtc || !crtc->enabled)
return;
drmmode_crtc = crtc->driver_private;
+ scanout_id = drmmode_crtc->scanout_id ^ 1;
if (drmmode_crtc->scanout_update_pending ||
- !drmmode_crtc->scanout[drmmode_crtc->scanout_id].pixmap ||
+ !drmmode_crtc->scanout[scanout_id].pixmap ||
drmmode_crtc->dpms_mode != DPMSModeOn)
return;
- scanout_id = drmmode_crtc->scanout_id ^ 1;
if (!radeon_prime_scanout_do_update(crtc, scanout_id))
return;
+ fb = radeon_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+ if (!fb) {
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "Failed to get FB for PRIME flip.\n");
+ return;
+ }
+
drm_queue_seq = radeon_drm_queue_alloc(crtc,
RADEON_DRM_QUEUE_CLIENT_DEFAULT,
- RADEON_DRM_QUEUE_ID_DEFAULT,
- NULL,
+ RADEON_DRM_QUEUE_ID_DEFAULT, fb,
radeon_scanout_flip_handler,
radeon_scanout_flip_abort, TRUE);
if (drm_queue_seq == RADEON_DRM_QUEUE_ERROR) {
@@ -847,18 +858,9 @@ radeon_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
return;
}
- drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending,
- radeon_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap));
- if (!drmmode_crtc->flip_pending) {
- xf86DrvMsg(scrn->scrnIndex, X_WARNING,
- "Failed to get FB for PRIME flip.\n");
- radeon_drm_abort_entry(drm_queue_seq);
- return;
- }
-
if (drmmode_page_flip_target_relative(pRADEONEnt, drmmode_crtc,
- drmmode_crtc->flip_pending->handle,
- 0, drm_queue_seq, 1) != 0) {
+ fb->handle, 0, drm_queue_seq, 1)
+ != 0) {
if (!(drmmode_crtc->scanout_status & DRMMODE_SCANOUT_FLIP_FAILED)) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
"flip queue failed in %s: %s, TearFree inactive\n",
@@ -877,6 +879,7 @@ radeon_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
drmmode_crtc->scanout_id = scanout_id;
drmmode_crtc->scanout_update_pending = drm_queue_seq;
+ drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending, fb);
}
static void
@@ -1137,6 +1140,7 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info,
RADEONEntPtr pRADEONEnt = RADEONEntPriv(scrn);
uintptr_t drm_queue_seq;
unsigned scanout_id;
+ struct drmmode_fb *fb;
if (drmmode_crtc->scanout_update_pending ||
drmmode_crtc->flip_pending ||
@@ -1152,10 +1156,16 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info,
radeon_cs_flush_indirect(scrn);
RegionEmpty(region);
+ fb = radeon_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+ if (!fb) {
+ xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+ "Failed to get FB for scanout flip.\n");
+ return;
+ }
+
drm_queue_seq = radeon_drm_queue_alloc(xf86_crtc,
RADEON_DRM_QUEUE_CLIENT_DEFAULT,
- RADEON_DRM_QUEUE_ID_DEFAULT,
- NULL,
+ RADEON_DRM_QUEUE_ID_DEFAULT, fb,
radeon_scanout_flip_handler,
radeon_scanout_flip_abort, TRUE);
if (drm_queue_seq == RADEON_DRM_QUEUE_ERROR) {
@@ -1164,18 +1174,9 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info,
return;
}
- drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending,
- radeon_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap));
- if (!drmmode_crtc->flip_pending) {
- xf86DrvMsg(scrn->scrnIndex, X_WARNING,
- "Failed to get FB for scanout flip.\n");
- radeon_drm_abort_entry(drm_queue_seq);
- return;
- }
-
if (drmmode_page_flip_target_relative(pRADEONEnt, drmmode_crtc,
- drmmode_crtc->flip_pending->handle,
- 0, drm_queue_seq, 1) != 0) {
+ fb->handle, 0, drm_queue_seq, 1)
+ != 0) {
if (!(drmmode_crtc->scanout_status & DRMMODE_SCANOUT_FLIP_FAILED)) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
"flip queue failed in %s: %s, TearFree inactive\n",
@@ -1201,6 +1202,7 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info,
drmmode_crtc->scanout_id = scanout_id;
drmmode_crtc->scanout_update_pending = drm_queue_seq;
+ drmmode_fb_reference(pRADEONEnt->fd, &drmmode_crtc->flip_pending, fb);
}
static void RADEONBlockHandler_KMS(BLOCKHANDLER_ARGS_DECL)
commit dcd3527299c1f6d6faa401c565fa884f4d8f3287
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:08:35 2019 +0100
Don't allow TearFree scanout flips to complete in the same vblank period
We were using a relative target of 0, meaning "complete the flip ASAP".
This could result in the flip sometimes, but not always completing in
the same vertical blank period where the corresponding drawing occurred,
potentially causing judder artifacts with applications updating their
window contents synchronized to the display refresh. A good way to test
this is the vsynctester.com site in a windowed browser, where the judder
results in the large "VSYNC" text intermittently appearing red or cyan
instead of the expected gray.
To avoid this, use a relative target MSC of 1, meaning that if a
vertical blank period is in progress, the flip will only complete in the
next one.
Reported by Julian Tempel and Brandon Wright in
https://bugs.freedesktop.org/106175 .
(Ported from amdgpu commit a1b479c7d0066c481af920f297d6af9009dda11e)
diff --git a/src/radeon_kms.c b/src/radeon_kms.c
index 67f42e0f..8b965622 100644
--- a/src/radeon_kms.c
+++ b/src/radeon_kms.c
@@ -858,7 +858,7 @@ radeon_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
if (drmmode_page_flip_target_relative(pRADEONEnt, drmmode_crtc,
drmmode_crtc->flip_pending->handle,
- 0, drm_queue_seq, 0) != 0) {
+ 0, drm_queue_seq, 1) != 0) {
if (!(drmmode_crtc->scanout_status & DRMMODE_SCANOUT_FLIP_FAILED)) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
"flip queue failed in %s: %s, TearFree inactive\n",
@@ -1175,7 +1175,7 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info,
if (drmmode_page_flip_target_relative(pRADEONEnt, drmmode_crtc,
drmmode_crtc->flip_pending->handle,
- 0, drm_queue_seq, 0) != 0) {
+ 0, drm_queue_seq, 1) != 0) {
if (!(drmmode_crtc->scanout_status & DRMMODE_SCANOUT_FLIP_FAILED)) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
"flip queue failed in %s: %s, TearFree inactive\n",
commit 274703087f80342f51fa69c935bb9a1cb0c4ae47
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:06:50 2019 +0100
glamor: Avoid glamor_create_pixmap for pixmaps backing windows
If the compositing manager uses direct rendering (as is usually the case
these days), the storage of a pixmap allocated by glamor_create_pixmap
needs to be reallocated for sharing it with the compositing manager.
Instead, allocate pixmap storage which can be shared directly.
(Ported from amdgpu commit bf326f2ea19daa6c8da23d6788ff301ae70b8e69)
diff --git a/src/radeon_glamor.c b/src/radeon_glamor.c
index f1098381..3e676f2d 100644
--- a/src/radeon_glamor.c
+++ b/src/radeon_glamor.c
@@ -238,7 +238,7 @@ radeon_glamor_create_pixmap(ScreenPtr screen, int w, int h, int depth,
if (info->shadow_primary) {
if (usage != CREATE_PIXMAP_USAGE_BACKING_PIXMAP)
return fbCreatePixmap(screen, w, h, depth, usage);
- } else {
+ } else if (usage != CREATE_PIXMAP_USAGE_BACKING_PIXMAP) {
pixmap = glamor_create_pixmap(screen, w, h, depth, usage);
if (pixmap)
return pixmap;
commit 6d1dfe2523e900517bd1e8743c87d6990a82c800
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:04:41 2019 +0100
dri2: Flush in dri2_create_buffer2 after calling glamor_set_pixmap_bo
To make sure the client can't use the shared pixmap storage for direct
rendering first, which could produce garbage.
Bugzilla: https://bugs.freedesktop.org/109235
(Ported from amdgpu commit ebd32b1c07208f8dbe853e089f5e4b7c6a7a658a)
diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index 922ed4fb..b5d6835c 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -252,6 +252,12 @@ radeon_dri2_create_buffer2(ScreenPtr pScreen,
} else if (is_glamor_pixmap) {
pixmap = radeon_glamor_set_pixmap_bo(drawable, pixmap);
pixmap->refcnt++;
+
+ /* The copy operation from radeon_glamor_set_pixmap_bo needs to
+ * be flushed to the kernel driver before the client starts
+ * using the pixmap storage for direct rendering.
+ */
+ radeon_cs_flush_indirect(pScrn);
}
if (!radeon_get_flink_name(pRADEONEnt, pixmap, &buffers->name))
commit 77d7abf46446522e686c6b6f1e4857458589ef37
Author: Michel Dänzer <michel.daenzer at amd.com>
Date: Mon Jan 28 18:00:20 2019 +0100
dri3: Flush if necessary in dri3_fd_from_pixmap
To make sure the client can't use the shared pixmap storage for direct
rendering first, which could produce garbage.
Bugzilla: https://bugs.freedesktop.org/109235
(Ported from amdgpu commit d168532ee739f7e33a2798051e64ba445dd3859f)
diff --git a/src/radeon_dri3.c b/src/radeon_dri3.c
index 25078bae..73353bf5 100644
--- a/src/radeon_dri3.c
+++ b/src/radeon_dri3.c
@@ -37,6 +37,7 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
+#include <gbm.h>
#include <errno.h>
#include <libgen.h>
@@ -218,8 +219,34 @@ static int radeon_dri3_fd_from_pixmap(ScreenPtr screen,
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
RADEONInfoPtr info = RADEONPTR(scrn);
- if (info->use_glamor)
- return glamor_fd_from_pixmap(screen, pixmap, stride, size);
+ if (info->use_glamor) {
+ Bool need_flush = TRUE;
+ int ret = -1;
+#if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,19,99,904,0)
+ struct gbm_bo *gbm_bo = glamor_gbm_bo_from_pixmap(screen, pixmap);
+
+ if (gbm_bo) {
+ ret = gbm_bo_get_fd(gbm_bo);
+ gbm_bo_destroy(gbm_bo);
+
+ if (ret >= 0)
+ need_flush = FALSE;
+ }
+#endif
+
+ if (ret < 0)
+ ret = glamor_fd_from_pixmap(screen, pixmap, stride, size);
+
+ /* glamor might have needed to reallocate the pixmap storage and
+ * copy the pixmap contents to the new storage. The copy
+ * operation needs to be flushed to the kernel driver before the
+ * client starts using the pixmap storage for direct rendering.
+ */
+ if (ret >= 0 && need_flush)
+ radeon_cs_flush_indirect(scrn);
+
+ return ret;
+ }
#endif
bo = radeon_get_pixmap_bo(pixmap);
More information about the xorg-commit
mailing list