The miPointerSpriteFunc swcursor code expects there to only be a single
framebuffer and when the cursor moves it will undo the damage of the
previous draw, potentially overwriting what ever is there in a new
framebuffer installed after a flip.
This leads to all kind of artifacts, so we need to disable pageflipping
when a swcursor is used.
The code for this has shamelessly been copied from the xf86-video-amdgpu
code.
Fixes: https://gitlab.freedesktop.org/xorg/xserver/issues/828
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Fix the following compiler warning:
drmmode_display.c: In function ‘drmmode_create_bo’:
drmmode_display.c:1019:9: warning: ISO C90 forbids mixed declarations and code [
1019 | uint32_t num_modifiers;
| ^~~~~~~~
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
When the pixmapPrivateKeyRec was moved from a global to being embedded
inside the drmmode_rec these 2 where missed, clean them up.
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Noticed when porting this logic to xf86-video-nouveau, and valgrind
complained about conditional jump based on uninitialized data.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Believe it or not, somehow we've never done this in legacy mode! We
currently simply change the DPMS property on the CRTC's output's
respective DRM connector, but this means that we're just setting the
CRTC as inactive-not disabled. From the perspective of the kernel, this
means that any shared resources used by the CRTC are still in use.
This can cause problems for drivers that are not yet fully atomic,
despite using the atomic helpers internally. For instance: if CRTC-1 and
CRTC-2 are still enabled and use shared resources within the kernel (an
MST topology, for example), and then userspace tries to go enable CRTC-3
on the same topology this might suddenly fail if CRTC-3 needs the shared
resources CRTC-1 and CRTC-2 are using. While I don't know of any
situations in the mainline kernel that actually trigger this, future
plans for reworking the atomic check of MST drivers are absolutely
going to make this into a real issue (they already are in my WIP
branches for the kernel).
So: actually do the right thing here and disable CRTCs when they're not
going to be used anymore, even in legacy mode.
Signed-off-by: Lyude Paul <lyude@redhat.com>
The X will be crashed on the system with other DDX driver,
such as amdgpu.
show the log like:
randr: falling back to unsynchronized pixmap sharing
(EE)
(EE) Backtrace:
(EE) 0: /usr/lib/xorg/Xorg (xorg_backtrace+0x4e)
(EE) 1: /usr/lib/xorg/Xorg (0x55cb0151a000+0x1b5ce9)
(EE) 2: /lib/x86_64-linux-gnu/libpthread.so.0 (0x7f1587a1d000+0x11390)
(EE)
(EE) Segmentation fault at address 0x0
(EE)
The issue is that modesetting as the master, and amdgpu as the slave.
Thus, when the master attempts to access pSlavePixPriv in ms_dirty_update(),
problems result due to the fact that it's accessing AMD's 'ppriv' using the
modesetting structure definition.
Apart from fixing crash issue, the patch fix other issue in master interface
in which driver should refer to master pixmap.
Signed-off-by: Jim Qu <Jim.Qu@amd.com>
Reviewed-by: Alex Goins <agoins@nvidia.com>
Instead of having every video driver loop over any pending leases to
free them during CloseScreen, do this up in the DIX layer by
terminating leases when a leased CRTC or Output is destroyed and
(just to make sure), also terminating leases in RRCloseScreen. The
latter should "never" get invoked as any lease should be associated
with a resource which was destroyed.
This is required as by the time the driver's CloseScreen function is
invoked, we've already freed all of the DIX randr structures and no
longer have any way to reference the leases
Signed-off-by: Keith Packard <keithp@keithp.com>
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=106960
Cc: Thomas Hellstrom <thellstrom@vmware.com>
The recent rewrite of modesetting driver broke the 24bpp support.
As typically found on cirrus KMS, it leads to a blank screen, spewing
the error like:
failed to add fb -22
(EE) modeset(0): failed to set mode: Invalid argument
The culript is that the wrong bpp value of the front buffer is passed
to drmModeAddFB(). Fix it by replacing with the back buffer bpp,
drmmode->kbpp.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Dirsch <sndirsch@suse.de>
Reviewed-by: Adam Jackson <ajax@redhat.com>
When setting DefaultDepth to 16 in the Screen section, the current
code requests a 32 bpp framebuffer, however the X-Server seems to
assumes 16 bpp.
Fixes commit 21217d0216 ("modesetting: Implement 32->24 bpp
conversion in shadow update")
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
If we're using atomic modesetting, then we're also using universal
planes, and so the lease we create needs to include the plane.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
We don't want universal_planes unless we're using atomic APIs for
modesetting, and the kernel already enables universal_planes
automatically when atomic is enabled.
If we enable universal_planes when we're not using atomic, then we
won't have selected a plane for each crtc, and this will break lease
creation which requires planes for each output when universal_planes
is enabled.
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This lets an application open a suitable DRM device and pass the file
descriptor to the mode setting driver through an X server command line
option, '-masterfd'.
There's a companion application, xlease, which creates a DRM master by
leasing an output from another X server. That is available at
git clone git://people.freedesktop.org/~keithp/xlease
v2:
Always print usage, but note that it can't be used if
setuid/gid
Suggested-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
So, this did actually work on older kernels at one point in time,
however it seems that this working was a result of some of the Linux
kernel's atomic modesetting helpers not preserving the CRTC's enabled
state in the right spots. This was fixed in:
846c7dfc1193 ("drm/atomic: Try to preserve the crtc enabled state in drm_atomic_remove_fb, v2")
As a result, atomic commits which simply disassociate a DRM connector
with it's CRTC while leaving the CRTC in an enabled state aren't enough
to disable the CRTC, and result in the atomic commit failing. This
currently can cause issues with MST hotplugging where X will end up
failing to disable the MST outputs after they've left the system. A
simple reproducer:
- Start up Xorg
- Connect an MST hub with displays connected to it
- Remove the hub
- Now there should be CRTCs stuck on the orphaned MST connectors, and X
won't be able to reclaim them.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
drmmode_shadow_allocate() still uses drmModeAddFB() which may fail if
the format is not as expected, preventing from using a rotated output.
Change it to use the new function drmmode_bo_import() which takes care
of calling the drmModeAddFB2() API.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106715
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Tested-by: Tomas Pelka <tpelka@redhat.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
We don't want DRM file descriptors to leak to child processes.
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Modifiers support needs gbm as a dependency. Without setting the dependency
included headers are not found reliably and the build might fail if the
headers are not placed in the default system include paths.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
The old 32-Bit wraparound handling didn't actually work, due to some
integer casting bug, and the mapping was ill equipped to deal with input
from the new true 64-bit GetCrtcSequence/QueueCrtcSequence api's
introduced in Linux 4.15.
For 32-Bit truncated input from pageflip events and old vblank events
and old drmWaitVblank ioctl, implement new wraparound handling, which
also allows to deal with wraparound in the other direction, e.g., if a
32-Bit truncated sequence value is passed in, whose true 64-Bit
in-kernel hw value is within 2^30 counts of the previous processed
value, but whose 32-bit truncated sequence value happens to lie just
above or below a 2^32 boundary, iow. one of the two values 'sequence'
vs. 'msc_prev' lies above a 2^32 border, the other one below it.
The method is directly translated from Mesa's proven implementation of
the INTEL_swap_events extension, where a true underlying 64-Bit wide
swapbuffers count (SBC) needs to get reconstructed from a 32-Bit LSB
truncated SBC transported over the X11 protocol wire. Same conditions
apply, ie. successive true 64-Bit SBC values are close to each other,
but don't always get received in strictly monotonically increasing
order. See Mesa commit cc5ddd584d17abd422ae4d8e83805969485740d9 ("glx:
Handle out-of-sequence swap completion events correctly. (v2)") for
explanation.
Additionally add a separate path for true 64-bit msc input originating
from Linux 4.15+ drmCrtcGetSequence/QueueSequence ioctl's and
corresponding 64-bit vblank events. True 64-bit msc's don't need
remapping and must be passed through.
As a reliability bonus, they are also used here to update the tracking
values msc_prev and ms_high with perfect 64-Bit ground truth as baseline
for mapping msc from pageflip completion events, because pageflip events
are always 32-bit wide, even when the new kernel api's are used. Because
each pageflip(-event) is always preceeded close in time (and vblank
count) by a drmCrtcQueueSequence queued event or drmCrtcGetSequence
query as part of DRI2 or DRI3+Present swap scheduling, we can be certain
that each pageflip event will get its truncated 32-bit msc remapped
reliably to the true 64-bit msc of flip completion whenever the sequence
api is available, ie. on Linux 4.15 or later.
Note: In principle at least the 32-bit mapping path could also be
backported to earlier server branches, as this seems to be broken for at
least server 1.16 to 1.19.
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Cc: Keith Packard <keithp@keithp.com>
Cc: Michel Dänzer <michel.daenzer@amd.com>
The function is ported from intel-ddx uxa backend around 2013, where its
stated purpose was to apply a vblank_offset to msc values to correct for
problems with those kernel provided msc values. Some (somewhat magic and
puzzling to myself) heuristic tried to guess if provided values were
unreasonable and tried to adapt the corrective vblank_offset to account
for that.
Except: It wasn't applied to kernel provided msc values, but the values
delivered by clients via DRI2 or Present, so valid client targetmsc
values, e.g., requesting a vblank event > 1000 vblanks in the future,
triggered the offset correction in arbitrarily wrong ways, leading to
wrong msc values being returned and thereby vblank events queued to the
kernel for the wrong time. This causes glXSwapBuffersMscOML and
glXWaitForMscOML to swap / return immediately whenever a swap/wait in >
1000 vblanks is requested.
The original code was also written to only deal with 32 bit mscs, but
server 1.20 modesetting ddx can now use new Linux 4.15+ kernel vblank
api to process true 64 bit msc's, which may confuse the heuristic even
more due to 32 bit integer truncation/wrapping.
This code caused various problems in the intel-ddx in the past since
year 2013, and was removed there in 2015 by Chris Wilson in commit
42ebe2ef9646be5c4586868cf332b4cd79bb4618:
" uxa: Remove the filtering of bogus Present MSC values
If the intention was to filter the return values from the kernel, the
filtering would have been applied to the kernel values and not to the
incoming values from Present. This filtering introduces crazy integer
promotion and truncation bugs all because Present feeds garbage into its
vblank requests.
"
Indeed, i found a Mesa bug yesterday which can cause Mesa's
PresentPixmap request to spuriously feed garbage targetMSC's into the
driver under some conditions. However, while other video drivers seem to
cope relatively well with that, modesetting ddx causes KDE-5's
plasmashell to lock up badly quite frequently, and my suspicion is that
the code removed in this commit is one major source of the extra
fragility.
Also my own tests fail for any swap scheduled more than 1000 vblanks
into the future, which is not uncommon for some scientific applications.
Iow. modesetting's swap scheduling seems to be more robust without this
function afaics.
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Keith Packard <keithp@keithp.com>
Tested-by: Mike Lothian <mike@fireburn.co.uk>
GBM objects were never destroyed after looking for format and
modifier compatibility when deciding whether flipping or copying
a presented pixmap.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106106
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Use the DRM_CAP_ADDFB2_MODIFIERS query to make sure the kms
driver supports modifiers in the addfb2 ioctl, and fall back
to addfb ioctl without modifiers if modifiers are unsupported.
E.g., as of Linux 4.17, nouveau-kms so far does not suppport
modifiers and gets angry if drmModeAddFB2WithModifiers() is
called (-> failure to set a video mode -> blank screen), but
Mesa's nvc0+ gallium driver causes gbm_bo_get_modifier() to
return a valid modifier by translating the default tiling of
bo's created via gbm_bo_create() into a modifier other than
DRM_FORMAT_MOD_INVALID (see Mesa's nvc0_miptree_get_modifier()).
Testing for != DRM_FORMAT_MOD_INVALID is apparently not
sufficient for safe use of drmModeAddFB2WithModifiers.
Bonus: Handle potential failure of populate_format_modifiers().
The required DRM_CAP is defined since libdrm v2.4.65, and we
require v2.4.89+ for the server, so we can use it unconditionally.
Tested on intel-kms, radeon-kms, nouveau-kms. Fixes failure on
NVidia Pascal.
Fixes: 2f807c2324 ("modesetting: Add support for multi-plane pixmaps when page-flipping")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Reviewed-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
ms_queue_vblank() returns false on failure.
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Frank Binns <frank.binns@imgtec.com>
Tested-by: Mike Lothian <mike@fireburn.co.uk>
We would fail to get the FB ID if it wasn't already imported, since we
were checking to see if the pointer was NULL (it never was) rather than
if the content of the pointer was 0.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Olivier Fourdan <ofourdan@redhat.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
drmmode_crtc_set_mode has a loop nested inside another loop, where both
of them were using 'i' as the loop iterator. Rename it to avoid an
infinite loop.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-and-Tested-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Non-atomic kms drivers like radeon-kms (or nouveau-kms with
default setting of "atomic ioctl disabled") don't export
any formats, so num_formats == 0.
Some atomic drivers (nouveau-kms with boot param nouveau.atomic=1,
or intel-kms on, e.g., Linux 4.13) expose num_formats == 0, or
don't expose any modifiers, so num_modifiers == 0.
Let the drmmode_is_format_supported() check pass in these cases
to allow page flipping, as it works just fine.
Tested on NV-96 for nouveau, HD-5770 for radeon, Intel Ivybridge
with Linux 4.13 and drm-next to fix page flipping.
Fixes: 9d147305b4 ("modesetting: Check if buffer format is supported when flipping")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
We need to make sure that the atomic commit are consistent
or else the kernel will reject it. For example, when moving
a CRTC from one output to another one, the first output CRTC_ID
property needs to be reset. Also if the second output was using
another CRTC beforehands, it needs to be disabled to avoid an
inconsistent state.
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Tested-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
CRTCs and outputs needs to be enabled/disabled when the current
DPMS mode is changed. We also try to do it in an atomic commit
when possible.
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Tested-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Keep track of whether or not we fed modifiers into GBM when we allocated
a BO. We'll use this later inside Glamor, to reallocate buffer storage
if we allocate buffer storage using modifiers, and a non-modifier-aware
client requests an export of that pixmap.
This makes it possible to run a compositing manager on an old GLX/EGL
stack on top of an X server which allocates internal buffer storage
using exotic modifiers from modifier-aware GBM/EGL/KMS.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Keep track of whether or not we fed modifiers into GBM when we allocated
a BO. We'll use this later inside Glamor, to reallocate buffer storage
if we allocate buffer storage using modifiers, and a non-modifier-aware
client requests an export of that pixmap.
This makes it possible to run a compositing manager on an old GLX/EGL
stack on top of an X server which allocates internal buffer storage
using exotic modifiers from modifier-aware GBM/EGL/KMS.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
The framebuffer can include multiple CRTCs in multi-monitors
setup. So we shouldn't use the buffer size but the CRTC size
instead. Rotated displays are shadowed, so we don't need to
worry about it there.
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Fixes a regression caused by modifiers support. For some hw to
continue working even if not supporting ARGB8888 and ARGB2101010
formats, we assume that all imported BOs are opaque.
Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Otherwise the same content is shown on all outputs.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Daniel Stone <daniels@collabora.comM>
drmmode_output_dpms is called especially with !output->crtc found in
xf86DisableUnusedFunctions so we have to guard for it, else the server
segfaults:
0 0x00007fdc1706054b in drmmode_output_dpms (output=0x55e15243c210, mode=3) at
drmmode_display.c:2243
1 0x000055e1500b6873 in xf86DisableUnusedFunctions (pScrn=0x55e152133f00) at
xf86Crtc.c:3021
2 0x000055e1500be940 in xf86RandR12CrtcSet (pScreen=<optimized out>,
randr_crtc=0x55e1524b2b90, randr_mode=0x0, x=0, y=0, rotation=<optimized out>,
num_randr_outputs=0, randr_outputs=0x0) at xf86RandR12.c:1244
3 0x000055e1500fa1c2 in RRCrtcSet (crtc=<optimized out>, mode=0x0, x=0, y=0,
rotation=rotation@entry=1, numOutputs=numOutputs@entry=0, outputs=0x0) at
rrcrtc.c:763
4 0x000055e1500fba9e in ProcRRSetCrtcConfig (client=0x55e152bfae50) at
rrcrtc.c:1390
5 0x000055e150044008 in Dispatch () at dispatch.c:478
6 0x000055e150047ff8 in dix_main (argc=13, argv=0x7ffc68561038,
envp=<optimized out>) at main.c:276
7 0x00007fdc1a0c6a87 in __libc_start_main () at /lib64/libc.so.6
8 0x000055e150031d0a in _start () at ../sysdeps/x86_64/start.S:120
Fixes: ba0c75177 ("modesetting: Fix up some XXX from removing GLAMOR_HAS_DRM_*")
Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Reviewed-by: Adam Jackson <ajax@redhat.com>
We don't use flink in the GetFB import path anymore, as we do an
FD-based import instead.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
A cosmetic change for automake (though we have to replicate some of
xorg-macros.m4 in manpages.am now), but meson's configure_file() wants
@-delimited strings.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
The check for "no modifier specified" in drmmode_is_format_supported()
should check for DRM_FORMAT_MOD_INVALID, not for zero, as zero actually
means DRM_FORMAT_MOD_LINEAR.
This allows page-flipping again when appropriate, as
tested under nouveau and ati drivers.
Fixes: 9d147305b4 ("modesetting: Check if buffer format is supported when flipping")
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
All the macros are available in the libdrm that we depend on.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
We already require libdrm 2.4.89 which provides the definition plus
guarding kernel UABI like that is generally a bad idea.
See previous commit for details why :-)
Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
The macro was available in libdrm for ages. Furthermore having a guard
like this is a very bad idea.
Building on an old server will result in a missing run-time functionality.
Since it's UABI one can use a local fallback, old kernels will return
-EINVAL and the fallback path will kick in.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
With earlier commit the required version was bumped to 2.4.89, thus the
guards always evaluate to true.
Fixes: e4e3447603 ("Add RandR leases with modesetting driver support
[v6]")
Cc: Keith Packard <keithp@keithp.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Louis-Francis Ratté-Boulianne <lfrb@collabora.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This reverts commit 8c455db0eb.
Since xf86platformBus.h is only included when XSERVER_PLATFORM_BUS is
defined, and configure.ac only defines that on systems with udev, this
commit breaks the build on non-udev systems like Solaris.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>