backends/drm: work around amdgpu vrr cursor bug differently

Instead of not-delaying cursor updates with adaptive sync, this forces a
software cursor instead. That way, the functionality works the same on all
the vendors.

For testing potential driver fixes, the environment variable
KWIN_DRM_DONT_FORCE_AMD_SW_CURSOR=1 can be used to disable this workaround
This commit is contained in:
Xaver Hugl 2024-03-09 01:17:57 +01:00
parent 380795423d
commit 656313455b
4 changed files with 19 additions and 8 deletions

View file

@ -28,13 +28,7 @@ DrmCommitThread::DrmCommitThread(DrmGpu *gpu, const QString &name)
return;
}
static bool delayVrrCursorUpdatesEnvSet = false;
static const bool delayVrrCursorUpdatesEnv = qEnvironmentVariableIntValue("KWIN_DRM_DELAY_VRR_CURSOR_UPDATES", &delayVrrCursorUpdatesEnvSet) == 1;
// amdgpu doesn't handle this correctly, so it's off by default for amd gpus
// See https://gitlab.freedesktop.org/drm/amd/-/issues/2186 for more details
const bool delayVrrCursorUpdates = (!delayVrrCursorUpdatesEnvSet && !gpu->isAmdgpu()) || delayVrrCursorUpdatesEnv;
m_thread.reset(QThread::create([this, delayVrrCursorUpdates]() {
m_thread.reset(QThread::create([this]() {
const auto thread = QThread::currentThread();
gainRealTime();
while (true) {
@ -74,7 +68,7 @@ DrmCommitThread::DrmCommitThread(DrmGpu *gpu, const QString &name)
}
continue;
}
if (m_commits.front()->isCursorOnly() && m_vrr && delayVrrCursorUpdates) {
if (m_commits.front()->isCursorOnly() && m_vrr) {
// wait for a primary plane commit to be in, while still enforcing
// a minimum cursor refresh rate of 30Hz
const auto cursorTarget = m_lastPageflip + std::chrono::duration_cast<std::chrono::nanoseconds>(1s) / 30;

View file

@ -48,6 +48,9 @@ EglGbmCursorLayer::EglGbmCursorLayer(EglGbmBackend *eglBackend, DrmPipeline *pip
std::optional<OutputLayerBeginFrameInfo> EglGbmCursorLayer::beginFrame()
{
if (m_pipeline->amdgpuVrrWorkaroundActive()) {
return std::nullopt;
}
// note that this allows blending to happen in sRGB or PQ encoding.
// That's technically incorrect, but it looks okay and is intentionally allowed
// as the hardware cursor is more important than an incorrectly blended cursor edge

View file

@ -406,6 +406,9 @@ bool DrmPipeline::updateCursor()
if (needsModeset() || !m_pending.crtc || !m_pending.active) {
return false;
}
if (amdgpuVrrWorkaroundActive() && m_cursorLayer->isEnabled()) {
return false;
}
// explicitly check for the cursor plane and not for AMS, as we might not always have one
if (m_pending.crtc->cursorPlane()) {
// test the full state, to take pending commits into account
@ -428,6 +431,12 @@ bool DrmPipeline::updateCursor()
}
}
bool DrmPipeline::amdgpuVrrWorkaroundActive() const
{
static const bool s_env = qEnvironmentVariableIntValue("KWIN_DRM_DONT_FORCE_AMD_SW_CURSOR") == 1;
return !s_env && gpu()->isAmdgpu() && (m_pending.presentationMode == PresentationMode::AdaptiveSync || m_pending.presentationMode == PresentationMode::AdaptiveAsync);
}
void DrmPipeline::applyPendingChanges()
{
m_next = m_pending;

View file

@ -134,6 +134,11 @@ public:
void setColorDescription(const ColorDescription &description);
void setIccProfile(const std::shared_ptr<IccProfile> &profile);
/**
* amdgpu drops cursor updates with adaptive sync: https://gitlab.freedesktop.org/drm/amd/-/issues/2186
*/
bool amdgpuVrrWorkaroundActive() const;
enum class CommitMode {
Test,
TestAllowModeset,