Differential D18570 Diff 56300 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()) {
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Line(s)313 if (m_planes.isEmpty()) {
306 m_atomicModeSetting = false; 315​ m_atomicModeSetting = false;
307 } 316​ }
308 } 317​ }
309 } else { 318​ } else {
310 qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode."; 319​ qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode.";
311 } 320​ }
312 } 321​ }
313 322​
314 DrmScopedPointer<drmModeRes> resources(drmModeGetResources(m_fd)); 323​ DrmScopedPointer<drmModeRes> 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(); 324​ drmModeRes *res = resources.data();
316 if (!resources) { 325​ if (!resources) {
317 qCWarning(KWIN_DRM) << "drmModeGetResources failed"; 326​ qCWarning(KWIN_DRM) << "drmModeGetResources failed";
318 return; 327​ return;
319 } 328​ }
320 329​
321 for (int i = 0; i < res->count_connectors; ++i) { 330​ for (int i = 0; i < res->count_connectors; ++i) {
322 m_connectors << new DrmConnector(res->connectors[i], m_fd); 331​ m_connectors << new DrmConnector(res->connectors[i], m_fd);
▲ Show 20 Lines • Show All 274 Lines • ▼ Show 20 Line(s)
597DrmOutput *DrmBackend::findOutput(const QByteArray &uuid) 606​DrmOutput *DrmBackend::findOutput(const QByteArray &uuid)
598{ 607​{
599 auto it = std::find_if(m_outputs.constBegin(), m_outputs.constEnd(), [uuid] (DrmOutput *o) { 608​ auto it = std::find_if(m_outputs.constBegin(), m_outputs.constEnd(), [uuid] (DrmOutput *o) {
600 return o->m_uuid == uuid; 609​ return o->m_uuid == uuid;
601 }); 610​ });
602 if (it != m_outputs.constEnd()) { 611​ if (it != m_outputs.constEnd()) {
603 return *it; 612​ return *it;
604 } 613​ }
605 return nullptr; 614​ return nullptr;
zzagUnsubmitted
Done

Functions have opening braces on the start of a line.

zzag: Functions have opening braces on the start of a line.
606} 615​}
607 616​
608void DrmBackend::present(DrmBuffer *buffer, DrmOutput *output) 617​bool DrmBackend::present(DrmBuffer *buffer, DrmOutput *output)
609{ 618​{
610 if (!buffer || buffer->bufferId() == 0) { 619​ if (!buffer || buffer->bufferId() == 0) {
611 if (m_deleteBufferAfterPageFlip) { 620​ if (m_deleteBufferAfterPageFlip) {
612 delete buffer; 621​ delete buffer;
613 } 622​ }
614 return; 623​ return false;
615 } 624​ }
616 625​
617 if (output->present(buffer)) { 626​ if (output->present(buffer)) {
618 m_pageFlipsPending++; 627​ m_pageFlipsPending++;
619 if (m_pageFlipsPending == 1 && Compositor::self()) { 628​ if (m_pageFlipsPending == 1 && Compositor::self()) {
620 Compositor::self()->aboutToSwapBuffers(); 629​ Compositor::self()->aboutToSwapBuffers();
621 } 630​ }
631​ return true;
622 } else if (m_deleteBufferAfterPageFlip) { 632​ } else if (m_deleteBufferAfterPageFlip) {
623 delete buffer; 633​ delete buffer;
624 } 634​ }
635​ return false;
625} 636​}
626 637​
627void DrmBackend::initCursor() 638​void DrmBackend::initCursor()
628{ 639​{
640​
641​#if HAVE_EGL_STREAMS
642​ // Hardware cursors aren't currently supported with EGLStream backend,
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…
643​ // possibly an NVIDIA driver bug
644​ if (m_useEglStreams) {
645​ setSoftWareCursor(true);
646​ }
647​#endif
648​
629 m_cursorEnabled = waylandServer()->seat()->hasPointer(); 649​ m_cursorEnabled = waylandServer()->seat()->hasPointer();
630 connect(waylandServer()->seat(), &KWayland::Server::SeatInterface::hasPointerChanged, this, 650​ connect(waylandServer()->seat(), &KWayland::Server::SeatInterface::hasPointerChanged, this,
631 [this] { 651​ [this] {
632 m_cursorEnabled = waylandServer()->seat()->hasPointer(); 652​ m_cursorEnabled = waylandServer()->seat()->hasPointer();
633 if (usesSoftwareCursor()) { 653​ if (usesSoftwareCursor()) {
634 return; 654​ return;
635 } 655​ }
636 for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { 656​ for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) {
Show All 40 Lines
677void DrmBackend::updateCursor() 697​void DrmBackend::updateCursor()
678{ 698​{
679 if (usesSoftwareCursor()) { 699​ if (usesSoftwareCursor()) {
680 return; 700​ return;
681 } 701​ }
682 if (isCursorHidden()) { 702​ if (isCursorHidden()) {
683 return; 703​ return;
684 } 704​ }
685 const QImage &cursorImage = softwareCursor(); 705​ 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()) { 706​ if (cursorImage.isNull()) {
687 doHideCursor(); 707​ doHideCursor();
688 return; 708​ return;
689 } 709​ }
690 for (auto it = m_outputs.constBegin(); it != m_outputs.constEnd(); ++it) { 710​ 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(); 711​ (*it)->updateCursor();
692 } 712​ }
693 713​
694 setCursor(); 714​ setCursor();
695 moveCursor(); 715​ moveCursor();
696} 716​}
697 717​
698void DrmBackend::doShowCursor() 718​void DrmBackend::doShowCursor()
Show All 24 Lines
723Screens *DrmBackend::createScreens(QObject *parent) 743​Screens *DrmBackend::createScreens(QObject *parent)
724{ 744​{
725 return new DrmScreens(this, parent); 745​ return new DrmScreens(this, parent);
726} 746​}
727 747​
728QPainterBackend *DrmBackend::createQPainterBackend() 748​QPainterBackend *DrmBackend::createQPainterBackend()
729{ 749​{
730 m_deleteBufferAfterPageFlip = false; 750​ m_deleteBufferAfterPageFlip = false;
731 return new DrmQPainterBackend(this); 751​ return new DrmQPainterBackend(this);
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…
732} 752​}
733 753​
734OpenGLBackend *DrmBackend::createOpenGLBackend() 754​OpenGLBackend *DrmBackend::createOpenGLBackend()
735{ 755​{
756​#if HAVE_EGL_STREAMS
757​ if (m_useEglStreams) {
758​ m_deleteBufferAfterPageFlip = false;
759​ return new EglStreamBackend(this);
760​ }
761​#endif
762​
romanggUnsubmitted
Done

else keyword and curly braces are unnecessary.

romangg: `else` keyword and curly braces are unnecessary.
736#if HAVE_GBM 763​#if HAVE_GBM
737 m_deleteBufferAfterPageFlip = true; 764​ m_deleteBufferAfterPageFlip = true;
738 return new EglGbmBackend(this); 765​ return new EglGbmBackend(this);
739#else 766​#else
740 return Platform::createOpenGLBackend(); 767​ return Platform::createOpenGLBackend();
741#endif 768​#endif
742} 769​}
743 770​
Show All 24 Lines
768} 795​}
769 796​
770QVector<CompositingType> DrmBackend::supportedCompositors() const 797​QVector<CompositingType> DrmBackend::supportedCompositors() const
771{ 798​{
772 if (selectedCompositor() != NoCompositing) { 799​ if (selectedCompositor() != NoCompositing) {
773 return {selectedCompositor()}; 800​ return {selectedCompositor()};
774 } 801​ }
775#if HAVE_GBM 802​#if HAVE_GBM
776 return QVector<CompositingType>{OpenGLCompositing, QPainterCompositing}; 803​ return QVector<CompositingType>{OpenGLCompositing, QPainterCompositing};
804​#elif HAVE_EGL_STREAMS
805​ return m_useEglStreams ?
806​ QVector<CompositingType>{OpenGLCompositing, QPainterCompositing} :
807​ QVector<CompositingType>{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.
777#else 808​#else
778 return QVector<CompositingType>{QPainterCompositing}; 809​ return QVector<CompositingType>{QPainterCompositing};
779#endif 810​#endif
780} 811​}
781 812​
782QString DrmBackend::supportInformation() const 813​QString DrmBackend::supportInformation() const
783{ 814​{
784 QString supportInfo; 815​ QString supportInfo;
785 QDebug s(&supportInfo); 816​ QDebug s(&supportInfo);
786 s.nospace(); 817​ s.nospace();
787 s << "Name: " << "DRM" << endl; 818​ s << "Name: " << "DRM" << endl;
788 s << "Active: " << m_active << endl; 819​ s << "Active: " << m_active << endl;
789 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
790 return supportInfo; 824​ return supportInfo;
791} 825​}
792 826​
793} 827​}