| Message ID | 1523460149-1740-9-git-send-email-alexandru-cosmin.gheorghe@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 11, 2018 at 04:22:19PM +0100, Alexandru Gheorghe wrote: > drmModeEncoder has a field called possible_clones. It's a bit mask > which tells if the encoder could be simultaneously connected, to the > same CRTC, with the encoders specified in the possible_clones mask. > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > --- > drmencoder.cpp | 8 ++++++++ > drmencoder.h | 4 ++++ > drmresources.cpp | 9 ++++++++- > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drmencoder.cpp b/drmencoder.cpp > index 1da7ec3..ff675f5 100644 > --- a/drmencoder.cpp > +++ b/drmencoder.cpp > @@ -39,6 +39,14 @@ DrmCrtc *DrmEncoder::crtc() const { > return crtc_; > } > > +bool DrmEncoder::can_clone(DrmEncoder *encoder) { > + return possible_clones_.find(encoder) != possible_clones_.end(); > +} The find() call is probably enough to justify CamelCase for this function. FTR, I _hate_ this part of the style guide and wish I had just picked one or the other. To improve readability, can you also change the name of "encoder" to "possible_clone" like below so it's super obvious what this does? > + > +void DrmEncoder::add_possible_clone(DrmEncoder *possible_clone) { > + possible_clones_[possible_clone] = true; > +} > + > void DrmEncoder::set_crtc(DrmCrtc *crtc) { > crtc_ = crtc; > set_display(crtc->display()); > diff --git a/drmencoder.h b/drmencoder.h > index 7e06691..5e7c010 100644 > --- a/drmencoder.h > +++ b/drmencoder.h > @@ -21,6 +21,7 @@ > > #include <stdint.h> > #include <vector> > +#include <map> Alphabetical > #include <xf86drmMode.h> > > namespace android { > @@ -43,6 +44,8 @@ class DrmEncoder { > const std::vector<DrmCrtc *> &possible_crtcs() const { > return possible_crtcs_; > } > + bool can_clone(DrmEncoder *encoder); > + void add_possible_clone(DrmEncoder *possible_clone); > > private: > uint32_t id_; > @@ -50,6 +53,7 @@ class DrmEncoder { > int display_; > > std::vector<DrmCrtc *> possible_crtcs_; > + std::map<DrmEncoder *, bool> possible_clones_; > }; > } > > diff --git a/drmresources.cpp b/drmresources.cpp > index a5ddda0..39f50be 100644 > --- a/drmresources.cpp > +++ b/drmresources.cpp > @@ -97,6 +97,7 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, > crtcs_.emplace_back(std::move(crtc)); > } > > + std::vector<int> possible_clones; > for (int i = 0; !ret && i < res->count_encoders; ++i) { > drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]); > if (!e) { > @@ -117,12 +118,18 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, > > std::unique_ptr<DrmEncoder> enc( > new DrmEncoder(e, current_crtc, possible_crtcs)); > - > + possible_clones.push_back(e->possible_clones); > drmModeFreeEncoder(e); > > encoders_.emplace_back(std::move(enc)); > } > > + for (uint32_t i = 0; i < encoders_.size(); i++) { > + for (uint32_t j = 0; j < encoders_.size(); j++) for (auto &enc: encoders_) { for (auto &clone: encoders_) { Or something similarly C++'y, looping through indices is sooo last decade :-) > + if (possible_clones[i] & (1 << j)) > + encoders_[i]->add_possible_clone(encoders_[j].get()); > + } > + > for (int i = 0; !ret && i < res->count_connectors; ++i) { > drmModeConnectorPtr c = drmModeGetConnector(fd(), res->connectors[i]); > if (!c) { > -- > 2.7.4 >
Hi Sean, On Mon, Apr 16, 2018 at 04:19:14PM -0400, Sean Paul wrote: > On Wed, Apr 11, 2018 at 04:22:19PM +0100, Alexandru Gheorghe wrote: > > drmModeEncoder has a field called possible_clones. It's a bit mask > > which tells if the encoder could be simultaneously connected, to the > > same CRTC, with the encoders specified in the possible_clones mask. > > > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > > --- > > drmencoder.cpp | 8 ++++++++ > > drmencoder.h | 4 ++++ > > drmresources.cpp | 9 ++++++++- > > 3 files changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/drmencoder.cpp b/drmencoder.cpp > > index 1da7ec3..ff675f5 100644 > > --- a/drmencoder.cpp > > +++ b/drmencoder.cpp > > @@ -39,6 +39,14 @@ DrmCrtc *DrmEncoder::crtc() const { > > return crtc_; > > } > > > > +bool DrmEncoder::can_clone(DrmEncoder *encoder) { > > + return possible_clones_.find(encoder) != possible_clones_.end(); > > +} > > The find() call is probably enough to justify CamelCase for this function. FTR, > I _hate_ this part of the style guide and wish I had just picked one or the > other. > > To improve readability, can you also change the name of "encoder" to > "possible_clone" like below so it's super obvious what this does? > Sure, will do. > > + > > +void DrmEncoder::add_possible_clone(DrmEncoder *possible_clone) { > > + possible_clones_[possible_clone] = true; > > +} > > + > > void DrmEncoder::set_crtc(DrmCrtc *crtc) { > > crtc_ = crtc; > > set_display(crtc->display()); > > diff --git a/drmencoder.h b/drmencoder.h > > index 7e06691..5e7c010 100644 > > --- a/drmencoder.h > > +++ b/drmencoder.h > > @@ -21,6 +21,7 @@ > > > > #include <stdint.h> > > #include <vector> > > +#include <map> > > Alphabetical Sure, wil do. > > > #include <xf86drmMode.h> > > > > namespace android { > > @@ -43,6 +44,8 @@ class DrmEncoder { > > const std::vector<DrmCrtc *> &possible_crtcs() const { > > return possible_crtcs_; > > } > > + bool can_clone(DrmEncoder *encoder); > > + void add_possible_clone(DrmEncoder *possible_clone); > > > > private: > > uint32_t id_; > > @@ -50,6 +53,7 @@ class DrmEncoder { > > int display_; > > > > std::vector<DrmCrtc *> possible_crtcs_; > > + std::map<DrmEncoder *, bool> possible_clones_; > > }; > > } > > > > diff --git a/drmresources.cpp b/drmresources.cpp > > index a5ddda0..39f50be 100644 > > --- a/drmresources.cpp > > +++ b/drmresources.cpp > > @@ -97,6 +97,7 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, > > crtcs_.emplace_back(std::move(crtc)); > > } > > > > + std::vector<int> possible_clones; > > for (int i = 0; !ret && i < res->count_encoders; ++i) { > > drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]); > > if (!e) { > > @@ -117,12 +118,18 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, > > > > std::unique_ptr<DrmEncoder> enc( > > new DrmEncoder(e, current_crtc, possible_crtcs)); > > - > > + possible_clones.push_back(e->possible_clones); > > drmModeFreeEncoder(e); > > > > encoders_.emplace_back(std::move(enc)); > > } > > > > + for (uint32_t i = 0; i < encoders_.size(); i++) { > > + for (uint32_t j = 0; j < encoders_.size(); j++) > > for (auto &enc: encoders_) { > for (auto &clone: encoders_) { > > Or something similarly C++'y, looping through indices is sooo last decade :-) Oldie but goldie. I do need the index in order to check the possible clones mask. I will try to find something more millennial/Generation z :). > > > > + if (possible_clones[i] & (1 << j)) > > + encoders_[i]->add_possible_clone(encoders_[j].get()); > > + } > > + > > for (int i = 0; !ret && i < res->count_connectors; ++i) { > > drmModeConnectorPtr c = drmModeGetConnector(fd(), res->connectors[i]); > > if (!c) { > > -- > > 2.7.4 > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS
diff --git a/drmencoder.cpp b/drmencoder.cpp index 1da7ec3..ff675f5 100644 --- a/drmencoder.cpp +++ b/drmencoder.cpp @@ -39,6 +39,14 @@ DrmCrtc *DrmEncoder::crtc() const { return crtc_; } +bool DrmEncoder::can_clone(DrmEncoder *encoder) { + return possible_clones_.find(encoder) != possible_clones_.end(); +} + +void DrmEncoder::add_possible_clone(DrmEncoder *possible_clone) { + possible_clones_[possible_clone] = true; +} + void DrmEncoder::set_crtc(DrmCrtc *crtc) { crtc_ = crtc; set_display(crtc->display()); diff --git a/drmencoder.h b/drmencoder.h index 7e06691..5e7c010 100644 --- a/drmencoder.h +++ b/drmencoder.h @@ -21,6 +21,7 @@ #include <stdint.h> #include <vector> +#include <map> #include <xf86drmMode.h> namespace android { @@ -43,6 +44,8 @@ class DrmEncoder { const std::vector<DrmCrtc *> &possible_crtcs() const { return possible_crtcs_; } + bool can_clone(DrmEncoder *encoder); + void add_possible_clone(DrmEncoder *possible_clone); private: uint32_t id_; @@ -50,6 +53,7 @@ class DrmEncoder { int display_; std::vector<DrmCrtc *> possible_crtcs_; + std::map<DrmEncoder *, bool> possible_clones_; }; } diff --git a/drmresources.cpp b/drmresources.cpp index a5ddda0..39f50be 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -97,6 +97,7 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, crtcs_.emplace_back(std::move(crtc)); } + std::vector<int> possible_clones; for (int i = 0; !ret && i < res->count_encoders; ++i) { drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]); if (!e) { @@ -117,12 +118,18 @@ int DrmResources::Init(ResourceManager *resource_manager, char *path, std::unique_ptr<DrmEncoder> enc( new DrmEncoder(e, current_crtc, possible_crtcs)); - + possible_clones.push_back(e->possible_clones); drmModeFreeEncoder(e); encoders_.emplace_back(std::move(enc)); } + for (uint32_t i = 0; i < encoders_.size(); i++) { + for (uint32_t j = 0; j < encoders_.size(); j++) + if (possible_clones[i] & (1 << j)) + encoders_[i]->add_possible_clone(encoders_[j].get()); + } + for (int i = 0; !ret && i < res->count_connectors; ++i) { drmModeConnectorPtr c = drmModeGetConnector(fd(), res->connectors[i]); if (!c) {
drmModeEncoder has a field called possible_clones. It's a bit mask which tells if the encoder could be simultaneously connected, to the same CRTC, with the encoders specified in the possible_clones mask. Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> --- drmencoder.cpp | 8 ++++++++ drmencoder.h | 4 ++++ drmresources.cpp | 9 ++++++++- 3 files changed, 20 insertions(+), 1 deletion(-)