Differential D18570 Diff 50573 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​#include "egl_stream_backend.h"
38// KWayland 39​// KWayland
39#include <KWayland/Server/seat_interface.h> 40​#include <KWayland/Server/seat_interface.h>
40#include <KWayland/Server/outputconfiguration_interface.h> 41​#include <KWayland/Server/outputconfiguration_interface.h>
41// KF5 42​// KF5
42#include <KConfigGroup> 43​#include <KConfigGroup>
43#include <KCoreAddons> 44​#include <KCoreAddons>
44#include <KLocalizedString> 45​#include <KLocalizedString>
45#include <KSharedConfig> 46​#include <KSharedConfig>
Show All 23 Lines
69{ 70​{
70 71​
71DrmBackend::DrmBackend(QObject *parent) 72​DrmBackend::DrmBackend(QObject *parent)
72 : Platform(parent) 73​ : Platform(parent)
73 , m_udev(new Udev) 74​ , m_udev(new Udev)
74 , m_udevMonitor(m_udev->monitor()) 75​ , m_udevMonitor(m_udev->monitor())
75 , m_dpmsFilter() 76​ , m_dpmsFilter()
76{ 77​{
78​ if (qEnvironmentVariableIsSet("KWIN_DRM_USE_EGLDEVICE")) {
79​ m_useEglDevice = true;
80​ }
77 setSupportsGammaControl(true); 81​ setSupportsGammaControl(true);
78 handleOutputs(); 82​ handleOutputs();
79} 83​}
80 84​
81DrmBackend::~DrmBackend() 85​DrmBackend::~DrmBackend()
82{ 86​{
83#if HAVE_GBM 87​#if HAVE_GBM
84 if (m_gbmDevice) { 88​ if (m_gbmDevice) {
▲ Show 20 Lines • Show All 160 Lines • ▼ Show 20 Line(s)
245void DrmBackend::openDrm() 249​void DrmBackend::openDrm()
246{ 250​{
247 connect(LogindIntegration::self(), &LogindIntegration::sessionActiveChanged, this, &DrmBackend::activate); 251​ connect(LogindIntegration::self(), &LogindIntegration::sessionActiveChanged, this, &DrmBackend::activate);
248 UdevDevice::Ptr device = m_udev->primaryGpu(); 252​ UdevDevice::Ptr device = m_udev->primaryGpu();
249 if (!device) { 253​ if (!device) {
250 qCWarning(KWIN_DRM) << "Did not find a GPU"; 254​ qCWarning(KWIN_DRM) << "Did not find a GPU";
251 return; 255​ return;
252 } 256​ }
253 int fd = LogindIntegration::self()->takeDevice(device->devNode()); 257​ m_devNode = device->devNode();
258​ int fd = LogindIntegration::self()->takeDevice(m_devNode.constData());
254 if (fd < 0) { 259​ if (fd < 0) {
255 qCWarning(KWIN_DRM) << "failed to open drm device at" << device->devNode(); 260​ qCWarning(KWIN_DRM) << "failed to open drm device at" << m_devNode;
256 return; 261​ return;
257 } 262​ }
258 m_fd = fd; 263​ m_fd = fd;
259 m_active = true; 264​ m_active = true;
260 QSocketNotifier *notifier = new QSocketNotifier(m_fd, QSocketNotifier::Read, this); 265​ QSocketNotifier *notifier = new QSocketNotifier(m_fd, QSocketNotifier::Read, this);
261 connect(notifier, &QSocketNotifier::activated, this, 266​ connect(notifier, &QSocketNotifier::activated, this,
262 [this] { 267​ [this] {
263 if (!LogindIntegration::self()->isActiveSession()) { 268​ if (!LogindIntegration::self()->isActiveSession()) {
264 return; 269​ return;
265 } 270​ }
266 drmEventContext e; 271​ drmEventContext e;
267 memset(&e, 0, sizeof e); 272​ memset(&e, 0, sizeof e);
268 e.version = KWIN_DRM_EVENT_CONTEXT_VERSION; 273​ e.version = KWIN_DRM_EVENT_CONTEXT_VERSION;
269 e.page_flip_handler = pageFlipHandler; 274​ e.page_flip_handler = pageFlipHandler;
270 drmHandleEvent(m_fd, &e); 275​ drmHandleEvent(m_fd, &e);
271 } 276​ }
272 ); 277​ );
273 m_drmId = device->sysNum(); 278​ m_drmId = device->sysNum();
274 279​
275 // trying to activate Atomic Mode Setting (this means also Universal Planes) 280​ // trying to activate Atomic Mode Setting (this means also Universal Planes)
276 if (!qEnvironmentVariableIsSet("KWIN_DRM_NO_AMS")) { 281​ // note: Atomic Mode Setting is not currently supported with EGLStream backend
282​ if (!qEnvironmentVariableIsSet("KWIN_DRM_NO_AMS") && !m_useEglDevice) {
277 if (drmSetClientCap(m_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) { 283​ if (drmSetClientCap(m_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) {
278 qCDebug(KWIN_DRM) << "Using Atomic Mode Setting."; 284​ qCDebug(KWIN_DRM) << "Using Atomic Mode Setting.";
279 m_atomicModeSetting = true; 285​ m_atomicModeSetting = true;
280 286​
281 ScopedDrmPointer<drmModePlaneRes, &drmModeFreePlaneResources> planeResources(drmModeGetPlaneResources(m_fd)); 287​ ScopedDrmPointer<drmModePlaneRes, &drmModeFreePlaneResources> planeResources(drmModeGetPlaneResources(m_fd));
282 if (!planeResources) { 288​ if (!planeResources) {
283 qCWarning(KWIN_DRM) << "Failed to get plane resources. Falling back to legacy mode"; 289​ qCWarning(KWIN_DRM) << "Failed to get plane resources. Falling back to legacy mode";
284 m_atomicModeSetting = false; 290​ m_atomicModeSetting = false;
Show All 21 Lines310 if (m_planes.isEmpty()) {
306 m_atomicModeSetting = false; 312​ m_atomicModeSetting = false;
307 } 313​ }
308 } 314​ }
309 } else { 315​ } else {
310 qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode."; 316​ qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode.";
311 } 317​ }
312 } 318​ }
313 319​
314 ScopedDrmPointer<_drmModeRes, &drmModeFreeResources> resources(drmModeGetResources(m_fd)); 320​ 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(); 321​ drmModeRes *res = resources.data();
316 if (!resources) { 322​ if (!resources) {
317 qCWarning(KWIN_DRM) << "drmModeGetResources failed"; 323​ qCWarning(KWIN_DRM) << "drmModeGetResources failed";
318 return; 324​ return;
319 } 325​ }
320 326​
321 for (int i = 0; i < res->count_connectors; ++i) { 327​ for (int i = 0; i < res->count_connectors; ++i) {
322 m_connectors << new DrmConnector(res->connectors[i], m_fd); 328​ m_connectors << new DrmConnector(res->connectors[i], m_fd);
▲ Show 20 Lines • Show All 277 Lines • ▼ Show 20 Line(s)605 auto it = std::find_if(m_outputs.constBegin(), m_outputs.constEnd(), [uuid] (DrmOutput *o) {
600 return o->m_uuid == uuid; 606​ return o->m_uuid == uuid;
601 }); 607​ });
602 if (it != m_outputs.constEnd()) { 608​ if (it != m_outputs.constEnd()) {
603 return *it; 609​ return *it;
604 } 610​ }
605 return nullptr; 611​ return nullptr;
606} 612​}
607 613​
614​void DrmBackend::queuePageFlip()
zzagUnsubmitted
Done

Functions have opening braces on the start of a line.

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

else keyword and curly braces are unnecessary.

romangg: `else` keyword and curly braces are unnecessary.
739#else 759​#else
740 return Platform::createOpenGLBackend(); 760​ return Platform::createOpenGLBackend();
741#endif 761​#endif
742} 762​ }
763​}
743 764​
744DrmDumbBuffer *DrmBackend::createBuffer(const QSize &size) 765​DrmDumbBuffer *DrmBackend::createBuffer(const QSize &size)
745{ 766​{
746 DrmDumbBuffer *b = new DrmDumbBuffer(m_fd, size); 767​ DrmDumbBuffer *b = new DrmDumbBuffer(m_fd, size);
747 return b; 768​ return b;
748} 769​}
749 770​
750#if HAVE_GBM 771​#if HAVE_GBM
Show All 14 Lines785 for (auto it = m_enabledOutputs.constBegin(); it != m_enabledOutputs.constEnd(); ++it) {
765 enabled = enabled || (*it)->isDpmsEnabled(); 786​ enabled = enabled || (*it)->isDpmsEnabled();
766 } 787​ }
767 setOutputsEnabled(enabled); 788​ setOutputsEnabled(enabled);
768} 789​}
769 790​
770QVector<CompositingType> DrmBackend::supportedCompositors() const 791​QVector<CompositingType> DrmBackend::supportedCompositors() const
771{ 792​{
772#if HAVE_GBM 793​#if HAVE_GBM
773 return QVector<CompositingType>{OpenGLCompositing, QPainterCompositing}; 794​ 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 795​#else
775 return QVector<CompositingType>{QPainterCompositing}; 796​ return QVector<CompositingType>{QPainterCompositing};
776#endif 797​#endif
777} 798​}
778 799​
779QString DrmBackend::supportInformation() const 800​QString DrmBackend::supportInformation() const
780{ 801​{
781 QString supportInfo; 802​ QString supportInfo;
782 QDebug s(&supportInfo); 803​ QDebug s(&supportInfo);
783 s.nospace(); 804​ s.nospace();
784 s << "Name: " << "DRM" << endl; 805​ s << "Name: " << "DRM" << endl;
785 s << "Active: " << m_active << endl; 806​ s << "Active: " << m_active << endl;
786 s << "Atomic Mode Setting: " << m_atomicModeSetting << endl; 807​ s << "Atomic Mode Setting: " << m_atomicModeSetting << endl;
808​ s << "Using EGL Device: " << m_useEglDevice << endl;
787 return supportInfo; 809​ return supportInfo;
788} 810​}
789 811​
790} 812​}