Commit Graph

495 Commits

Author SHA1 Message Date
Michel Dänzer
2798244be7 Make drmmode_cm_enabled an inline function
So that it can be used outside of drmmode_display.c as well.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-03-04 17:39:12 +01:00
Michel Dänzer
72653455e4 Revert "Remove set but unused amdgpu_dri2::pKernelDRMVersion"
This reverts commit 720a61000a.

We're going to make use of it now.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-03-04 17:31:45 +01:00
Michel Dänzer
28cd209ebf Revert "gitlab-ci: Only run docker-image stage if relevant source files change"
This reverts commit 9c23076b9e.

Some scenarios have come to light where this failed to ensure the docker
image exists:

* If the master branch of a forked repository is used for an MR which
  doesn't modify .gitlab-ci.yml, the docker-image job may not run.
* If the docker-image job of the first pipeline in a forked repository
  is cancelled or fails for any reason, and .gitlab-ci.yml isn't
  modified for the next pipeline run.
2019-03-01 10:09:21 +00:00
Michel Dänzer
09be74a3d1 dri2: Call drm_queue_handle_deferred in dri2_deferred_event
drm_queue_handler just puts the event on the signalled list; without
calling drm_queue_handle_deferred, actual processing of the event may be
delayed indefinitely, e.g. until another event arrives from the kernel.

This could result in DRI2 clients hanging during DPMS off.

Fixes: 739181c8d3 "Add amdgpu_drm_handle_event wrapper for
                      drmHandleEvent"
Reviewed-by: Aaron Liu <aaron.liu@amd.com>
Tested-by: Aaron Liu <aaron.liu@amd.com>
2019-03-01 09:53:38 +00:00
Michel Dänzer
a636f42b49 present: Check that flip and screen pixmap pitches match
If they don't, flipping will result in corrupted display.

Test case:

* Run Xorg at 1920x1080 with no window manager
* glxgears -geometry 2048x1080

The Present extension code in xserver 1.21 will check for this.

Tested-by: Jax Lin <jax.lin@amd.com>
2019-02-27 18:43:27 +01:00
Michel Dänzer
bd090f389f Call amdgpu_present_set_screen_vrr from amdgpu_vrr_property_update
If the window is currently flipping.

This might make a difference when the property gets disabled: Variable
refresh will now be disabled immediately in that case, instead of only
when the window can no longer use page flipping at all.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-02-22 17:45:04 +01:00
Michel Dänzer
d9be5d712d Make use of property request wrappers for VRR property
Instead of scanning for PropertyNotify events. Reasons:

* Works even if no client listens to PropertyNotify events for the
  window.
* No overhead on delivery of unrelated events, and no overhead at all
  if Option "VariableRefresh" is disabled.

v2:
* Use shorter variable name amdgpu_vrr_atom.
* Call MakeAtom regardless of info->instance_id, for robustness vs VRR
  being enabled in some but not all AMDGPU screens.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-02-22 17:44:30 +01:00
Michel Dänzer
ef8fbe33b7 Wrap change/delete window property request handlers
Preparation for the following change.

v2:
* Add comments explaining what the wrappers are wrapping.
* Use global amdgpu_property_vectors_wrapped to keep track of whether
  the vectors need to be (un)wrapped, for robustness against VRR being
  enabled in some but not all AMDGPU screens.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-02-20 17:57:16 +01:00
Michel Dänzer
09a45ff8fe Don't enable the VRR support code for GPU screens
Windows aren't associated with GPU screens, and amdgpu_present_flip is
never called for them, so VRR can never actually be enabled for them.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-02-20 16:37:13 +01:00
Michel Dänzer
2a3d00dc7e Don't register a window private if VRR is disabled
It's not used in that case.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2019-02-12 18:58:23 +01:00
Michel Dänzer
5f91be77e0 gitlab-ci: Don't rely on $CI_PROJECT_NAME
The name of a forked repository can be changed later, in which case this
would fail to refer to the main repository.

Pointed out by Eric Engestrom in
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/224 .
2019-02-11 18:32:07 +01:00
Michel Dänzer
9c23076b9e gitlab-ci: Only run docker-image stage if relevant source files change
Otherwise there's normally no need to run it. It will also run when a
new branch is created, which ensures that the docker image always exists
(e.g. in a newly forked repository).

Inspired by https://gitlab.freedesktop.org/mesa/mesa/merge_requests/143
2019-02-07 17:35:13 +01:00
Michel Dänzer
9045fb310f 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.

Acked-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-25 17:54:53 +01:00
Michel Dänzer
3ff2cc225f 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.

v2:
* Use ErrorF instead of xf86DrvMsg with hard-coded screen 0.

Bugzilla: https://bugs.freedesktop.org/109364
Reviewed-by: Alex Deucher <alexander.deucher@amd.com> # v1
2019-01-25 17:49:52 +01:00
Michel Dänzer
e72a02ba1d 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) AMDGPU(0): flip queue failed: Device or resource busy
 (WW) AMDGPU(0): Page flip failed: Device or resource busy
 (EE) AMDGPU(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 .

Acked-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-17 11:34:04 +01:00
Michel Dänzer
a1b479c7d0 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 .

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-16 18:39:56 +01:00
Michel Dänzer
bf326f2ea1 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.

Acked-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-16 18:39:11 +01:00
Michel Dänzer
ebd32b1c07 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
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-16 18:38:40 +01:00
Michel Dänzer
d168532ee7 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
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-11 18:04:26 +01:00
Michel Dänzer
2058c4c469 Only call drmmode_validate_leases if RandR is enabled
It would crash if RandR is disabled, e.g. because Xinerama is enabled.

Bugzilla: https://bugs.freedesktop.org/109230
(Ported from radeon commit b1c01698f577577e4a88bad0ae08fb5d998e7ebb)
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-10 17:25:05 +01:00
Michel Dänzer
f3c0939a0c Only call drmmode_uevent_init if RandR is enabled
There's no point in listening for hotplug events if RandR is disabled,
as there's no other mechanism for them to be propagated. We were already
mostly ignoring them in that case.

Inspired by
1a489142c8
(via https://bugs.freedesktop.org/109230#c11).
(Ported from radeon commit 38db1bbcfc019c92884c7819a6630c70e543f6b2)
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2019-01-10 17:00:12 +01:00
Michel Dänzer
f3ddda618e gitlab-ci: Use kaniko instead of docker-in-docker for image generation
kaniko can also work in unprivileged runners.

Based on v2 of
https://gitlab.freedesktop.org/xorg/xserver/merge_requests/92 .
Acked-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-20 10:28:32 +01:00
Michel Dänzer
b689dc5081 Remove superfluous vrr_flipping field and clean up related code
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-20 10:08:13 +01:00
Michel Dänzer
233a0be82d Don't clear info->flip_window in present_unflip
present_unflip can get called between present_check_flip and
present_flip, in which case the latter would pass a NULL WindowPtr to
the former, resulting in a crash.

present_flip should never get called for a window which has already been
destroyed, so there's no need to clear info->flip_window.

Bugzilla: https://bugs.freedesktop.org/109067
Fixes: 2d18b37159 "Check last flip window instead of screen root
                      before flipping"
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-19 18:56:37 +01:00
Mario Kleiner
d4eab5d108 Fix crash when page flipping in multi-X-Screen/Zaphod mode
amdgpu_do_pageflip() indexed the flipdata->fb[] array
indexing over config->num_crtc, but the flip completion
routines, e.g., drmmode_flip_handler(), index that array
via the crtc hw id from drmmode_get_crtc_id(crtc).

This is mismatched and causes indexing into the wrong
array slot at flip completion -> Server crash.

Always use drmmode_get_crtc_id(crtc) for indexing into
the array to fix this.

Tested on a dual-X-Screen setup with one video output
assigned to each X-Screen, page-flipping an OpenGL app
on either of both X-Screens. This used to crash when
flipping on X-Screen 1, now it doesn't anymore.

Fixes: 9b6782c821 "Store FB for each CRTC in drmmode_flipdata_rec"
(Ported from radeon commit 0058fd2ebf4c900b12f129984e98886a7ac84b2f)
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
2018-12-19 18:03:27 +01:00
Michel Dänzer
0d60233d26 Use two HW cursor buffers per CRTC
Switch to the other buffer when xf86_config->cursor changes. Avoids
these issues possible when re-using the same buffer:

* The HW may intermittently display a mix of the old and new cursor
  images.
* If the hotspot changes, the HW may intermittently display the new
  cursor image at the location corresponding to the old image's hotspot.

Bugzilla: https://bugs.freedesktop.org/108832
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:40:40 +01:00
Michel Dänzer
b04697de52 Update cursor position in drmmode_show_cursor if hotspot changed
The cursor position is updated to be consistent with the new hotspot in
the same ioctl call.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:40:03 +01:00
Michel Dänzer
b344e1559e Use drmIoctl in drmmode_show_cursor
This should be functionally equivalent to what drmModeSetCursor(2) do
behind the scenes, but allows for new tricks in following changes.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:39:16 +01:00
Michel Dänzer
e95044e453 Drop AMDGPUInfoRec::cursor_buffer array
Not needed or even useful for anything.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:38:40 +01:00
Michel Dänzer
13c85e8a13 Don't use GBM for allocating HW cursor BOs
GBM doesn't really buy us anything for the cursor BOs. This simplifies
the code and following changes.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:37:56 +01:00
Michel Dänzer
bcfa6c258f Automatically try re-enabling TearFree after a flip failed
Specifically, after both the page flip and vblank ioctls failed, but
then the vblank ioctl started working again. This can happen
intermittently e.g. when hotplugging a DP display. Previously, TearFree
would stay disabled in that case until a modeset was triggered somehow.

Bugzilla: https://bugs.freedesktop.org/103791
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:17:30 +01:00
Michel Dänzer
4e7a24ac5a Cancel pending scanout update in drmmode_crtc_scanout_update
drmmode_crtc_scanout_update does the equivalent of a scanout update,
so no need to do it again. This might also avoid issues if there's a
pending scanout update at this point.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:17:02 +01:00
Michel Dänzer
500fadb162 Perform scanout buffer update immediately if drmmode_wait_vblank fails
Otherwise the damaged screen contents may never be displayed in that
case.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-12-13 12:16:27 +01:00
Michel Dänzer
be862ed459 Generate docker image as part of CI pipeline
This removes the dependency on an externally generated docker image, and
should make it easier to update the docker image or make other changes
related to it.

v2:
* If the image doesn't exist, try pulling it from the main repo's
  registry.
* Use debian:testing-slim as the base, might result in a slightly
  smaller image.

v3:
* Prevent installation of packages which are only recommended, for an
  even smaller image.
* Add recommendation to remove new image from source repository in
  favour of the main repository's.

Acked-by: Alex Deucher <alexander.deucher@amd.com> # v2
2018-12-11 11:47:16 +01:00
Nicholas Kazlauskas
b11ee02c45 Support CRTC variable refresh for windows using Present flips
This patch adds support for setting the CRTC variable refresh property
for suitable windows flipping via the Present extension.

The "VariableRefresh" Option is added to AMDGPU in this patch. This
option defaults to false, and must be set to "true" in an X conf
file for variable refresh support in the driver.

In order for a window to be suitable for variable refresh it must have
the _VARIABLE_REFRESH property with a 32-bit CARDINAL value of 1.

Then the window must pass the checks required to be suitable for
Present extension flips - it must cover the entire X screen and no
other window may already be flipping.

With these conditions met every CRTC for the X screen will have their
variable refresh property set to true.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2018-11-30 15:40:34 +00:00
Nicholas Kazlauskas
2d18b37159 Check last flip window instead of screen root before flipping
A significant amount of time can pass between the X call into
check_flip for a window and when amdgpu_present_flip actually occurs.
To ensure that flipping is still possible there was an additional check
performed on screen->root in amdgpu_present_flip - but what should
be checked instead is the window itself. This only really worked before
because X ensures that the window has the same dimensions as the screen
to allow for present extension flipping.

This patch tracks the flip window between calls to check_flip and flip
and uses that window instead of screen->root.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
2018-11-30 15:40:34 +00:00
Michel Dänzer
13c94a373b Skip gamma correction of cursor data if premultiplied R/G/B > alpha
The un-premultiplied R/G/B values would overflow the gamma LUT, so just
pass through the data unchanged, and leave it up to the HW how to
interpret such weird premultiplied alpha pixels.

Bugzilla: https://bugs.freedesktop.org/108355
2018-11-20 10:14:37 +01:00
Michel Dänzer
51ba6dddee Move deferred vblank events to separate drm_vblank_deferred list
It was still possible for nested xorg_list_for_each_entry_safe loops
to occur over the drm_vblank_signalled list, which could mess up that
list. Moving deferred events to a separate list allows processing the
drm_vblank_signalled list without xorg_list_for_each_entry_safe.

v2:
* Refactor drm_handle_vblank_signalled helper function, less code
  duplication => better readability (Alex Deucher)

Bugzilla: https://bugs.freedesktop.org/108600
Acked-by: Alex Deucher <alexander.deucher@amd.com>
2018-11-16 15:47:44 +00:00
Michel Dänzer
e2c7369cae Explicitly keep track of whether a DRM event is for a flip or not
When an async flip is performed, and TearFree is enabled on the CRTC
used for timing, we schedule a vblank event for completing the page
flip. The DRM event queuing code treated this event like a vblank event,
but it needs to be treated like a page flip event.

Acked-by: Alex Deucher <alexander.deucher@amd.com>
2018-11-16 15:47:44 +00:00
Michel Dänzer
eda571222f Use drm_abort_one in drm_queue_handler
At this point, we've already established that e->handler is NULL, no
need to check again in drm_queue_handle_one. This also makes it clearer
what's happening.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-11-16 15:47:44 +00:00
Michel Dänzer
426f9a4965 Relax detection of non-premultiplied alpha cursor data
The stricter detection broke the cursor in some games. Apparently those
use cursor data with premultiplied alpha, but with some pixels having
r/g/b values larger than the alpha value (which corresponds to original
r/g/b values > 1.0), triggering the workaround.

Relax the detection to match what's in the X server since 1.18.4, but
keep the workaround for older versions.

Bugzilla: https://bugs.freedesktop.org/108650
Acked-by: Alex Deucher <alexander.deucher@amd.com>
2018-11-15 16:10:03 +01:00
Michel Dänzer
a9da219e13 Add README.md to EXTRA_DIST
Otherwise it isn't included in the generated tarballs.

Suggested-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-11-14 09:58:46 +01:00
Michel Dänzer
1cb338253a man: This driver supports colour depths 8, 15 and 16
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-10-29 11:04:55 +01:00
Michel Dänzer
0734cdf544 glamor: Can work at depth >= 15 with current xserver Git master
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-10-29 10:57:03 +01:00
Michel Dänzer
ad6dfb0124 Detect and fix up non-premultiplied cursor data
X server >= 1.18 already had code for this, but it only caught cases
where some pixels have 0 for alpha and non-0 for a non-alpha component.
Turns out some apps (e.g. the Civilization VI game) use
non-premultiplied cursor data which doesn't have such pixels, but can
still result in visual artifacts.

This uses the method suggested by Kamil in
https://bugs.freedesktop.org/92309#c19: check for pixels where any
colour component value is larger than the alpha value, which isn't
possible with premultiplied alpha.

There can still be non-premultiplied data which won't be caught by this,
but that should result in slightly incorrect colours and/or blending at
the worst, not wildly incorrect colours such as shown in the bug report
below.

v2:
* Disable the check with current xserver Git master, which already does
  the same check now.

Bugzilla: https://bugs.freedesktop.org/108355
Suggested-by: Kamil Páral <kamil.paral@gmail.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
2018-10-25 09:15:56 +02:00
Michel Dänzer
c9d43c1deb Allow up to six instances in Zaphod mode
Corresponding to up to six CRTCs being available in the hardware.

v2:
* Move instance overflow check from PreInit to the probe hooks, in
  order to further minimize wasted effort.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com> # v1
2018-10-11 17:14:26 +02:00
Michel Dänzer
aa572683d8 Fix condition for calling set_pixmap_bo in drmmode_xf86crtc_resize
This matches CreateScreenResources_KMS.

Fixes crash when resizing the screen (e.g. using xrandr) with depth <
24.

Bugzilla: https://bugs.freedesktop.org/104914
Acked-by: Alex Deucher <alexander.deucher@amd.com>
2018-10-10 17:28:35 +02:00
Michel Dänzer
05a1ba9abc Add GitLab CI configuration
Builds the driver against all supported versions of xserver, with both
gcc and clang for xserver >= 1.18 (older versions cause warnings with
clang). Compiler warnings are treated as errors.

The xserver 1.15 build uses standalone glamor, the xserver 1.13 & 1.14
builds use --disable-glamor.

With the latest xserver version, make install and make distcheck are
tested as well.
2018-10-02 18:01:14 +02:00
Michel Dänzer
babbd38057 Fix --disable-glamor build
We were still relying on the glamor.h header being picked up implicitly.
2018-10-02 17:55:03 +02:00
Michel Dänzer
b6ee7f92cf Cast return value of amdgpu_get_marketing_name to char*
Avoids compiler warning with xserver < 1.16:

../../src/amdgpu_kms.c: In function ‘AMDGPUPreInitChipType_KMS’:
../../src/amdgpu_kms.c:1203:17: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  pScrn->chipset = amdgpu_get_marketing_name(pAMDGPUEnt->pDev);
                 ^
2018-10-02 17:52:10 +02:00