Re: [PATCH hwc v2 08/18] drm_hwcomposer: Parse and store possible_clones information
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
> > ---
> > 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
> > #include
> > +#include
>
> Alphabetical
Sure, wil do.
>
> > #include
> >
> > namespace android {
> > @@ -43,6 +44,8 @@ class DrmEncoder {
> >const std::vector &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 possible_crtcs_;
> > + std::map 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 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 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
--
Cheers,
Alex G
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH hwc v2 08/18] drm_hwcomposer: Parse and store possible_clones information
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
> ---
> 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
> #include
> +#include
Alphabetical
> #include
>
> namespace android {
> @@ -43,6 +44,8 @@ class DrmEncoder {
>const std::vector &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 possible_crtcs_;
> + std::map 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 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 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
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH hwc v2 08/18] drm_hwcomposer: Parse and store possible_clones information
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
---
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();
+}
+
+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
#include
+#include
#include
namespace android {
@@ -43,6 +44,8 @@ class DrmEncoder {
const std::vector &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 possible_crtcs_;
+ std::map 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 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 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) {
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
