[PATCH hwc v1] [RFC] drm_hwcomposer: Flatten composition using writeback connector
Sean Paul
seanpaul at chromium.org
Mon Mar 19 18:03:54 UTC 2018
On Tue, Mar 13, 2018 at 04:21:20PM +0000, Alexandru Gheorghe wrote:
> This patchset tries to add support for using writeback connector to
> flatten a scene when it doesn't change for a while. This idea had
> been floated around on IRC here [1] and here [2].
>
> Developed on top of the latest writeback series, sent by Liviu here
> [3].
>
> Probably the patch should/could be broken in more patches, but since I
> want to put this out there to get feedback, I kept them as a single
> patch for now.
Thank you for your patch, it's looking good. Feel free to submit the v2 in
multiple patches. Also note that we've migrated to gitlab, the new tree is
located here:
https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer
>
> This change could be summarize as follow:
> - Attach a writeback connector to the CRTC that's controlling a
> display.
> - Detect the scene did not change for a while(60 vblanks).
> - Re-commit scene and get the composited scene through the writeback
> connector.
> - Commit the whole scene as a single plane.
>
> Some areas that I consider important and I could use some
> feedback/ideas:
>
> 1. Building the pipeline.
> Currently, drm_hwcomposer allows to connect just a single connector
> to a crtc. For now, I decided to treat the writeback connector as a
> separate field inside DrmCrtc. I'm not sure if it's a good idea to try
> to handle this in a unified way, since the writeback connector is such
> a special connector. Regarding the allocation of writeback connectors,
> my idea was to allocate writeback connector to the primary display
> first and then continue allocating while respecting the display number. 0
> gets a writeback before 1 and so on.
>
> 2. Heuristic for triggering the flattening.
> I just created a VSyncWorker which will trigger the flattening of the
> scene if it doesn't change for 60 consecutive vsyncs. The countdown
> gets reset every time ValidateDisplay is called. This is a relatively
> basic heuristic, so I'm open to suggestions.
>
> 3. Locking scheme corner cases.
> The Vsynworker is a separate thread which will contend with
> SurfaceFlinger for showing things on the screen. I tried to limit the
> race window by resetting the countdown on ValidateDisplay and
> explicitely checking that we still need to use the flatten scene before
> commiting to get the writeback result or before applying the flattened
> scene.
>
> 4. Building the DrmDisplayComposition for the flattened scene.
> I kind of lost myself in all types of layers/planes and compositions,
> so I'm not sure if I'm correctly building the DrmDisplayComposition
> object for the FlattenScene, it works and shows what I expect on the
> screen. So, any feedback here is appreciated.
>
> 5. I see there is a drmcompositorworker.cpp which implemented the same
> idea using the GPU, however that seems to not be used anymore, does
> anyone know the rationale behind it?
>
> Some unfinished/untested things:
> - Make sure the DrmFrameBuffer allocates one of the formats reported
> in WRITEBACK_PIXEL_FORMATS.
> - I'm using a hacked setup where, when needed it, the GL compositing is
> done by Surfaceflinger, so I'm not sure how well this changes are
> getting along with the GLCompositorWorker.
>
> [1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2018-02-23
> [2] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2018-02-09
> [3] https://lists.freedesktop.org/archives/dri-devel/2018-February/167703.html
>
> Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe at arm.com>
> ---
> drmconnector.cpp | 36 ++++++++++-
> drmconnector.h | 8 +++
> drmcrtc.cpp | 11 +++-
> drmcrtc.h | 8 ++-
> drmdisplaycompositor.cpp | 164 +++++++++++++++++++++++++++++++++++++++++++++--
> drmdisplaycompositor.h | 16 +++--
> drmencoder.cpp | 15 +++++
> drmencoder.h | 7 +-
> drmhwctwo.cpp | 1 +
> drmresources.cpp | 56 +++++++++++++++-
> drmresources.h | 1 +
> 11 files changed, 306 insertions(+), 17 deletions(-)
>
> diff --git a/drmconnector.cpp b/drmconnector.cpp
> index 145518f..2ed4f23 100644
> --- a/drmconnector.cpp
> +++ b/drmconnector.cpp
> @@ -52,6 +52,23 @@ int DrmConnector::Init() {
> ALOGE("Could not get CRTC_ID property\n");
> return ret;
> }
> + if (writeback()) {
> + ret = drm_->GetConnectorProperty(*this, "WRITEBACK_PIXEL_FORMATS", &writeback_pixel_formats_);
> + if (ret) {
> + ALOGE("Could not get WRITEBACK_PIXEL_FORMATS connector_id = %d\n", id_);
> + return ret;
> + }
> + ret = drm_->GetConnectorProperty(*this, "WRITEBACK_FB_ID", &writeback_fb_id_);
> + if (ret) {
> + ALOGE("Could not get WRITEBACK_FB_ID connector_id = %d\n", id_);
> + return ret;
> + }
> + ret = drm_->GetConnectorProperty(*this, "WRITEBACK_OUT_FENCE_PTR", &writeback_out_fence_);
> + if (ret) {
> + ALOGE("Could not get WRITEBACK_OUT_FENCE_PTR connector_id = %d\n", id_);
> + return ret;
> + }
> + }
> return 0;
> }
>
> @@ -78,8 +95,13 @@ bool DrmConnector::external() const {
> type_ == DRM_MODE_CONNECTOR_VGA;
> }
>
> +#define DRM_MODE_CONNECTOR_WRITEBACK 18
> +bool DrmConnector::writeback() const {
> + return type_ == DRM_MODE_CONNECTOR_WRITEBACK;
> +}
> +
> bool DrmConnector::valid_type() const {
> - return internal() || external();
> + return internal() || external() || writeback();
> }
>
> int DrmConnector::UpdateModes() {
> @@ -130,6 +152,18 @@ const DrmProperty &DrmConnector::crtc_id_property() const {
> return crtc_id_property_;
> }
>
> +const DrmProperty &DrmConnector::writeback_pixel_formats() const {
> + return writeback_pixel_formats_;
> +}
> +
> +const DrmProperty &DrmConnector::writeback_fb_id() const {
> + return writeback_fb_id_;
> +}
> +
> +const DrmProperty &DrmConnector::writeback_out_fence() const {
> + return writeback_out_fence_;
> +}
> +
> DrmEncoder *DrmConnector::encoder() const {
> return encoder_;
> }
> diff --git a/drmconnector.h b/drmconnector.h
> index 5601e06..ad18762 100644
> --- a/drmconnector.h
> +++ b/drmconnector.h
> @@ -28,6 +28,7 @@
> namespace android {
>
> class DrmResources;
> +class DrmCrtc;
Not needed?
>
> class DrmConnector {
> public:
> @@ -46,6 +47,7 @@ class DrmConnector {
>
> bool internal() const;
> bool external() const;
> + bool writeback() const;
> bool valid_type() const;
>
> int UpdateModes();
> @@ -58,6 +60,9 @@ class DrmConnector {
>
> const DrmProperty &dpms_property() const;
> const DrmProperty &crtc_id_property() const;
> + const DrmProperty &writeback_pixel_formats() const;
> + const DrmProperty &writeback_fb_id() const;
> + const DrmProperty &writeback_out_fence() const;
>
> const std::vector<DrmEncoder *> &possible_encoders() const {
> return possible_encoders_;
> @@ -88,6 +93,9 @@ class DrmConnector {
>
> DrmProperty dpms_property_;
> DrmProperty crtc_id_property_;
> + DrmProperty writeback_pixel_formats_;
> + DrmProperty writeback_fb_id_;
> + DrmProperty writeback_out_fence_;
>
> std::vector<DrmEncoder *> possible_encoders_;
> };
> diff --git a/drmcrtc.cpp b/drmcrtc.cpp
> index 1b354fe..f8c9f25 100644
> --- a/drmcrtc.cpp
> +++ b/drmcrtc.cpp
> @@ -31,7 +31,8 @@ DrmCrtc::DrmCrtc(DrmResources *drm, drmModeCrtcPtr c, unsigned pipe)
> id_(c->crtc_id),
> pipe_(pipe),
> display_(-1),
> - mode_(&c->mode) {
> + mode_(&c->mode),
> + writeback_conn_(nullptr) {
> }
>
> int DrmCrtc::Init() {
> @@ -75,6 +76,14 @@ bool DrmCrtc::can_bind(int display) const {
> return display_ == -1 || display_ == display;
> }
>
> +DrmConnector* DrmCrtc::writeback_conn() const {
> + return writeback_conn_;
> +}
> +
> +void DrmCrtc::set_writeback_conn(DrmConnector* writeback_conn) {
> + writeback_conn_ = writeback_conn;
> +}
> +
> const DrmProperty &DrmCrtc::active_property() const {
> return active_property_;
> }
> diff --git a/drmcrtc.h b/drmcrtc.h
> index c5a5599..bbd8d37 100644
> --- a/drmcrtc.h
> +++ b/drmcrtc.h
> @@ -22,11 +22,11 @@
>
> #include <stdint.h>
> #include <xf86drmMode.h>
> -
> +#include "drmconnector.h"
Please retain whitespace (comment applies below as well)
> namespace android {
>
> class DrmResources;
> -
> +class DrmConnector;
> class DrmCrtc {
> public:
> DrmCrtc(DrmResources *drm, drmModeCrtcPtr c, unsigned pipe);
> @@ -39,7 +39,9 @@ class DrmCrtc {
> unsigned pipe() const;
>
> int display() const;
> + DrmConnector* writeback_conn() const;
I think a better way would be to associate the display with the connector as it
does for a traditional connector. This might be as simple as just adding a
!conn->writeback() check to GetConnectorForDisplay() and adding a new
GetWritebackConnectorForDisplay() function to DrmResources.
> void set_display(int display);
> + void set_writeback_conn(DrmConnector*);
>
> bool can_bind(int display) const;
>
> @@ -55,7 +57,7 @@ class DrmCrtc {
> int display_;
>
> DrmMode mode_;
> -
> + DrmConnector *writeback_conn_;
> DrmProperty active_property_;
> DrmProperty mode_property_;
> DrmProperty out_fence_ptr_property_;
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index e556e86..9783852 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -16,7 +16,6 @@
>
> #define ATRACE_TAG ATRACE_TAG_GRAPHICS
> #define LOG_TAG "hwc-drm-display-compositor"
> -
More unrelated whitespace changes
> #include "drmdisplaycompositor.h"
>
> #include <pthread.h>
> @@ -36,9 +35,24 @@
> #include "drmplane.h"
> #include "drmresources.h"
> #include "glworker.h"
> +static const uint32_t kWaitWritebackFence = 100; //ms
>
> namespace android {
>
> +class CompositorVsyncCallback : public VsyncCallback {
> + public:
> + CompositorVsyncCallback(DrmDisplayCompositor *compositor)
> + :compositor_(compositor) {
> + }
> +
> + void Callback(int display, int64_t timestamp) {
> + compositor_->Vsync(display, timestamp);
> + }
> +
> + private:
> + DrmDisplayCompositor *compositor_;
> +};
> +
The GLCompositor already has flattening, so we should tie into that. I assume
writeback is going to be more efficient than using GPU (given that the GPU is
probably turned off in these scenarios), so you're probably safe to use
writeback when available and fall back to GL if it's not.
> void SquashState::Init(DrmHwcLayer *layers, size_t num_layers) {
> generation_number_++;
> valid_history_ = 0;
> @@ -183,7 +197,8 @@ DrmDisplayCompositor::DrmDisplayCompositor()
> framebuffer_index_(0),
> squash_framebuffer_index_(0),
> dump_frames_composited_(0),
> - dump_last_timestamp_ns_(0) {
> + dump_last_timestamp_ns_(0),
> + flatten_countdown_(FLATTEN_COUNTDOWN_INIT) {
> struct timespec ts;
> if (clock_gettime(CLOCK_MONOTONIC, &ts))
> return;
> @@ -193,7 +208,7 @@ DrmDisplayCompositor::DrmDisplayCompositor()
> DrmDisplayCompositor::~DrmDisplayCompositor() {
> if (!initialized_)
> return;
> -
> + vsync_worker_.Exit();
> int ret = pthread_mutex_lock(&lock_);
> if (ret)
> ALOGE("Failed to acquire compositor lock %d", ret);
> @@ -223,6 +238,9 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) {
> }
>
> initialized_ = true;
> + vsync_worker_.Init(drm_, display_);
> + auto callback = std::make_shared<CompositorVsyncCallback>(this);
> + vsync_worker_.RegisterCallback(callback);
> return 0;
> }
>
> @@ -482,7 +500,7 @@ int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) {
> }
>
> int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> - bool test_only) {
> + bool test_only, DrmDisplayComposition *writeback_comp) {
> ATRACE_CALL();
>
> int ret = 0;
> @@ -491,7 +509,8 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> std::vector<DrmCompositionPlane> &comp_planes =
> display_comp->composition_planes();
> uint64_t out_fences[drm_->crtcs().size()];
> -
> + int writeback_fence = -1;
> + DrmFramebuffer *writeback_fb = nullptr;
> DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
> if (!connector) {
> ALOGE("Could not locate connector for display %d", display_);
> @@ -508,6 +527,32 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> ALOGE("Failed to allocate property set");
> return -ENOMEM;
> }
> + DrmConnector *writeback_conn = crtc->writeback_conn();
> + if (writeback_comp != nullptr) {
> + if (writeback_conn == nullptr)
FWIW, we use NULL everywhere else, so let's stick with that.
> + return -EINVAL;
> + writeback_fb = &framebuffers_[framebuffer_index_];
> + framebuffer_index_ = (framebuffer_index_ + 1) % DRM_DISPLAY_BUFFERS;
> + ret = PrepareFramebuffer(*writeback_fb, writeback_comp);
> + if (ret) {
> + ALOGE("Failed to prepare framebuffer for pre-composite %d", ret);
> + return ret;
> + }
> + if (writeback_conn->writeback_fb_id().id() == 0 || writeback_conn->writeback_out_fence().id() == 0)
> + return -EINVAL;
> + ret = drmModeAtomicAddProperty(pset, writeback_conn->id(), writeback_conn->writeback_fb_id().id(),
> + writeback_comp->layers().back().buffer->fb_id);
> + if (ret < 0) {
> + ALOGE("Failed to add writeback_fb_id");
> + return ret;
> + }
> + ret = drmModeAtomicAddProperty(pset, writeback_conn->id(), writeback_conn->writeback_out_fence().id(),
> + (uint64_t) &writeback_fence);
> + if (ret < 0) {
> + ALOGE("Failed to add writeback_out_fence");
> + return ret;
> + }
> + }
>
> if (crtc->out_fence_ptr_property().id() != 0) {
> ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->out_fence_ptr_property().id(),
> @@ -537,6 +582,12 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> drmModeAtomicFree(pset);
> return ret;
> }
> + if (writeback_conn != nullptr) {
> + ret = drmModeAtomicAddProperty(pset, writeback_conn->id(), writeback_conn->crtc_id_property().id(), crtc->id());
> + if (ret < 0) {
> + ALOGE("Failed to attach writeback");
> + }
> + }
> }
>
> for (DrmCompositionPlane &comp_plane : comp_planes) {
> @@ -691,6 +742,17 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> if (test_only)
> flags |= DRM_MODE_ATOMIC_TEST_ONLY;
>
> + // There is a race when we recommit a scene to get the writeback result
> + // to shorten that race, check if we still need that result
> + bool abort_commit = false;
> + if (writeback_comp != nullptr) {
> + AutoLock lock(&lock_, "CommitFrame");
> + lock.Lock();
> + abort_commit = !CountdownExpired();
> + }
> + if (abort_commit)
> + return -EINVAL;
> +
> ret = drmModeAtomicCommit(drm_->fd(), pset, flags, drm_);
> if (ret) {
> if (test_only)
> @@ -729,6 +791,14 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> display_comp->set_out_fence((int) out_fences[crtc->pipe()]);
> }
>
> + if (writeback_fence >= 0 && writeback_fb != nullptr) {
> + ret = sync_wait(writeback_fence, kWaitWritebackFence);
> + if (ret) {
> + ALOGE("Failed to wait on writeback fence");
> + }
> + close(writeback_fence);
> + }
> +
> return ret;
> }
>
> @@ -784,7 +854,7 @@ void DrmDisplayCompositor::ClearDisplay() {
> }
>
> void DrmDisplayCompositor::ApplyFrame(
> - std::unique_ptr<DrmDisplayComposition> composition, int status) {
> + std::unique_ptr<DrmDisplayComposition> composition, int status, bool start_countdown) {
> int ret = status;
>
> if (!ret)
> @@ -807,6 +877,8 @@ void DrmDisplayCompositor::ApplyFrame(
> ALOGE("Failed to acquire lock for active_composition swap");
>
> active_composition_.swap(composition);
> + flatten_countdown_ = FLATTEN_COUNTDOWN_INIT;
> + vsync_worker_.VSyncControl(start_countdown);
>
> if (!ret)
> ret = pthread_mutex_unlock(&lock_);
> @@ -878,6 +950,65 @@ int DrmDisplayCompositor::ApplyComposition(
> return ret;
> }
>
> +int DrmDisplayCompositor::FlattenScene() {
> + std::unique_ptr<DrmDisplayComposition> comp = CreateComposition();
> + DrmDisplayComposition *src = active_composition_.get();
> + DrmDisplayComposition *dst = comp.get();
> + std::vector<DrmCompositionPlane> &src_planes = src->composition_planes();
> + if (src == nullptr || dst == nullptr)
> + return -EINVAL;
> + size_t src_planes_with_layer = std::count_if(
> + src_planes.begin(), src_planes.end(), [](DrmCompositionPlane &p) {
> + return p.type() != DrmCompositionPlane::Type::kDisable;
> + });
> +
> + if (src_planes_with_layer <= 1)
> + return -EALREADY;
> + int ret = dst->Init(drm_, src->crtc(), src->importer(), src->planner(),
> + src->frame_no());
> + if (ret)
> + return ret;
> + DrmCompositionPlane squashed_comp(DrmCompositionPlane::Type::kPrecomp, NULL,
> + src->crtc());
> + for (DrmCompositionPlane &comp_plane : src_planes) {
> + if (comp_plane.plane() == NULL) {
> + ALOGE("Skipping squash all because of NULL plane");
> + ret = -EINVAL;
> + }
> + if (comp_plane.plane()->type() == DRM_PLANE_TYPE_PRIMARY)
> + squashed_comp.set_plane(comp_plane.plane());
> + else
> + dst->AddPlaneDisable(comp_plane.plane());
> + }
> + ret = CommitFrame(active_composition_.get(), 0, dst);
> + if (ret || dst->layers().size() != 1) {
> + ALOGE("Failed to flatten scene using writeback");
> + return -EINVAL;
> + }
> + squashed_comp.source_layers().push_back(0);
> +
> + ret = dst->AddPlaneComposition(std::move(squashed_comp));
> + if (ret) {
> + ALOGE("Failed to add flatten scene");
> + return ret;
> + }
> + ret = dst->FinalizeComposition();
> + if (ret) {
> + ALOGE("Failed to finalize composition");
> + return ret;
> + }
> + // If countdown it's still expired ApplyFlattenScene
> + AutoLock lock(&lock_, "FlattenScene");
> + lock.Lock();
> + if (CountdownExpired()) {
> + lock.Unlock();
> + ApplyFrame(std::move(comp), 0, false);
> + } else {
> + return -EAGAIN;
> + }
> + return 0;
> +}
> +
> int DrmDisplayCompositor::SquashAll() {
> AutoLock lock(&lock_, "compositor");
> int ret = lock.Lock();
> @@ -1026,6 +1157,27 @@ move_layers_back:
> return ret;
> }
>
> +bool DrmDisplayCompositor::CountdownExpired() const {
> + return flatten_countdown_ <= 0;
> +}
> +
> +void DrmDisplayCompositor::ResetCountdown() {
> + AutoLock lock(&lock_, "ResetCountdown");
> + lock.Lock();
> + flatten_countdown_ = FLATTEN_COUNTDOWN_INIT;
> +}
> +
> +void DrmDisplayCompositor::Vsync(int display, int64_t timestamp) {
> + AutoLock lock(&lock_, "VSync");
> + lock.Lock();
> + flatten_countdown_--;
> + if (CountdownExpired()) {
> + lock.Unlock();
> + int ret = FlattenScene();
> + ALOGI("Vsync: Flatten scene for display %d at timestamp %" PRIu64 " ret = %d \n", display, timestamp, ret);
> + }
> +}
> +
> void DrmDisplayCompositor::Dump(std::ostringstream *out) const {
> int ret = pthread_mutex_lock(&lock_);
> if (ret)
> diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> index f1965fb..4a5696a 100644
> --- a/drmdisplaycompositor.h
> +++ b/drmdisplaycompositor.h
> @@ -29,11 +29,15 @@
>
> #include <hardware/hardware.h>
> #include <hardware/hwcomposer.h>
> +#include <vsyncworker.h>
>
> // One for the front, one for the back, and one for cases where we need to
> // squash a frame that the hw can't display with hw overlays.
> #define DRM_DISPLAY_BUFFERS 3
>
> +// If a scene is still for this number of vblanks flatten it to reduce power consumption.
> +#define FLATTEN_COUNTDOWN_INIT 60
> +
> namespace android {
>
> class GLWorkerCompositor;
> @@ -91,7 +95,8 @@ class DrmDisplayCompositor {
> int Composite();
> int SquashAll();
> void Dump(std::ostringstream *out) const;
> -
> + void Vsync(int display, int64_t timestamp);
> + void ResetCountdown();
> std::tuple<uint32_t, uint32_t, int> GetActiveModeResolution();
>
> SquashState *squash_state() {
> @@ -118,15 +123,16 @@ class DrmDisplayCompositor {
> int ApplySquash(DrmDisplayComposition *display_comp);
> int ApplyPreComposite(DrmDisplayComposition *display_comp);
> int PrepareFrame(DrmDisplayComposition *display_comp);
> - int CommitFrame(DrmDisplayComposition *display_comp, bool test_only);
> + int CommitFrame(DrmDisplayComposition *display_comp, bool test_only, DrmDisplayComposition *writeback_comp = nullptr);
> int SquashFrame(DrmDisplayComposition *src, DrmDisplayComposition *dst);
> int ApplyDpms(DrmDisplayComposition *display_comp);
> int DisablePlanes(DrmDisplayComposition *display_comp);
>
> void ClearDisplay();
> void ApplyFrame(std::unique_ptr<DrmDisplayComposition> composition,
> - int status);
> -
> + int status, bool start_countdown = true);
> + int FlattenScene();
> + bool CountdownExpired() const;
> std::tuple<int, uint32_t> CreateModeBlob(const DrmMode &mode);
>
> DrmResources *drm_;
> @@ -155,6 +161,8 @@ class DrmDisplayCompositor {
> // we need to reset them on every Dump() call.
> mutable uint64_t dump_frames_composited_;
> mutable uint64_t dump_last_timestamp_ns_;
> + VSyncWorker vsync_worker_;
> + int64_t flatten_countdown_;
> };
> }
>
> diff --git a/drmencoder.cpp b/drmencoder.cpp
> index 3d762f3..3df06a2 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,19 @@ 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..6b39505 100644
> --- a/drmencoder.h
> +++ b/drmencoder.h
> @@ -25,6 +25,8 @@
>
> namespace android {
>
> +class DrmCrtc;
> +
> class DrmEncoder {
> public:
> DrmEncoder(drmModeEncoderPtr e, DrmCrtc *current_crtc,
> @@ -36,7 +38,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 +48,7 @@ class DrmEncoder {
> private:
> uint32_t id_;
> DrmCrtc *crtc_;
> + int display_;
>
> std::vector<DrmCrtc *> possible_crtcs_;
> };
> diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp
> index dfca1a6..65337cc 100644
> --- a/drmhwctwo.cpp
> +++ b/drmhwctwo.cpp
> @@ -700,6 +700,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types,
> break;
> }
> }
> + compositor_.ResetCountdown();
> return *num_types ? HWC2::Error::HasChanges : HWC2::Error::None;
> }
>
> diff --git a/drmresources.cpp b/drmresources.cpp
> index 32dd376..880cef2 100644
> --- a/drmresources.cpp
> +++ b/drmresources.cpp
> @@ -33,6 +33,8 @@
> #include <cutils/log.h>
> #include <cutils/properties.h>
>
> +#define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 4
> +
Presumably this should come from libdrm headers?
> namespace android {
>
> DrmResources::DrmResources() : event_listener_(this) {
> @@ -65,6 +67,11 @@ int DrmResources::Init() {
> return ret;
> }
>
> + ret = drmSetClientCap(fd(), DRM_CLIENT_CAP_WRITEBACK_CONNECTORS, 1);
> + if (ret) {
> + ALOGI("Failed to set writeback cap %d", ret);
> + ret = 0;
> + }
> drmModeResPtr res = drmModeGetResources(fd());
> if (!res) {
> ALOGE("Failed to get DrmResources resources");
> @@ -77,6 +84,7 @@ int DrmResources::Init() {
> std::pair<uint32_t, uint32_t>(res->max_width, res->max_height);
>
> bool found_primary = false;
> + int primary_index = 0;
> int display_num = 1;
>
> for (int i = 0; !ret && i < res->count_crtcs; ++i) {
> @@ -162,18 +170,22 @@ int DrmResources::Init() {
> if (conn->internal() && !found_primary) {
> conn->set_display(0);
> found_primary = true;
> - } else {
> + } else if (conn->external()) {
> + if (!found_primary) primary_index++;
This is against style guide (and same below).
> conn->set_display(display_num);
> ++display_num;
> }
> }
>
> // Then look for primary amongst external connectors
> + if (!found_primary) primary_index = 0;
> for (auto &conn : connectors_) {
> if (conn->external() && !found_primary) {
> conn->set_display(0);
> found_primary = true;
> + break;
> }
> + if (!found_primary) primary_index++;
> }
>
> if (res)
> @@ -220,12 +232,29 @@ int DrmResources::Init() {
> }
>
> for (auto &conn : connectors_) {
> + if (conn->writeback())
> + continue;
> ret = CreateDisplayPipe(conn.get());
> if (ret) {
> ALOGE("Failed CreateDisplayPipe %d with %d", conn->id(), ret);
> return ret;
> }
> }
> + /* Allocate writebacks according to the display number */
> + /* 0 should get a writeback before 1 */
> + for (auto &writeback_conn : connectors_) {
> + if (!writeback_conn->writeback())
> + continue;
> + int index = primary_index;
> + do {
> + if (connectors_[index]->display() < 0)
> + continue;
> + ret = AttachWriteback(writeback_conn.get(), connectors_[index].get());
> + if (!ret)
> + break;
> + index = (index + 1) % connectors_.size();
> + } while (index != primary_index);
> + }
I think you can avoid all of the primary_index gymnastics if you just make
allocating the writeback connector part of CreateDisplayPipe (which dovetails
nicely into my suggestion up above to allocate a display to the writeback
connector.
> return 0;
> }
>
> @@ -314,6 +343,31 @@ int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
> return -ENODEV;
> }
>
> +/*
> + * Attach writeback connector to the CRTC linked to the display_conn
> + *
> + */
> +int DrmResources::AttachWriteback(DrmConnector *writeback_conn, DrmConnector *display_conn) {
> + int ret = -EINVAL;
> + if (display_conn->writeback())
> + return -EINVAL;
> + if (!display_conn->encoder() || ! display_conn->encoder()->crtc())
> + return -EINVAL;
> + DrmCrtc *display_crtc = display_conn->encoder()->crtc();
> + if (display_crtc->writeback_conn() != nullptr)
> + return -EINVAL;
> + for (DrmEncoder *writeback_enc : writeback_conn->possible_encoders()) {
> + // Use just encoders which had not been bound already.
> + if (writeback_enc->can_bind(display_crtc->display())) {
> + writeback_enc->set_crtc(display_crtc);
> + writeback_conn->set_encoder(writeback_enc);
> + display_crtc->set_writeback_conn(writeback_conn);
> + ret = 0;
> + }
> + }
> + return ret;
> +}
> +
> int DrmResources::CreatePropertyBlob(void *data, size_t length,
> uint32_t *blob_id) {
> struct drm_mode_create_blob create_blob;
> diff --git a/drmresources.h b/drmresources.h
> index 4cca48c..ad285ec 100644
> --- a/drmresources.h
> +++ b/drmresources.h
> @@ -78,6 +78,7 @@ class DrmResources {
> DrmProperty *property);
>
> int CreateDisplayPipe(DrmConnector *connector);
> + int AttachWriteback(DrmConnector *writeback_conn, DrmConnector *display_conn);
>
> UniqueFd fd_;
> uint32_t mode_id_ = 0;
> --
> 2.7.4
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
More information about the dri-devel
mailing list