Differential D18570 Diff 51224 plugins/platforms/drm/drm_backend.cpp

Changeset View

Standalone View

View Options

plugins/platforms/drm/drm_backend.cpp

Show All 29 Lines
30#include "scene_qpainter_drm_backend.h" 30​#include "scene_qpainter_drm_backend.h"
31#include "screens_drm.h" 31​#include "screens_drm.h"
32#include "udev.h" 32​#include "udev.h"
33#include "wayland_server.h" 33​#include "wayland_server.h"
34#if HAVE_GBM 34​#if HAVE_GBM
35#include "egl_gbm_backend.h" 35​#include "egl_gbm_backend.h"
36#include <gbm.h> 36​#include <gbm.h>
37#endif 37​#endif
38​#if HAVE_EGL_STREAMS
39​#include "egl_stream_backend.h"
40​#endif
38// KWayland 41​// KWayland
39#include <KWayland/Server/seat_interface.h> 42​#include <KWayland/Server/seat_interface.h>
40#include <KWayland/Server/outputconfiguration_interface.h> 43​#include <KWayland/Server/outputconfiguration_interface.h>
41// KF5 44​// KF5
42#include <KConfigGroup> 45​#include <KConfigGroup>
43#include <KCoreAddons> 46​#include <KCoreAddons>
44#include <KLocalizedString> 47​#include <KLocalizedString>
45#include <KSharedConfig> 48​#include <KSharedConfig>
Show All 23 Lines
69{ 72​{
70 73​
71DrmBackend::DrmBackend(QObject *parent) 74​DrmBackend::DrmBackend(QObject *parent)
72 : Platform(parent) 75​ : Platform(parent)
73 , m_udev(new Udev) 76​ , m_udev(new Udev)
74 , m_udevMonitor(m_udev->monitor()) 77​ , m_udevMonitor(m_udev->monitor())
75 , m_dpmsFilter() 78​ , m_dpmsFilter()
76{ 79​{
80​#if HAVE_EGL_STREAMS
81​ if (qEnvironmentVariableIsSet("KWIN_DRM_USE_EGL_STREAMS")) {
82​ m_useEglStreams = true;
83​ }
84​#endif
77 setSupportsGammaControl(true); 85​ setSupportsGammaControl(true);
78 handleOutputs(); 86​ handleOutputs();
79} 87​}
80 88​
81DrmBackend::~DrmBackend() 89​DrmBackend::~DrmBackend()
82{ 90​{
83#if HAVE_GBM 91​#if HAVE_GBM
84 if (m_gbmDevice) { 92​ if (m_gbmDevice) {
▲ Show 20 Lines • Show All 160 Lines • ▼ Show 20 Line(s)
245void DrmBackend::openDrm() 253​void DrmBackend::openDrm()
246{ 254​{
247 connect(LogindIntegration::self(), &LogindIntegration::sessionActiveChanged, this, &DrmBackend::activate); 255​ connect(LogindIntegration::self(), &LogindIntegration::sessionActiveChanged, this, &DrmBackend::activate);
248 UdevDevice::Ptr device = m_udev->primaryGpu(); 256​ UdevDevice::Ptr device = m_udev->primaryGpu();
249 if (!device) { 257​ if (!device) {
250 qCWarning(KWIN_DRM) << "Did not find a GPU"; 258​ qCWarning(KWIN_DRM) << "Did not find a GPU";
251 return; 259​ return;
252 } 260​ }
253 int fd = LogindIntegration::self()->takeDevice(device->devNode()); 261​ m_devNode = device->devNode();
262​ int fd = LogindIntegration::self()->takeDevice(m_devNode.constData());
254 if (fd < 0) { 263​ if (fd < 0) {
255 qCWarning(KWIN_DRM) << "failed to open drm device at" << device->devNode(); 264​ qCWarning(KWIN_DRM) << "failed to open drm device at" << m_devNode;
256 return; 265​ return;
257 } 266​ }
258 m_fd = fd; 267​ m_fd = fd;
259 m_active = true; 268​ m_active = true;
260 QSocketNotifier *notifier = new QSocketNotifier(m_fd, QSocketNotifier::Read, this); 269​ QSocketNotifier *notifier = new QSocketNotifier(m_fd, QSocketNotifier::Read, this);
261 connect(notifier, &QSocketNotifier::activated, this, 270​ connect(notifier, &QSocketNotifier::activated, this,
262 [this] { 271​ [this] {
263 if (!LogindIntegration::self()->isActiveSession()) { 272​ if (!LogindIntegration::self()->isActiveSession()) {
264 return; 273​ return;
265 } 274​ }
266 drmEventContext e; 275​ drmEventContext e;
267 memset(&e, 0, sizeof e); 276​ memset(&e, 0, sizeof e);
268 e.version = KWIN_DRM_EVENT_CONTEXT_VERSION; 277​ e.version = KWIN_DRM_EVENT_CONTEXT_VERSION;
269 e.page_flip_handler = pageFlipHandler; 278​ e.page_flip_handler = pageFlipHandler;
270 drmHandleEvent(m_fd, &e); 279​ drmHandleEvent(m_fd, &e);
271 } 280​ }
272 ); 281​ );
273 m_drmId = device->sysNum(); 282​ m_drmId = device->sysNum();
274 283​
275 // trying to activate Atomic Mode Setting (this means also Universal Planes) 284​ // trying to activate Atomic Mode Setting (this means also Universal Planes)
276 if (!qEnvironmentVariableIsSet("KWIN_DRM_NO_AMS")) { 285​ // note: Atomic Mode Setting is not currently supported with EGLStream backend
286​ if (!qEnvironmentVariableIsSet("KWIN_DRM_NO_AMS")
287​#if HAVE_EGL_STREAMS
288​ && !m_useEglStreams
289​#endif
290​ ) {
277 if (drmSetClientCap(m_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) { 291​ if (drmSetClientCap(m_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) {
278 qCDebug(KWIN_DRM) << "Using Atomic Mode Setting."; 292​ qCDebug(KWIN_DRM) << "Using Atomic Mode Setting.";
279 m_atomicModeSetting = true; 293​ m_atomicModeSetting = true;
280 294​
281 ScopedDrmPointer<drmModePlaneRes, &drmModeFreePlaneResources> planeResources(drmModeGetPlaneResources(m_fd)); 295​ ScopedDrmPointer<drmModePlaneRes, &drmModeFreePlaneResources> planeResources(drmModeGetPlaneResources(m_fd));
282 if (!planeResources) { 296​ if (!planeResources) {
283 qCWarning(KWIN_DRM) << "Failed to get plane resources. Falling back to legacy mode"; 297​ qCWarning(KWIN_DRM) << "Failed to get plane resources. Falling back to legacy mode";
284 m_atomicModeSetting = false; 298​ m_atomicModeSetting = false;
Show All 21 Lines318 if (m_planes.isEmpty()) {
306 m_atomicModeSetting = false; 320​ m_atomicModeSetting = false;
307 } 321​ }
308 } 322​ }
309 } else { 323​ } else {
310 qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode."; 324​ qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode.";
311 } 325​ }
312 } 326​ }
313 327​
314 ScopedDrmPointer<_drmModeRes, &drmModeFreeResources> resources(drmModeGetResources(m_fd)); 328​ ScopedDrmPointer<_drmModeRes, &drmModeFreeResources> resources(drmModeGetResources(m_fd));
davidedmundsonUnsubmitted
Not Done

I know this isn't related to your patch, and I don't want to take it off-topic, but when I run on my nvidia machine I get a null value from drmResources.

I get the same failure with a simple self-contained libDRM test.

I have a valid FD from logind.
/dev/card0 exists, drmGetVersion works fine.

I'm running Nvidia driver version 415.27

Any ideas?

davidedmundson: I know this isn't related to your patch, and I don't want to take it off-topic, but when I run…
ekurzingerAuthorUnsubmitted
Not Done

Hmm, interesting. Could you confirm that the nvidia_drm module is loaded with the option "modeset=1"? We currently don't enable the DRM driver by default (although that will probably change eventually).

# cat /sys/module/nvidia_drm/parameters/modeset

should print "Y"

ekurzinger: Hmm, interesting. Could you confirm that the nvidia_drm module is loaded with the option…
315 drmModeRes *res = resources.data(); 329​ drmModeRes *res = resources.data();
316 if (!resources) { 330​ if (!resources) {
317 qCWarning(KWIN_DRM) << "drmModeGetResources failed"; 331​ qCWarning(KWIN_DRM) << "drmModeGetResources failed";
318 return; 332​ return;
319 } 333​ }
320 334​
321 for (int i = 0; i < res->count_connectors; ++i) { 335​ for (int i = 0; i < res->count_connectors; ++i) {
322 m_connectors << new DrmConnector(res->connectors[i], m_fd); 336​ m_connectors << new DrmConnector(res->connectors[i], m_fd);
▲ Show 20 Lines • Show All 269 Lines • ▼ Show 20 Line(s)605 if (it != m_outputs.constEnd()) {
592 return *it; 606​ return *it;
593 } 607​ }
594 return nullptr; 608​ return nullptr;
595} 609​}
596 610​
597DrmOutput *DrmBackend::findOutput(const QByteArray &uuid) 611​DrmOutput *DrmBackend::findOutput(const QByteArray &uuid)
598{ 612​{
599 auto it = std::find_if(m_outputs.constBegin(), m_outputs.constEnd(), [uuid] (DrmOutput *o) { 613​ auto it = std::find_if(m_outputs.constBegin(), m_outputs.constEnd(), [uuid] (DrmOutput *o) {
600 return o->m_uuid == uuid; 614​ return o->m_uuid == uuid;
zzagUnsubmitted
Done

Functions have opening braces on the start of a line.

zzag: Functions have opening braces on the start of a line.
601 }); 615​ });
602 if (it != m_outputs.constEnd()) { 616​ if (it != m_outputs.constEnd()) {
603 return *it; 617​ return *it;
604 } 618​ }
605 return nullptr; 619​ return nullptr;
606} 620​}
607 621​
622​void DrmBackend::queuePageFlip()
623​{
624​ ++m_pageFlipsPending;
625​ if (m_pageFlipsPending == 1 && Compositor::self()) {
626​ Compositor::self()->aboutToSwapBuffers();
627​ }
628​}
629​
608void DrmBackend::present(DrmBuffer *buffer, DrmOutput *output) 630​void DrmBackend::present(DrmBuffer *buffer, DrmOutput *output)
609{ 631​{
610 if (!buffer || buffer->bufferId() == 0) { 632​ if (!buffer || buffer->bufferId() == 0) {
611 if (m_deleteBufferAfterPageFlip) { 633​ if (m_deleteBufferAfterPageFlip) {
612 delete buffer; 634​ delete buffer;
613 } 635​ }
614 return; 636​ return;
615 } 637​ }
616 638​
617 if (output->present(buffer)) { 639​ if (output->present(buffer)) {
618 m_pageFlipsPending++; 640​ queuePageFlip();
619 if (m_pageFlipsPending == 1 && Compositor::self()) { ​
620 Compositor::self()->aboutToSwapBuffers(); ​
621 } ​
622 } else if (m_deleteBufferAfterPageFlip) { 641​ } else if (m_deleteBufferAfterPageFlip) {
623 delete buffer; 642​ delete buffer;
624 } 643​ }
625} 644​}
626 645​
627void DrmBackend::initCursor() 646​void DrmBackend::initCursor()
628{ 647​{
648​#if HAVE_EGL_STREAMS
649​ // Hardware cursors aren't currently supported with EGLStream backend
650​ if (m_useEglStreams) {
cfeckUnsubmitted
Not Done

A was searching for the definition of this function, because it is named wrongly (should be setSoftwareCursor), but it seems to be a mistake in the base class. If it's private API, it might be worth to rename it.

cfeck: A was searching for the definition of this function, because it is named wrongly (should be…
romanggUnsubmitted
Not Done

Not in this diff though. Same holds for other suggestions below in this file, which are not directly related to this diff.

romangg: Not in this diff though. Same holds for other suggestions below in this file, which are not…
ekurzingerAuthorUnsubmitted
Not Done

yeah, while I agree with these suggestions I think it would be best to limit this commit to code related to the new backend as opposed to general cleanup. I'll address the style issues you noticed in egl_stream_backend.cpp, though (thanks for catching those).

ekurzinger: yeah, while I agree with these suggestions I think it would be best to limit this commit to…
651​ setSoftWareCursor(true);
652​ }
653​#endif
654​
629 m_cursorEnabled = waylandServer()->seat()->hasPointer(); 655​ m_cursorEnabled = waylandServer()->seat()->hasPointer();
630 connect(waylandServer()->seat(), &KWayland::Server::SeatInterface::hasPointerChanged, this, 656​ connect(waylandServer()->seat(), &KWayland::Server::SeatInterface::hasPointerChanged, this,
631 [this] { 657​ [this] {
632 m_cursorEnabled = waylandServer()->seat()->hasPointer(); 658​ m_cursorEnabled = waylandServer()->seat()->hasPointer();
633 if (usesSoftwareCursor()) { 659​ if (usesSoftwareCursor()) {
634 return; 660​ return;
635 } 661​ }
636 for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { 662​ for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) {
Show All 40 Lines
677void DrmBackend::updateCursor() 703​void DrmBackend::updateCursor()
678{ 704​{
679 if (usesSoftwareCursor()) { 705​ if (usesSoftwareCursor()) {
680 return; 706​ return;
681 } 707​ }
682 if (isCursorHidden()) { 708​ if (isCursorHidden()) {
683 return; 709​ return;
684 } 710​ }
685 const QImage &cursorImage = softwareCursor(); 711​ const QImage &cursorImage = softwareCursor();
cfeckUnsubmitted
Not Done

For implicitely shared Qt classes, it might be dangerous to use a reference, because that won't increase the shared-ref counter. Please just omit the &.

cfeck: For implicitely shared Qt classes, it might be dangerous to use a reference, because that won't…
686 if (cursorImage.isNull()) { 712​ if (cursorImage.isNull()) {
687 doHideCursor(); 713​ doHideCursor();
688 return; 714​ return;
689 } 715​ }
690 for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { 716​ for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) {
cfeckUnsubmitted
Not Done

If for (const &output : m_outputs) also works here (and at other places), you could improve readability.

cfeck: If `for (const &output : m_outputs)` also works here (and at other places), you could improve…
691 (*it)->updateCursor(); 717​ (*it)->updateCursor();
692 } 718​ }
693 719​
694 setCursor(); 720​ setCursor();
695 moveCursor(); 721​ moveCursor();
696} 722​}
697 723​
698void DrmBackend::doShowCursor() 724​void DrmBackend::doShowCursor()
Show All 18 Lines740{
717 } 743​ }
718 for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { 744​ for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) {
719 (*it)->moveCursor(Cursor::pos()); 745​ (*it)->moveCursor(Cursor::pos());
720 } 746​ }
721} 747​}
722 748​
723Screens *DrmBackend::createScreens(QObject *parent) 749​Screens *DrmBackend::createScreens(QObject *parent)
724{ 750​{
725 return new DrmScreens(this, parent); 751​ return new DrmScreens(this, parent);
zzagUnsubmitted
Not Done

It would be great if EGLStreams were optional.

zzag: It would be great if EGLStreams were optional.
graesslinUnsubmitted
Not Done

Wouldn't it make more sense to try the available backends? E.g first create EglGbmBackend, if that fails create EglStreamBackend - comparable to how we load the compositor.

graesslin: Wouldn't it make more sense to try the available backends? E.g first create EglGbmBackend, if…
ekurzingerAuthorUnsubmitted
Not Done

If Mesa is available, creating an EglGbmBackend should always succeed, I believe. On systems running the NVIDIA driver it will just fall back to using llvmpipe-based software rendering (of course, with abysmal performance).

We could attempt to create the EglStreamBackend first, which should fail on any non-NVIDIA systems, and then fall back to GBM (and then QPainter). Alternatively, I suppose we would need some way to detect if the NVIDIA driver is present. There are probably a few ways to do this but they might be a little hacky.

And re: zzag I can totally make the EGLStream stuff compile-time optional if that would be preferred, I'll work on an amended version of the change and get it up shortly.

ekurzinger: If Mesa is available, creating an EglGbmBackend should always succeed, I believe. On systems…
726} 752​}
727 753​
728QPainterBackend *DrmBackend::createQPainterBackend() 754​QPainterBackend *DrmBackend::createQPainterBackend()
729{ 755​{
730 m_deleteBufferAfterPageFlip = false; 756​ m_deleteBufferAfterPageFlip = false;
731 return new DrmQPainterBackend(this); 757​ return new DrmQPainterBackend(this);
732} 758​}
733 759​
734OpenGLBackend *DrmBackend::createOpenGLBackend() 760​OpenGLBackend *DrmBackend::createOpenGLBackend()
735{ 761​{
762​#if HAVE_EGL_STREAMS
763​ if (m_useEglStreams) {
764​ m_deleteBufferAfterPageFlip = false;
765​ return new EglStreamBackend(this);
766​ } else
767​#endif
768​ {
romanggUnsubmitted
Done

else keyword and curly braces are unnecessary.

romangg: `else` keyword and curly braces are unnecessary.
736#if HAVE_GBM 769​#if HAVE_GBM
737 m_deleteBufferAfterPageFlip = true; 770​ m_deleteBufferAfterPageFlip = true;
738 return new EglGbmBackend(this); 771​ return new EglGbmBackend(this);
739#else 772​#else
740 return Platform::createOpenGLBackend(); 773​ return Platform::createOpenGLBackend();
741#endif 774​#endif
742} 775​ }
776​}
743 777​
744DrmDumbBuffer *DrmBackend::createBuffer(const QSize &size) 778​DrmDumbBuffer *DrmBackend::createBuffer(const QSize &size)
745{ 779​{
746 DrmDumbBuffer *b = new DrmDumbBuffer(m_fd, size); 780​ DrmDumbBuffer *b = new DrmDumbBuffer(m_fd, size);
747 return b; 781​ return b;
748} 782​}
749 783​
750#if HAVE_GBM 784​#if HAVE_GBM
Show All 13 Lines793{
764 for (auto it = m_enabledOutputs.constBegin(); it != m_enabledOutputs.constEnd(); ++it) { 798​ for (auto it = m_enabledOutputs.constBegin(); it != m_enabledOutputs.constEnd(); ++it) {
765 enabled = enabled || (*it)->isDpmsEnabled(); 799​ enabled = enabled || (*it)->isDpmsEnabled();
766 } 800​ }
767 setOutputsEnabled(enabled); 801​ setOutputsEnabled(enabled);
768} 802​}
769 803​
770QVector<CompositingType> DrmBackend::supportedCompositors() const 804​QVector<CompositingType> DrmBackend::supportedCompositors() const
771{ 805​{
772#if HAVE_GBM 806​#if HAVE_GBM || HAVE_EGL_STREAMS
773 return QVector<CompositingType>{OpenGLCompositing, QPainterCompositing}; 807​ return QVector<CompositingType>{OpenGLCompositing, QPainterCompositing};
romanggUnsubmitted
Done

In case of HAVE_EGL_STREAMS not if m_useEglStreams is false.

romangg: In case of `HAVE_EGL_STREAMS` not if `m_useEglStreams` is false.
774#else 808​#else
775 return QVector<CompositingType>{QPainterCompositing}; 809​ return QVector<CompositingType>{QPainterCompositing};
776#endif 810​#endif
777} 811​}
778 812​
779QString DrmBackend::supportInformation() const 813​QString DrmBackend::supportInformation() const
780{ 814​{
781 QString supportInfo; 815​ QString supportInfo;
782 QDebug s(&supportInfo); 816​ QDebug s(&supportInfo);
783 s.nospace(); 817​ s.nospace();
784 s << "Name: " << "DRM" << endl; 818​ s << "Name: " << "DRM" << endl;
785 s << "Active: " << m_active << endl; 819​ s << "Active: " << m_active << endl;
786 s << "Atomic Mode Setting: " << m_atomicModeSetting << endl; 820​ s << "Atomic Mode Setting: " << m_atomicModeSetting << endl;
821​#if HAVE_EGL_STREAMS
822​ s << "Using EGL Streams: " << m_useEglStreams << endl;
823​#endif
787 return supportInfo; 824​ return supportInfo;
788} 825​}
789 826​
790} 827​}