drm_hwcomposer: Rework UserPropertyBlob to use RAII
Makes code simpler and leak-free.
Signed-off-by: Roman Stratiienko <roman.o.stratiienko@globallogic.com>
diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp
index 4539664..576c533 100644
--- a/compositor/DrmDisplayCompositor.cpp
+++ b/compositor/DrmDisplayCompositor.cpp
@@ -51,11 +51,6 @@
DrmDisplayCompositor::~DrmDisplayCompositor() {
if (!initialized_)
return;
- DrmDevice *drm = resource_manager_->GetDrmDevice(display_);
- if (mode_.blob_id)
- drm->DestroyPropertyBlob(mode_.blob_id);
- if (mode_.old_blob_id)
- drm->DestroyPropertyBlob(mode_.old_blob_id);
active_composition_.reset();
}
@@ -161,9 +156,9 @@
return -EINVAL;
}
- if (mode_.needs_modeset &&
+ if (mode_.blob &&
(!crtc->active_property().AtomicSet(*pset, 1) ||
- !crtc->mode_property().AtomicSet(*pset, mode_.blob_id) ||
+ !crtc->mode_property().AtomicSet(*pset, *mode_.blob) ||
!connector->crtc_id_property().AtomicSet(*pset, crtc->id()))) {
return -EINVAL;
}
@@ -210,14 +205,7 @@
}
}
- if (!test_only && mode_.needs_modeset) {
- ret = drm->DestroyPropertyBlob(mode_.old_blob_id);
- if (ret) {
- ALOGE("Failed to destroy old mode property blob %" PRIu32 "/%d",
- mode_.old_blob_id, ret);
- return ret;
- }
-
+ if (!test_only && mode_.blob) {
/* TODO: Add dpms to the pset when the kernel supports it */
ret = ApplyDpms(display_comp);
if (ret) {
@@ -226,9 +214,7 @@
}
connector->set_active_mode(mode_.mode);
- mode_.old_blob_id = mode_.blob_id;
- mode_.blob_id = 0;
- mode_.needs_modeset = false;
+ mode_.old_blob = std::move(mode_.blob);
}
if (crtc->out_fence_ptr_property()) {
@@ -256,21 +242,14 @@
return 0;
}
-std::tuple<int, uint32_t> DrmDisplayCompositor::CreateModeBlob(
- const DrmMode &mode) {
+auto DrmDisplayCompositor::CreateModeBlob(const DrmMode &mode)
+ -> DrmModeUserPropertyBlobUnique {
struct drm_mode_modeinfo drm_mode {};
mode.ToDrmModeModeInfo(&drm_mode);
- uint32_t id = 0;
DrmDevice *drm = resource_manager_->GetDrmDevice(display_);
- int ret = drm->CreatePropertyBlob(&drm_mode, sizeof(struct drm_mode_modeinfo),
- &id);
- if (ret) {
- ALOGE("Failed to create mode property blob %d", ret);
- return std::make_tuple(ret, 0);
- }
- ALOGE("Create blob_id %" PRIu32 "\n", id);
- return std::make_tuple(ret, id);
+ return drm->RegisterUserPropertyBlob(&drm_mode,
+ sizeof(struct drm_mode_modeinfo));
}
void DrmDisplayCompositor::ClearDisplay() {
@@ -327,15 +306,11 @@
return ret;
case DRM_COMPOSITION_TYPE_MODESET:
mode_.mode = composition->display_mode();
- if (mode_.blob_id)
- resource_manager_->GetDrmDevice(display_)->DestroyPropertyBlob(
- mode_.blob_id);
- std::tie(ret, mode_.blob_id) = CreateModeBlob(mode_.mode);
- if (ret) {
+ mode_.blob = CreateModeBlob(mode_.mode);
+ if (!mode_.blob) {
ALOGE("Failed to create mode blob for display %d", display_);
- return ret;
+ return -EINVAL;
}
- mode_.needs_modeset = true;
return 0;
default:
ALOGE("Unknown composition type %d", composition->type());
diff --git a/compositor/DrmDisplayCompositor.h b/compositor/DrmDisplayCompositor.h
index 9f0c0d9..3227e12 100644
--- a/compositor/DrmDisplayCompositor.h
+++ b/compositor/DrmDisplayCompositor.h
@@ -57,10 +57,9 @@
private:
struct ModeState {
- bool needs_modeset = false;
DrmMode mode;
- uint32_t blob_id = 0;
- uint32_t old_blob_id = 0;
+ DrmModeUserPropertyBlobUnique blob;
+ DrmModeUserPropertyBlobUnique old_blob;
};
DrmDisplayCompositor(const DrmDisplayCompositor &) = delete;
@@ -72,7 +71,7 @@
void ApplyFrame(std::unique_ptr<DrmDisplayComposition> composition,
int status);
- std::tuple<int, uint32_t> CreateModeBlob(const DrmMode &mode);
+ auto CreateModeBlob(const DrmMode &mode) -> DrmModeUserPropertyBlobUnique;
ResourceManager *resource_manager_;
int display_;
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index b103fd8..1753ddc 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -500,8 +500,8 @@
return -EINVAL;
}
-int DrmDevice::CreatePropertyBlob(void *data, size_t length,
- uint32_t *blob_id) const {
+auto DrmDevice::RegisterUserPropertyBlob(void *data, size_t length) const
+ -> DrmModeUserPropertyBlobUnique {
struct drm_mode_create_blob create_blob {};
create_blob.length = length;
create_blob.data = (__u64)data;
@@ -509,24 +509,21 @@
int ret = drmIoctl(fd(), DRM_IOCTL_MODE_CREATEPROPBLOB, &create_blob);
if (ret) {
ALOGE("Failed to create mode property blob %d", ret);
- return ret;
+ return DrmModeUserPropertyBlobUnique();
}
- *blob_id = create_blob.blob_id;
- return 0;
-}
-int DrmDevice::DestroyPropertyBlob(uint32_t blob_id) const {
- if (!blob_id)
- return 0;
-
- struct drm_mode_destroy_blob destroy_blob {};
- destroy_blob.blob_id = (__u32)blob_id;
- int ret = drmIoctl(fd(), DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy_blob);
- if (ret) {
- ALOGE("Failed to destroy mode property blob %" PRIu32 "/%d", blob_id, ret);
- return ret;
- }
- return 0;
+ return DrmModeUserPropertyBlobUnique(
+ new uint32_t(create_blob.blob_id), [this](const uint32_t *it) {
+ struct drm_mode_destroy_blob destroy_blob {};
+ destroy_blob.blob_id = (__u32)*it;
+ int err = drmIoctl(fd(), DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy_blob);
+ if (err != 0) {
+ ALOGE("Failed to destroy mode property blob %" PRIu32 "/%d", *it,
+ err);
+ }
+ // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
+ delete it;
+ });
}
DrmEventListener *DrmDevice::event_listener() {
diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index 9cbc7df..81c60cd 100644
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -79,8 +79,9 @@
const std::vector<std::unique_ptr<DrmCrtc>> &crtcs() const;
uint32_t next_mode_id();
- int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id) const;
- int DestroyPropertyBlob(uint32_t blob_id) const;
+ auto RegisterUserPropertyBlob(void *data, size_t length) const
+ -> DrmModeUserPropertyBlobUnique;
+
bool HandlesDisplay(int display) const;
void RegisterHotplugHandler(DrmEventHandler *handler) {
event_listener_.RegisterHotplugHandler(handler);
diff --git a/drm/DrmUnique.h b/drm/DrmUnique.h
index 4d248bf..282528b 100644
--- a/drm/DrmUnique.h
+++ b/drm/DrmUnique.h
@@ -68,6 +68,8 @@
});
}
+using DrmModeUserPropertyBlobUnique = DUniquePtr<uint32_t /*id*/>;
+
using DrmModePropertyBlobUnique = DUniquePtr<drmModePropertyBlobRes>;
auto inline MakeDrmModePropertyBlobUnique(int fd, uint32_t blob_id) {
return DrmModePropertyBlobUnique(drmModeGetPropertyBlob(fd, blob_id),