From 93b5e13308bfc8e5b546fff63d926f26ec3c4a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Tue, 20 Oct 2015 13:16:05 +0200 Subject: [PATCH] [hwcomposer] Rework the vsync code This changes how we synchronize through vsync. We use a mutex and a wait condition to synchronize the threads. When presenting the frame our main gui thread blocks and will be woken up by the vsync event (or a timeout of max 1 frame time slot). In order to minimize the blocked time we use the blocksForRetrace functionality from the GLX compositor. Given this change we no longer need to tell the compositor that we are swapping the frame, it's blocked anyway. Also we don't need the failsafe QTimer anymore. With this change applied on a Nexus 5 it's succeeding the "Martin tortures phone test". It doesn't tear anymore and has a smooth experience. I'm rather disappointed by the fact that we need to block in order to get vsync. This means Android/hwcomposer is as bad as GLX. So much for the "Android stack is so awesome", in fact it's not. Anybody thinking it's awesome should compare to DRM/KMS and especially atomic modesetting. Yes it's possible to present frames without tearing and without having to block the rendering thread. Reviewed-By: Marco Martin and Bhushan Shah --- .../hwcomposer/egl_hwcomposer_backend.cpp | 1 + backends/hwcomposer/hwcomposer_backend.cpp | 40 ++++++------------- backends/hwcomposer/hwcomposer_backend.h | 16 ++++---- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/backends/hwcomposer/egl_hwcomposer_backend.cpp b/backends/hwcomposer/egl_hwcomposer_backend.cpp index 8ea4314df1..52aa46fba1 100644 --- a/backends/hwcomposer/egl_hwcomposer_backend.cpp +++ b/backends/hwcomposer/egl_hwcomposer_backend.cpp @@ -36,6 +36,7 @@ EglHwcomposerBackend::EglHwcomposerBackend(HwcomposerBackend *backend) // EGL is always direct rendering setIsDirectRendering(true); setSyncsToVBlank(true); + setBlocksForRetrace(true); } EglHwcomposerBackend::~EglHwcomposerBackend() diff --git a/backends/hwcomposer/hwcomposer_backend.cpp b/backends/hwcomposer/hwcomposer_backend.cpp index f9b2cc8368..fa6fbb1f01 100644 --- a/backends/hwcomposer/hwcomposer_backend.cpp +++ b/backends/hwcomposer/hwcomposer_backend.cpp @@ -28,8 +28,6 @@ along with this program. If not, see . #include #include #include -// Qt -#include // hybris/android #include #include @@ -43,12 +41,8 @@ namespace KWin HwcomposerBackend::HwcomposerBackend(QObject *parent) : AbstractBackend(parent) - , m_vsyncFailSafeTimer(new QTimer(this)) { handleOutputs(); - m_vsyncFailSafeTimer->setSingleShot(true); - m_vsyncFailSafeTimer->setInterval(1000); - connect(m_vsyncFailSafeTimer, &QTimer::timeout, this, &HwcomposerBackend::vsync); } HwcomposerBackend::~HwcomposerBackend() @@ -127,9 +121,7 @@ void HwcomposerBackend::init() if (disp != 0) { return; } - QMetaObject::invokeMethod(dynamic_cast(waylandServer()->backend()), - "vsync", - Qt::QueuedConnection); + dynamic_cast(waylandServer()->backend())->wakeVSync(); }; procs->hotplug = [] (const struct hwc_procs* procs, int disp, int connected) { Q_UNUSED(procs) @@ -148,6 +140,9 @@ void HwcomposerBackend::init() } m_displaySize = output->pixelSize(); m_refreshRate = output->refreshRate(); + if (m_refreshRate != 0) { + m_vsyncInterval = 1000000/m_refreshRate; + } qCDebug(KWIN_HWCOMPOSER) << "Display size:" << m_displaySize; qCDebug(KWIN_HWCOMPOSER) << "Refresh rate:" << m_refreshRate; @@ -201,27 +196,18 @@ OpenGLBackend *HwcomposerBackend::createOpenGLBackend() return new EglHwcomposerBackend(this); } -void HwcomposerBackend::present() +void HwcomposerBackend::waitVSync() { - if (m_pageFlipPending) { - return; - } - m_pageFlipPending = true; - if (Compositor::self()) { - m_vsyncFailSafeTimer->start(); - Compositor::self()->aboutToSwapBuffers(); - } + m_vsyncMutex.lock(); + m_vsyncWaitCondition.wait(&m_vsyncMutex, m_vsyncInterval); + m_vsyncMutex.unlock(); } -void HwcomposerBackend::vsync() +void HwcomposerBackend::wakeVSync() { - m_vsyncFailSafeTimer->stop(); - if (m_pageFlipPending) { - m_pageFlipPending = false; - if (Compositor::self()) { - Compositor::self()->bufferSwapComplete(); - } - } + m_vsyncMutex.lock(); + m_vsyncWaitCondition.wakeAll(); + m_vsyncMutex.unlock(); } static void initLayer(hwc_layer_1_t *layer, const hwc_rect_t &rect) @@ -279,7 +265,7 @@ HwcomposerWindow::~HwcomposerWindow() void HwcomposerWindow::present(HWComposerNativeWindowBuffer *buffer) { - m_backend->present(); + m_backend->waitVSync(); hwc_composer_device_1_t *device = m_backend->device(); auto fblayer = &m_list[0]->hwLayers[1]; diff --git a/backends/hwcomposer/hwcomposer_backend.h b/backends/hwcomposer/hwcomposer_backend.h index 54b1bfa430..a6d85ea35f 100644 --- a/backends/hwcomposer/hwcomposer_backend.h +++ b/backends/hwcomposer/hwcomposer_backend.h @@ -21,6 +21,9 @@ along with this program. If not, see . #define KWIN_HWCOMPOSER_BACKEND_H #include "abstract_backend.h" +#include +#include + // libhybris #include // needed as hwcomposer_window.h includes EGL which on non-arm includes Xlib @@ -32,8 +35,6 @@ typedef struct hwc_composer_device_1 hwc_composer_device_1_t; class HWComposerNativeWindowBuffer; -class QTimer; - namespace KWin { @@ -61,14 +62,12 @@ public: hwc_composer_device_1_t *device() const { return m_device; } - void present(); int refreshRate() const { return m_refreshRate; } void enableVSync(bool enable); - -public Q_SLOTS: - void vsync(); + void waitVSync(); + void wakeVSync(); private Q_SLOTS: void toggleBlankOutput(); @@ -77,10 +76,11 @@ private: QSize m_displaySize; hwc_composer_device_1_t *m_device = nullptr; bool m_outputBlank = true; - bool m_pageFlipPending = false; int m_refreshRate = 60000; - QTimer *m_vsyncFailSafeTimer; + int m_vsyncInterval = 16; bool m_hasVsync = false; + QMutex m_vsyncMutex; + QWaitCondition m_vsyncWaitCondition; }; class HwcomposerWindow : public HWComposerNativeWindow