| Message ID | 1523460149-1740-6-git-send-email-alexandru-cosmin.gheorghe@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote: > Use the newly added ResourceManager for creating and detecting all the > drm devices instead of assuming that there is only one device. > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > --- > drmhwctwo.cpp | 34 +++++++++++++--------------------- > drmhwctwo.h | 4 +--- > drmresources.cpp | 25 ++++++++++++++++++------- > drmresources.h | 14 +++++++++++--- > 4 files changed, 43 insertions(+), 34 deletions(-) > > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp > index dfca1a6..cddd5da 100644 > --- a/drmhwctwo.cpp > +++ b/drmhwctwo.cpp > @@ -58,40 +58,32 @@ DrmHwcTwo::DrmHwcTwo() { > } > > HWC2::Error DrmHwcTwo::Init() { > - int ret = drm_.Init(); > + int ret = resource_manager_.Init(); > if (ret) { > - ALOGE("Can't initialize drm object %d", ret); > + ALOGE("Can't initialize the resource manager %d", ret); > return HWC2::Error::NoResources; > } > > - importer_.reset(Importer::CreateInstance(&drm_)); > - if (!importer_) { > - ALOGE("Failed to create importer instance"); > + DrmResources *drm = resource_manager_.GetDrmResources(HWC_DISPLAY_PRIMARY); > + std::shared_ptr<Importer> importer = > + resource_manager_.GetImporter(HWC_DISPLAY_PRIMARY); > + if (!drm || !importer) { > + ALOGE("Failed to get a valid drmresource and importer"); > return HWC2::Error::NoResources; > } > + displays_.emplace( > + std::piecewise_construct, std::forward_as_tuple(HWC_DISPLAY_PRIMARY), > + std::forward_as_tuple(drm, importer, resource_manager_.GetGralloc(), > + HWC_DISPLAY_PRIMARY, HWC2::DisplayType::Physical)); > > - ret = hw_get_module(GRALLOC_HARDWARE_MODULE_ID, > - (const hw_module_t **)&gralloc_); > - if (ret) { > - ALOGE("Failed to open gralloc module %d", ret); > - return HWC2::Error::NoResources; > - } > - > - displays_.emplace(std::piecewise_construct, > - std::forward_as_tuple(HWC_DISPLAY_PRIMARY), > - std::forward_as_tuple(&drm_, importer_, gralloc_, > - HWC_DISPLAY_PRIMARY, > - HWC2::DisplayType::Physical)); > - > - DrmCrtc *crtc = drm_.GetCrtcForDisplay(static_cast<int>(HWC_DISPLAY_PRIMARY)); > + DrmCrtc *crtc = drm->GetCrtcForDisplay(static_cast<int>(HWC_DISPLAY_PRIMARY)); > if (!crtc) { > ALOGE("Failed to get crtc for display %d", > static_cast<int>(HWC_DISPLAY_PRIMARY)); > return HWC2::Error::BadDisplay; > } > - > std::vector<DrmPlane *> display_planes; > - for (auto &plane : drm_.planes()) { > + for (auto &plane : drm->planes()) { > if (plane->GetCrtcSupported(*crtc)) > display_planes.push_back(plane.get()); > } > diff --git a/drmhwctwo.h b/drmhwctwo.h > index 0490e2a..beb5d2d 100644 > --- a/drmhwctwo.h > +++ b/drmhwctwo.h > @@ -262,9 +262,7 @@ class DrmHwcTwo : public hwc2_device_t { > HWC2::Error RegisterCallback(int32_t descriptor, hwc2_callback_data_t data, > hwc2_function_pointer_t function); > > - DrmResources drm_; > - std::shared_ptr<Importer> importer_; // Shared with HwcDisplay > - const gralloc_module_t *gralloc_; > + ResourceManager resource_manager_; > std::map<hwc2_display_t, HwcDisplay> displays_; > std::map<HWC2::Callback, HwcCallback> callbacks_; > }; > diff --git a/drmresources.cpp b/drmresources.cpp > index 32dd376..a5ddda0 100644 > --- a/drmresources.cpp > +++ b/drmresources.cpp > @@ -42,10 +42,9 @@ DrmResources::~DrmResources() { > event_listener_.Exit(); > } > > -int DrmResources::Init() { > - char path[PROPERTY_VALUE_MAX]; > - property_get("hwc.drm.device", path, "/dev/dri/card0"); > - > +int DrmResources::Init(ResourceManager *resource_manager, char *path, > + int start_display_index) { > + resource_manager_ = resource_manager; You can avoid the backpointer if you just pass the RM to the right places (looks like compositor + composition). Bonus points if you can remove drm_ from those objects once you've done that. > /* TODO: Use drmOpenControl here instead */ > fd_.Set(open(path, O_RDWR)); > if (fd() < 0) { > @@ -76,8 +75,8 @@ int DrmResources::Init() { > max_resolution_ = > std::pair<uint32_t, uint32_t>(res->max_width, res->max_height); > > - bool found_primary = false; > - int display_num = 1; > + bool found_primary = start_display_index != 0; > + int display_num = found_primary ? start_display_index : 1; This could use a comment. AFAICT, you're assuming the primary display will always be in the first drm device, which is fine, but should be explained _somewhere_. > > for (int i = 0; !ret && i < res->count_crtcs; ++i) { > drmModeCrtcPtr c = drmModeGetCrtc(fd(), res->crtcs[i]); > @@ -161,9 +160,11 @@ int DrmResources::Init() { > for (auto &conn : connectors_) { > if (conn->internal() && !found_primary) { > conn->set_display(0); > + displays_[0] = 0; > found_primary = true; > } else { > conn->set_display(display_num); > + displays_[display_num] = display_num; > ++display_num; > } > } > @@ -171,7 +172,9 @@ int DrmResources::Init() { > // Then look for primary amongst external connectors > for (auto &conn : connectors_) { > if (conn->external() && !found_primary) { > + displays_.erase(conn->display()); > conn->set_display(0); > + displays_[0] = 0; > found_primary = true; > } > } > @@ -226,7 +229,11 @@ int DrmResources::Init() { > return ret; > } > } > - return 0; > + return displays_.size() ? displays_.rbegin()->first : -EINVAL; I'd rather not change the meaning of the return value (especially without a comment somewhere to let readers know this function doesn't follow the 0 || -ERRNO convention). Consider returning a pair of ret,display. > +} > + > +bool DrmResources::HandlesDisplay(int display) const { > + return displays_.find(display) != displays_.end(); > } > > DrmConnector *DrmResources::GetConnectorForDisplay(int display) const { > @@ -349,6 +356,10 @@ DrmEventListener *DrmResources::event_listener() { > return &event_listener_; > } > > +ResourceManager *DrmResources::resource_manager() { > + return resource_manager_; > +} > + > int DrmResources::GetProperty(uint32_t obj_id, uint32_t obj_type, > const char *prop_name, DrmProperty *property) { > drmModeObjectPropertiesPtr props; > diff --git a/drmresources.h b/drmresources.h > index 4cca48c..4cdcd87 100644 > --- a/drmresources.h > +++ b/drmresources.h > @@ -17,22 +17,26 @@ > #ifndef ANDROID_DRM_H_ > #define ANDROID_DRM_H_ > > +#include <stdint.h> > #include "drmconnector.h" > #include "drmcrtc.h" > #include "drmencoder.h" > #include "drmeventlistener.h" > #include "drmplane.h" > - > -#include <stdint.h> Why this change? > +#include "platform.h" > +#include "resourcemanager.h" > > namespace android { > > +class ResourceManager; > + > class DrmResources { > public: > DrmResources(); > ~DrmResources(); > > - int Init(); > + int Init(ResourceManager *resource_manager, char *path, > + int start_display_index); > > int fd() const { > return fd_.get(); > @@ -58,6 +62,7 @@ class DrmResources { > DrmCrtc *GetCrtcForDisplay(int display) const; > DrmPlane *GetPlane(uint32_t id) const; > DrmEventListener *event_listener(); > + ResourceManager *resource_manager(); > > int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, > DrmProperty *property); > @@ -71,6 +76,7 @@ class DrmResources { > > int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); > int DestroyPropertyBlob(uint32_t blob_id); > + bool HandlesDisplay(int display) const; > > private: > int TryEncoderForDisplay(int display, DrmEncoder *enc); > @@ -90,6 +96,8 @@ class DrmResources { > > std::pair<uint32_t, uint32_t> min_resolution_; > std::pair<uint32_t, uint32_t> max_resolution_; > + std::map<int, int> displays_; > + ResourceManager *resource_manager_; > }; > } > > -- > 2.7.4 >
Hi Sean, On Mon, Apr 16, 2018 at 03:54:02PM -0400, Sean Paul wrote: > On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote: > > Use the newly added ResourceManager for creating and detecting all the > > drm devices instead of assuming that there is only one device. > > > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > > --- > > drmhwctwo.cpp | 34 +++++++++++++--------------------- > > drmhwctwo.h | 4 +--- > > drmresources.cpp | 25 ++++++++++++++++++------- > > drmresources.h | 14 +++++++++++--- > > 4 files changed, 43 insertions(+), 34 deletions(-) > > > > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp > > index dfca1a6..cddd5da 100644 > > --- a/drmhwctwo.cpp > > +++ b/drmhwctwo.cpp > > @@ -58,40 +58,32 @@ DrmHwcTwo::DrmHwcTwo() { > > } > > > > HWC2::Error DrmHwcTwo::Init() { > > - int ret = drm_.Init(); > > + int ret = resource_manager_.Init(); > > if (ret) { > > - ALOGE("Can't initialize drm object %d", ret); > > + ALOGE("Can't initialize the resource manager %d", ret); > > return HWC2::Error::NoResources; > > } > > > > - importer_.reset(Importer::CreateInstance(&drm_)); > > - if (!importer_) { > > - ALOGE("Failed to create importer instance"); > > + DrmResources *drm = resource_manager_.GetDrmResources(HWC_DISPLAY_PRIMARY); > > + std::shared_ptr<Importer> importer = > > + resource_manager_.GetImporter(HWC_DISPLAY_PRIMARY); > > + if (!drm || !importer) { > > + ALOGE("Failed to get a valid drmresource and importer"); > > return HWC2::Error::NoResources; > > } > > + displays_.emplace( > > + std::piecewise_construct, std::forward_as_tuple(HWC_DISPLAY_PRIMARY), > > + std::forward_as_tuple(drm, importer, resource_manager_.GetGralloc(), > > + HWC_DISPLAY_PRIMARY, HWC2::DisplayType::Physical)); > > > > - ret = hw_get_module(GRALLOC_HARDWARE_MODULE_ID, > > - (const hw_module_t **)&gralloc_); > > - if (ret) { > > - ALOGE("Failed to open gralloc module %d", ret); > > - return HWC2::Error::NoResources; > > - } > > - > > - displays_.emplace(std::piecewise_construct, > > - std::forward_as_tuple(HWC_DISPLAY_PRIMARY), > > - std::forward_as_tuple(&drm_, importer_, gralloc_, > > - HWC_DISPLAY_PRIMARY, > > - HWC2::DisplayType::Physical)); > > - > > - DrmCrtc *crtc = drm_.GetCrtcForDisplay(static_cast<int>(HWC_DISPLAY_PRIMARY)); > > + DrmCrtc *crtc = drm->GetCrtcForDisplay(static_cast<int>(HWC_DISPLAY_PRIMARY)); > > if (!crtc) { > > ALOGE("Failed to get crtc for display %d", > > static_cast<int>(HWC_DISPLAY_PRIMARY)); > > return HWC2::Error::BadDisplay; > > } > > - > > std::vector<DrmPlane *> display_planes; > > - for (auto &plane : drm_.planes()) { > > + for (auto &plane : drm->planes()) { > > if (plane->GetCrtcSupported(*crtc)) > > display_planes.push_back(plane.get()); > > } > > diff --git a/drmhwctwo.h b/drmhwctwo.h > > index 0490e2a..beb5d2d 100644 > > --- a/drmhwctwo.h > > +++ b/drmhwctwo.h > > @@ -262,9 +262,7 @@ class DrmHwcTwo : public hwc2_device_t { > > HWC2::Error RegisterCallback(int32_t descriptor, hwc2_callback_data_t data, > > hwc2_function_pointer_t function); > > > > - DrmResources drm_; > > - std::shared_ptr<Importer> importer_; // Shared with HwcDisplay > > - const gralloc_module_t *gralloc_; > > + ResourceManager resource_manager_; > > std::map<hwc2_display_t, HwcDisplay> displays_; > > std::map<HWC2::Callback, HwcCallback> callbacks_; > > }; > > diff --git a/drmresources.cpp b/drmresources.cpp > > index 32dd376..a5ddda0 100644 > > --- a/drmresources.cpp > > +++ b/drmresources.cpp > > @@ -42,10 +42,9 @@ DrmResources::~DrmResources() { > > event_listener_.Exit(); > > } > > > > -int DrmResources::Init() { > > - char path[PROPERTY_VALUE_MAX]; > > - property_get("hwc.drm.device", path, "/dev/dri/card0"); > > - > > +int DrmResources::Init(ResourceManager *resource_manager, char *path, > > + int start_display_index) { > > + resource_manager_ = resource_manager; > > You can avoid the backpointer if you just pass the RM to the right places (looks > like compositor + composition). Bonus points if you can remove drm_ from those > objects once you've done that. That's the thing Compositor/Composition already had drm_, hence the need of the backpointer. Didn't want to touch that as well. I suppose there is no strong reason why both Compositor & Composition shouldn't have just a ResourceManager object. > > > /* TODO: Use drmOpenControl here instead */ > > fd_.Set(open(path, O_RDWR)); > > if (fd() < 0) { > > @@ -76,8 +75,8 @@ int DrmResources::Init() { > > max_resolution_ = > > std::pair<uint32_t, uint32_t>(res->max_width, res->max_height); > > > > - bool found_primary = false; > > - int display_num = 1; > > + bool found_primary = start_display_index != 0; > > + int display_num = found_primary ? start_display_index : 1; > > This could use a comment. AFAICT, you're assuming the primary display will > always be in the first drm device, which is fine, but should be explained > _somewhere_. Will do. > > > > > for (int i = 0; !ret && i < res->count_crtcs; ++i) { > > drmModeCrtcPtr c = drmModeGetCrtc(fd(), res->crtcs[i]); > > @@ -161,9 +160,11 @@ int DrmResources::Init() { > > for (auto &conn : connectors_) { > > if (conn->internal() && !found_primary) { > > conn->set_display(0); > > + displays_[0] = 0; > > found_primary = true; > > } else { > > conn->set_display(display_num); > > + displays_[display_num] = display_num; > > ++display_num; > > } > > } > > @@ -171,7 +172,9 @@ int DrmResources::Init() { > > // Then look for primary amongst external connectors > > for (auto &conn : connectors_) { > > if (conn->external() && !found_primary) { > > + displays_.erase(conn->display()); > > conn->set_display(0); > > + displays_[0] = 0; > > found_primary = true; > > } > > } > > @@ -226,7 +229,11 @@ int DrmResources::Init() { > > return ret; > > } > > } > > - return 0; > > + return displays_.size() ? displays_.rbegin()->first : -EINVAL; > > I'd rather not change the meaning of the return value (especially without a > comment somewhere to let readers know this function doesn't follow the 0 || > -ERRNO convention). Consider returning a pair of ret,display. I avoided pair because it looks so ugly, but suppose it's better than not knowing if it's an error code or something else. I will update it to return a pair. > > > +} > > + > > +bool DrmResources::HandlesDisplay(int display) const { > > + return displays_.find(display) != displays_.end(); > > } > > > > DrmConnector *DrmResources::GetConnectorForDisplay(int display) const { > > @@ -349,6 +356,10 @@ DrmEventListener *DrmResources::event_listener() { > > return &event_listener_; > > } > > > > +ResourceManager *DrmResources::resource_manager() { > > + return resource_manager_; > > +} > > + > > int DrmResources::GetProperty(uint32_t obj_id, uint32_t obj_type, > > const char *prop_name, DrmProperty *property) { > > drmModeObjectPropertiesPtr props; > > diff --git a/drmresources.h b/drmresources.h > > index 4cca48c..4cdcd87 100644 > > --- a/drmresources.h > > +++ b/drmresources.h > > @@ -17,22 +17,26 @@ > > #ifndef ANDROID_DRM_H_ > > #define ANDROID_DRM_H_ > > > > +#include <stdint.h> > > #include "drmconnector.h" > > #include "drmcrtc.h" > > #include "drmencoder.h" > > #include "drmeventlistener.h" > > #include "drmplane.h" > > - > > -#include <stdint.h> > > Why this change? I blame clang-format-diff-3.8 -i. I suppose it should be in a different commit. > > > +#include "platform.h" > > +#include "resourcemanager.h" > > > > namespace android { > > > > +class ResourceManager; > > + > > class DrmResources { > > public: > > DrmResources(); > > ~DrmResources(); > > > > - int Init(); > > + int Init(ResourceManager *resource_manager, char *path, > > + int start_display_index); > > > > int fd() const { > > return fd_.get(); > > @@ -58,6 +62,7 @@ class DrmResources { > > DrmCrtc *GetCrtcForDisplay(int display) const; > > DrmPlane *GetPlane(uint32_t id) const; > > DrmEventListener *event_listener(); > > + ResourceManager *resource_manager(); > > > > int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, > > DrmProperty *property); > > @@ -71,6 +76,7 @@ class DrmResources { > > > > int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); > > int DestroyPropertyBlob(uint32_t blob_id); > > + bool HandlesDisplay(int display) const; > > > > private: > > int TryEncoderForDisplay(int display, DrmEncoder *enc); > > @@ -90,6 +96,8 @@ class DrmResources { > > > > std::pair<uint32_t, uint32_t> min_resolution_; > > std::pair<uint32_t, uint32_t> max_resolution_; > > + std::map<int, int> displays_; > > + ResourceManager *resource_manager_; > > }; > > } > > > > -- > > 2.7.4 > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS
On Tue, Apr 17, 2018 at 02:43:17PM +0100, Alexandru-Cosmin Gheorghe wrote: > Hi Sean, > > On Mon, Apr 16, 2018 at 03:54:02PM -0400, Sean Paul wrote: > > On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote: > > > Use the newly added ResourceManager for creating and detecting all the > > > drm devices instead of assuming that there is only one device. > > > > > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > > > --- <snip /> > > > diff --git a/drmresources.cpp b/drmresources.cpp > > > index 32dd376..a5ddda0 100644 > > > --- a/drmresources.cpp > > > +++ b/drmresources.cpp > > > @@ -42,10 +42,9 @@ DrmResources::~DrmResources() { > > > event_listener_.Exit(); > > > } > > > > > > -int DrmResources::Init() { > > > - char path[PROPERTY_VALUE_MAX]; > > > - property_get("hwc.drm.device", path, "/dev/dri/card0"); > > > - > > > +int DrmResources::Init(ResourceManager *resource_manager, char *path, > > > + int start_display_index) { > > > + resource_manager_ = resource_manager; > > > > You can avoid the backpointer if you just pass the RM to the right places (looks > > like compositor + composition). Bonus points if you can remove drm_ from those > > objects once you've done that. > > That's the thing Compositor/Composition already had drm_, hence the > need of the backpointer. Didn't want to touch that as well. I suppose > there is no strong reason why both Compositor & Composition shouldn't > have just a ResourceManager object. Yeah, exactly what I was thinking. It'll be a bit more refactoring, but worth it IMO. <snip /> > > > diff --git a/drmresources.h b/drmresources.h > > > index 4cca48c..4cdcd87 100644 > > > --- a/drmresources.h > > > +++ b/drmresources.h > > > @@ -17,22 +17,26 @@ > > > #ifndef ANDROID_DRM_H_ > > > #define ANDROID_DRM_H_ > > > > > > +#include <stdint.h> > > > #include "drmconnector.h" > > > #include "drmcrtc.h" > > > #include "drmencoder.h" > > > #include "drmeventlistener.h" > > > #include "drmplane.h" > > > - > > > -#include <stdint.h> > > > > Why this change? > > I blame clang-format-diff-3.8 -i. I suppose it should be in a different > commit. I'd rather fix it at the source. If stdint.h include needs to move, that probably means one of our headers needs to include it. So let's figure out which needs it and add it there instead of shuffling things here. Sean > > > > > +#include "platform.h" > > > +#include "resourcemanager.h" > > > > > > namespace android { > > > > > > +class ResourceManager; > > > + > > > class DrmResources { > > > public: > > > DrmResources(); > > > ~DrmResources(); > > > > > > - int Init(); > > > + int Init(ResourceManager *resource_manager, char *path, > > > + int start_display_index); > > > > > > int fd() const { > > > return fd_.get(); > > > @@ -58,6 +62,7 @@ class DrmResources { > > > DrmCrtc *GetCrtcForDisplay(int display) const; > > > DrmPlane *GetPlane(uint32_t id) const; > > > DrmEventListener *event_listener(); > > > + ResourceManager *resource_manager(); > > > > > > int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, > > > DrmProperty *property); > > > @@ -71,6 +76,7 @@ class DrmResources { > > > > > > int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); > > > int DestroyPropertyBlob(uint32_t blob_id); > > > + bool HandlesDisplay(int display) const; > > > > > > private: > > > int TryEncoderForDisplay(int display, DrmEncoder *enc); > > > @@ -90,6 +96,8 @@ class DrmResources { > > > > > > std::pair<uint32_t, uint32_t> min_resolution_; > > > std::pair<uint32_t, uint32_t> max_resolution_; > > > + std::map<int, int> displays_; > > > + ResourceManager *resource_manager_; > > > }; > > > } > > > > > > -- > > > 2.7.4 > > > > > > > -- > > Sean Paul, Software Engineer, Google / Chromium OS > > -- > Cheers, > Alex G
On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote: > Use the newly added ResourceManager for creating and detecting all the > drm devices instead of assuming that there is only one device. > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > --- <snip /> > index 4cca48c..4cdcd87 100644 > --- a/drmresources.h > +++ b/drmresources.h > @@ -17,22 +17,26 @@ > #ifndef ANDROID_DRM_H_ > #define ANDROID_DRM_H_ > > +#include <stdint.h> > #include "drmconnector.h" > #include "drmcrtc.h" > #include "drmencoder.h" > #include "drmeventlistener.h" > #include "drmplane.h" > - > -#include <stdint.h> > +#include "platform.h" > +#include "resourcemanager.h" > > namespace android { > > +class ResourceManager; > + > class DrmResources { One more thing I've been thinking about. Let's rename this to DrmDevice now that we can have more than one. It's immediately obvious what a collection of DrmDevices is, it's less obvious if they're DrmResources. I think ResourceManager is Ok to keep, but if you think there's a better name I'm open to that. Sean > public: > DrmResources(); > ~DrmResources(); > > - int Init(); > + int Init(ResourceManager *resource_manager, char *path, > + int start_display_index); > > int fd() const { > return fd_.get(); > @@ -58,6 +62,7 @@ class DrmResources { > DrmCrtc *GetCrtcForDisplay(int display) const; > DrmPlane *GetPlane(uint32_t id) const; > DrmEventListener *event_listener(); > + ResourceManager *resource_manager(); > > int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, > DrmProperty *property); > @@ -71,6 +76,7 @@ class DrmResources { > > int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); > int DestroyPropertyBlob(uint32_t blob_id); > + bool HandlesDisplay(int display) const; > > private: > int TryEncoderForDisplay(int display, DrmEncoder *enc); > @@ -90,6 +96,8 @@ class DrmResources { > > std::pair<uint32_t, uint32_t> min_resolution_; > std::pair<uint32_t, uint32_t> max_resolution_; > + std::map<int, int> displays_; > + ResourceManager *resource_manager_; > }; > } > > -- > 2.7.4 >
diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index dfca1a6..cddd5da 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -58,40 +58,32 @@ DrmHwcTwo::DrmHwcTwo() { } HWC2::Error DrmHwcTwo::Init() { - int ret = drm_.Init(); + int ret = resource_manager_.Init(); if (ret) { - ALOGE("Can't initialize drm object %d", ret); + ALOGE("Can't initialize the resource manager %d", ret); return HWC2::Error::NoResources; } - importer_.reset(Importer::CreateInstance(&drm_)); - if (!importer_) { - ALOGE("Failed to create importer instance"); + DrmResources *drm = resource_manager_.GetDrmResources(HWC_DISPLAY_PRIMARY); + std::shared_ptr<Importer> importer = + resource_manager_.GetImporter(HWC_DISPLAY_PRIMARY); + if (!drm || !importer) { + ALOGE("Failed to get a valid drmresource and importer"); return HWC2::Error::NoResources; } + displays_.emplace( + std::piecewise_construct, std::forward_as_tuple(HWC_DISPLAY_PRIMARY), + std::forward_as_tuple(drm, importer, resource_manager_.GetGralloc(), + HWC_DISPLAY_PRIMARY, HWC2::DisplayType::Physical)); - ret = hw_get_module(GRALLOC_HARDWARE_MODULE_ID, - (const hw_module_t **)&gralloc_); - if (ret) { - ALOGE("Failed to open gralloc module %d", ret); - return HWC2::Error::NoResources; - } - - displays_.emplace(std::piecewise_construct, - std::forward_as_tuple(HWC_DISPLAY_PRIMARY), - std::forward_as_tuple(&drm_, importer_, gralloc_, - HWC_DISPLAY_PRIMARY, - HWC2::DisplayType::Physical)); - - DrmCrtc *crtc = drm_.GetCrtcForDisplay(static_cast<int>(HWC_DISPLAY_PRIMARY)); + DrmCrtc *crtc = drm->GetCrtcForDisplay(static_cast<int>(HWC_DISPLAY_PRIMARY)); if (!crtc) { ALOGE("Failed to get crtc for display %d", static_cast<int>(HWC_DISPLAY_PRIMARY)); return HWC2::Error::BadDisplay; } - std::vector<DrmPlane *> display_planes; - for (auto &plane : drm_.planes()) { + for (auto &plane : drm->planes()) { if (plane->GetCrtcSupported(*crtc)) display_planes.push_back(plane.get()); } diff --git a/drmhwctwo.h b/drmhwctwo.h index 0490e2a..beb5d2d 100644 --- a/drmhwctwo.h +++ b/drmhwctwo.h @@ -262,9 +262,7 @@ class DrmHwcTwo : public hwc2_device_t { HWC2::Error RegisterCallback(int32_t descriptor, hwc2_callback_data_t data, hwc2_function_pointer_t function); - DrmResources drm_; - std::shared_ptr<Importer> importer_; // Shared with HwcDisplay - const gralloc_module_t *gralloc_; + ResourceManager resource_manager_; std::map<hwc2_display_t, HwcDisplay> displays_; std::map<HWC2::Callback, HwcCallback> callbacks_; }; diff --git a/drmresources.cpp b/drmresources.cpp index 32dd376..a5ddda0 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -42,10 +42,9 @@ DrmResources::~DrmResources() { event_listener_.Exit(); } -int DrmResources::Init() { - char path[PROPERTY_VALUE_MAX]; - property_get("hwc.drm.device", path, "/dev/dri/card0"); - +int DrmResources::Init(ResourceManager *resource_manager, char *path, + int start_display_index) { + resource_manager_ = resource_manager; /* TODO: Use drmOpenControl here instead */ fd_.Set(open(path, O_RDWR)); if (fd() < 0) { @@ -76,8 +75,8 @@ int DrmResources::Init() { max_resolution_ = std::pair<uint32_t, uint32_t>(res->max_width, res->max_height); - bool found_primary = false; - int display_num = 1; + bool found_primary = start_display_index != 0; + int display_num = found_primary ? start_display_index : 1; for (int i = 0; !ret && i < res->count_crtcs; ++i) { drmModeCrtcPtr c = drmModeGetCrtc(fd(), res->crtcs[i]); @@ -161,9 +160,11 @@ int DrmResources::Init() { for (auto &conn : connectors_) { if (conn->internal() && !found_primary) { conn->set_display(0); + displays_[0] = 0; found_primary = true; } else { conn->set_display(display_num); + displays_[display_num] = display_num; ++display_num; } } @@ -171,7 +172,9 @@ int DrmResources::Init() { // Then look for primary amongst external connectors for (auto &conn : connectors_) { if (conn->external() && !found_primary) { + displays_.erase(conn->display()); conn->set_display(0); + displays_[0] = 0; found_primary = true; } } @@ -226,7 +229,11 @@ int DrmResources::Init() { return ret; } } - return 0; + return displays_.size() ? displays_.rbegin()->first : -EINVAL; +} + +bool DrmResources::HandlesDisplay(int display) const { + return displays_.find(display) != displays_.end(); } DrmConnector *DrmResources::GetConnectorForDisplay(int display) const { @@ -349,6 +356,10 @@ DrmEventListener *DrmResources::event_listener() { return &event_listener_; } +ResourceManager *DrmResources::resource_manager() { + return resource_manager_; +} + int DrmResources::GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name, DrmProperty *property) { drmModeObjectPropertiesPtr props; diff --git a/drmresources.h b/drmresources.h index 4cca48c..4cdcd87 100644 --- a/drmresources.h +++ b/drmresources.h @@ -17,22 +17,26 @@ #ifndef ANDROID_DRM_H_ #define ANDROID_DRM_H_ +#include <stdint.h> #include "drmconnector.h" #include "drmcrtc.h" #include "drmencoder.h" #include "drmeventlistener.h" #include "drmplane.h" - -#include <stdint.h> +#include "platform.h" +#include "resourcemanager.h" namespace android { +class ResourceManager; + class DrmResources { public: DrmResources(); ~DrmResources(); - int Init(); + int Init(ResourceManager *resource_manager, char *path, + int start_display_index); int fd() const { return fd_.get(); @@ -58,6 +62,7 @@ class DrmResources { DrmCrtc *GetCrtcForDisplay(int display) const; DrmPlane *GetPlane(uint32_t id) const; DrmEventListener *event_listener(); + ResourceManager *resource_manager(); int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, DrmProperty *property); @@ -71,6 +76,7 @@ class DrmResources { int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); int DestroyPropertyBlob(uint32_t blob_id); + bool HandlesDisplay(int display) const; private: int TryEncoderForDisplay(int display, DrmEncoder *enc); @@ -90,6 +96,8 @@ class DrmResources { std::pair<uint32_t, uint32_t> min_resolution_; std::pair<uint32_t, uint32_t> max_resolution_; + std::map<int, int> displays_; + ResourceManager *resource_manager_; }; }
Use the newly added ResourceManager for creating and detecting all the drm devices instead of assuming that there is only one device. Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> --- drmhwctwo.cpp | 34 +++++++++++++--------------------- drmhwctwo.h | 4 +--- drmresources.cpp | 25 ++++++++++++++++++------- drmresources.h | 14 +++++++++++--- 4 files changed, 43 insertions(+), 34 deletions(-)