platforms/drm: Avoid re-using blobs

Blobs are not reference counted if used by other drm master, if kwin
re-uses a deleted blob in an atomic commit, it will fail. For example,
on my computer, this happens when kwin starts after xorg.

Besides that, kwin may try to destroy blobs that it doesn't own, which
is not fatal but it's strange to do so.

CCBUG: 442603
CCBUG: 439873

src/plugins/platforms/drm/drm_object.cpp

@@ -153,18 +153,10 @@ bool DrmObject::updateProperties()
                 continue;
             }
             if (def.name == prop->name) {
-                drmModePropertyBlobRes *blob = nullptr;
-                if (prop->flags & DRM_MODE_PROP_BLOB) {
-                    blob = drmModeGetPropertyBlob(m_gpu->fd(), properties->prop_values[drmPropIndex]);
-                }
                 if (m_props[propIndex]) {
-                    if (prop->flags & DRM_MODE_PROP_BLOB) {
+                    m_props[propIndex]->setCurrent(properties->prop_values[drmPropIndex]);
-                        m_props[propIndex]->setCurrentBlob(blob);
-                    } else {
-                        m_props[propIndex]->setCurrent(properties->prop_values[drmPropIndex]);
-                    }
                 } else {
-                    m_props[propIndex] = new Property(this, prop.data(), properties->prop_values[drmPropIndex], def.enumNames, blob);
+                    m_props[propIndex] = new Property(this, prop.data(), properties->prop_values[drmPropIndex], def.enumNames);
                 }
                 found = true;
                 break;
@@ -189,15 +181,12 @@ bool DrmObject::updateProperties()
  * Definitions for DrmObject::Property
  */
 
-DrmObject::Property::Property(DrmObject *obj, drmModePropertyRes *prop, uint64_t val, const QVector<QByteArray> &enumNames, drmModePropertyBlobRes *blob)
+DrmObject::Property::Property(DrmObject *obj, drmModePropertyRes *prop, uint64_t val, const QVector<QByteArray> &enumNames)
     : m_propId(prop->prop_id)
     , m_propName(prop->name)
     , m_pending(val)
-    , m_pendingBlob(blob)
     , m_next(val)
-    , m_nextBlob(blob)
     , m_current(val)
-    , m_currentBlob(blob)
     , m_immutable(prop->flags & DRM_MODE_PROP_IMMUTABLE)
     , m_obj(obj)
 {
@@ -234,8 +223,8 @@ bool DrmObject::Property::setPendingBlob(void *blob, size_t length)
             return false;
         }
     }
-    if (m_pending && m_pending != m_current && m_pending != m_next) {
+    if (m_pendingBlob && m_pendingBlob != m_currentBlob && m_pendingBlob != m_nextBlob) {
-        drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_pending);
+        drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_pendingBlob->id);
         drmModeFreePropertyBlob(m_pendingBlob);
     }
     m_pending = id;
@@ -243,11 +232,6 @@ bool DrmObject::Property::setPendingBlob(void *blob, size_t length)
     return true;
 }
 
-drmModePropertyBlobRes *DrmObject::Property::pendingBlob() const
-{
-    return m_pendingBlob;
-}
-
 void DrmObject::Property::setCurrent(uint64_t value)
 {
     m_current = value;
@@ -260,40 +244,12 @@ uint64_t DrmObject::Property::current() const
 
 void DrmObject::Property::setCurrentBlob(drmModePropertyBlobRes *blob)
 {
-    if (!blob && !m_currentBlob) {
+    if (m_currentBlob && m_currentBlob != m_pendingBlob && m_currentBlob != m_nextBlob && m_currentBlob != blob) {
-        return;
+        drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_currentBlob->id);
-    }
-    if (!blob && m_pending == m_current) {
-        // the current blob might have been created by the prior drm master
-        // if it is, then the kernel has destroyed the blob and we must not continue using it
-        // -> simply create a copy
-        bool commit = m_pending == m_next;
-        setPendingBlob(m_currentBlob->data, m_currentBlob->length);
-        if (commit) {
-            commitPending();
-        }
-    }
-    if (!blob && m_next == m_current) {
-        // same as above, for the "next" blob
-        uint32_t id = 0;
-        if (drmModeCreatePropertyBlob(m_obj->m_gpu->fd(), m_currentBlob->data, m_currentBlob->length, &id) != 0) {
-            qCWarning(KWIN_DRM) << "Creating property blob failed!" << strerror(errno);
-        }
-        m_next = id;
-        m_nextBlob = drmModeGetPropertyBlob(m_obj->m_gpu->fd(), id);
-    }
-    uint32_t blobId = blob ? blob->id : 0;
-    if (m_current && m_current != m_pending && m_current != m_next && m_current != blobId) {
-        drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_current);
         drmModeFreePropertyBlob(m_currentBlob);
     }
     m_currentBlob = blob;
-    m_current = blobId;
+    m_current = blob ? blob->id : 0;
-}
-
-drmModePropertyBlobRes *DrmObject::Property::currentBlob() const
-{
-    return m_currentBlob;
 }
 
 void DrmObject::Property::commit()
@@ -314,8 +270,8 @@ void DrmObject::Property::commitPending()
         return;
     }
     if (m_pendingBlob || m_nextBlob) {
-        if (m_next && m_next != m_current) {
+        if (m_nextBlob && m_nextBlob != m_currentBlob) {
-            drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_next);
+            drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_nextBlob->id);
             drmModeFreePropertyBlob(m_nextBlob);
         }
         m_next = m_pending;
@@ -331,8 +287,8 @@ void DrmObject::Property::rollbackPending()
         return;
     }
     if (m_pendingBlob || m_nextBlob) {
-        if (m_pending && m_pending != m_current) {
+        if (m_pendingBlob && m_pendingBlob != m_currentBlob) {
-            drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_pending);
+            drmModeDestroyPropertyBlob(m_obj->m_gpu->fd(), m_pendingBlob->id);
             drmModeFreePropertyBlob(m_pendingBlob);
         }
         m_pending = m_next;

src/plugins/platforms/drm/drm_object.h

@@ -79,7 +79,7 @@ public:
     class Property
     {
     public:
-        Property(DrmObject *obj, drmModePropertyRes *prop, uint64_t val, const QVector<QByteArray> &enumNames, drmModePropertyBlobRes *blob);
+        Property(DrmObject *obj, drmModePropertyRes *prop, uint64_t val, const QVector<QByteArray> &enumNames);
         virtual ~Property();
 
         void initEnumMap(drmModePropertyRes *prop);
@@ -128,12 +128,10 @@ public:
         void setPending(uint64_t value);
         uint64_t pending() const;
         bool setPendingBlob(void *blob, size_t length);
-        drmModePropertyBlobRes *pendingBlob() const;
 
         void setCurrent(uint64_t value);
         uint64_t current() const;
         void setCurrentBlob(drmModePropertyBlobRes *blob);
-        drmModePropertyBlobRes *currentBlob() const;
 
         void commit();
         void commitPending();

src/plugins/platforms/drm/drm_object_connector.cpp

@@ -100,15 +100,18 @@ bool DrmConnector::init()
 
     // parse edid
     auto edidProp = getProp(PropertyIndex::Edid);
-    if (edidProp && edidProp->currentBlob() && edidProp->currentBlob()->data) {
+    if (edidProp) {
-        m_edid = Edid(edidProp->currentBlob()->data, edidProp->currentBlob()->length);
+        DrmScopedPointer<drmModePropertyBlobRes> blob(drmModeGetPropertyBlob(gpu()->fd(), edidProp->current()));
-        if (!m_edid.isValid()) {
+        if (blob && blob->data) {
-            qCWarning(KWIN_DRM, "Couldn't parse EDID for connector with id %d", id());
+            m_edid = Edid(blob->data, blob->length);
+            if (!m_edid.isValid()) {
+                qCWarning(KWIN_DRM, "Couldn't parse EDID for connector with id %d", id());
+            }
         }
+        deleteProp(PropertyIndex::Edid);
     } else {
         qCDebug(KWIN_DRM) << "Could not find edid for connector" << this;
     }
-    deleteProp(PropertyIndex::Edid);
 
     // check the physical size
     if (m_edid.physicalSize().isEmpty()) {

src/plugins/platforms/drm/drm_object_plane.cpp

@@ -74,25 +74,28 @@ bool DrmPlane::init()
 
         // read formats from blob if available and if modifiers are supported, and from the plane object if not
         if (auto formatProp = getProp(PropertyIndex::In_Formats); formatProp && gpu()->addFB2ModifiersSupported() && qEnvironmentVariableIntValue("KWIN_DRM_USE_MODIFIERS") == 1) {
-            auto blob = static_cast<drm_format_modifier_blob*>(formatProp->currentBlob()->data);
+            DrmScopedPointer<drmModePropertyBlobRes> propertyBlob(drmModeGetPropertyBlob(gpu()->fd(), formatProp->current()));
-            auto modifiers = reinterpret_cast<drm_format_modifier*>(reinterpret_cast<uint8_t*>(blob) + blob->modifiers_offset);
+            if (propertyBlob && propertyBlob->data) {
-            uint32_t *formatarr = reinterpret_cast<uint32_t*>(reinterpret_cast<uint8_t*>(blob) + blob->formats_offset);
+                auto blob = static_cast<drm_format_modifier_blob*>(propertyBlob->data);
+                auto modifiers = reinterpret_cast<drm_format_modifier*>(reinterpret_cast<uint8_t*>(blob) + blob->modifiers_offset);
+                uint32_t *formatarr = reinterpret_cast<uint32_t*>(reinterpret_cast<uint8_t*>(blob) + blob->formats_offset);
 
-            for (uint32_t f = 0; f < blob->count_formats; f++) {
+                for (uint32_t f = 0; f < blob->count_formats; f++) {
-                auto format = formatarr[f];
+                    auto format = formatarr[f];
-                QVector<uint64_t> mods;
+                    QVector<uint64_t> mods;
-                for (uint32_t m = 0; m < blob->count_modifiers; m++) {
+                    for (uint32_t m = 0; m < blob->count_modifiers; m++) {
-                    auto modifier = &modifiers[m];
+                        auto modifier = &modifiers[m];
-                    // The modifier advertisement blob is partitioned into groups of 64 formats
+                        // The modifier advertisement blob is partitioned into groups of 64 formats
-                    if (m < modifier->offset || m > modifier->offset + 63) {
+                        if (m < modifier->offset || m > modifier->offset + 63) {
-                        continue;
+                            continue;
+                        }
+                        if (!(modifier->formats & (1 << (f - modifier->offset)))) {
+                            continue;
+                        }
+                        mods << modifier->modifier;
                     }
-                    if (!(modifier->formats & (1 << (f - modifier->offset)))) {
+                    m_supportedFormats.insert(format, mods);
-                        continue;
-                    }
-                    mods << modifier->modifier;
                 }
-                m_supportedFormats.insert(format, mods);
             }
         } else {
             for (uint32_t i = 0; i < p->count_formats; i++) {