Fix some non-atomic modesetting calls to be atomic

This commit is contained in:
Adam Jackson 2019-04-23 19:39:25 -04:00
parent 324f637f1e
commit c0df45f1ac
5 changed files with 416 additions and 1 deletions

View File

@ -0,0 +1,64 @@
From 6915752b2da77c29bf8c861a6d4f2383e667323b Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Fri, 5 Oct 2018 14:07:46 -0400
Subject: [PATCH xserver 1/4] modesetting: Weaksauce atomic property debugging
This would be a good idea to have around for troubleshooting purposes,
but this particular patch is Not Good.
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
hw/xfree86/drivers/modesetting/drmmode_display.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index cb48aa46b..a8d989a24 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -415,6 +415,13 @@ drmmode_ConvertToKMode(ScrnInfoPtr scrn,
drmModeModeInfo * kmode, DisplayModePtr mode);
+static int
+drmmodeatomicaddproperty(drmModeAtomicReqPtr req, uint32_t obj, uint32_t prop, uint64_t value)
+{
+ ErrorF("Adding to %p: %x %x %lx\n", req, obj, prop, value);
+ return drmModeAtomicAddProperty(req, obj, prop, value);
+}
+
static int
plane_add_prop(drmModeAtomicReq *req, drmmode_crtc_private_ptr drmmode_crtc,
enum drmmode_plane_property prop, uint64_t val)
@@ -425,7 +432,8 @@ plane_add_prop(drmModeAtomicReq *req, drmmode_crtc_private_ptr drmmode_crtc,
if (!info)
return -1;
- ret = drmModeAtomicAddProperty(req, drmmode_crtc->plane_id,
+ ErrorF("Setting %s\n", info->name);
+ ret = drmmodeatomicaddproperty(req, drmmode_crtc->plane_id,
info->prop_id, val);
return (ret <= 0) ? -1 : 0;
}
@@ -467,7 +475,8 @@ crtc_add_prop(drmModeAtomicReq *req, drmmode_crtc_private_ptr drmmode_crtc,
if (!info)
return -1;
- ret = drmModeAtomicAddProperty(req, drmmode_crtc->mode_crtc->crtc_id,
+ ErrorF("Setting %s\n", info->name);
+ ret = drmmodeatomicaddproperty(req, drmmode_crtc->mode_crtc->crtc_id,
info->prop_id, val);
return (ret <= 0) ? -1 : 0;
}
@@ -482,7 +491,8 @@ connector_add_prop(drmModeAtomicReq *req, drmmode_output_private_ptr drmmode_out
if (!info)
return -1;
- ret = drmModeAtomicAddProperty(req, drmmode_output->output_id,
+ ErrorF("Setting %s\n", info->name);
+ ret = drmmodeatomicaddproperty(req, drmmode_output->output_id,
info->prop_id, val);
return (ret <= 0) ? -1 : 0;
}
--
2.20.1

View File

@ -0,0 +1,31 @@
From e262c8ae0039710422c11ae254ddb4ae7e6fac02 Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Fri, 5 Oct 2018 14:03:54 -0400
Subject: [PATCH xserver 2/4] modesetting: Propagate more failure in
drmmode_set_mode_major
It's possible that actually setting the mode would fail even though the
check succeeded. We would throw away the error in this case, which would
probably make recovery a bit difficult.
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
hw/xfree86/drivers/modesetting/drmmode_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index a8d989a24..33f6cea3b 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -1540,7 +1540,7 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
/* if we only tested the mode previously, really set it now */
if (can_test)
- drmmode_crtc_set_mode(crtc, FALSE);
+ ret = drmmode_crtc_set_mode(crtc, FALSE);
ms->pending_modeset = FALSE;
}
--
2.20.1

View File

@ -0,0 +1,64 @@
From e7f0961638818df8ee91edfae0378a02f7177e9f Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Fri, 5 Oct 2018 14:00:33 -0400
Subject: [PATCH xserver 3/4] modesetting: Factor out drmmode_target_output
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
.../drivers/modesetting/drmmode_display.c | 26 ++++++++++++-------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 33f6cea3b..7924aa396 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -757,6 +757,21 @@ drmmode_crtc_disable(xf86CrtcPtr crtc)
return ret;
}
+static xf86OutputPtr
+drmmode_target_output(ScrnInfoPtr scrn, xf86CrtcPtr crtc)
+{
+ xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
+ int o;
+
+ if (config->output[config->compat_output]->crtc == crtc)
+ return config->output[config->compat_output];
+
+ for (o = 0; o < config->num_output; o++)
+ if (config->output[o]->crtc == crtc)
+ return config->output[o];
+
+ return NULL;
+}
static int
drmmode_crtc_set_mode(xf86CrtcPtr crtc, Bool test_only)
{
@@ -3448,7 +3463,6 @@ drmmode_set_desired_modes(ScrnInfoPtr pScrn, drmmode_ptr drmmode, Bool set_hw)
xf86CrtcPtr crtc = config->crtc[c];
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
xf86OutputPtr output = NULL;
- int o;
/* Skip disabled CRTCs */
if (!crtc->enabled) {
@@ -3459,15 +3473,7 @@ drmmode_set_desired_modes(ScrnInfoPtr pScrn, drmmode_ptr drmmode, Bool set_hw)
continue;
}
- if (config->output[config->compat_output]->crtc == crtc)
- output = config->output[config->compat_output];
- else {
- for (o = 0; o < config->num_output; o++)
- if (config->output[o]->crtc == crtc) {
- output = config->output[o];
- break;
- }
- }
+ output = drmmode_target_output(pScrn, crtc);
/* paranoia */
if (!output)
continue;
--
2.20.1

View File

@ -0,0 +1,245 @@
From e4e389447c59c89d39a5a78a5f19cdfd22d7699c Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Fri, 5 Oct 2018 14:09:35 -0400
Subject: [PATCH xserver 4/4] modesetting: Use atomic instead of per-crtc walks
where we can
drmmode_set_desired_modes (reachable from CreateScreenResources,
EnterVT, etc.) currently works by doing ->set_mode_major on each CRTC.
This is silly, as atomic lets us configure every CRTC at once. It's also
fragile, because we're not trying very hard to ensure the intermediate
states are valid.
This patch introduces drmmode_set_mode_atomic, which blasts the entire
RANDR state into the kernel in an... atomic... fashion. We change
drmmode_set_desired_modes and drmmode_xf86crtc_resize to use this
instead of walking each CRTC. We also change drmmode_crtc_set_mode to
use this, so that client RANDR requests (operating CRTC-at-a-time, since
RANDR doesn't have atomic changes yet) use the same code paths.
Signed-off-by: Adam Jackson <ajax@redhat.com>
---
.../drivers/modesetting/drmmode_display.c | 179 +++++++++++-------
1 file changed, 109 insertions(+), 70 deletions(-)
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 7924aa396..020678155 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -772,6 +772,96 @@ drmmode_target_output(ScrnInfoPtr scrn, xf86CrtcPtr crtc)
return NULL;
}
+
+static int
+drmmode_set_mode_atomic(ScrnInfoPtr scrn, modesettingPtr ms, Bool test_only)
+{
+ xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
+ drmModeAtomicReq *req = drmModeAtomicAlloc();
+ uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
+ int i, j, ret = 0;
+
+ if (!req)
+ return 1;
+
+ for (i = 0; i < xf86_config->num_crtc; i++) {
+ xf86CrtcPtr crtc = xf86_config->crtc[i];
+ drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+ Bool active = crtc->enabled;
+ uint32_t fb_id;
+ int x, y;
+ drmModeModeInfo kmode;
+
+ if (active) {
+ drmmode_ConvertToKMode(crtc->scrn, &kmode, &crtc->desiredMode);
+ ret |= drm_mode_ensure_blob(crtc, kmode);
+
+ /* XXX this probably doesn't belong here */
+ if (!crtc->mode.HDisplay && crtc->desiredMode.HDisplay)
+ memcpy(&crtc->mode, &crtc->desiredMode, sizeof(crtc->mode));
+ }
+
+ ret |= !drmmode_crtc_get_fb_id(crtc, &fb_id, &x, &y);
+ ret |= crtc_add_prop(req, drmmode_crtc,
+ DRMMODE_CRTC_ACTIVE, active);
+ ret |= crtc_add_prop(req, drmmode_crtc,
+ DRMMODE_CRTC_MODE_ID,
+ active ? drmmode_crtc->current_mode->blob_id : 0);
+ ret |= plane_add_props(req, crtc, active ? fb_id : 0, x, y);
+ }
+
+ for (i = 0; i < xf86_config->num_output; i++) {
+ xf86OutputPtr output = xf86_config->output[i];
+ drmmode_output_private_ptr drmmode_output = output->driver_private;
+ drmmode_crtc_private_ptr drm_crtc =
+ output->crtc ? output->crtc->driver_private : NULL;
+
+ if (drmmode_output->output_id == -1)
+ continue;
+
+ ret |= connector_add_prop(req, drmmode_output,
+ DRMMODE_CONNECTOR_CRTC_ID,
+ drm_crtc ? drm_crtc->mode_crtc->crtc_id : 0);
+ }
+
+ if (test_only)
+ flags |= DRM_MODE_ATOMIC_TEST_ONLY;
+
+ if (ret == 0)
+ ret = drmModeAtomicCommit(ms->fd, req, flags, NULL);
+
+ if (ret == 0 && !test_only) {
+ for (i = 0; i < xf86_config->num_crtc; i++) {
+ xf86CrtcPtr crtc = xf86_config->crtc[i];
+ for (j = 0; i < xf86_config->num_output; i++) {
+ xf86OutputPtr output = xf86_config->output[j];
+ drmmode_output_private_ptr drmmode_output =
+ output->driver_private;
+
+ if (output->crtc == crtc)
+ drmmode_output->current_crtc = crtc;
+ else if (drmmode_output->current_crtc == crtc)
+ drmmode_output->current_crtc = NULL;
+ }
+ }
+ }
+
+ if (ret == 0 && test_only) {
+ for (i = 0; i < xf86_config->num_crtc; i++) {
+ xf86CrtcPtr crtc = xf86_config->crtc[i];
+ crtc->mode = crtc->desiredMode;
+ crtc->rotation = crtc->desiredRotation;
+ crtc->x = crtc->desiredX;
+ crtc->y = crtc->desiredY;
+ if (!xf86CrtcRotate(crtc))
+ ret = 1;
+ }
+ }
+
+ drmModeAtomicFree(req);
+ return ret;
+}
+
static int
drmmode_crtc_set_mode(xf86CrtcPtr crtc, Bool test_only)
{
@@ -786,73 +876,12 @@ drmmode_crtc_set_mode(xf86CrtcPtr crtc, Bool test_only)
int x, y;
int i, ret = 0;
+ if (ms->atomic_modeset)
+ return drmmode_set_mode_atomic(crtc->scrn, ms, test_only);
+
if (!drmmode_crtc_get_fb_id(crtc, &fb_id, &x, &y))
return 1;
- if (ms->atomic_modeset) {
- drmModeAtomicReq *req = drmModeAtomicAlloc();
- Bool active;
- uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
-
- if (!req)
- return 1;
-
- ret |= crtc_add_dpms_props(req, crtc, DPMSModeOn, &active);
- ret |= plane_add_props(req, crtc, active ? fb_id : 0, x, y);
-
- /* Orphaned CRTCs need to be disabled right now in atomic mode */
- for (i = 0; i < xf86_config->num_crtc; i++) {
- xf86CrtcPtr other_crtc = xf86_config->crtc[i];
- drmmode_crtc_private_ptr other_drmmode_crtc = other_crtc->driver_private;
- int lost_outputs = 0;
- int remaining_outputs = 0;
- int j;
-
- if (other_crtc == crtc)
- continue;
-
- for (j = 0; j < xf86_config->num_output; j++) {
- xf86OutputPtr output = xf86_config->output[j];
- drmmode_output_private_ptr drmmode_output = output->driver_private;
-
- if (drmmode_output->current_crtc == other_crtc) {
- if (output->crtc == crtc)
- lost_outputs++;
- else
- remaining_outputs++;
- }
- }
-
- if (lost_outputs > 0 && remaining_outputs == 0) {
- ret |= crtc_add_prop(req, other_drmmode_crtc,
- DRMMODE_CRTC_ACTIVE, 0);
- ret |= crtc_add_prop(req, other_drmmode_crtc,
- DRMMODE_CRTC_MODE_ID, 0);
- }
- }
-
- if (test_only)
- flags |= DRM_MODE_ATOMIC_TEST_ONLY;
-
- if (ret == 0)
- ret = drmModeAtomicCommit(ms->fd, req, flags, NULL);
-
- if (ret == 0 && !test_only) {
- for (i = 0; i < xf86_config->num_output; i++) {
- xf86OutputPtr output = xf86_config->output[i];
- drmmode_output_private_ptr drmmode_output = output->driver_private;
-
- if (output->crtc == crtc)
- drmmode_output->current_crtc = crtc;
- else if (drmmode_output->current_crtc == crtc)
- drmmode_output->current_crtc = NULL;
- }
- }
-
- drmModeAtomicFree(req);
- return ret;
- }
-
output_ids = calloc(sizeof(uint32_t), xf86_config->num_output);
if (!output_ids)
return -1;
@@ -3199,14 +3228,19 @@ drmmode_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
if (!drmmode_glamor_handle_new_screen_pixmap(drmmode))
goto fail;
- for (i = 0; i < xf86_config->num_crtc; i++) {
- xf86CrtcPtr crtc = xf86_config->crtc[i];
+ if (ms->atomic_modeset) {
+ if (drmmode_set_mode_atomic(scrn, ms, FALSE))
+ goto fail;
+ } else {
+ for (i = 0; i < xf86_config->num_crtc; i++) {
+ xf86CrtcPtr crtc = xf86_config->crtc[i];
- if (!crtc->enabled)
- continue;
+ if (!crtc->enabled)
+ continue;
- drmmode_set_mode_major(crtc, &crtc->mode,
- crtc->rotation, crtc->x, crtc->y);
+ drmmode_set_mode_major(crtc, &crtc->mode,
+ crtc->rotation, crtc->x, crtc->y);
+ }
}
if (old_fb_id) {
@@ -3457,8 +3491,13 @@ Bool
drmmode_set_desired_modes(ScrnInfoPtr pScrn, drmmode_ptr drmmode, Bool set_hw)
{
xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
+ modesettingPtr ms = modesettingPTR(pScrn);
int c;
+ /* drmmode_set_mode_atomic returns 0 on success, we return TRUE */
+ if (ms->atomic_modeset)
+ return !drmmode_set_mode_atomic(pScrn, ms, /* test_only = */ !set_hw);
+
for (c = 0; c < config->num_crtc; c++) {
xf86CrtcPtr crtc = config->crtc[c];
drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
--
2.20.1

View File

@ -46,7 +46,7 @@
Summary: X.Org X11 X server
Name: xorg-x11-server
Version: 1.20.4
Release: 3%{?gitdate:.%{gitdate}}%{?dist}
Release: 4%{?gitdate:.%{gitdate}}%{?dist}
URL: http://www.x.org
License: MIT
@ -99,6 +99,14 @@ Patch10: 0001-xwayland-present-Destroy-sync_callback-in-xwl_presen.patch
# https://gitlab.freedesktop.org/xorg/xserver/merge_requests/152
Patch11: 0001-dix-leave-last.valuators-alone-on-slave-switch.patch
# test for https://bugzilla.redhat.com/show_bug.cgi?id=1697591
# see also https://gitlab.freedesktop.org/xorg/xserver/merge_requests/36
#Patch21: 0001-modesetting-Weaksauce-atomic-property-debugging.patch
Patch22: 0002-modesetting-Propagate-more-failure-in-drmmode_set_mo.patch
Patch23: 0003-modesetting-Factor-out-drmmode_target_output.patch
Patch24: 0004-modesetting-Use-atomic-instead-of-per-crtc-walks-whe.patch
BuildRequires: systemtap-sdt-devel
BuildRequires: git
BuildRequires: automake autoconf libtool pkgconfig
@ -523,6 +531,9 @@ find %{inst_srcdir}/hw/xfree86 -name \*.c -delete
%changelog
* Tue Apr 23 2019 Adam Jackson <ajax@redhat.com> - 1.20.4-4
- Fix some non-atomic modesetting calls to be atomic
* Wed Mar 27 2019 Peter Hutterer <peter.hutterer@redhat.com> 1.20.4-3
- Fix a Qt scrolling bug, don't reset the valuator on slave switch