| Message ID | 1523460149-1740-8-git-send-email-alexandru-cosmin.gheorghe@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 11, 2018 at 04:22:18PM +0100, Alexandru Gheorghe wrote: > In the current implementation TryEncoderForDisplay just looks > at the crtc linked to the display, if that's not assigned to > a display it means the encoder could be used, otherwise iterate > to the list of possible_crtcs and find one which is not used. > > This logic works fine when you have just one encoder connected to a > crtc but with two or more, like is the case when we attach a writeback > connector, we need to know if we already assigned the encoder to a > display. > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > --- > drmencoder.cpp | 14 ++++++++++++++ > drmencoder.h | 4 ++++ > 2 files changed, 18 insertions(+) > > diff --git a/drmencoder.cpp b/drmencoder.cpp > index 3d762f3..1da7ec3 100644 > --- a/drmencoder.cpp > +++ b/drmencoder.cpp > @@ -27,6 +27,7 @@ DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc, > const std::vector<DrmCrtc *> &possible_crtcs) > : id_(e->encoder_id), > crtc_(current_crtc), > + display_(-1), > possible_crtcs_(possible_crtcs) { > } > > @@ -40,5 +41,18 @@ DrmCrtc *DrmEncoder::crtc() const { > > void DrmEncoder::set_crtc(DrmCrtc *crtc) { > crtc_ = crtc; > + set_display(crtc->display()); > +} > + > +int DrmEncoder::display() const { > + return display_; > +} > + > +void DrmEncoder::set_display(int display) { > + display_ = display; > +} Instead of adding this, just call set_crtc() in TryEncoderForDisplay() for the already-bound case. That way we only have one entry point for this. > + > +bool DrmEncoder::can_bind(int display) const { > + return display_ == -1 || display_ == display; > } > } > diff --git a/drmencoder.h b/drmencoder.h > index 58ccbfb..7e06691 100644 > --- a/drmencoder.h > +++ b/drmencoder.h > @@ -36,6 +36,9 @@ class DrmEncoder { > > DrmCrtc *crtc() const; > void set_crtc(DrmCrtc *crtc); > + bool can_bind(int display) const; > + void set_display(int display); > + int display() const; > > const std::vector<DrmCrtc *> &possible_crtcs() const { > return possible_crtcs_; > @@ -44,6 +47,7 @@ class DrmEncoder { > private: > uint32_t id_; > DrmCrtc *crtc_; > + int display_; > > std::vector<DrmCrtc *> possible_crtcs_; > }; > -- > 2.7.4 >
Hi Sean, On Mon, Apr 16, 2018 at 04:02:07PM -0400, Sean Paul wrote: > On Wed, Apr 11, 2018 at 04:22:18PM +0100, Alexandru Gheorghe wrote: > > In the current implementation TryEncoderForDisplay just looks > > at the crtc linked to the display, if that's not assigned to > > a display it means the encoder could be used, otherwise iterate > > to the list of possible_crtcs and find one which is not used. > > > > This logic works fine when you have just one encoder connected to a > > crtc but with two or more, like is the case when we attach a writeback > > connector, we need to know if we already assigned the encoder to a > > display. > > > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > > --- > > drmencoder.cpp | 14 ++++++++++++++ > > drmencoder.h | 4 ++++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/drmencoder.cpp b/drmencoder.cpp > > index 3d762f3..1da7ec3 100644 > > --- a/drmencoder.cpp > > +++ b/drmencoder.cpp > > @@ -27,6 +27,7 @@ DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc, > > const std::vector<DrmCrtc *> &possible_crtcs) > > : id_(e->encoder_id), > > crtc_(current_crtc), > > + display_(-1), > > possible_crtcs_(possible_crtcs) { > > } > > > > @@ -40,5 +41,18 @@ DrmCrtc *DrmEncoder::crtc() const { > > > > void DrmEncoder::set_crtc(DrmCrtc *crtc) { > > crtc_ = crtc; > > + set_display(crtc->display()); > > +} > > + > > +int DrmEncoder::display() const { > > + return display_; > > +} > > + > > +void DrmEncoder::set_display(int display) { > > + display_ = display; > > +} > > Instead of adding this, just call set_crtc() in TryEncoderForDisplay() for the > already-bound case. That way we only have one entry point for this. Fair enough, I will remove set_display. > > > + > > +bool DrmEncoder::can_bind(int display) const { > > + return display_ == -1 || display_ == display; > > } > > } > > diff --git a/drmencoder.h b/drmencoder.h > > index 58ccbfb..7e06691 100644 > > --- a/drmencoder.h > > +++ b/drmencoder.h > > @@ -36,6 +36,9 @@ class DrmEncoder { > > > > DrmCrtc *crtc() const; > > void set_crtc(DrmCrtc *crtc); > > + bool can_bind(int display) const; > > + void set_display(int display); > > + int display() const; > > > > const std::vector<DrmCrtc *> &possible_crtcs() const { > > return possible_crtcs_; > > @@ -44,6 +47,7 @@ class DrmEncoder { > > private: > > uint32_t id_; > > DrmCrtc *crtc_; > > + int display_; > > > > std::vector<DrmCrtc *> possible_crtcs_; > > }; > > -- > > 2.7.4 > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS
diff --git a/drmencoder.cpp b/drmencoder.cpp index 3d762f3..1da7ec3 100644 --- a/drmencoder.cpp +++ b/drmencoder.cpp @@ -27,6 +27,7 @@ DrmEncoder::DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc, const std::vector<DrmCrtc *> &possible_crtcs) : id_(e->encoder_id), crtc_(current_crtc), + display_(-1), possible_crtcs_(possible_crtcs) { } @@ -40,5 +41,18 @@ DrmCrtc *DrmEncoder::crtc() const { void DrmEncoder::set_crtc(DrmCrtc *crtc) { crtc_ = crtc; + set_display(crtc->display()); +} + +int DrmEncoder::display() const { + return display_; +} + +void DrmEncoder::set_display(int display) { + display_ = display; +} + +bool DrmEncoder::can_bind(int display) const { + return display_ == -1 || display_ == display; } } diff --git a/drmencoder.h b/drmencoder.h index 58ccbfb..7e06691 100644 --- a/drmencoder.h +++ b/drmencoder.h @@ -36,6 +36,9 @@ class DrmEncoder { DrmCrtc *crtc() const; void set_crtc(DrmCrtc *crtc); + bool can_bind(int display) const; + void set_display(int display); + int display() const; const std::vector<DrmCrtc *> &possible_crtcs() const { return possible_crtcs_; @@ -44,6 +47,7 @@ class DrmEncoder { private: uint32_t id_; DrmCrtc *crtc_; + int display_; std::vector<DrmCrtc *> possible_crtcs_; };
In the current implementation TryEncoderForDisplay just looks at the crtc linked to the display, if that's not assigned to a display it means the encoder could be used, otherwise iterate to the list of possible_crtcs and find one which is not used. This logic works fine when you have just one encoder connected to a crtc but with two or more, like is the case when we attach a writeback connector, we need to know if we already assigned the encoder to a display. Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> --- drmencoder.cpp | 14 ++++++++++++++ drmencoder.h | 4 ++++ 2 files changed, 18 insertions(+)