From 19147f5f85bd87a1fedb460939dfdcef76ed7242 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 24 Oct 2016 17:14:32 +0200 Subject: [PATCH] [platformx/x11] Add a freeze protection against OpenGL Summary: With nouveau driver it can happen that KWin gets frozen when first trying to render with OpenGL. This results in a freeze of the complete desktop as the compositor is non functional. Our OpenGL breakage detection is only able to detect crashes, but not freezes. This change improves it by also added a freeze protection. In the PreInit stage a thread is started with a QTimer of 15 sec. If the timer fires, qFatal is triggered to terminate KWin. This can only happen if the creation of the OpenGL compositor takes longer than said 15 sec. In the PostInit stage the timer gets deleted and the thread stopeed again. Thus if a freeze is detected the OpenGL unsafe protection is written into the config. KWin aborts and gets restarted by DrKonqui. The new KWin instance will no longer try to activate the freezing OpenGL as the protection is set. If KWin doesn't freeze the protection is removed from the config as we are used to. Check for freezes for the first n frames, not just the first This patch changes the freeze detection code to detect freezes in the first 30 frames (by default, users can change that with the KWIN_MAX_FRAMES_TESTED environment variable). This detects successfully the freezes associated to nouveau drivers in https://bugzilla.suse.com/show_bug.cgi?id=1005323 Reviewers: davidedmundson, #plasma, #kwin, graesslin Reviewed By: #plasma, #kwin, graesslin Subscribers: luebking, graesslin, kwin, plasma-devel, davidedmundson Tags: #plasma Differential Revision: https://phabricator.kde.org/D3132 --- composite.cpp | 16 +++++++- composite.h | 1 + platform.h | 5 ++- .../platforms/x11/standalone/x11_platform.cpp | 39 ++++++++++++++++++- .../platforms/x11/standalone/x11_platform.h | 2 + 5 files changed, 60 insertions(+), 3 deletions(-) diff --git a/composite.cpp b/composite.cpp index 729044081b..41b948e249 100644 --- a/composite.cpp +++ b/composite.cpp @@ -135,6 +135,9 @@ Compositor::Compositor(QObject* workspace) } ); + if (qEnvironmentVariableIsSet("KWIN_MAX_FRAMES_TESTED")) + m_framesToTestForSafety = qEnvironmentVariableIntValue("KWIN_MAX_FRAMES_TESTED"); + // register DBus new CompositorDBusInterface(this); } @@ -207,7 +210,6 @@ void Compositor::slotCompositingOptionsInitialized() m_scene = SceneOpenGL::createScene(this); - // TODO: Add 30 second delay to protect against screen freezes as well kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PostInit); if (m_scene && !m_scene->initFailed()) { @@ -734,7 +736,19 @@ void Compositor::performCompositing() // clear all repaints, so that post-pass can add repaints for the next repaint repaints_region = QRegion(); + if (m_framesToTestForSafety > 0 && (m_scene->compositingType() & OpenGLCompositing)) { + kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PreFrame); + } m_timeSinceLastVBlank = m_scene->paint(repaints, windows); + if (m_framesToTestForSafety > 0) { + if (m_scene->compositingType() & OpenGLCompositing) { + kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PostFrame); + } + m_framesToTestForSafety--; + if (m_framesToTestForSafety == 0 && (m_scene->compositingType() & OpenGLCompositing)) { + kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PostLastGuardedFrame); + } + } m_timeSinceStart += m_timeSinceLastVBlank; if (waylandServer()) { diff --git a/composite.h b/composite.h index a1690e268e..a7aee56416 100644 --- a/composite.h +++ b/composite.h @@ -239,6 +239,7 @@ private: Scene *m_scene; bool m_bufferSwapPending; bool m_composeAtSwapCompletion; + int m_framesToTestForSafety = 3; KWIN_SINGLETON_VARIABLE(Compositor, s_compositor) }; diff --git a/platform.h b/platform.h index 3991f5fe34..de0b3bffcc 100644 --- a/platform.h +++ b/platform.h @@ -142,7 +142,10 @@ public: virtual bool openGLCompositingIsBroken() const; enum class OpenGLSafePoint { PreInit, - PostInit + PostInit, + PreFrame, + PostFrame, + PostLastGuardedFrame }; /** * This method is invoked before and after creating the OpenGL rendering Scene. diff --git a/plugins/platforms/x11/standalone/x11_platform.cpp b/plugins/platforms/x11/standalone/x11_platform.cpp index 8212d1e0e0..05bdd3982b 100644 --- a/plugins/platforms/x11/standalone/x11_platform.cpp +++ b/plugins/platforms/x11/standalone/x11_platform.cpp @@ -37,6 +37,7 @@ along with this program. If not, see . #include #include +#include #include #include @@ -200,12 +201,48 @@ void X11StandalonePlatform::createOpenGLSafePoint(OpenGLSafePoint safePoint) switch (safePoint) { case OpenGLSafePoint::PreInit: group.writeEntry(unsafeKey, true); + group.sync(); + // Deliberately continue with PreFrame + case OpenGLSafePoint::PreFrame: + if (m_openGLFreezeProtectionThread == nullptr) { + Q_ASSERT(m_openGLFreezeProtection == nullptr); + m_openGLFreezeProtectionThread = new QThread(this); + m_openGLFreezeProtectionThread->setObjectName("FreezeDetector"); + m_openGLFreezeProtectionThread->start(); + m_openGLFreezeProtection = new QTimer; + m_openGLFreezeProtection->setInterval(15000); + m_openGLFreezeProtection->setSingleShot(true); + m_openGLFreezeProtection->start(); + m_openGLFreezeProtection->moveToThread(m_openGLFreezeProtectionThread); + connect(m_openGLFreezeProtection, &QTimer::timeout, m_openGLFreezeProtection, + [] { + const QString unsafeKey(QLatin1String("OpenGLIsUnsafe") + (kwinApp()->isX11MultiHead() ? QString::number(kwinApp()->x11ScreenNumber()) : QString())); + auto group = KConfigGroup(kwinApp()->config(), "Compositing"); + group.writeEntry(unsafeKey, true); + group.sync(); + qFatal("Freeze in OpenGL initialization detected"); + }, Qt::DirectConnection); + } else { + Q_ASSERT(m_openGLFreezeProtection); + QMetaObject::invokeMethod(m_openGLFreezeProtection, "start", Qt::QueuedConnection); + } break; case OpenGLSafePoint::PostInit: group.writeEntry(unsafeKey, false); + group.sync(); + // Deliberately continue with PostFrame + case OpenGLSafePoint::PostFrame: + QMetaObject::invokeMethod(m_openGLFreezeProtection, "stop", Qt::QueuedConnection); + break; + case OpenGLSafePoint::PostLastGuardedFrame: + m_openGLFreezeProtection->deleteLater(); + m_openGLFreezeProtection = nullptr; + m_openGLFreezeProtectionThread->quit(); + m_openGLFreezeProtectionThread->wait(); + delete m_openGLFreezeProtectionThread; + m_openGLFreezeProtectionThread = nullptr; break; } - group.sync(); } } diff --git a/plugins/platforms/x11/standalone/x11_platform.h b/plugins/platforms/x11/standalone/x11_platform.h index e3a670e0bf..b90b759a4c 100644 --- a/plugins/platforms/x11/standalone/x11_platform.h +++ b/plugins/platforms/x11/standalone/x11_platform.h @@ -62,6 +62,8 @@ private: static bool hasGlx(); XInputIntegration *m_xinputIntegration = nullptr; + QThread *m_openGLFreezeProtectionThread = nullptr; + QTimer *m_openGLFreezeProtection = nullptr; };