From 880837be13cec8784a771b0b32ebb30be6da676f Mon Sep 17 00:00:00 2001
From: Bernd Weimer <bernd.weimer@qt.io>
Date: Thu, 11 May 2023 15:17:20 +0200
Subject: Fix some more obvious issues in GPU reader

Reference count has been fixed to properly terminate underlying GPU
memory tracing process, no warning is printed any more when the process
finishes and parsing intel_gpu_top output has been adapted to pick the
actual load value (subject to future changes in the tool).

Change-Id: I7f02e7b48b826da2d6b10f1d67470b560a3c7c21
Pick-to: 6.5 6.2 5.15
Reviewed-by: Robert Griebl <robert.griebl@qt.io>
---
 src/monitor-lib/systemreader.cpp  | 41 +++++++++++++--------------------------
 src/shared-main-lib/gpustatus.cpp |  5 +++++
 src/shared-main-lib/gpustatus.h   |  1 +
 3 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/src/monitor-lib/systemreader.cpp b/src/monitor-lib/systemreader.cpp
index 6db7891b..3f27404f 100644
--- a/src/monitor-lib/systemreader.cpp
+++ b/src/monitor-lib/systemreader.cpp
@@ -171,22 +171,19 @@ public:
             setArguments({ qSL("dmon"), qSL("--select"), qSL("u") });
         }
 
-        QObject::connect(this, static_cast<void(QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished),
-                         this, [this](int exitCode, QProcess::ExitStatus) {
-            if (m_refCount) {
-                qCWarning(LogSystem) << "Failed to run GPU monitoring tool:"
-                                     << program() << arguments().join(qSL(" ")) << " - "
-                                     << "exited with code:" << exitCode;
-            }
+        connect(this, static_cast<void(QProcess::*)(QProcess::ProcessError error)>(&QProcess::errorOccurred),
+                this, [this](QProcess::ProcessError error) {
+            if (m_refCount)
+                qCWarning(LogSystem) << "GPU monitoring tool:" << program() << "caused error:" << error;
         });
 
-        QObject::connect(this, &QProcess::readyReadStandardOutput, this, [this]() {
+        connect(this, &QProcess::readyReadStandardOutput, this, [this]() {
             while (canReadLine()) {
                 const QByteArray str = readLine();
                 if (str.isEmpty() || (str.at(0) == '#'))
                     continue;
 
-                int pos = 0;
+                int pos = (GpuVendor::get() == GpuVendor::Intel) ? 50 : 0;
                 QVector<qreal> values;
 
                 while (pos < str.size() && values.size() < 2) {
@@ -207,11 +204,11 @@ public:
 
                 switch (GpuVendor::get()) {
                 case GpuVendor::Intel:
-                    if (values.size() >= 2)
-                        m_lastValue = values.at(1) / 100;
+                    if (values.size() > 0)
+                        m_lastValue = values.at(0) / 100;
                     break;
                 case GpuVendor::Nvidia:
-                    if (values.size() >= 2) {
+                    if (values.size() > 1) {
                         if (qFuzzyIsNull(values.at(0)))  // hardcoded to first gfx card
                             m_lastValue = values.at(1) / 100;
                     }
@@ -226,25 +223,15 @@ public:
 
     void ref()
     {
-        if (m_refCount.ref()) {
-            if (!isRunning()) {
-                start(QIODevice::ReadOnly);
-                if (!waitForStarted(2000)) {
-                    qCWarning(LogSystem) << "Could not start GPU monitoring tool:"
-                                         << program() << arguments().join(qSL(" ")) << " - "
-                                         << errorString();
-                }
-            }
-        }
+        if (m_refCount.ref() && !isRunning())
+            start(QIODevice::ReadOnly);
     }
 
     void deref()
     {
-        if (!m_refCount.deref()) {
-            if (isRunning()) {
-                kill();
-                waitForFinished();
-            }
+        if (!m_refCount.deref() && isRunning()) {
+            kill();
+            waitForFinished();
         }
     }
 
diff --git a/src/shared-main-lib/gpustatus.cpp b/src/shared-main-lib/gpustatus.cpp
index 6ee73d48..7c829894 100644
--- a/src/shared-main-lib/gpustatus.cpp
+++ b/src/shared-main-lib/gpustatus.cpp
@@ -57,6 +57,11 @@ GpuStatus::GpuStatus(QObject *parent)
     m_gpuReader->setActive(true);
 }
 
+GpuStatus::~GpuStatus()
+{
+    m_gpuReader->setActive(false);
+}
+
 /*!
     \qmlproperty real GpuStatus::gpuLoad
     \readonly
diff --git a/src/shared-main-lib/gpustatus.h b/src/shared-main-lib/gpustatus.h
index d36c6741..5ed0d1e3 100644
--- a/src/shared-main-lib/gpustatus.h
+++ b/src/shared-main-lib/gpustatus.h
@@ -24,6 +24,7 @@ class GpuStatus : public QObject
 
 public:
     GpuStatus(QObject *parent = nullptr);
+    ~GpuStatus() override;
 
     qreal gpuLoad() const;
 
-- 
cgit v1.2.1