Move Edid and DPMS into DrmConnector

src/plugins/platforms/drm/drm_object.h

@@ -137,6 +137,13 @@ protected:
         const QByteArray &name() const {
             return m_propName;
         }
+        /**
+         * while this is usually automatically set, some properties
+         * (like DPMS) are not meant to be used in AMS
+         */
+        void setImmutable() {
+            m_immutable = true;
+        }
         bool isImmutable() const {
             return m_immutable;
         }

src/plugins/platforms/drm/drm_object_connector.cpp

@@ -9,6 +9,11 @@
 #include "drm_object_connector.h"
 #include "drm_pointer.h"
 #include "logging.h"
+#include "edid.h"
+
+#include <main.h>
+// frameworks
+#include <KConfigGroup>
 
 namespace KWin
 {
@@ -31,10 +36,54 @@ bool DrmConnector::init()
 {
     qCDebug(KWIN_DRM) << "Creating connector" << m_id;
 
-    return initProps({
+    if (!initProps({
-        PropertyDefinition(QByteArrayLiteral("CRTC_ID")),
+            PropertyDefinition(QByteArrayLiteral("CRTC_ID")),
-        PropertyDefinition(QByteArrayLiteral("non-desktop")),
+            PropertyDefinition(QByteArrayLiteral("non-desktop")),
-    }, DRM_MODE_OBJECT_CONNECTOR);
+            PropertyDefinition(QByteArrayLiteral("DPMS")),
+            PropertyDefinition(QByteArrayLiteral("EDID")),
+        }, DRM_MODE_OBJECT_CONNECTOR)) {
+        return false;
+    }
+
+    if (auto dpmsProp = m_props[static_cast<uint32_t>(PropertyIndex::Dpms)]) {
+        // the dpms property makes atomic commits fail
+        // for legacy it will be explicitly set
+        dpmsProp->setImmutable();
+    } else {
+        qCWarning(KWIN_DRM) << "Could not find DPMS property!";
+    }
+
+    // parse edid
+    if (auto edidProp = m_props[static_cast<uint32_t>(PropertyIndex::Edid)]) {
+        m_edid.reset(new Edid(edidProp->blob()->data, edidProp->blob()->length));
+        if (!m_edid->isValid()) {
+            qCWarning(KWIN_DRM, "Couldn't parse EDID for connector with id %d", id());
+        }
+        deleteProp(PropertyIndex::Edid);
+    } else {
+        qCWarning(KWIN_DRM) << "Could not find edid for connector" << this;
+    }
+
+    // check the physical size
+    if (m_edid->physicalSize().isEmpty()) {
+        m_physicalSize = QSize(m_conn->mmWidth, m_conn->mmHeight);
+    } else {
+        m_physicalSize = m_edid->physicalSize();
+    }
+
+    // the size might be completely borked. E.g. Samsung SyncMaster 2494HS reports 160x90 while in truth it's 520x292
+    // as this information is used to calculate DPI info, it's going to result in everything being huge
+    const QByteArray unknown = QByteArrayLiteral("unknown");
+    KConfigGroup group = kwinApp()->config()->group("EdidOverwrite").group(m_edid->eisaId().isEmpty() ? unknown : m_edid->eisaId())
+                                                       .group(m_edid->monitorName().isEmpty() ? unknown : m_edid->monitorName())
+                                                       .group(m_edid->serialNumber().isEmpty() ? unknown : m_edid->serialNumber());
+    if (group.hasKey("PhysicalSize")) {
+        const QSize overwriteSize = group.readEntry("PhysicalSize", m_physicalSize);
+        qCWarning(KWIN_DRM) << "Overwriting monitor physical size for" << m_edid->eisaId() << "/" << m_edid->monitorName() << "/" << m_edid->serialNumber() << " from " << m_physicalSize << "to " << overwriteSize;
+        m_physicalSize = overwriteSize;
+    }
+
+    return true;
 }
 
 bool DrmConnector::isConnected()
@@ -46,4 +95,49 @@ bool DrmConnector::isConnected()
     return con->connection == DRM_MODE_CONNECTED;
 }
 
+static QHash<int, QByteArray> s_connectorNames = {
+    {DRM_MODE_CONNECTOR_Unknown, QByteArrayLiteral("Unknown")},
+    {DRM_MODE_CONNECTOR_VGA, QByteArrayLiteral("VGA")},
+    {DRM_MODE_CONNECTOR_DVII, QByteArrayLiteral("DVI-I")},
+    {DRM_MODE_CONNECTOR_DVID, QByteArrayLiteral("DVI-D")},
+    {DRM_MODE_CONNECTOR_DVIA, QByteArrayLiteral("DVI-A")},
+    {DRM_MODE_CONNECTOR_Composite, QByteArrayLiteral("Composite")},
+    {DRM_MODE_CONNECTOR_SVIDEO, QByteArrayLiteral("SVIDEO")},
+    {DRM_MODE_CONNECTOR_LVDS, QByteArrayLiteral("LVDS")},
+    {DRM_MODE_CONNECTOR_Component, QByteArrayLiteral("Component")},
+    {DRM_MODE_CONNECTOR_9PinDIN, QByteArrayLiteral("DIN")},
+    {DRM_MODE_CONNECTOR_DisplayPort, QByteArrayLiteral("DP")},
+    {DRM_MODE_CONNECTOR_HDMIA, QByteArrayLiteral("HDMI-A")},
+    {DRM_MODE_CONNECTOR_HDMIB, QByteArrayLiteral("HDMI-B")},
+    {DRM_MODE_CONNECTOR_TV, QByteArrayLiteral("TV")},
+    {DRM_MODE_CONNECTOR_eDP, QByteArrayLiteral("eDP")},
+    {DRM_MODE_CONNECTOR_VIRTUAL, QByteArrayLiteral("Virtual")},
+    {DRM_MODE_CONNECTOR_DSI, QByteArrayLiteral("DSI")},
+#ifdef DRM_MODE_CONNECTOR_DPI
+    {DRM_MODE_CONNECTOR_DPI, QByteArrayLiteral("DPI")},
+#endif
+};
+
+QString DrmConnector::connectorName() const
+{
+    return s_connectorNames.value(m_conn->connector_type, QByteArrayLiteral("Unknown")) + QStringLiteral("-") + QString::number(m_conn->connector_type_id);
+}
+
+QString DrmConnector::modelName() const
+{
+    return connectorName() + m_edid->nameString();
+}
+
+bool DrmConnector::isInternal() const
+{
+    return m_conn->connector_type == DRM_MODE_CONNECTOR_LVDS || m_conn->connector_type == DRM_MODE_CONNECTOR_eDP
+                || m_conn->connector_type == DRM_MODE_CONNECTOR_DSI;
+}
+
+QSize DrmConnector::physicalSize() const
+{
+    return m_physicalSize;
+}
+
+
 }

src/plugins/platforms/drm/drm_object_connector.h

@@ -9,11 +9,15 @@
 #ifndef KWIN_DRM_OBJECT_CONNECTOR_H
 #define KWIN_DRM_OBJECT_CONNECTOR_H
 
+#include <QSize>
+
 #include "drm_object.h"
 
 namespace KWin
 {
 
+class Edid;
+
 class DrmConnector : public DrmObject
 {
 public:
@@ -26,6 +30,8 @@ public:
     enum class PropertyIndex : uint32_t {
         CrtcId = 0,
         NonDesktop = 1,
+        Dpms = 2,
+        Edid = 3,
         Count
     };
 
@@ -42,9 +48,28 @@ public:
         }
         return prop->value();
     }
+
+    Property *dpms() const {
+        return m_props[static_cast<uint32_t>(PropertyIndex::Dpms)];
+    }
+
+    Edid *edid() const {
+        return m_edid.get();
+    }
+
+    QString connectorName() const;
+    QString modelName() const;
+
+    bool isInternal() const;
+    QSize physicalSize() const;
+
 private:
     DrmScopedPointer<drmModeConnector> m_conn;
     QVector<uint32_t> m_encoders;
+
+    QScopedPointer<Edid> m_edid;
+    QSize m_physicalSize = QSize(-1, -1);
+
 };
 
 }

src/plugins/platforms/drm/drm_output.cpp

@@ -10,6 +10,7 @@
 #include "drm_backend.h"
 #include "drm_object_crtc.h"
 #include "drm_object_connector.h"
+#include "edid.h"
 
 #include "composite.h"
 #include "cursor.h"
@@ -21,10 +22,6 @@
 #include "wayland_server.h"
 // KWayland
 #include <KWaylandServer/output_interface.h>
-// KF5
-#include <KConfigGroup>
-#include <KLocalizedString>
-#include <KSharedConfig>
 // Qt
 #include <QMatrix4x4>
 #include <QCryptographicHash>
@@ -175,29 +172,6 @@ void DrmOutput::moveCursor()
     drmModeMoveCursor(m_gpu->fd(), m_crtc->id(), pos.x(), pos.y());
 }
 
-static QHash<int, QByteArray> s_connectorNames = {
-    {DRM_MODE_CONNECTOR_Unknown, QByteArrayLiteral("Unknown")},
-    {DRM_MODE_CONNECTOR_VGA, QByteArrayLiteral("VGA")},
-    {DRM_MODE_CONNECTOR_DVII, QByteArrayLiteral("DVI-I")},
-    {DRM_MODE_CONNECTOR_DVID, QByteArrayLiteral("DVI-D")},
-    {DRM_MODE_CONNECTOR_DVIA, QByteArrayLiteral("DVI-A")},
-    {DRM_MODE_CONNECTOR_Composite, QByteArrayLiteral("Composite")},
-    {DRM_MODE_CONNECTOR_SVIDEO, QByteArrayLiteral("SVIDEO")},
-    {DRM_MODE_CONNECTOR_LVDS, QByteArrayLiteral("LVDS")},
-    {DRM_MODE_CONNECTOR_Component, QByteArrayLiteral("Component")},
-    {DRM_MODE_CONNECTOR_9PinDIN, QByteArrayLiteral("DIN")},
-    {DRM_MODE_CONNECTOR_DisplayPort, QByteArrayLiteral("DP")},
-    {DRM_MODE_CONNECTOR_HDMIA, QByteArrayLiteral("HDMI-A")},
-    {DRM_MODE_CONNECTOR_HDMIB, QByteArrayLiteral("HDMI-B")},
-    {DRM_MODE_CONNECTOR_TV, QByteArrayLiteral("TV")},
-    {DRM_MODE_CONNECTOR_eDP, QByteArrayLiteral("eDP")},
-    {DRM_MODE_CONNECTOR_VIRTUAL, QByteArrayLiteral("Virtual")},
-    {DRM_MODE_CONNECTOR_DSI, QByteArrayLiteral("DSI")},
-#ifdef DRM_MODE_CONNECTOR_DPI
-    {DRM_MODE_CONNECTOR_DPI, QByteArrayLiteral("DPI")},
-#endif
-};
-
 namespace {
 quint64 refreshRateForMode(_drmModeModeInfo *m)
 {
@@ -219,15 +193,12 @@ quint64 refreshRateForMode(_drmModeModeInfo *m)
 
 bool DrmOutput::init(drmModeConnector *connector)
 {
-    initEdid(connector);
-    initDpms(connector);
     initUuid();
     if (m_gpu->atomicModeSetting() && !m_primaryPlane) {
         return false;
     }
 
-    setInternal(connector->connector_type == DRM_MODE_CONNECTOR_LVDS || connector->connector_type == DRM_MODE_CONNECTOR_eDP
+    setInternal(m_conn->isInternal());
-                || connector->connector_type == DRM_MODE_CONNECTOR_DSI);
     setDpmsSupported(true);
     initOutputDevice(connector);
 
@@ -244,39 +215,14 @@ void DrmOutput::initUuid()
 {
     QCryptographicHash hash(QCryptographicHash::Md5);
     hash.addData(QByteArray::number(m_conn->id()));
-    hash.addData(m_edid.eisaId());
+    hash.addData(m_conn->edid()->eisaId());
-    hash.addData(m_edid.monitorName());
+    hash.addData(m_conn->edid()->monitorName());
-    hash.addData(m_edid.serialNumber());
+    hash.addData(m_conn->edid()->serialNumber());
     m_uuid = hash.result().toHex().left(10);
 }
 
 void DrmOutput::initOutputDevice(drmModeConnector *connector)
 {
-    QString manufacturer;
-    if (!m_edid.vendor().isEmpty()) {
-        manufacturer = QString::fromLatin1(m_edid.vendor());
-    } else if (!m_edid.eisaId().isEmpty()) {
-        manufacturer = QString::fromLatin1(m_edid.eisaId());
-    }
-
-    QString connectorName = s_connectorNames.value(connector->connector_type, QByteArrayLiteral("Unknown")) + QStringLiteral("-") + QString::number(connector->connector_type_id);
-    QString modelName;
-
-    if (!m_edid.monitorName().isEmpty()) {
-        QString m = QString::fromLatin1(m_edid.monitorName());
-        if (!m_edid.serialNumber().isEmpty()) {
-            m.append('/');
-            m.append(QString::fromLatin1(m_edid.serialNumber()));
-        }
-        modelName = m;
-    } else if (!m_edid.serialNumber().isEmpty()) {
-        modelName = QString::fromLatin1(m_edid.serialNumber());
-    } else {
-        modelName = i18n("unknown");
-    }
-
-    const QString model = connectorName + QStringLiteral("-") + modelName;
-
     // read in mode information
     QVector<KWaylandServer::OutputDeviceInterface::Mode> modes;
     for (int i = 0; i < connector->count_modes; ++i) {
@@ -299,20 +245,8 @@ void DrmOutput::initOutputDevice(drmModeConnector *connector)
         modes << mode;
     }
 
-    QSize physicalSize = !m_edid.physicalSize().isEmpty() ? m_edid.physicalSize() : QSize(connector->mmWidth, connector->mmHeight);
+    setName(m_conn->connectorName());
-    // the size might be completely borked. E.g. Samsung SyncMaster 2494HS reports 160x90 while in truth it's 520x292
+    initInterfaces(m_conn->modelName(), m_conn->edid()->manufacturerString(), m_uuid, m_conn->physicalSize(), modes, m_conn->edid()->raw());
-    // as this information is used to calculate DPI info, it's going to result in everything being huge
-    const QByteArray unknown = QByteArrayLiteral("unknown");
-    KConfigGroup group = kwinApp()->config()->group("EdidOverwrite").group(m_edid.eisaId().isEmpty() ? unknown : m_edid.eisaId())
-                                                       .group(m_edid.monitorName().isEmpty() ? unknown : m_edid.monitorName())
-                                                       .group(m_edid.serialNumber().isEmpty() ? unknown : m_edid.serialNumber());
-    if (group.hasKey("PhysicalSize")) {
-        const QSize overwriteSize = group.readEntry("PhysicalSize", physicalSize);
-        qCWarning(KWIN_DRM) << "Overwriting monitor physical size for" << m_edid.eisaId() << "/" << m_edid.monitorName() << "/" << m_edid.serialNumber() << " from " << physicalSize << "to " << overwriteSize;
-        physicalSize = overwriteSize;
-    }
-    setName(connectorName);
-    initInterfaces(model, manufacturer, m_uuid, physicalSize, modes, m_edid.raw());
 }
 
 bool DrmOutput::isCurrentMode(const drmModeModeInfo *mode) const
@@ -333,28 +267,6 @@ bool DrmOutput::isCurrentMode(const drmModeModeInfo *mode) const
         && mode->type        == m_mode.type
         && qstrcmp(mode->name, m_mode.name) == 0;
 }
-void DrmOutput::initEdid(drmModeConnector *connector)
-{
-    DrmScopedPointer<drmModePropertyBlobRes> edid;
-    for (int i = 0; i < connector->count_props; ++i) {
-        DrmScopedPointer<drmModePropertyRes> property(drmModeGetProperty(m_gpu->fd(), connector->props[i]));
-        if (!property) {
-            continue;
-        }
-        if ((property->flags & DRM_MODE_PROP_BLOB) && qstrcmp(property->name, "EDID") == 0) {
-            edid.reset(drmModeGetPropertyBlob(m_gpu->fd(), connector->prop_values[i]));
-        }
-    }
-    if (!edid) {
-        qCWarning(KWIN_DRM) << "Could not find edid for connector" << connector << connector->connector_id;
-        return;
-    }
-
-    m_edid = Edid(edid->data, edid->length);
-    if (!m_edid.isValid()) {
-        qCWarning(KWIN_DRM, "Couldn't parse EDID for connector with id %d", m_conn->id());
-    }
-}
 
 bool DrmOutput::initCursor(const QSize &cursorSize)
 {
@@ -371,20 +283,6 @@ bool DrmOutput::initCursor(const QSize &cursorSize)
     return true;
 }
 
-void DrmOutput::initDpms(drmModeConnector *connector)
-{
-    for (int i = 0; i < connector->count_props; ++i) {
-        DrmScopedPointer<drmModePropertyRes> property(drmModeGetProperty(m_gpu->fd(), connector->props[i]));
-        if (!property) {
-            continue;
-        }
-        if (qstrcmp(property->name, "DPMS") == 0) {
-            m_dpms.swap(property);
-            break;
-        }
-    }
-}
-
 void DrmOutput::updateEnablement(bool enable)
 {
     if (enable) {
@@ -471,7 +369,7 @@ static KWaylandServer::OutputInterface::DpmsMode toWaylandDpmsMode(DrmOutput::Dp
 
 void DrmOutput::updateDpms(KWaylandServer::OutputInterface::DpmsMode mode)
 {
-    if (m_dpms.isNull() || !isEnabled()) {
+    if (!m_conn->dpms() || !isEnabled()) {
         return;
     }
 
@@ -534,7 +432,7 @@ void DrmOutput::dpmsFinishOff()
 bool DrmOutput::dpmsLegacyApply()
 {
     if (drmModeConnectorSetProperty(m_gpu->fd(), m_conn->id(),
-                                    m_dpms->prop_id, uint64_t(m_dpmsModePending)) < 0) {
+                                    m_conn->dpms()->propId(), uint64_t(m_dpmsModePending)) < 0) {
         m_dpmsModePending = m_dpmsMode;
         qCWarning(KWIN_DRM) << "Setting DPMS failed";
         return false;

src/plugins/platforms/drm/drm_output.h

@@ -13,7 +13,6 @@
 #include "drm_pointer.h"
 #include "drm_object.h"
 #include "drm_object_plane.h"
-#include "edid.h"
 
 #include <QObject>
 #include <QPoint>
@@ -116,8 +115,6 @@ private:
 
     bool presentLegacy(const QSharedPointer<DrmBuffer> &buffer);
     bool setModeLegacy(DrmBuffer *buffer);
-    void initEdid(drmModeConnector *connector);
-    void initDpms(drmModeConnector *connector);
     void initOutputDevice(drmModeConnector *connector);
 
     bool isCurrentMode(const drmModeModeInfo *mode) const;
@@ -150,8 +147,6 @@ private:
     DrmCrtc *m_crtc = nullptr;
     bool m_lastGbm = false;
     drmModeModeInfo m_mode;
-    Edid m_edid;
-    DrmScopedPointer<drmModePropertyRes> m_dpms;
     DpmsMode m_dpmsMode = DpmsMode::On;
     DpmsMode m_dpmsModePending = DpmsMode::On;
     QByteArray m_uuid;

src/plugins/platforms/drm/edid.cpp

@@ -14,6 +14,8 @@
 #include <QFile>
 #include <QStandardPaths>
 
+#include <KLocalizedString>
+
 namespace KWin
 {
 
@@ -211,4 +213,31 @@ QByteArray Edid::raw() const
     return m_raw;
 }
 
+QString Edid::manufacturerString() const
+{
+    QString manufacturer;
+    if (!m_vendor.isEmpty()) {
+        manufacturer = QString::fromLatin1(m_vendor);
+    } else if (!m_eisaId.isEmpty()) {
+        manufacturer = QString::fromLatin1(m_eisaId);
+    }
+    return manufacturer;
+}
+
+QString Edid::nameString() const
+{
+    if (!m_monitorName.isEmpty()) {
+        QString m = QString::fromLatin1(m_monitorName);
+        if (!m_serialNumber.isEmpty()) {
+            m.append('/');
+            m.append(QString::fromLatin1(m_serialNumber));
+        }
+        return m;
+    } else if (!m_serialNumber.isEmpty()) {
+        return QString::fromLatin1(m_serialNumber);
+    } else {
+        return i18n("unknown");
+    }
+}
+
 } // namespace KWin

src/plugins/platforms/drm/edid.h

@@ -61,6 +61,17 @@ public:
      */
     QByteArray raw() const;
 
+    /**
+     * returns the vendor if included, the EISA ID if not
+     */
+    QString manufacturerString() const;
+
+    /**
+     * returns a string representing the monitor name
+     * Can be a serial number or "unknown" if the name is empty
+     */
+    QString nameString() const;
+
 private:
     QSize m_physicalSize;
     QByteArray m_vendor;