From a8055e454623be8c25bdd304940b15731ec2c6cf Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Sat, 20 Mar 2021 19:18:52 +0100 Subject: [PATCH] Refactor DRM presentation Presentation doesn't have to go through DrmBackend and by moving DrmGpu::deleteBufferAfterPageflip into DrmBuffer some code can be simplified --- src/plugins/platforms/drm/drm_backend.cpp | 18 ----- src/plugins/platforms/drm/drm_backend.h | 2 - src/plugins/platforms/drm/drm_buffer.h | 3 + src/plugins/platforms/drm/drm_buffer_gbm.h | 3 + src/plugins/platforms/drm/drm_gpu.cpp | 2 - src/plugins/platforms/drm/drm_gpu.h | 9 --- src/plugins/platforms/drm/drm_object_crtc.cpp | 4 +- .../platforms/drm/drm_object_plane.cpp | 11 +--- src/plugins/platforms/drm/drm_object_plane.h | 1 - src/plugins/platforms/drm/drm_output.cpp | 65 +++++++++---------- src/plugins/platforms/drm/egl_gbm_backend.cpp | 2 +- .../platforms/drm/egl_stream_backend.cpp | 2 +- .../drm/scene_qpainter_drm_backend.cpp | 2 +- 13 files changed, 44 insertions(+), 80 deletions(-) diff --git a/src/plugins/platforms/drm/drm_backend.cpp b/src/plugins/platforms/drm/drm_backend.cpp index 1cd61f81c0..3f764c73ae 100644 --- a/src/plugins/platforms/drm/drm_backend.cpp +++ b/src/plugins/platforms/drm/drm_backend.cpp @@ -516,23 +516,6 @@ DrmOutput *DrmBackend::findOutput(quint32 connector) return nullptr; } -bool DrmBackend::present(DrmBuffer *buffer, DrmOutput *output) -{ - if (!buffer || buffer->bufferId() == 0) { - if (output->gpu()->deleteBufferAfterPageFlip()) { - delete buffer; - } - return false; - } - - if (output->present(buffer)) { - return true; - } else if (output->gpu()->deleteBufferAfterPageFlip()) { - delete buffer; - } - return false; -} - void DrmBackend::initCursor() { @@ -640,7 +623,6 @@ void DrmBackend::moveCursor() QPainterBackend *DrmBackend::createQPainterBackend() { - m_gpus.at(0)->setDeleteBufferAfterPageFlip(false); return new DrmQPainterBackend(this, m_gpus.at(0)); } diff --git a/src/plugins/platforms/drm/drm_backend.h b/src/plugins/platforms/drm/drm_backend.h index d513c19687..24d20c0aa8 100644 --- a/src/plugins/platforms/drm/drm_backend.h +++ b/src/plugins/platforms/drm/drm_backend.h @@ -61,8 +61,6 @@ public: void init() override; void prepareShutdown() override; - bool present(DrmBuffer *buffer, DrmOutput *output); - Outputs outputs() const override; Outputs enabledOutputs() const override; QVector drmOutputs() const { diff --git a/src/plugins/platforms/drm/drm_buffer.h b/src/plugins/platforms/drm/drm_buffer.h index c7f524d0ca..5db8c2f4bb 100644 --- a/src/plugins/platforms/drm/drm_buffer.h +++ b/src/plugins/platforms/drm/drm_buffer.h @@ -23,6 +23,9 @@ public: virtual ~DrmBuffer() = default; virtual bool needsModeChange(DrmBuffer *b) const {Q_UNUSED(b) return false;} + virtual bool shouldDeleteAfterPageflip() const { + return false; + } quint32 bufferId() const { return m_bufferId; diff --git a/src/plugins/platforms/drm/drm_buffer_gbm.h b/src/plugins/platforms/drm/drm_buffer_gbm.h index 99c7a738d0..7a70d4bed9 100644 --- a/src/plugins/platforms/drm/drm_buffer_gbm.h +++ b/src/plugins/platforms/drm/drm_buffer_gbm.h @@ -40,6 +40,9 @@ public: return true; } } + bool shouldDeleteAfterPageflip() const override { + return true; + } bool hasBo() const { return m_bo != nullptr; diff --git a/src/plugins/platforms/drm/drm_gpu.cpp b/src/plugins/platforms/drm/drm_gpu.cpp index 47cdf4303c..ab6019e182 100644 --- a/src/plugins/platforms/drm/drm_gpu.cpp +++ b/src/plugins/platforms/drm/drm_gpu.cpp @@ -62,8 +62,6 @@ DrmGpu::DrmGpu(DrmBackend *backend, QByteArray devNode, int fd, int drmId) : m_b DrmScopedPointer version(drmGetVersion(fd)); m_useEglStreams = strstr(version->name, "nvidia-drm"); - m_deleteBufferAfterPageFlip = !m_useEglStreams; - m_socketNotifier = new QSocketNotifier(fd, QSocketNotifier::Read, this); connect(m_socketNotifier, &QSocketNotifier::activated, this, &DrmGpu::dispatchEvents); } diff --git a/src/plugins/platforms/drm/drm_gpu.h b/src/plugins/platforms/drm/drm_gpu.h index 6f79252510..9f32133f16 100644 --- a/src/plugins/platforms/drm/drm_gpu.h +++ b/src/plugins/platforms/drm/drm_gpu.h @@ -58,10 +58,6 @@ public: return m_useEglStreams; } - bool deleteBufferAfterPageFlip() const { - return m_deleteBufferAfterPageFlip; - } - QByteArray devNode() const { return m_devNode; } @@ -86,10 +82,6 @@ public: m_eglDisplay = display; } - void setDeleteBufferAfterPageFlip(bool deleteBuffer) { - m_deleteBufferAfterPageFlip = deleteBuffer; - } - DrmDumbBuffer *createBuffer(const QSize &size) const { return new DrmDumbBuffer(m_fd, size); } @@ -132,7 +124,6 @@ private: const int m_drmId; bool m_atomicModeSetting; bool m_useEglStreams; - bool m_deleteBufferAfterPageFlip; gbm_device* m_gbmDevice; EGLDisplay m_eglDisplay = EGL_NO_DISPLAY; clockid_t m_presentationClock; diff --git a/src/plugins/platforms/drm/drm_object_crtc.cpp b/src/plugins/platforms/drm/drm_object_crtc.cpp index 08126c3d45..80ea75944e 100644 --- a/src/plugins/platforms/drm/drm_object_crtc.cpp +++ b/src/plugins/platforms/drm/drm_object_crtc.cpp @@ -67,7 +67,7 @@ bool DrmCrtc::initProps() void DrmCrtc::flipBuffer() { - if (m_currentBuffer && m_gpu->deleteBufferAfterPageFlip() && m_currentBuffer != m_nextBuffer) { + if (m_currentBuffer && m_currentBuffer->shouldDeleteAfterPageflip() && m_currentBuffer != m_nextBuffer) { delete m_currentBuffer; } m_currentBuffer = m_nextBuffer; @@ -94,7 +94,7 @@ bool DrmCrtc::blank(DrmOutput *output) } if (output->setModeLegacy(m_blackBuffer)) { - if (m_currentBuffer && m_gpu->deleteBufferAfterPageFlip()) { + if (m_currentBuffer && m_currentBuffer->shouldDeleteAfterPageflip()) { delete m_currentBuffer; delete m_nextBuffer; } diff --git a/src/plugins/platforms/drm/drm_object_plane.cpp b/src/plugins/platforms/drm/drm_object_plane.cpp index 3df8955d92..a23c90fa72 100644 --- a/src/plugins/platforms/drm/drm_object_plane.cpp +++ b/src/plugins/platforms/drm/drm_object_plane.cpp @@ -164,16 +164,11 @@ DrmPlane::Transformations DrmPlane::transformation() void DrmPlane::flipBuffer() { + if (m_current != m_next && m_current && m_current->shouldDeleteAfterPageflip()) { + delete m_current; + } m_current = m_next; m_next = nullptr; } -void DrmPlane::flipBufferWithDelete() -{ - if (m_current != m_next) { - delete m_current; - } - flipBuffer(); -} - } diff --git a/src/plugins/platforms/drm/drm_object_plane.h b/src/plugins/platforms/drm/drm_object_plane.h index 43acce2aa4..109efc140d 100644 --- a/src/plugins/platforms/drm/drm_object_plane.h +++ b/src/plugins/platforms/drm/drm_object_plane.h @@ -87,7 +87,6 @@ public: Transformations transformation(); void flipBuffer(); - void flipBufferWithDelete(); Transformations supportedTransformations() const { return m_supportedTransformations; diff --git a/src/plugins/platforms/drm/drm_output.cpp b/src/plugins/platforms/drm/drm_output.cpp index 94c6352b57..1fef950819 100644 --- a/src/plugins/platforms/drm/drm_output.cpp +++ b/src/plugins/platforms/drm/drm_output.cpp @@ -70,7 +70,7 @@ void DrmOutput::teardown() if (m_primaryPlane) { // TODO: when having multiple planes, also clean up these - if (m_gpu->deleteBufferAfterPageFlip()) { + if (m_primaryPlane->current() && m_primaryPlane->current()->shouldDeleteAfterPageflip()) { delete m_primaryPlane->current(); } m_primaryPlane->setCurrent(nullptr); @@ -670,41 +670,23 @@ void DrmOutput::pageFlipped() if (!m_crtc) { return; } - // Egl based surface buffers get destroyed, QPainter based dumb buffers not - // TODO: split up DrmOutput in two for dumb and egl/gbm surface buffer compatible subclasses completely? - if (m_gpu->deleteBufferAfterPageFlip()) { - if (m_gpu->atomicModeSetting()) { - if (!m_primaryPlane->next()) { - // on manual vt switch - // TODO: when we later use overlay planes it might happen, that we have a page flip with only - // damage on one of these, and therefore the primary plane has no next buffer - // -> Then we don't want to return here! - if (m_primaryPlane->current()) { - m_primaryPlane->current()->releaseGbm(); - } - return; + if (m_gpu->atomicModeSetting()) { + if (!m_primaryPlane->next()) { + if (m_primaryPlane->current()) { + m_primaryPlane->current()->releaseGbm(); } - for (DrmPlane *p : m_nextPlanesFlipList) { - p->flipBufferWithDelete(); - } - m_nextPlanesFlipList.clear(); - } else { - if (!m_crtc->next()) { - // on manual vt switch - if (DrmBuffer *b = m_crtc->current()) { - b->releaseGbm(); - } - } - m_crtc->flipBuffer(); + return; } + for (DrmPlane *p : m_nextPlanesFlipList) { + p->flipBuffer(); + } + m_nextPlanesFlipList.clear(); } else { - if (m_gpu->atomicModeSetting()){ - for (DrmPlane *p : m_nextPlanesFlipList) { - p->flipBuffer(); + if (!m_crtc->next()) { + // on manual vt switch + if (DrmBuffer *b = m_crtc->current()) { + b->releaseGbm(); } - m_nextPlanesFlipList.clear(); - } else { - m_crtc->flipBuffer(); } m_crtc->flipBuffer(); } @@ -716,14 +698,25 @@ void DrmOutput::pageFlipped() bool DrmOutput::present(DrmBuffer *buffer) { + if (!buffer || buffer->bufferId() == 0) { + if (buffer && buffer->shouldDeleteAfterPageflip()) { + delete buffer; + } + return false; + } if (m_dpmsModePending != DpmsMode::On) { return false; } + bool result; if (m_gpu->atomicModeSetting()) { - return presentAtomically(buffer); + result = presentAtomically(buffer); } else { - return presentLegacy(buffer); + result = presentLegacy(buffer); } + if (!result && buffer->shouldDeleteAfterPageflip()) { + delete buffer; + } + return result; } bool DrmOutput::dpmsAtomicOff() @@ -976,8 +969,10 @@ bool DrmOutput::atomicReqModesetPopulate(drmModeAtomicReq *req, bool enable) m_primaryPlane->setValue(int(DrmPlane::PropertyIndex::CrtcH), targetRect.height()); m_primaryPlane->setValue(int(DrmPlane::PropertyIndex::CrtcId), m_crtc->id()); } else { - if (m_gpu->deleteBufferAfterPageFlip()) { + if (m_primaryPlane->current() && m_primaryPlane->current()->shouldDeleteAfterPageflip()) { delete m_primaryPlane->current(); + } + if (m_primaryPlane->next() && m_primaryPlane->next()->shouldDeleteAfterPageflip()) { delete m_primaryPlane->next(); } m_primaryPlane->setCurrent(nullptr); diff --git a/src/plugins/platforms/drm/egl_gbm_backend.cpp b/src/plugins/platforms/drm/egl_gbm_backend.cpp index 59983279cf..980ebe2f45 100644 --- a/src/plugins/platforms/drm/egl_gbm_backend.cpp +++ b/src/plugins/platforms/drm/egl_gbm_backend.cpp @@ -602,7 +602,7 @@ bool EglGbmBackend::presentOnOutput(Output &output, const QRegion &damagedRegion } Q_EMIT output.output->outputChange(damagedRegion); - if (!m_backend->present(output.buffer, output.output)) { + if (!output.output->present(output.buffer)) { return false; } diff --git a/src/plugins/platforms/drm/egl_stream_backend.cpp b/src/plugins/platforms/drm/egl_stream_backend.cpp index f04fcfe608..76c520df22 100644 --- a/src/plugins/platforms/drm/egl_stream_backend.cpp +++ b/src/plugins/platforms/drm/egl_stream_backend.cpp @@ -446,7 +446,7 @@ bool EglStreamBackend::presentOnOutput(EglStreamBackend::Output &o) return false; } - return m_backend->present(o.buffer, o.output); + return o.output->present(o.buffer); } SceneOpenGLTexturePrivate *EglStreamBackend::createBackendTexture(SceneOpenGLTexture *texture) diff --git a/src/plugins/platforms/drm/scene_qpainter_drm_backend.cpp b/src/plugins/platforms/drm/scene_qpainter_drm_backend.cpp index 500e10329b..295e746de6 100644 --- a/src/plugins/platforms/drm/scene_qpainter_drm_backend.cpp +++ b/src/plugins/platforms/drm/scene_qpainter_drm_backend.cpp @@ -114,7 +114,7 @@ void DrmQPainterBackend::endFrame(int screenId, int mask, const QRegion &damage) const Output &rendererOutput = m_outputs[screenId]; DrmOutput *drmOutput = rendererOutput.output; - if (!m_backend->present(rendererOutput.buffer[rendererOutput.index], drmOutput)) { + if (!drmOutput->present(rendererOutput.buffer[rendererOutput.index])) { RenderLoopPrivate *renderLoopPrivate = RenderLoopPrivate::get(drmOutput->renderLoop()); renderLoopPrivate->notifyFrameFailed(); }