Differential D2370 Diff 6384 plugins/platforms/drm/drm_object.cpp

Changeset View

Standalone View

View Options

plugins/platforms/drm/drm_object.cpp

  • This file was added.
1​/********************************************************************
2​ KWin - the KDE window manager
3​ This file is part of the KDE project.
4​
5​Copyright (C) 2016 Roman Gilg <subdiff@gmail.com>
6​
7​This program is free software; you can redistribute it and/or modify
8​it under the terms of the GNU General Public License as published by
9​the Free Software Foundation; either version 2 of the License, or
10​(at your option) any later version.
11​
12​This program is distributed in the hope that it will be useful,
13​but WITHOUT ANY WARRANTY; without even the implied warranty of
14​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15​GNU General Public License for more details.
16​
17​You should have received a copy of the GNU General Public License
18​along with this program. If not, see <http://www.gnu.org/licenses/>.
19​*********************************************************************/
20​#include "drm_object.h"
21​#include "logging.h"
22​
23​namespace KWin
24​{
25​
26​/*
27​ * Defintions for class DrmObject
28​ */
29​
30​DrmObject::DrmObject(uint32_t object_id, int fd)
31​ : m_fd(fd)
32​ , m_id(object_id)
33​{
34​}
35​
36​DrmObject::~DrmObject()
37​{
38​ foreach(Property* p, m_props)
39​ delete p;
40​}
41​
42​void DrmObject::initProp(int n, drmModeObjectProperties *properties, QVector<QByteArray> enumNames)
43​{
44​ m_props.resize(m_propsNames.size());
45​ for (unsigned int i = 0; i < properties->count_props; ++i) {
46​ drmModePropertyRes *prop = drmModeGetProperty(m_fd, properties->props[i]);
47​ if (!prop) {
48​ continue;
49​ }
50​ if (prop->name == m_propsNames[n]) {
51​ qCDebug(KWIN_DRM).nospace() << m_id << ": " << prop->name << "' (id " << prop->prop_id
graesslinUnsubmitted
Done

I think you don't need the explicit conversion to QByteArray - or you could use qstrcmp.

graesslin: I think you don't need the explicit conversion to QByteArray - or you could use qstrcmp.
52​ << "): " << properties->prop_values[i];
53​ m_props[n] = new Property(prop, properties->prop_values[i], enumNames);
54​ }
55​ drmModeFreeProperty(prop);
56​ }
57​}
58​
59​void DrmObject::setPropValue(int index, uint64_t new_value)
60​{
61​ Q_ASSERT(index < m_props.size());
62​ m_props[index]->setValue(new_value);
graesslinUnsubmitted
Done

nitpick: coding style

graesslin: nitpick: coding style
63​ return;
64​}
65​
graesslinUnsubmitted
Done

maybe remove the debug spam instead of commenting :-)

graesslin: maybe remove the debug spam instead of commenting :-)
romanggAuthorUnsubmitted
Done

Fine! But watching the changing property values on repaints can be really helpful and now I have to remember where to put it in emergency case. Also I found my comment line to be very edgy. :D

romangg: Fine! But watching the changing property values on repaints can be really helpful and now I…
66​bool DrmObject::atomicAddProperty(drmModeAtomicReq *req, int prop, uint64_t value)
67​{
68​ uint32_t mask = 1U << prop;
69​ if ((m_propsPending | m_propsValid) & mask && value == propValue(prop)) {
70​ // no change necessary, don't add property for next atomic commit
71​ return true;
72​ }
73​ if (drmModeAtomicAddProperty(req, m_id, m_props[prop]->propId(), value) < 0) {
74​ // error when adding property
75​ return false;
76​ }
77​ m_propsPending |= mask;
78​ m_propsValid &= ~mask;
79​ // adding property was successful
80​ return true;
81​}
82​
83​/*
84​ * Defintions for struct Prop
85​ */
86​
87​DrmObject::Property::Property(drmModePropertyRes *prop, uint64_t val, QVector<QByteArray> enumNames)
88​ : m_propId(prop->prop_id)
89​ , m_propName(prop->name)
90​ , m_value(val)
91​{
92​ if (!enumNames.isEmpty()) {
93​ qCDebug(KWIN_DRM) << m_propName << " has enums:" << enumNames;
graesslinUnsubmitted
Done

nitpick: missing whitespaces

graesslin: nitpick: missing whitespaces
94​ m_enumNames = enumNames;
95​ initEnumMap(prop);
96​ }
97​}
98​
99​DrmObject::Property::~Property() = default;
100​
graesslinUnsubmitted
Done

just as a hint: you can simplify that as:

DrmObject::Prop::~Prop() = default;
graesslin: just as a hint: you can simplify that as: DrmObject::Prop::~Prop() = default;
romanggAuthorUnsubmitted
Done

Awesome!

romangg: Awesome!
101​void DrmObject::Property::initEnumMap(drmModePropertyRes *prop)
102​{
103​ if (!(prop->flags & DRM_MODE_PROP_ENUM) || prop->count_enums < 1) {
104​ qCWarning(KWIN_DRM) << "Property '" << prop->name << "' ( id ="
105​ << m_propId << ") should be enum valued, but it is not.";
106​ return;
107​ }
108​
109​ int nameCount = m_enumNames.size();
110​ m_enumMap.resize(nameCount);
111​
112​ qCDebug(KWIN_DRM).nospace() << "Test all " << prop->count_enums <<
113​ " possible enums" <<":";
114​
115​ for (int i = 0; i < prop->count_enums; i++) {
116​
117​ struct drm_mode_property_enum *en = &prop->enums[i];
118​ int j = 0;
119​
120​ while (QByteArray(en->name) != m_enumNames[j]) {
121​ j++;
122​ if (j == nameCount) {
123​ break;
graesslinUnsubmitted
Done

can we be sure this loop terminates?

graesslin: can we be sure this loop terminates?
romanggAuthorUnsubmitted
Done
  • nameCount = m_enumNames.size()
  • initEnumMap called <=> !enumNames.isEmpty()
  • So nameCount >= 1
  • On j== nameCount the loop breaks, whereas 1<=j<=nameCount
romangg: - nameCount = m_enumNames.size() - initEnumMap called <=> !enumNames.isEmpty() - So nameCount…
124​ }
125​ }
graesslinUnsubmitted
Done

nitpick: whitespaces and missing braces

graesslin: nitpick: whitespaces and missing braces
126​
127​ if (j == nameCount) {
128​ qCWarning(KWIN_DRM).nospace() << m_propName << " has unrecognized enum '" << en->name << "'";
129​ } else {
130​ qCDebug(KWIN_DRM).nospace() << "Enum '"
131​ << en->name << "': runtime-value = " << en->value;
132​ m_enumMap[j] = en->value;
133​ }
134​ }
135​
136​ if (KWIN_DRM().isDebugEnabled()) {
137​ for (int i = 0; i < m_enumMap.size(); i++) {
138​ if (m_value == m_enumMap[i]) {
139​ qCDebug(KWIN_DRM) << "=>" << m_propName << "with mapped enum value" << m_enumNames[i];
graesslinUnsubmitted
Done

do we need this loop? All it does is print out debug values...

graesslin: do we need this loop? All it does is print out debug values...
romanggAuthorUnsubmitted
Done

This is meant by the TODO above. I wanted to do something like:

if (debug) {
    for (...) {
    ...
    }
}

But I didn't succeed in this. The problem was, that, although QLoggingCategory has an isEnabled() method, I couldn't apply it on the macros in logging.h in some meaningful way. If you know otherwise, please tell me.

In general the debug statement is really useful, since otherwise you wouldn't know which enum value the object has.
On the other side if there is no easy solution it's also fine with me, if we simply remove the loop.

romangg: This is meant by the TODO above. I wanted to do something like: ``` if (debug) { for (...)…
graesslinUnsubmitted
Done

Apparently there is QLoggingCategory::isDebugEnabled which should allow exactly that.

graesslin: Apparently there is QLoggingCategory::isDebugEnabled which should allow exactly that.
140​ }
graesslinUnsubmitted
Done

nitpick whitespaces.

graesslin: nitpick whitespaces.
141​ }
142​ }
143​}
144​
145​}