drm_hwcomposer: Cleanup DRM atomic commit
Create and use DrmPlane::AtomicSet() wrapper.
Signed-off-by: Roman Stratiienko <roman.o.stratiienko@globallogic.com>
diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp
index 142f574..bcb90cf 100644
--- a/compositor/DrmDisplayCompositor.cpp
+++ b/compositor/DrmDisplayCompositor.cpp
@@ -177,13 +177,9 @@
->composition_planes();
for (DrmCompositionPlane &comp_plane : comp_planes) {
DrmPlane *plane = comp_plane.plane();
- ret = drmModeAtomicAddProperty(pset.get(), plane->id(),
- plane->crtc_property().id(), 0) < 0 ||
- drmModeAtomicAddProperty(pset.get(), plane->id(),
- plane->fb_property().id(), 0) < 0;
- if (ret) {
- ALOGE("Failed to add plane %d disable to pset", plane->id());
- return ret;
+ if (!plane->crtc_property().AtomicSet(*pset, 0) ||
+ !plane->fb_property().AtomicSet(*pset, 0)) {
+ return -EINVAL;
}
}
DrmDevice *drm = resource_manager_->GetDrmDevice(display_);
@@ -219,40 +215,23 @@
return -ENODEV;
}
- auto pset_unique = MakeDrmModeAtomicReqUnique();
- auto *pset = pset_unique.get();
+ auto pset = MakeDrmModeAtomicReqUnique();
if (!pset) {
ALOGE("Failed to allocate property set");
return -ENOMEM;
}
- if (crtc->out_fence_ptr_property().id() != 0) {
- ret = drmModeAtomicAddProperty(pset, crtc->id(),
- crtc->out_fence_ptr_property().id(),
- (uint64_t)&out_fences[crtc->pipe()]);
- if (ret < 0) {
- ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret);
- return ret;
- }
+ if (crtc->out_fence_ptr_property() &&
+ !crtc->out_fence_ptr_property()
+ .AtomicSet(*pset, (uint64_t)&out_fences[crtc->pipe()])) {
+ return -EINVAL;
}
- if (mode_.needs_modeset) {
- ret = drmModeAtomicAddProperty(pset, crtc->id(),
- crtc->active_property().id(), 1);
- if (ret < 0) {
- ALOGE("Failed to add crtc active to pset\n");
- return ret;
- }
-
- ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(),
- mode_.blob_id) < 0 ||
- drmModeAtomicAddProperty(pset, connector->id(),
- connector->crtc_id_property().id(),
- crtc->id()) < 0;
- if (ret) {
- ALOGE("Failed to add blob %d to pset", mode_.blob_id);
- return ret;
- }
+ if (mode_.needs_modeset &&
+ (!crtc->active_property().AtomicSet(*pset, 1) ||
+ !crtc->mode_property().AtomicSet(*pset, mode_.blob_id) ||
+ !connector->crtc_id_property().AtomicSet(*pset, crtc->id()))) {
+ return -EINVAL;
}
for (DrmCompositionPlane &comp_plane : comp_planes) {
@@ -292,7 +271,7 @@
source_crop = layer.source_crop;
alpha = layer.alpha;
- if (plane->blend_property().id()) {
+ if (plane->blend_property()) {
switch (layer.blending) {
case DrmHwcBlending::kPreMult:
std::tie(blend, ret) = plane->blend_property().GetEnumValueWithName(
@@ -310,19 +289,14 @@
}
}
- if (plane->zpos_property().id() &&
- !plane->zpos_property().is_immutable()) {
+ if (plane->zpos_property() && !plane->zpos_property().is_immutable()) {
uint64_t min_zpos = 0;
// Ignore ret and use min_zpos as 0 by default
std::tie(std::ignore, min_zpos) = plane->zpos_property().range_min();
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->zpos_property().id(),
- source_layers.front() + min_zpos) < 0;
- if (ret) {
- ALOGE("Failed to add zpos property %d to plane %d",
- plane->zpos_property().id(), plane->id());
+ if (!plane->zpos_property().AtomicSet(*pset, source_layers.front() +
+ min_zpos)) {
break;
}
}
@@ -342,19 +316,12 @@
rotation |= DRM_MODE_ROTATE_0;
if (fence_fd >= 0) {
- uint32_t prop_id = plane->in_fence_fd_property().id();
- if (prop_id == 0) {
- ALOGE("Failed to get IN_FENCE_FD property id");
- break;
- }
- ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd);
- if (ret < 0) {
- ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret);
+ if (!plane->in_fence_fd_property().AtomicSet(*pset, fence_fd)) {
break;
}
}
- if (plane->color_encoding_propery().id()) {
+ if (plane->color_encoding_propery()) {
switch (layer.dataspace & HAL_DATASPACE_STANDARD_MASK) {
case HAL_DATASPACE_STANDARD_BT709:
std::tie(color_encoding,
@@ -378,7 +345,7 @@
}
}
- if (plane->color_range_property().id()) {
+ if (plane->color_range_property()) {
switch (layer.dataspace & HAL_DATASPACE_RANGE_MASK) {
case HAL_DATASPACE_RANGE_FULL:
std::tie(color_range,
@@ -396,103 +363,60 @@
// Disable the plane if there's no framebuffer
if (fb_id == UINT32_MAX) {
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->crtc_property().id(), 0) < 0 ||
- drmModeAtomicAddProperty(pset, plane->id(),
- plane->fb_property().id(), 0) < 0;
- if (ret) {
- ALOGE("Failed to add plane %d disable to pset", plane->id());
+ if (!plane->crtc_property().AtomicSet(*pset, 0) ||
+ !plane->fb_property().AtomicSet(*pset, 0)) {
break;
}
continue;
}
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->crtc_property().id(), crtc->id()) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->fb_property().id(), fb_id) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->crtc_x_property().id(),
- display_frame.left) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->crtc_y_property().id(),
- display_frame.top) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->crtc_w_property().id(),
- display_frame.right - display_frame.left) <
- 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->crtc_h_property().id(),
- display_frame.bottom - display_frame.top) <
- 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->src_x_property().id(),
- (int)(source_crop.left) << 16) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->src_y_property().id(),
- (int)(source_crop.top) << 16) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->src_w_property().id(),
- (int)(source_crop.right - source_crop.left)
- << 16) < 0;
- ret |= drmModeAtomicAddProperty(pset, plane->id(),
- plane->src_h_property().id(),
- (int)(source_crop.bottom - source_crop.top)
- << 16) < 0;
- if (ret) {
- ALOGE("Failed to add plane %d to set", plane->id());
+ if (!plane->crtc_property().AtomicSet(*pset, crtc->id()) ||
+ !plane->fb_property().AtomicSet(*pset, fb_id) ||
+ !plane->crtc_x_property().AtomicSet(*pset, display_frame.left) ||
+ !plane->crtc_y_property().AtomicSet(*pset, display_frame.top) ||
+ !plane->crtc_w_property().AtomicSet(*pset, display_frame.right -
+ display_frame.left) ||
+ !plane->crtc_h_property().AtomicSet(*pset, display_frame.bottom -
+ display_frame.top) ||
+ !plane->src_x_property().AtomicSet(*pset, (int)(source_crop.left)
+ << 16) ||
+ !plane->src_y_property().AtomicSet(*pset, (int)(source_crop.top)
+ << 16) ||
+ !plane->src_w_property()
+ .AtomicSet(*pset, (int)(source_crop.right - source_crop.left)
+ << 16) ||
+ !plane->src_h_property()
+ .AtomicSet(*pset, (int)(source_crop.bottom - source_crop.top)
+ << 16)) {
break;
}
- if (plane->rotation_property().id()) {
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->rotation_property().id(),
- rotation) < 0;
- if (ret) {
- ALOGE("Failed to add rotation property %d to plane %d",
- plane->rotation_property().id(), plane->id());
+ if (plane->rotation_property()) {
+ if (!plane->rotation_property().AtomicSet(*pset, rotation)) {
break;
}
}
- if (plane->alpha_property().id()) {
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->alpha_property().id(), alpha) < 0;
- if (ret) {
- ALOGE("Failed to add alpha property %d to plane %d",
- plane->alpha_property().id(), plane->id());
+ if (plane->alpha_property()) {
+ if (!plane->alpha_property().AtomicSet(*pset, alpha)) {
break;
}
}
- if (plane->blend_property().id() && blend != UINT64_MAX) {
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->blend_property().id(), blend) < 0;
- if (ret) {
- ALOGE("Failed to add pixel blend mode property %d to plane %d",
- plane->blend_property().id(), plane->id());
+ if (plane->blend_property() && blend != UINT64_MAX) {
+ if (!plane->blend_property().AtomicSet(*pset, blend)) {
break;
}
}
- if (plane->color_encoding_propery().id() && color_encoding != UINT64_MAX) {
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->color_encoding_propery().id(),
- color_encoding) < 0;
- if (ret) {
- ALOGE("Failed to add COLOR_ENCODING property %d to plane %d",
- plane->color_encoding_propery().id(), plane->id());
+ if (plane->color_encoding_propery() && color_encoding != UINT64_MAX) {
+ if (!plane->color_encoding_propery().AtomicSet(*pset, color_encoding)) {
break;
}
}
- if (plane->color_range_property().id() && color_range != UINT64_MAX) {
- ret = drmModeAtomicAddProperty(pset, plane->id(),
- plane->color_range_property().id(),
- color_range) < 0;
- if (ret) {
- ALOGE("Failed to add COLOR_RANGE property %d to plane %d",
- plane->color_range_property().id(), plane->id());
+ if (plane->color_range_property() && color_range != UINT64_MAX) {
+ if (!plane->color_range_property().AtomicSet(*pset, color_range)) {
break;
}
}
@@ -503,7 +427,7 @@
if (test_only)
flags |= DRM_MODE_ATOMIC_TEST_ONLY;
- ret = drmModeAtomicCommit(drm->fd(), pset, flags, drm);
+ ret = drmModeAtomicCommit(drm->fd(), pset.get(), flags, drm);
if (ret) {
if (!test_only)
ALOGE("Failed to commit pset ret=%d\n", ret);
@@ -532,7 +456,7 @@
mode_.needs_modeset = false;
}
- if (crtc->out_fence_ptr_property().id()) {
+ if (crtc->out_fence_ptr_property()) {
display_comp->out_fence_ = UniqueFd((int)out_fences[crtc->pipe()]);
}
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index bd4b1e4..01327f8 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -547,7 +547,7 @@
for (int i = 0; !found && (size_t)i < props->count_props; ++i) {
drmModePropertyPtr p = drmModeGetProperty(fd(), props->props[i]);
if (!strcmp(p->name, prop_name)) {
- property->Init(p, props->prop_values[i]);
+ property->Init(obj_id, p, props->prop_values[i]);
found = true;
}
drmModeFreeProperty(p);
diff --git a/drm/DrmProperty.cpp b/drm/DrmProperty.cpp
index 8e6f7e5..32f1c62 100644
--- a/drm/DrmProperty.cpp
+++ b/drm/DrmProperty.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
+#define LOG_TAG "hwc-drm-property"
+
#include "DrmProperty.h"
#include <xf86drmMode.h>
@@ -23,6 +26,7 @@
#include <string>
#include "DrmDevice.h"
+#include "utils/log.h"
namespace android {
@@ -30,11 +34,13 @@
: value_(e->value), name_(e->name) {
}
-DrmProperty::DrmProperty(drmModePropertyPtr p, uint64_t value) {
- Init(p, value);
+DrmProperty::DrmProperty(uint32_t obj_id, drmModePropertyPtr p,
+ uint64_t value) {
+ Init(obj_id, p, value);
}
-void DrmProperty::Init(drmModePropertyPtr p, uint64_t value) {
+void DrmProperty::Init(uint32_t obj_id, drmModePropertyPtr p, uint64_t value) {
+ obj_id_ = obj_id;
id_ = p->prop_id;
flags_ = p->flags;
name_ = p->name;
@@ -131,4 +137,19 @@
return std::make_tuple(UINT64_MAX, -EINVAL);
}
+
+auto DrmProperty::AtomicSet(drmModeAtomicReq &pset, uint64_t value) const
+ -> bool {
+ if (id_ == 0) {
+ ALOGE("AtomicSet() is called on non-initialized property!");
+ return false;
+ }
+ if (drmModeAtomicAddProperty(&pset, obj_id_, id_, value) < 0) {
+ ALOGE("Failed to add obj_id: %u, prop_id: %u (%s) to pset", obj_id_, id_,
+ name_.c_str());
+ return false;
+ }
+ return true;
+}
+
} // namespace android
diff --git a/drm/DrmProperty.h b/drm/DrmProperty.h
index 70678fd..8db480a 100644
--- a/drm/DrmProperty.h
+++ b/drm/DrmProperty.h
@@ -37,11 +37,11 @@
class DrmProperty {
public:
DrmProperty() = default;
- DrmProperty(drmModePropertyPtr p, uint64_t value);
+ DrmProperty(uint32_t obj_id, drmModePropertyPtr p, uint64_t value);
DrmProperty(const DrmProperty &) = delete;
DrmProperty &operator=(const DrmProperty &) = delete;
- void Init(drmModePropertyPtr p, uint64_t value);
+ auto Init(uint32_t obj_id, drmModePropertyPtr p, uint64_t value) -> void;
std::tuple<uint64_t, int> GetEnumValueWithName(const std::string &name) const;
uint32_t id() const;
@@ -54,6 +54,13 @@
std::tuple<int, uint64_t> range_min() const;
std::tuple<int, uint64_t> range_max() const;
+ [[nodiscard]] auto AtomicSet(drmModeAtomicReq &pset, uint64_t value) const
+ -> bool;
+
+ operator bool() const {
+ return id_ != 0;
+ }
+
private:
class DrmPropertyEnum {
public:
@@ -64,6 +71,7 @@
std::string name_;
};
+ uint32_t obj_id_ = 0;
uint32_t id_ = 0;
DrmPropertyType type_ = DRM_PROPERTY_TYPE_INVALID;