xserver: Branch 'server-1.20-branch' - 2 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Dec 15 18:37:03 UTC 2021


 hw/xfree86/drivers/modesetting/driver.c  |    8 ++++++++
 hw/xfree86/drivers/modesetting/driver.h  |    1 +
 hw/xfree86/drivers/modesetting/present.c |    7 +++++--
 randr/rrlease.c                          |    2 +-
 4 files changed, 15 insertions(+), 3 deletions(-)

New commits:
commit 5ff3310b69463cf85ea5eec76634cd7377a935e5
Author: Mario Kleiner <mario.kleiner.de at gmail.com>
Date:   Sat Aug 28 02:27:20 2021 +0200

    modesetting: Allow Present flips with mismatched stride on atomic drivers.
    
    When using DRI3+Present with PRIME render offload, sometimes there is
    a mismatch between the stride of the to-be-presented Pixmap and the
    frontbuffer. The current code would reject a pageflip present in this
    case if atomic modesetting is not enabled, ie. always, as atomic
    modesetting is disabled by default due to brokeness in the current
    modesetting-ddx.
    
    Fullscreen presents without page flipping however trigger the copy
    path as fallback, which causes not only unreliable presentation timing
    and degraded performance, but also massive tearing artifacts due to
    rendering to the framebuffer without any hardware sync to vblank.
    Tearing is extra awful on modesetting-ddx because glamor afaics seems
    to use drawing of a textured triangle strip for the copy implementation,
    not a dedicated blitter engine. The rasterization pattern creates extra
    awful tearing artifacts.
    
    We can do better: According to a tip from Michel Daenzer (thanks!),
    at least atomic modesetting capable kms drivers should be able to
    reliably change scanout stride during a pageflip, even if atomic
    modesetting is not actually enabled for the modesetting client.
    
    This commit adds detection logic to find out if the underlying kms
    driver is atomic_modeset_capable, and if so, it no longer rejects
    page flip presents on mismatched stride between new Pixmap and
    frontbuffer.
    
    We (ab)use a call to drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 0);
    for this purpose. The call itself has no practical effect, as it
    requests disabling atomic mode, although atomic mode is disabled by
    default. However, the return value of drmSetClientCap() tells us if the
    underlying kms driver is atomic modesetting capable: An atomic driver
    will return 0 for success. A legacy non-atomic driver will return a
    non-zero error code, either -EINVAL for early atomic Linux versions
    4.0 - 4.19 (or for non-atomic Linux 3.x and earlier), or -EOPNOTSUPP
    for Linux 4.20 and later.
    
    Testing on a MacBookPro 2017 with Intel Kabylake display server gpu +
    AMD Polaris11 as prime renderoffload gpu, X-Server master + Mesa 21.0.3
    show improvement from unbearable tearing to perfect, despite a stride
    mismatch between display gpu and Pixmap of 11776 Bytes vs. 11520
    Bytes. That this is correct behaviour was also confirmed by comparing the
    behaviour and .check_flip implementation of the patched modesetting-ddx
    against the current intel-ddx SNA Present implementation.
    
    Please consider merging this patch before the server-1.21 branch point.
    This patch could also be cherry-picked into the server 1.20 branch to
    fix the same limitation.
    
    Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
    (cherry picked from commit 8f8ebf870b55c9875b7cfd8ef69c1df02d589b4a)

diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
index ec4189a2c..5dc8e202c 100644
--- a/hw/xfree86/drivers/modesetting/driver.c
+++ b/hw/xfree86/drivers/modesetting/driver.c
@@ -1040,6 +1040,14 @@ PreInit(ScrnInfoPtr pScrn, int flags)
 #endif
     }
 
+    /*
+     * Use "atomic modesetting disable" request to detect if the kms driver is
+     * atomic capable, regardless if we will actually use atomic modesetting.
+     * This is effectively a no-op, we only care about the return status code.
+     */
+    ret = drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 0);
+    ms->atomic_modeset_capable = (ret == 0);
+
     if (xf86ReturnOptValBool(ms->drmmode.Options, OPTION_ATOMIC, FALSE)) {
         ret = drmSetClientCap(ms->fd, DRM_CLIENT_CAP_ATOMIC, 1);
         ms->atomic_modeset = (ret == 0);
diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index a99f37871..28810c063 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -108,6 +108,7 @@ typedef struct _modesettingRec {
      * Page flipping stuff.
      *  @{
      */
+    Bool atomic_modeset_capable;
     Bool atomic_modeset;
     Bool pending_modeset;
     /** @} */
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index 186309a29..f7a475356 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -252,8 +252,11 @@ ms_present_check_unflip(RRCrtcPtr crtc,
     if (num_crtcs_on == 0)
         return FALSE;
 
-    /* Check stride, can't change that on flip */
-    if (!ms->atomic_modeset &&
+    /*
+     * Check stride, can't change that reliably on flip on some drivers, unless
+     * the kms driver is atomic_modeset_capable.
+     */
+    if (!ms->atomic_modeset_capable &&
         pixmap->devKind != drmmode_bo_get_pitch(&ms->drmmode.front_bo))
         return FALSE;
 
commit 574fe59ef0669d4754884a115016d393d187848a
Author: Mario Kleiner <mario.kleiner.de at gmail.com>
Date:   Mon Oct 18 08:14:04 2021 +0200

    Fix RandR leasing for more than 1 simultaneously active lease.
    
    Due to a switched order of parameters in the xorg_list_add()
    call inside ProcRRCreateLease(), adding a new lease for RandR
    output leasing does not actually add the new RRLeasePtr lease
    record to the list of existing leases for a X-Screen, but instead
    replaces the existing list with a new list that has the new lease
    as the only element, and probably leaks a bit of memory.
    
    Therefore the server "forgets" all active leases for a screen,
    except for the last added lease. If multiple leases are created
    in a session, then destruction of all leases but the last one
    will fail in many cases, e.g., during server shutdown in
    RRCloseScreen(), or resource destruction, e.g., in
    RRCrtcDestroyResource().
    
    Most importantly, it fails if a client simply close(fd)'es the
    DRM master descriptor to release a lease, quits, gets killed or
    crashes. In this case the kernel will destroy the lease and shut
    down the display output, then send a lease event via udev to the
    ddx, which e.g., in the modesetting-ddx will trigger a call to
    drmmode_validate_leases().
    
    That function is supposed to detect the released lease and tell
    the server to terminate the lease on the server side as well,
    via xf86CrtcLeaseTerminated(), but this doesn't happen for all
    the leases the server has forgotten. The end result is a dead
    video output, as the server won't reinitialize the crtc's
    corresponding to the terminated but forgotten lease.
    
    This bug was observed when using the amdvlk AMD OSS Vulkan
    driver and trying to lease multiple VKDisplay's, and also
    under Mesa radv, as both Mesa Vulkan/WSI/Display and amdvlk
    terminate leases by simply close()ing the lease fd, not by
    sending explicit RandR protocol requests to free leases.
    
    Leasing worked, but ending a session with multiple active
    leases ended in a lot of unpleasant darkness.
    
    Fixing the wrong argument order to xorg_list_add() fixes the
    problem. Tested on single-X-Screen and dual-X-Screen setups,
    with one, two or three active leases.
    
    Please merge this for the upcoming server 21.1 branch.
    Merging into server 1.20 would also make a lot of sense.
    
    Fixes: e4e3447603b5fd3a38a92c3f972396d1f81168ad
    Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
    Reviewed-by: Keith Packard <keithp at keithp.com>
    (cherry picked from commit f467f85ca1f780d5c7cf3c20888e399708d761ac)

diff --git a/randr/rrlease.c b/randr/rrlease.c
index e25d9ca99..11ba96f24 100644
--- a/randr/rrlease.c
+++ b/randr/rrlease.c
@@ -295,7 +295,7 @@ ProcRRCreateLease(ClientPtr client)
     if (rc != Success)
         goto bail_lease;
 
-    xorg_list_add(&scr_priv->leases, &lease->list);
+    xorg_list_add(&lease->list, &scr_priv->leases);
 
     if (!AddResource(stuff->lid, RRLeaseType, lease)) {
         close(fd);


More information about the xorg-commit mailing list