Add quirk for nvidia blob buggy i2c ddc/ci

daemon/backends/upower/ddcutilbrightness.cpp

@@ -17,49 +17,165 @@
  *
  */
 
+#include <QFile>
 #include <powerdevil_debug.h>
 #include "ddcutilbrightness.h"
 
 DDCutilBrightness::DDCutilBrightness()
-    : m_usedVcp({0x10})
+    :
+#ifdef WITH_DDCUTIL
+    m_dlist(nullptr),
+#endif
+    m_usedVcp({0x10})
 {
     m_setBrightnessEventFilter.setInterval(100);
     m_setBrightnessEventFilter.setSingleShot(true);
     connect(&m_setBrightnessEventFilter, &QTimer::timeout, this, &DDCutilBrightness::setBrightnessAfterFilter);
 }
 
+#ifdef WITH_DDCUTIL
+bool DDCutilBrightness::isI2CBusOwnedByNVIDIABlob(int i2c_busno) const
+{
+    /* /sys/bus/i2c/devices/i2c-7/device/uevent
+     DRIVER=nvidia
+     PCI_CLASS=30000
+     PCI_ID=10DE:1B80
+     PCI_SUBSYS_ID=3842:6282
+     PCI_SLOT_NAME=0000:01:00.0
+     MODALIAS=pci:v000010DEd00001B80sv00003842sd00006282bc03sc00i00
+    */
+
+    QFile f(QString("/sys/bus/i2c/devices/i2c-%1/device/uevent").arg(i2c_busno));
+    if (!f.open(QFile::ReadOnly)) {
+        qCDebug(POWERDEVIL).nospace() << "Can't open " << f.fileName() << ": " << f.errorString();
+        return false;
+    }
+
+    QByteArray data = f.readAll();
+    f.close();
+
+    if (data.indexOf("DRIVER=nvidia\n") >= 0) {
+        qCDebug(POWERDEVIL) << "I2C bus" << i2c_busno << "owned by NVIDIA BLOB";
+        return true;
+    }
+
+    return false;
+}
+
+int DDCutilBrightness::displayHandleErrorCount(const QVector<DDCADisplayHandle> &prevList, const DDCA_Display_Info &h)
+{
+    for (const auto &dh : prevList) {
+        if (h.path.io_mode == dh.displayInfo.path.io_mode) {
+            bool match = false;
+
+            switch (h.path.io_mode)
+            {
+                case DDCA_IO_I2C:
+                    match = h.path.path.i2c_busno == dh.displayInfo.path.path.i2c_busno;
+                    break;
+
+                case DDCA_IO_USB:
+                    match = h.path.path.hiddev_devno == dh.displayInfo.path.path.hiddev_devno;
+                    break;
+
+                case DDCA_IO_ADL:
+                    match = h.path.path.adlno.iAdapterIndex == dh.displayInfo.path.path.adlno.iAdapterIndex &&
+                            h.path.path.adlno.iDisplayIndex == dh.displayInfo.path.path.adlno.iDisplayIndex;
+                    break;
+            }
+
+            if (match)
+                return dh.errorCount;
+        }
+    }
+
+    return 0;
+}
+
+void DDCutilBrightness::resetHandlers()
+{
+    // close old display handlers
+    for (const auto &dh : m_displayHandleList) {
+        ddca_close_display(dh.handle);
+    }
+
+    m_displayHandleList.clear();
+    m_supportedVcp_perDisp.clear();
+
+    // if prev m_dlist is presented - close, to prevent memory leaks
+    if (m_dlist) {
+        ddca_free_display_info_list(m_dlist);
+        m_dlist = nullptr;
+    }
+}
+#endif
+
 void DDCutilBrightness::detect()
 {
 #ifdef WITH_DDCUTIL
     DDCA_Status rc;
 
+    // save old display list for error tracking
+    QVector<DDCADisplayHandle> oldDisplayHandlerList = m_displayHandleList;
+
     qCDebug(POWERDEVIL) << "Check for monitors using ddca_get_displays()...";
-    // Inquire about detected monitors.
-    DDCA_Display_Info_List * dlist = nullptr;
-    ddca_get_display_info_list2(true, &dlist);
-    qCInfo(POWERDEVIL)  << "[DDCutilBrightness] " << dlist->ct << "display(s) were detected";
 
-    for (int iDisp = 0; iDisp < dlist->ct; ++iDisp) {
-        DDCA_Display_Handle dh = nullptr;  // initialize to avoid clang analyzer warning
+    // close all handlers and clear all lists
+    resetHandlers();
+
+    ddca_get_display_info_list2(true, &m_dlist);
+    qCInfo(POWERDEVIL)  << "[DDCutilBrightness] " << m_dlist->ct << "display(s) were detected";
+
+    for (int iDisp = 0; iDisp < m_dlist->ct; ++iDisp) {
+
+        // check error count for this *BUS* and ignore when limit reached
+        if (displayHandleErrorCount(oldDisplayHandlerList, m_dlist->info[iDisp]) > 6) {
+            // Ignore this bus, due to error rate limit
+            qCWarning(POWERDEVIL) << m_dlist->info[iDisp].model_name << "start's to ignore due error rate limit";
+            continue;
+        }
 
-        qCDebug(POWERDEVIL) << "Create a Display Identifier for display"<<iDisp <<
-        " : "<< dlist->info[iDisp].model_name;
+        qCDebug(POWERDEVIL) << "Create a Display Identifier for display" << iDisp <<
+            " : "<< m_dlist->info[iDisp].model_name;
+
+        DDCADisplayHandle displayHandle;
+        displayHandle.displayInfo = m_dlist->info[iDisp];
+
+        // check whether i2c bus owned by nvidia blob
+        if (m_dlist->info[iDisp].path.io_mode == DDCA_IO_I2C) {
+            if (isI2CBusOwnedByNVIDIABlob(m_dlist->info[iDisp].path.path.i2c_busno)) {
+                displayHandle.quirks |= DDCADisplayHandle::QUIRK_NVIDIA_BLOB_I2C;
+            }
+        }
 
-        m_displayInfoList.append(dlist->info[iDisp]);
 
         qCDebug(POWERDEVIL) << "Opening the display reference, creating a display handle...";
-        rc = ddca_open_display2(dlist->info[iDisp].dref, true, &dh);
+        rc = ddca_open_display2(m_dlist->info[iDisp].dref, true, &displayHandle.handle);
         if (rc != 0) {
             qCWarning(POWERDEVIL) << "[DDCutilBrightness]: ddct_open_display"<< rc;
             continue;
         }
         
         DDCA_Feature_List vcpList;
-        ddca_get_feature_list_by_dref(DDCA_SUBSET_COLOR, dh, false, &vcpList);
+        ddca_get_feature_list_by_dref(DDCA_SUBSET_COLOR, displayHandle.handle, false, &vcpList);
         QVector<uint16_t> tmpVcpList;
         for (int iVcp = 0; iVcp < m_usedVcp.count(); ++iVcp) {
             DDCA_Non_Table_Vcp_Value returnValue;
-            rc = ddca_get_non_table_vcp_value(dh,m_usedVcp.value(iVcp), &returnValue);
+            rc = ddca_get_non_table_vcp_value(displayHandle.handle, m_usedVcp.value(iVcp), &returnValue);
+            qCDebug(POWERDEVIL) << "ddca_get_non_table_vcp_value return" << rc;
+
+            // give a chance for a buggy nvidia i2c
+            if (displayHandle.quirks & DDCADisplayHandle::QUIRK_NVIDIA_BLOB_I2C) {
+                if (rc < 0) {
+                    displayHandle.errorCount = displayHandleErrorCount(oldDisplayHandlerList, displayHandle.displayInfo) + 1;
+                    qCDebug(POWERDEVIL) << "error count" << displayHandle.errorCount;
+                    m_displayHandleList.append(displayHandle);
+
+                    ddca_close_display(displayHandle.handle);
+                    displayHandle.handle = nullptr;
+                    return detect();
+                }
+            }
 
             if (rc < 0) {
                 qCDebug(POWERDEVIL) << "[DDCutilBrightness]: This monitor does not seem to support " << m_usedVcp[iVcp];
@@ -71,12 +187,13 @@ void DDCutilBrightness::detect()
         //we only store displays that actually support the features we want.
         if (tmpVcpList.contains(0x10)) {
             qCDebug(POWERDEVIL) << "Display supports Brightness, adding handle to list";
-            m_displayHandleList.append(dh);
+            m_displayHandleList.append(displayHandle);
             m_supportedVcp_perDisp.value(iDisp).append(tmpVcpList);
         }
     }
+
 #else
-    qCInfo(POWERDEVIL)  << "[DDCutilBrightness] compiled without DDC/CI support";
+    qCInfo(POWERDEVIL) << "[DDCutilBrightness] compiled without DDC/CI support";
     return;
 #endif
 }
@@ -101,7 +218,7 @@ long DDCutilBrightness::brightness()
         DDCA_Status rc;
         DDCA_Non_Table_Vcp_Value returnValue;
 
-        rc = ddca_get_non_table_vcp_value(m_displayHandleList.at(0),
+        rc = ddca_get_non_table_vcp_value(m_displayHandleList.at(0).handle,
                                 0x10, &returnValue);
         qCDebug(POWERDEVIL) << "[DDCutilBrightness::brightness]: ddca_get_vcp_value returned" << rc;
 
@@ -122,7 +239,7 @@ long DDCutilBrightness::brightnessMax()
     DDCA_Status rc;
     DDCA_Non_Table_Vcp_Value returnValue;
 
-    rc = ddca_get_non_table_vcp_value(m_displayHandleList.at(0)
+    rc = ddca_get_non_table_vcp_value(m_displayHandleList.at(0).handle
                                     , 0x10, &returnValue);
     qCDebug(POWERDEVIL) << "[DDCutilBrightness::brightnessMax]: ddca_get_vcp_value returned" << rc;
 
@@ -149,15 +266,28 @@ void DDCutilBrightness::setBrightnessAfterFilter()
 #ifdef WITH_DDCUTIL
     DDCA_Status rc;
     for (int iDisp = 0; iDisp < m_displayHandleList.count(); ++iDisp) {          //FIXME: we set the same brightness to all monitors, plugin architecture needed
-        qCDebug(POWERDEVIL) << "[DDCutilBrightness]: setting brightness "<<m_tmpCurrentBrightness;
+        DDCADisplayHandle &h = m_displayHandleList[iDisp];
+
+        qCDebug(POWERDEVIL) << "[DDCutilBrightness]: setting brightness " << m_tmpCurrentBrightness;
         uint8_t newsh = (uint16_t)m_tmpCurrentBrightness >> 8;
         uint8_t newsl = (uint16_t)m_tmpCurrentBrightness & 0x00ff;
-        rc = ddca_set_non_table_vcp_value(m_displayHandleList.at(iDisp), 0x10,
-                                           newsh, newsl);
+        rc = ddca_set_non_table_vcp_value(h.handle, 0x10, newsh, newsl);
+        qCDebug(POWERDEVIL) << "ddca_set_non_table_vcp_value return" << rc;
+
+        if (h.quirks & DDCADisplayHandle::QUIRK_NVIDIA_BLOB_I2C) {
+            // on nvidia blob response for this command might be NULL
+            // but actually brightness is set, so just ignore this return codes
+            if (rc == DDCRC_NULL_RESPONSE ||
+                rc == DDCRC_ALL_RESPONSES_NULL ||
+                // that may return when display come to dpms off
+                rc == DDCRC_RETRIES) {
+                rc = 0;
+            }
+        }
 
         if (rc < 0) {
-            qCWarning(POWERDEVIL) << "[DDCutilBrightness::setBrightness] failed, trying to detect()";
-            detect();
+            qCWarning(POWERDEVIL) << "[DDCutilBrightness::setBrightness] failed, trying to detect()" << rc;
+            return detect();
         } else {
             m_lastBrightnessKnown = m_tmpCurrentBrightness;
         }

daemon/backends/upower/ddcutilbrightness.h

@@ -26,11 +26,26 @@
 
 #ifdef WITH_DDCUTIL
 #include <ddcutil_c_api.h>
+#include <ddcutil_status_codes.h>
 #endif
 
 class DDCutilBrightness: public QObject
 {
     Q_OBJECT
+
+#ifdef WITH_DDCUTIL
+    struct DDCADisplayHandle {
+        enum : int {
+            QUIRK_NVIDIA_BLOB_I2C = (1 << 1),
+        };
+
+        DDCA_Display_Handle handle;
+        DDCA_Display_Info   displayInfo;
+        int quirks = 0;
+        int errorCount = 0;
+    };
+#endif
+
 public:
     DDCutilBrightness();
     void detect();
@@ -44,8 +59,15 @@ private Q_SLOTS:
 
 private:
 #ifdef WITH_DDCUTIL
-    QVector<DDCA_Display_Handle> m_displayHandleList;
-    QVector<DDCA_Display_Info> m_displayInfoList;
+    void resetHandlers();
+    bool isI2CBusOwnedByNVIDIABlob(int i2c_busno) const;
+    int displayHandleErrorCount(const QVector<DDCADisplayHandle> &prevList, const DDCA_Display_Info &);
+#endif
+
+private:
+#ifdef WITH_DDCUTIL
+    QVector<DDCADisplayHandle> m_displayHandleList;
+    DDCA_Display_Info_List *m_dlist;
 #endif //ifdef WITH_DDCUTIL
     //Per display properties
     //destription mapped to vcp values for easy retrieval