From 82c2324b5675ea2d5e3b962f270bda1d186e7326 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Mon, 4 Apr 2022 20:19:05 +0200 Subject: [PATCH] backends/drm: fetch immutable blobs in DrmProperty If the blob is fetched while there is no kernel-visible reference to it, the driver may re-use the blob ID. When DrmProperty is created or updated, KWin holds a reference on the blob via drmModeObjectProperties, so this should prevent any possible issues. CCBUG: 449285 --- src/backends/drm/drm_object_connector.cpp | 13 +++----- src/backends/drm/drm_object_plane.cpp | 39 +++++++++++------------ src/backends/drm/drm_property.cpp | 27 +++++++++++++++- src/backends/drm/drm_property.h | 7 +++- 4 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/backends/drm/drm_object_connector.cpp b/src/backends/drm/drm_object_connector.cpp index a0045290df..0cdbd70b4f 100644 --- a/src/backends/drm/drm_object_connector.cpp +++ b/src/backends/drm/drm_object_connector.cpp @@ -337,16 +337,11 @@ bool DrmConnector::updateProperties() } // parse edid - auto edidProp = getProp(PropertyIndex::Edid); - if (edidProp) { - DrmScopedPointer blob(drmModeGetPropertyBlob(gpu()->fd(), edidProp->current())); - if (blob && blob->data) { - m_edid = Edid(blob->data, blob->length); - if (!m_edid.isValid()) { - qCWarning(KWIN_DRM) << "Couldn't parse EDID for connector" << this; - } + if (const auto edidProp = getProp(PropertyIndex::Edid); edidProp && edidProp->immutableBlob()) { + m_edid = Edid(edidProp->immutableBlob()->data, edidProp->immutableBlob()->length); + if (!m_edid.isValid()) { + qCWarning(KWIN_DRM) << "Couldn't parse EDID for connector" << this; } - deleteProp(PropertyIndex::Edid); } else { qCDebug(KWIN_DRM) << "Could not find edid for connector" << this; } diff --git a/src/backends/drm/drm_object_plane.cpp b/src/backends/drm/drm_object_plane.cpp index 6967fdd50d..c4dcca96d4 100644 --- a/src/backends/drm/drm_object_plane.cpp +++ b/src/backends/drm/drm_object_plane.cpp @@ -73,29 +73,26 @@ bool DrmPlane::init() checkSupport(5, Transformation::ReflectY); // 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()) { - DrmScopedPointer propertyBlob(drmModeGetPropertyBlob(gpu()->fd(), formatProp->current())); - if (propertyBlob && propertyBlob->data) { - auto blob = static_cast(propertyBlob->data); - auto modifiers = reinterpret_cast(reinterpret_cast(blob) + blob->modifiers_offset); - uint32_t *formatarr = reinterpret_cast(reinterpret_cast(blob) + blob->formats_offset); - - for (uint32_t f = 0; f < blob->count_formats; f++) { - auto format = formatarr[f]; - QVector mods; - for (uint32_t m = 0; m < blob->count_modifiers; m++) { - auto modifier = &modifiers[m]; - // The modifier advertisement blob is partitioned into groups of 64 formats - if (m < modifier->offset || m > modifier->offset + 63) { - continue; - } - if (!(modifier->formats & (1 << (f - modifier->offset)))) { - continue; - } - mods << modifier->modifier; + if (const auto formatProp = getProp(PropertyIndex::In_Formats); formatProp && formatProp->immutableBlob() && gpu()->addFB2ModifiersSupported()) { + auto blob = static_cast(formatProp->immutableBlob()->data); + auto modifiers = reinterpret_cast(reinterpret_cast(blob) + blob->modifiers_offset); + uint32_t *formatarr = reinterpret_cast(reinterpret_cast(blob) + blob->formats_offset); + + for (uint32_t f = 0; f < blob->count_formats; f++) { + auto format = formatarr[f]; + QVector mods; + for (uint32_t m = 0; m < blob->count_modifiers; m++) { + auto modifier = &modifiers[m]; + // The modifier advertisement blob is partitioned into groups of 64 formats + if (m < modifier->offset || m > modifier->offset + 63) { + continue; } - m_supportedFormats.insert(format, mods); + if (!(modifier->formats & (1 << (f - modifier->offset)))) { + continue; + } + mods << modifier->modifier; } + m_supportedFormats.insert(format, mods); } } else { for (uint32_t i = 0; i < p->count_formats; i++) { diff --git a/src/backends/drm/drm_property.cpp b/src/backends/drm/drm_property.cpp index 28af8ec07f..8ab0dc0e5a 100644 --- a/src/backends/drm/drm_property.cpp +++ b/src/backends/drm/drm_property.cpp @@ -24,6 +24,7 @@ DrmProperty::DrmProperty(DrmObject *obj, drmModePropertyRes *prop, uint64_t val, , m_next(val) , m_current(val) , m_immutable(prop->flags & DRM_MODE_PROP_IMMUTABLE) + , m_isBlob(prop->flags & DRM_MODE_PROP_BLOB) , m_obj(obj) { if (!enumNames.isEmpty()) { @@ -35,6 +36,7 @@ DrmProperty::DrmProperty(DrmObject *obj, drmModePropertyRes *prop, uint64_t val, m_minValue = prop->values[0]; m_maxValue = prop->values[1]; } + updateBlob(); } DrmProperty::~DrmProperty() = default; @@ -110,7 +112,10 @@ bool DrmProperty::needsCommit() const void DrmProperty::setCurrent(uint64_t value) { - m_current = value; + if (m_current != value) { + updateBlob(); + m_current = value; + } } uint64_t DrmProperty::current() const @@ -122,10 +127,12 @@ QVector DrmProperty::enumNames() const { return m_enumNames; } + bool DrmProperty::hasEnum(uint64_t value) const { return m_enumMap.contains(value); } + bool DrmProperty::hasAllEnums() const { return m_enumMap.count() == m_enumNames.count(); @@ -166,4 +173,22 @@ uint64_t DrmProperty::maxValue() const return m_maxValue; } +void DrmProperty::updateBlob() +{ + if (m_immutable && m_isBlob) { + if (m_current != 0) { + m_immutableBlob.reset(drmModeGetPropertyBlob(m_obj->gpu()->fd(), m_current)); + if (m_immutableBlob && (!m_immutableBlob->data || !m_immutableBlob->length)) { + m_immutableBlob.reset(); + } + } else { + m_immutableBlob.reset(); + } + } +} + +drmModePropertyBlobRes *DrmProperty::immutableBlob() const +{ + return m_immutableBlob.get(); +} } diff --git a/src/backends/drm/drm_property.h b/src/backends/drm/drm_property.h index 48bdeafdc2..1ae547f41e 100644 --- a/src/backends/drm/drm_property.h +++ b/src/backends/drm/drm_property.h @@ -9,9 +9,9 @@ */ #pragma once +#include "drm_pointer.h" #include - #include #include #include @@ -59,6 +59,7 @@ public: void setCurrent(uint64_t value); uint64_t current() const; + drmModePropertyBlobRes *immutableBlob() const; uint64_t minValue() const; uint64_t maxValue() const; @@ -78,6 +79,8 @@ public: } private: + void updateBlob(); + uint32_t m_propId = 0; QByteArray m_propName; @@ -89,6 +92,7 @@ private: uint64_t m_next = 0; // the value currently set for or by the kernel uint64_t m_current = 0; + DrmScopedPointer m_immutableBlob; uint64_t m_minValue = -1; uint64_t m_maxValue = -1; @@ -96,6 +100,7 @@ private: QMap m_enumMap; QVector m_enumNames; const bool m_immutable; + const bool m_isBlob; bool m_legacy = false; const DrmObject *m_obj; }; -- GitLab