backends/drm: Add a check for ranged properties

Include a warning if we are ever setting an invalid value, this can save
some of us quite some time in the future, since failing to fall in the
range makes the whole pipeline become rejected.
wilder/Plasma/6.2
Aleix Pol Gonzalez 2 years ago committed by Aleix Pol Gonzalez
parent 0db46e3966
commit d55fa39ebf
  1. 1
      src/backends/drm/drm_commit.cpp
  2. 26
      src/backends/drm/drm_property.cpp
  3. 15
      src/backends/drm/drm_property.h

@ -44,6 +44,7 @@ DrmAtomicCommit::DrmAtomicCommit(const QList<DrmPipeline *> &pipelines)
void DrmAtomicCommit::addProperty(const DrmProperty &prop, uint64_t value) void DrmAtomicCommit::addProperty(const DrmProperty &prop, uint64_t value)
{ {
prop.checkValueInRange(value);
m_properties[prop.drmObject()->id()][prop.propId()] = value; m_properties[prop.drmObject()->id()][prop.propId()] = value;
} }

@ -42,9 +42,7 @@ void DrmProperty::update(DrmPropertyList &propertyList)
const auto &[prop, value] = *opt; const auto &[prop, value] = *opt;
m_propId = prop->prop_id; m_propId = prop->prop_id;
m_current = value; m_current = value;
m_immutable = prop->flags & DRM_MODE_PROP_IMMUTABLE; m_flags = prop->flags;
m_isBlob = prop->flags & DRM_MODE_PROP_BLOB;
m_isBitmask = prop->flags & DRM_MODE_PROP_BITMASK;
if (prop->flags & DRM_MODE_PROP_RANGE) { if (prop->flags & DRM_MODE_PROP_RANGE) {
Q_ASSERT(prop->count_values > 1); Q_ASSERT(prop->count_values > 1);
m_minValue = prop->values[0]; m_minValue = prop->values[0];
@ -58,7 +56,7 @@ void DrmProperty::update(DrmPropertyList &propertyList)
struct drm_mode_property_enum *en = &prop->enums[i]; struct drm_mode_property_enum *en = &prop->enums[i];
int j = m_enumNames.indexOf(QByteArray(en->name)); int j = m_enumNames.indexOf(QByteArray(en->name));
if (j >= 0) { if (j >= 0) {
if (m_isBitmask) { if (m_flags & DRM_MODE_PROP_BITMASK) {
m_enumToPropertyMap[1 << j] = 1 << en->value; m_enumToPropertyMap[1 << j] = 1 << en->value;
m_propertyToEnumMap[1 << en->value] = 1 << j; m_propertyToEnumMap[1 << en->value] = 1 << j;
} else { } else {
@ -68,7 +66,7 @@ void DrmProperty::update(DrmPropertyList &propertyList)
} }
} }
} }
if (m_immutable && m_isBlob) { if ((m_flags & DRM_MODE_PROP_IMMUTABLE) && (m_flags & DRM_MODE_PROP_BLOB)) {
if (m_current != 0) { if (m_current != 0) {
m_immutableBlob.reset(drmModeGetPropertyBlob(m_obj->gpu()->fd(), m_current)); m_immutableBlob.reset(drmModeGetPropertyBlob(m_obj->gpu()->fd(), m_current));
if (m_immutableBlob && (!m_immutableBlob->data || !m_immutableBlob->length)) { if (m_immutableBlob && (!m_immutableBlob->data || !m_immutableBlob->length)) {
@ -108,12 +106,17 @@ const QByteArray &DrmProperty::name() const
bool DrmProperty::isImmutable() const bool DrmProperty::isImmutable() const
{ {
return m_immutable; return m_flags & DRM_MODE_PROP_IMMUTABLE;
} }
bool DrmProperty::isBitmask() const bool DrmProperty::isBitmask() const
{ {
return m_isBitmask; return m_flags & DRM_MODE_PROP_BITMASK;
}
uint64_t DrmProperty::maxValue() const
{
return m_maxValue;
} }
uint64_t DrmProperty::minValue() const uint64_t DrmProperty::minValue() const
@ -121,9 +124,14 @@ uint64_t DrmProperty::minValue() const
return m_minValue; return m_minValue;
} }
uint64_t DrmProperty::maxValue() const void DrmProperty::checkValueInRange(uint64_t value) const
{ {
return m_maxValue; if (Q_UNLIKELY(m_flags & DRM_MODE_PROP_RANGE && (value > m_maxValue || value < m_minValue))) {
qCWarning(KWIN_DRM) << "Range property value out of bounds." << m_propName << " value:" << value << "min:" << m_minValue << "max:" << m_maxValue;
}
if (Q_UNLIKELY(m_flags & DRM_MODE_PROP_SIGNED_RANGE && (int64_t(value) > int64_t(m_maxValue) || int64_t(value) < int64_t(m_minValue)))) {
qCWarning(KWIN_DRM) << "Signed range property value out of bounds." << m_propName << " value:" << int64_t(value) << "min:" << int64_t(m_minValue) << "max:" << int64_t(m_maxValue);
}
} }
drmModePropertyBlobRes *DrmProperty::immutableBlob() const drmModePropertyBlobRes *DrmProperty::immutableBlob() const

@ -42,6 +42,11 @@ public:
uint64_t maxValue() const; uint64_t maxValue() const;
bool isValid() const; bool isValid() const;
/**
* Prints a warning and returns false if @p value is unacceptable for the property
*/
void checkValueInRange(uint64_t value) const;
void update(DrmPropertyList &propertyList); void update(DrmPropertyList &propertyList);
bool setPropertyLegacy(uint64_t value); bool setPropertyLegacy(uint64_t value);
@ -60,9 +65,7 @@ protected:
QMap<uint64_t, uint64_t> m_enumToPropertyMap; QMap<uint64_t, uint64_t> m_enumToPropertyMap;
QMap<uint64_t, uint64_t> m_propertyToEnumMap; QMap<uint64_t, uint64_t> m_propertyToEnumMap;
bool m_immutable = false; uint32_t m_flags;
bool m_isBlob = false;
bool m_isBitmask = false;
}; };
template<typename Enum> template<typename Enum>
@ -82,7 +85,7 @@ public:
bool hasEnum(Enum value) const bool hasEnum(Enum value) const
{ {
const uint64_t integerValue = static_cast<uint64_t>(value); const uint64_t integerValue = static_cast<uint64_t>(value);
if (m_isBitmask) { if (isBitmask()) {
for (uint64_t mask = 1; integerValue >= mask && mask != 0; mask <<= 1) { for (uint64_t mask = 1; integerValue >= mask && mask != 0; mask <<= 1) {
if ((integerValue & mask) && !m_enumToPropertyMap.contains(mask)) { if ((integerValue & mask) && !m_enumToPropertyMap.contains(mask)) {
return false; return false;
@ -96,7 +99,7 @@ public:
Enum enumForValue(uint64_t value) const Enum enumForValue(uint64_t value) const
{ {
if (m_isBitmask) { if (isBitmask()) {
uint64_t ret = 0; uint64_t ret = 0;
for (uint64_t mask = 1; value >= mask && mask != 0; mask <<= 1) { for (uint64_t mask = 1; value >= mask && mask != 0; mask <<= 1) {
if (value & mask) { if (value & mask) {
@ -112,7 +115,7 @@ public:
uint64_t valueForEnum(Enum enumValue) const uint64_t valueForEnum(Enum enumValue) const
{ {
const uint64_t integer = static_cast<uint64_t>(enumValue); const uint64_t integer = static_cast<uint64_t>(enumValue);
if (m_isBitmask) { if (isBitmask()) {
uint64_t set = 0; uint64_t set = 0;
for (uint64_t mask = 1; integer >= mask && mask != 0; mask <<= 1) { for (uint64_t mask = 1; integer >= mask && mask != 0; mask <<= 1) {
if (integer & mask) { if (integer & mask) {

Loading…
Cancel
Save