diff --git a/plugins/platforms/drm/drm_object.h b/plugins/platforms/drm/drm_object.h --- a/plugins/platforms/drm/drm_object.h +++ b/plugins/platforms/drm/drm_object.h @@ -17,8 +17,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . *********************************************************************/ -#ifndef KWIN_DRM_OBJECT_H -#define KWIN_DRM_OBJECT_H +#pragma once #include #include @@ -36,11 +35,18 @@ class DrmObject { public: - // creates drm object by its id delivered by the kernel + /** + * Create DRM object representation. + * @param object_id provided by the kernel + * @param fd of the DRM device + */ DrmObject(uint32_t object_id, int fd); - virtual ~DrmObject(); + /** + * Must be called to query necessary data directly after creation. + * @return true when initializing was successful + */ virtual bool atomicInit() = 0; uint32_t id() const { @@ -54,32 +60,37 @@ m_output = output; } - bool propHasEnum(int prop, uint64_t value) const { - auto property = m_props.at(prop); - return property ? property->hasEnum(value) : false; - } - - void setValue(int prop, uint64_t new_value) - { - Q_ASSERT(prop < m_props.size()); - auto property = m_props.at(prop); - if (property) { - property->setValue(new_value); - } - } - int fd() const { return m_fd; } - virtual bool atomicPopulate(drmModeAtomicReq *req); + /** + * Populate an atomic request with data of this object. + * @param req the atomic request + * @return true when the request was successfully populated + */ + virtual bool atomicPopulate(drmModeAtomicReq *req) const; + + void setValue(int prop, uint64_t new_value); + bool propHasEnum(int prop, uint64_t value) const; protected: - virtual bool initProps() = 0; // only derived classes know names and quantity of properties + /** + * Initialize properties of object. Only derived classes know names and quantities of + * properties. + * + * @return true when properties have been initialized successfully + */ + virtual bool initProps() = 0; + void setPropertyNames(QVector &&vector); - void initProp(int n, drmModeObjectProperties *properties, QVector enumNames = QVector(0)); + void initProp(int n, drmModeObjectProperties *properties, + QVector enumNames = QVector(0)); + + bool doAtomicPopulate(drmModeAtomicReq *req, int firstProperty) const; + class Property; - bool atomicAddProperty(drmModeAtomicReq *req, Property *property); + bool atomicAddProperty(drmModeAtomicReq *req, Property *property) const; int m_fd; const uint32_t m_id; @@ -96,7 +107,14 @@ void initEnumMap(drmModePropertyRes *prop); - uint64_t enumMap(int n) { + /** + * For properties of enum type the enum map identifies the kernel runtime values, + * which must be queried beforehand. + * + * @param n the index to the enum + * @return the runtime enum value corresponding with enum index @param n + */ + uint64_t enumMap(int n) const { return m_enumMap[n]; // TODO: test on index out of bounds? } bool hasEnum(uint64_t value) const { @@ -129,8 +147,5 @@ QVector m_propsNames; }; - } -#endif - diff --git a/plugins/platforms/drm/drm_object.cpp b/plugins/platforms/drm/drm_object.cpp --- a/plugins/platforms/drm/drm_object.cpp +++ b/plugins/platforms/drm/drm_object.cpp @@ -37,8 +37,9 @@ DrmObject::~DrmObject() { - foreach(Property* p, m_props) + for (auto *p : m_props) { delete p; + } } void DrmObject::setPropertyNames(QVector &&vector) @@ -50,42 +51,64 @@ void DrmObject::initProp(int n, drmModeObjectProperties *properties, QVector enumNames) { for (unsigned int i = 0; i < properties->count_props; ++i) { - DrmScopedPointer prop( - drmModeGetProperty(fd(), properties->props[i])); + DrmScopedPointer prop( drmModeGetProperty(fd(), properties->props[i]) ); if (!prop) { + qCWarning(KWIN_DRM) << "Getting property" << i << "failed"; continue; } + if (prop->name == m_propsNames[n]) { qCDebug(KWIN_DRM).nospace() << m_id << ": " << prop->name << "' (id " << prop->prop_id << "): " << properties->prop_values[i]; m_props[n] = new Property(prop.data(), properties->prop_values[i], enumNames); } } } -bool DrmObject::atomicAddProperty(drmModeAtomicReq *req, Property *property) +bool DrmObject::atomicPopulate(drmModeAtomicReq *req) const { - if (drmModeAtomicAddProperty(req, m_id, property->propId(), property->value()) <= 0) { - qCWarning(KWIN_DRM) << "Adding property" << property->name() << "to atomic commit failed for object" << this; - return false; - } - return true; + return doAtomicPopulate(req, 0); } -bool DrmObject::atomicPopulate(drmModeAtomicReq *req) +bool DrmObject::doAtomicPopulate(drmModeAtomicReq *req, int firstProperty) const { bool ret = true; - for (int i = 0; i < m_props.size(); i++) { + for (int i = firstProperty; i < m_props.size(); i++) { auto property = m_props.at(i); if (!property) { continue; } ret &= atomicAddProperty(req, property); } if (!ret) { - qCWarning(KWIN_DRM) << "Failed to populate atomic plane" << m_id; + qCWarning(KWIN_DRM) << "Failed to populate atomic object" << m_id; + return false; + } + return true; +} + +void DrmObject::setValue(int prop, uint64_t new_value) +{ + Q_ASSERT(prop < m_props.size()); + auto property = m_props.at(prop); + if (property) { + property->setValue(new_value); + } +} + +bool DrmObject::propHasEnum(int prop, uint64_t value) const +{ + auto property = m_props.at(prop); + return property ? property->hasEnum(value) : false; +} + +bool DrmObject::atomicAddProperty(drmModeAtomicReq *req, Property *property) const +{ + if (drmModeAtomicAddProperty(req, m_id, property->propId(), property->value()) <= 0) { + qCWarning(KWIN_DRM) << "Adding property" << property->name() + << "to atomic commit failed for object" << this; return false; } return true; @@ -101,7 +124,7 @@ , m_value(val) { if (!enumNames.isEmpty()) { - qCDebug(KWIN_DRM) << m_propName << " has enums:" << enumNames; + qCDebug(KWIN_DRM) << m_propName << " can have enums:" << enumNames; m_enumNames = enumNames; initEnumMap(prop); } @@ -111,43 +134,48 @@ void DrmObject::Property::initEnumMap(drmModePropertyRes *prop) { - if (!((prop->flags & DRM_MODE_PROP_ENUM) || (prop->flags & DRM_MODE_PROP_BITMASK)) || prop->count_enums < 1) { + if ( ( !(prop->flags & DRM_MODE_PROP_ENUM) && !(prop->flags & DRM_MODE_PROP_BITMASK) ) + || prop->count_enums < 1 ) { qCWarning(KWIN_DRM) << "Property '" << prop->name << "' ( id =" << m_propId << ") should be enum valued, but it is not."; return; } - int nameCount = m_enumNames.size(); + const int nameCount = m_enumNames.size(); m_enumMap.resize(nameCount); - qCDebug(KWIN_DRM).nospace() << "Test all " << prop->count_enums << - " possible enums" <<":"; + qCDebug(KWIN_DRM).nospace() << "Available are " << prop->count_enums << + " enums. Query their runtime values:"; for (int i = 0; i < prop->count_enums; i++) { - struct drm_mode_property_enum *en = &prop->enums[i]; int j = 0; while (QByteArray(en->name) != m_enumNames[j]) { j++; if (j == nameCount) { + qCWarning(KWIN_DRM).nospace() << m_propName << " has unrecognized enum '" + << en->name << "'"; break; } } - if (j == nameCount) { - qCWarning(KWIN_DRM).nospace() << m_propName << " has unrecognized enum '" << en->name << "'"; - } else { - qCDebug(KWIN_DRM).nospace() << "Enum '" - << en->name << "': runtime-value = " << en->value; + if (j < nameCount) { + qCDebug(KWIN_DRM).nospace() << "Enum '" << en->name + << "': runtime-value = " << en->value; m_enumMap[j] = en->value; } } if (KWIN_DRM().isDebugEnabled()) { for (int i = 0; i < m_enumMap.size(); i++) { if (m_value == m_enumMap[i]) { - qCDebug(KWIN_DRM) << "=>" << m_propName << "with mapped enum value" << m_enumNames[i]; + // TODO: This does not work with bitmask properties, because from kernel we get the + // values for some reason as the shift distance instead of the full value. + // See: https://github.com/torvalds/linux/blob/6794862a/drivers/ + // gpu/drm/drm_blend.c#L267 + qCDebug(KWIN_DRM) << "=>" << m_propName + << "with mapped enum value" << m_enumNames[i]; } } } diff --git a/plugins/platforms/drm/drm_object_plane.h b/plugins/platforms/drm/drm_object_plane.h --- a/plugins/platforms/drm/drm_object_plane.h +++ b/plugins/platforms/drm/drm_object_plane.h @@ -17,11 +17,10 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . *********************************************************************/ -#ifndef KWIN_DRM_OBJECT_PLANE_H -#define KWIN_DRM_OBJECT_PLANE_H +#pragma once #include "drm_object.h" -// drm + #include namespace KWin @@ -60,12 +59,12 @@ }; enum class Transformation { - Rotate0 = 1 << 0, - Rotate90 = 1 << 1, - Rotate180 = 1 << 2, - Rotate270 = 1 << 3, - ReflectX = 1 << 4, - ReflectY = 1 << 5 + Rotate0 = 1 << 0, + Rotate90 = 1 << 1, + Rotate180 = 1 << 2, + Rotate270 = 1 << 3, + ReflectX = 1 << 4, + ReflectY = 1 << 5 }; Q_DECLARE_FLAGS(Transformations, Transformation); @@ -93,14 +92,15 @@ void setTransformation(Transformations t); Transformations transformation(); - bool atomicPopulate(drmModeAtomicReq *req) override; void flipBuffer(); void flipBufferWithDelete(); Transformations supportedTransformations() const { return m_supportedTransformations; } + bool atomicPopulate(drmModeAtomicReq *req) const override; + private: DrmBuffer *m_current = nullptr; DrmBuffer *m_next = nullptr; @@ -118,5 +118,3 @@ Q_DECLARE_OPERATORS_FOR_FLAGS(KWin::DrmPlane::Transformations) -#endif - diff --git a/plugins/platforms/drm/drm_object_plane.cpp b/plugins/platforms/drm/drm_object_plane.cpp --- a/plugins/platforms/drm/drm_object_plane.cpp +++ b/plugins/platforms/drm/drm_object_plane.cpp @@ -60,6 +60,11 @@ return true; } +bool DrmPlane::atomicPopulate(drmModeAtomicReq *req) const +{ + return doAtomicPopulate(req, 1); +} + bool DrmPlane::initProps() { setPropertyNames( { @@ -106,20 +111,22 @@ } else if (j == int(PropertyIndex::Rotation)) { initProp(j, properties.data(), rotationNames); m_supportedTransformations = Transformations(); - auto testTransform = [j, this] (uint64_t value, Transformation t, const QString &name) { + + auto checkSupport = [j, this] (uint64_t value, Transformation t, const QString &name) { if (propHasEnum(j, value)) { qCDebug(KWIN_DRM) << name; m_supportedTransformations |= t; } }; qCDebug(KWIN_DRM).nospace() << "Supported Transformations on plane " << m_id << ":"; - testTransform(0, Transformation::Rotate0, "rotate-0"); - testTransform(1, Transformation::Rotate90, "rotate-90"); - testTransform(2, Transformation::Rotate180, "rotate-180"); - testTransform(3, Transformation::Rotate270, "rotate-270"); - testTransform(4, Transformation::ReflectX, "reflect-x-axis"); - testTransform(5, Transformation::ReflectY, "reflect-y-axis"); + checkSupport(0, Transformation::Rotate0, "rotate-0"); + checkSupport(1, Transformation::Rotate90, "rotate-90"); + checkSupport(2, Transformation::Rotate180, "rotate-180"); + checkSupport(3, Transformation::Rotate270, "rotate-270"); + checkSupport(4, Transformation::ReflectX, "reflect-x"); + checkSupport(5, Transformation::ReflectY, "reflect-y"); qCDebug(KWIN_DRM) << ""; + } else { initProp(j, properties.data()); } @@ -153,6 +160,8 @@ void DrmPlane::setTransformation(Transformations t) { + // TODO: When being pedantic, this should go through the enum mapping. Just remember + // that these are the shift distance, not the shifted value. if (auto property = m_props.at(int(PropertyIndex::Rotation))) { property->setValue(int(t)); } @@ -166,25 +175,6 @@ return Transformations(Transformation::Rotate0); } -bool DrmPlane::atomicPopulate(drmModeAtomicReq *req) -{ - bool ret = true; - - for (int i = 1; i < m_props.size(); i++) { - auto property = m_props.at(i); - if (!property) { - continue; - } - ret &= atomicAddProperty(req, property); - } - - if (!ret) { - qCWarning(KWIN_DRM) << "Failed to populate atomic plane" << m_id; - return false; - } - return true; -} - void DrmPlane::flipBuffer() { m_current = m_next;