On Wed, Apr 11, 2018 at 04:22:15PM +0100, Alexandru Gheorghe wrote:
Add a resource manager object that is responsible for detecting all
kms devices and allocates unique display numbers for every detected
display.
This is controlled by the value of hwc.drm.device property, if it ends
with a %, it will try to open minor devices until and error is detected.
E.g: /dev/dri/card%
I'm a bit on the fence as to whether to use the %, or add a new
hwc.drm.scan_devices property. I guess since we need to convey the path anyways
this is fine, but it really needs to be better documented.
Additionally, this will be used for finding an available writeback
connector that will be used for the flattening of the currently
displayed scene.
Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>
---
Android.mk | 1 +
resourcemanager.cpp | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
resourcemanager.h | 29 ++++++++++++++++++++++
3 files changed, 101 insertions(+)
create mode 100644 resourcemanager.cpp
create mode 100644 resourcemanager.h
diff --git a/Android.mk b/Android.mk
index 1add286..736fe24 100644
--- a/Android.mk
+++ b/Android.mk
@@ -52,6 +52,7 @@ LOCAL_C_INCLUDES := \
LOCAL_SRC_FILES := \
autolock.cpp \
+ resourcemanager.cpp \
drmresources.cpp \
drmconnector.cpp \
drmcrtc.cpp \
diff --git a/resourcemanager.cpp b/resourcemanager.cpp
new file mode 100644
index 0000000..e7b654e
--- /dev/null
+++ b/resourcemanager.cpp
@@ -0,0 +1,71 @@
+#include "resourcemanager.h"
+#include <cutils/log.h>
+#include <cutils/properties.h>
+
+namespace android {
+
+ResourceManager::ResourceManager() : gralloc_(NULL) {
+}
+
+int ResourceManager::Init() {
+ char path_pattern[PROPERTY_VALUE_MAX];
+ property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");
We probably also don't want to default to scanning, since that might change
behavior in existing boards.
+
+ uint8_t minor = 0;
Please use unsigned/int instead of fixed-size types. Unless the number of bits
is relevant to use of the variable, let the compiler handle it.
+ int last_display_index = 0;
Could we just call this num_displays? It was kind of hard for me to reason
through this, especially when it's call "start_display_index" when you jump into
drm::Init(). I also think drm->Init's return pair should return
<ret, displays_added> instead of <ret, display_index>, so each time you call
Init(), you're adding to num_displays.
+ int last_char = strlen(path_pattern) - 1;
+ do {
+ char path[PROPERTY_VALUE_MAX];
Please use string
+ if (path_pattern[last_char] == '%') {
+ path_pattern[last_char] = '\0';
+ snprintf(path, PROPERTY_VALUE_MAX, "%s%d", path_pattern, minor);
+ path_pattern[last_char] = '%';
This doesn't work for minor > 10.
+ } else {
+ snprintf(path, PROPERTY_VALUE_MAX, "%s", path_pattern);
+ }
+ std::unique_ptr<DrmResources> drm = std::make_unique<DrmResources>();
+ last_display_index = drm->Init(this, path, last_display_index);
+ if (last_display_index < 0) {
+ break;
+ }
+ std::shared_ptr<Importer> importer;
+ importer.reset(Importer::CreateInstance(drm.get()));
+ if (!importer) {
+ ALOGE("Failed to create importer instance");
+ break;
+ }
+ importers_.push_back(importer);
+ drms_.push_back(std::move(drm));
+ minor++;
+ last_display_index++;
+ } while (path_pattern[last_char] == '%');
+
+ if (!drms_.size()) {
+ ALOGE("Failed to find any working drm device");
+ return -EINVAL;
+ }
+
+ return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
+ (const hw_module_t **)&gralloc_);
+}
Consider the following:
int ResourceManager::AddDrmDevice(std::string path) {
std::unique_ptr<DrmDevice> drm = std::make_unique<DrmDevice>();
int displays_added, ret;
std::tie(displays_added, ret) = drm->Init(path.c_str(), num_displays_);
if (ret)
return ret;
std::shared_ptr<Importer> importer;
importer.reset(Importer::CreateInstance(drm.get()));
if (!importer) {
ALOGE("Failed to create importer instance");
return -ENODEV;
}
importers_.push_back(importer);
drms_.push_back(std::move(drm));
num_displays_ += displays_added;
return 0;
}
int ResourceManager::Init() {
char path_pattern[PROPERTY_VALUE_MAX];
int path_len = property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");
if (path_pattern[path_len - 1] != '%')
return AddDrmDevice(std::string(path_pattern);
path_pattern[path_len - 1] = '\0';
for (int ret = 0, idx = 0; !ret; ++idx) {
ostringstream path;
path << path_pattern << idx;
ret = AddDrmDevice(path.str());
}
if (!num_displays_) {
ALOGE("Failed to initialize any displays");
return -EINVAL;
}
return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
(const hw_module_t **)&gralloc_);
}
I think resolves the issues from the original patches and incorporates the
suggestions of drm->Init() returning the tuple of added displays, as well as
eliminating the backpointer.
+
+DrmResources *ResourceManager::GetDrmResources(int display) {
+ for (uint32_t i = 0; i < drms_.size(); i++) {
for (auto &drm_: drms_) {
+ if (drms_[i]->HandlesDisplay(display))
+ return drms_[i].get();
+ }
+ return NULL;
+}
+
+std::shared_ptr<Importer> ResourceManager::GetImporter(int display) {
+ for (uint32_t i = 0; i < drms_.size(); i++) {
Same here
+ if (drms_[i]->HandlesDisplay(display))
+ return importers_[i];
+ }
+ return NULL;
+}
+
+const gralloc_module_t *ResourceManager::GetGralloc() {
I think this should be called gralloc()
+ return gralloc_;
+}
+}
diff --git a/resourcemanager.h b/resourcemanager.h
new file mode 100644
index 0000000..b8caa9a
--- /dev/null
+++ b/resourcemanager.h
@@ -0,0 +1,29 @@
+#ifndef RESOURCEMANAGER_H
+#define RESOURCEMANAGER_H
+
+#include "drmresources.h"
+#include "platform.h"
+
+namespace android {
+
+class DrmResources;
+class Importer;
I think you need either the forward declarations or the includes, but not both?
+
+class ResourceManager {
+ public:
+ ResourceManager();
+ ResourceManager(const ResourceManager &) = delete;
+ ResourceManager &operator=(const ResourceManager &) = delete;
+ int Init();
+ DrmResources *GetDrmResources(int display);
+ std::shared_ptr<Importer> GetImporter(int display);
+ const gralloc_module_t *GetGralloc();
+
+ private:
+ std::vector<std::unique_ptr<DrmResources>> drms_;
+ std::vector<std::shared_ptr<Importer>> importers_;
+ const gralloc_module_t *gralloc_;
+};
+}
+
+#endif // RESOURCEMANAGER_H
--
2.7.4
