| Message ID | 20170927115841.29134-2-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27 September 2017 at 12:58, Robert Foss <robert.foss@collabora.com> wrote:
> 16 files changed, 93 insertions(+), 724 deletions(-)
Holly smokes, that's some amazing stat.
Please sir can I have some more ;-)
Question - this patch removes the threading implementation, while the
actual substitute lands with patches 2-6.
Did I get this right?
If so, ideally this patch will be the final in the series.
Guess it doesn't matter too much though.
-Emil
Hey Emil, On Wed, 2017-09-27 at 14:34 +0100, Emil Velikov wrote: > On 27 September 2017 at 12:58, Robert Foss <robert.foss@collabora.com > > wrote: > > > 16 files changed, 93 insertions(+), 724 deletions(-) > > Holly smokes, that's some amazing stat. > Please sir can I have some more ;-) > > Question - this patch removes the threading implementation, while the > actual substitute lands with patches 2-6. > Did I get this right? > > If so, ideally this patch will be the final in the series. > Guess it doesn't matter too much though. Ack. I'll submit v3 with this change tomorrow. Thanks! > > -Emil
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote: > From: Sean Paul <seanpaul@chromium.org> > > Since HWC2 doesn't require the use of threads to implement correct > synchronization, remove some of these threads. > My SoB seems to have been dropped (or perhaps I just forgot to add it in the original thread). At any rate, here you go: Signed-off-by: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > Android.mk | 3 - > drmcomposition.cpp | 166 ---------------------------------------- > drmcomposition.h | 79 ------------------- > drmcompositor.cpp | 106 -------------------------- > drmcompositor.h | 56 -------------- > drmcompositorworker.h | 41 ---------- > drmdisplaycomposition.cpp | 1 + > drmdisplaycomposition.h | 10 +++ > drmdisplaycompositor.cpp | 189 ++++------------------------------------------ > drmdisplaycompositor.h | 36 +-------- > drmeventlistener.cpp | 3 + > drmhwctwo.cpp | 6 +- > drmresources.cpp | 54 +------------ > drmresources.h | 5 -- > glworker.cpp | 52 +++++++++++-- > glworker.h | 10 +++ > 16 files changed, 93 insertions(+), 724 deletions(-) > delete mode 100644 drmcomposition.cpp > delete mode 100644 drmcomposition.h > delete mode 100644 drmcompositor.cpp > delete mode 100644 drmcompositor.h > delete mode 100644 drmcompositorworker.h > > diff --git a/Android.mk b/Android.mk > index aa95b44..5d16c2f 100644 > --- a/Android.mk > +++ b/Android.mk > @@ -57,9 +57,6 @@ LOCAL_C_INCLUDES := \ > LOCAL_SRC_FILES := \ > autolock.cpp \ > drmresources.cpp \ > - drmcomposition.cpp \ > - drmcompositor.cpp \ > - drmcompositorworker.cpp \ > drmconnector.cpp \ > drmcrtc.cpp \ > drmdisplaycomposition.cpp \ > diff --git a/drmcomposition.cpp b/drmcomposition.cpp > deleted file mode 100644 > index 1aaf920..0000000 > --- a/drmcomposition.cpp > +++ /dev/null > @@ -1,166 +0,0 @@ > -/* > - * Copyright (C) 2015 The Android Open Source Project > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#define LOG_TAG "hwc-drm-composition" > - > -#include "drmcomposition.h" > -#include "drmcrtc.h" > -#include "drmplane.h" > -#include "drmresources.h" > -#include "platform.h" > - > -#include <stdlib.h> > - > -#include <cutils/log.h> > -#include <cutils/properties.h> > -#include <sw_sync.h> > -#include <sync/sync.h> > - > -namespace android { > - > -DrmComposition::DrmComposition(DrmResources *drm, Importer *importer, > - Planner *planner) > - : drm_(drm), importer_(importer), planner_(planner) { > - char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; > - property_get("hwc.drm.use_overlay_planes", use_overlay_planes_prop, "1"); > - bool use_overlay_planes = atoi(use_overlay_planes_prop); > - > - for (auto &plane : drm->planes()) { > - if (plane->type() == DRM_PLANE_TYPE_PRIMARY) > - primary_planes_.push_back(plane.get()); > - else if (use_overlay_planes && plane->type() == DRM_PLANE_TYPE_OVERLAY) > - overlay_planes_.push_back(plane.get()); > - } > -} > - > -int DrmComposition::Init(uint64_t frame_no) { > - for (auto &conn : drm_->connectors()) { > - int display = conn->display(); > - composition_map_[display].reset(new DrmDisplayComposition()); > - if (!composition_map_[display]) { > - ALOGE("Failed to allocate new display composition\n"); > - return -ENOMEM; > - } > - > - // If the display hasn't been modeset yet, this will be NULL > - DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); > - > - int ret = composition_map_[display]->Init(drm_, crtc, importer_, planner_, > - frame_no); > - if (ret) { > - ALOGE("Failed to init display composition for %d", display); > - return ret; > - } > - } > - return 0; > -} > - > -int DrmComposition::SetLayers(size_t num_displays, > - DrmCompositionDisplayLayersMap *maps) { > - int ret = 0; > - for (size_t display_index = 0; display_index < num_displays; > - display_index++) { > - DrmCompositionDisplayLayersMap &map = maps[display_index]; > - int display = map.display; > - > - if (!drm_->GetConnectorForDisplay(display)) { > - ALOGE("Invalid display given to SetLayers %d", display); > - continue; > - } > - > - ret = composition_map_[display]->SetLayers( > - map.layers.data(), map.layers.size(), map.geometry_changed); > - if (ret) > - return ret; > - } > - > - return 0; > -} > - > -int DrmComposition::SetDpmsMode(int display, uint32_t dpms_mode) { > - return composition_map_[display]->SetDpmsMode(dpms_mode); > -} > - > -int DrmComposition::SetDisplayMode(int display, const DrmMode &display_mode) { > - return composition_map_[display]->SetDisplayMode(display_mode); > -} > - > -std::unique_ptr<DrmDisplayComposition> DrmComposition::TakeDisplayComposition( > - int display) { > - return std::move(composition_map_[display]); > -} > - > -int DrmComposition::Plan(std::map<int, DrmDisplayCompositor> &compositor_map) { > - int ret = 0; > - for (auto &conn : drm_->connectors()) { > - int display = conn->display(); > - DrmDisplayComposition *comp = GetDisplayComposition(display); > - ret = comp->Plan(compositor_map[display].squash_state(), &primary_planes_, > - &overlay_planes_); > - if (ret) { > - ALOGE("Failed to plan composition for dislay %d", display); > - return ret; > - } > - } > - > - return 0; > -} > - > -int DrmComposition::DisableUnusedPlanes() { > - for (auto &conn : drm_->connectors()) { > - int display = conn->display(); > - DrmDisplayComposition *comp = GetDisplayComposition(display); > - > - /* > - * Leave empty compositions alone > - * TODO: re-visit this and potentially disable leftover planes after the > - * active compositions have gobbled up all they can > - */ > - if (comp->type() == DRM_COMPOSITION_TYPE_EMPTY || > - comp->type() == DRM_COMPOSITION_TYPE_MODESET) > - continue; > - > - DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); > - if (!crtc) { > - ALOGE("Failed to find crtc for display %d", display); > - continue; > - } > - > - for (std::vector<DrmPlane *>::iterator iter = primary_planes_.begin(); > - iter != primary_planes_.end(); ++iter) { > - if ((*iter)->GetCrtcSupported(*crtc)) { > - comp->AddPlaneDisable(*iter); > - primary_planes_.erase(iter); > - break; > - } > - } > - for (std::vector<DrmPlane *>::iterator iter = overlay_planes_.begin(); > - iter != overlay_planes_.end();) { > - if ((*iter)->GetCrtcSupported(*crtc)) { > - comp->AddPlaneDisable(*iter); > - iter = overlay_planes_.erase(iter); > - } else { > - iter++; > - } > - } > - } > - return 0; > -} > - > -DrmDisplayComposition *DrmComposition::GetDisplayComposition(int display) { > - return composition_map_[display].get(); > -} > -} > diff --git a/drmcomposition.h b/drmcomposition.h > deleted file mode 100644 > index eae8cde..0000000 > --- a/drmcomposition.h > +++ /dev/null > @@ -1,79 +0,0 @@ > -/* > - * Copyright (C) 2015 The Android Open Source Project > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#ifndef ANDROID_DRM_COMPOSITION_H_ > -#define ANDROID_DRM_COMPOSITION_H_ > - > -#include "drmhwcomposer.h" > -#include "drmdisplaycomposition.h" > -#include "drmplane.h" > -#include "platform.h" > - > -#include <map> > -#include <vector> > - > -#include <hardware/hardware.h> > -#include <hardware/hwcomposer.h> > - > -namespace android { > - > -class DrmDisplayCompositor; > - > -struct DrmCompositionDisplayLayersMap { > - int display; > - bool geometry_changed = true; > - std::vector<DrmHwcLayer> layers; > - > - DrmCompositionDisplayLayersMap() = default; > - DrmCompositionDisplayLayersMap(DrmCompositionDisplayLayersMap &&rhs) = > - default; > -}; > - > -class DrmComposition { > - public: > - DrmComposition(DrmResources *drm, Importer *importer, Planner *planner); > - > - int Init(uint64_t frame_no); > - > - int SetLayers(size_t num_displays, DrmCompositionDisplayLayersMap *maps); > - int SetDpmsMode(int display, uint32_t dpms_mode); > - int SetDisplayMode(int display, const DrmMode &display_mode); > - > - std::unique_ptr<DrmDisplayComposition> TakeDisplayComposition(int display); > - DrmDisplayComposition *GetDisplayComposition(int display); > - > - int Plan(std::map<int, DrmDisplayCompositor> &compositor_map); > - int DisableUnusedPlanes(); > - > - private: > - DrmComposition(const DrmComposition &) = delete; > - > - DrmResources *drm_; > - Importer *importer_; > - Planner *planner_; > - > - std::vector<DrmPlane *> primary_planes_; > - std::vector<DrmPlane *> overlay_planes_; > - > - /* > - * This _must_ be read-only after it's passed to QueueComposition. Otherwise > - * locking is required to maintain consistency across the compositor threads. > - */ > - std::map<int, std::unique_ptr<DrmDisplayComposition>> composition_map_; > -}; > -} > - > -#endif // ANDROID_DRM_COMPOSITION_H_ > diff --git a/drmcompositor.cpp b/drmcompositor.cpp > deleted file mode 100644 > index c1f3ed8..0000000 > --- a/drmcompositor.cpp > +++ /dev/null > @@ -1,106 +0,0 @@ > -/* > - * Copyright (C) 2015 The Android Open Source Project > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#define LOG_TAG "hwc-drm-compositor" > - > -#include "drmcompositor.h" > -#include "drmdisplaycompositor.h" > -#include "drmresources.h" > -#include "platform.h" > - > -#include <sstream> > -#include <stdlib.h> > - > -#include <cutils/log.h> > - > -namespace android { > - > -DrmCompositor::DrmCompositor(DrmResources *drm) : drm_(drm), frame_no_(0) { > -} > - > -DrmCompositor::~DrmCompositor() { > -} > - > -int DrmCompositor::Init() { > - for (auto &conn : drm_->connectors()) { > - int display = conn->display(); > - int ret = compositor_map_[display].Init(drm_, display); > - if (ret) { > - ALOGE("Failed to initialize display compositor for %d", display); > - return ret; > - } > - } > - planner_ = Planner::CreateInstance(drm_); > - if (!planner_) { > - ALOGE("Failed to create planner instance for composition"); > - return -ENOMEM; > - } > - > - return 0; > -} > - > -std::unique_ptr<DrmComposition> DrmCompositor::CreateComposition( > - Importer *importer) { > - std::unique_ptr<DrmComposition> composition( > - new DrmComposition(drm_, importer, planner_.get())); > - int ret = composition->Init(++frame_no_); > - if (ret) { > - ALOGE("Failed to initialize drm composition %d", ret); > - return nullptr; > - } > - return composition; > -} > - > -int DrmCompositor::QueueComposition( > - std::unique_ptr<DrmComposition> composition) { > - int ret; > - > - ret = composition->Plan(compositor_map_); > - if (ret) > - return ret; > - > - ret = composition->DisableUnusedPlanes(); > - if (ret) > - return ret; > - > - for (auto &conn : drm_->connectors()) { > - int display = conn->display(); > - int ret = compositor_map_[display].QueueComposition( > - composition->TakeDisplayComposition(display)); > - if (ret) { > - ALOGE("Failed to queue composition for display %d (%d)", display, ret); > - return ret; > - } > - } > - > - return 0; > -} > - > -int DrmCompositor::Composite() { > - /* > - * This shouldn't be called, we should be calling Composite() on the display > - * compositors directly. > - */ > - ALOGE("Calling base drm compositor Composite() function"); > - return -EINVAL; > -} > - > -void DrmCompositor::Dump(std::ostringstream *out) const { > - *out << "DrmCompositor stats:\n"; > - for (auto &conn : drm_->connectors()) > - compositor_map_[conn->display()].Dump(out); > -} > -} > diff --git a/drmcompositor.h b/drmcompositor.h > deleted file mode 100644 > index 19271b5..0000000 > --- a/drmcompositor.h > +++ /dev/null > @@ -1,56 +0,0 @@ > -/* > - * Copyright (C) 2015 The Android Open Source Project > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#ifndef ANDROID_DRM_COMPOSITOR_H_ > -#define ANDROID_DRM_COMPOSITOR_H_ > - > -#include "drmcomposition.h" > -#include "drmdisplaycompositor.h" > -#include "platform.h" > - > -#include <map> > -#include <memory> > -#include <sstream> > - > -namespace android { > - > -class DrmCompositor { > - public: > - DrmCompositor(DrmResources *drm); > - ~DrmCompositor(); > - > - int Init(); > - > - std::unique_ptr<DrmComposition> CreateComposition(Importer *importer); > - > - int QueueComposition(std::unique_ptr<DrmComposition> composition); > - int Composite(); > - void Dump(std::ostringstream *out) const; > - > - private: > - DrmCompositor(const DrmCompositor &) = delete; > - > - DrmResources *drm_; > - std::unique_ptr<Planner> planner_; > - > - uint64_t frame_no_; > - > - // mutable for Dump() propagation > - mutable std::map<int, DrmDisplayCompositor> compositor_map_; > -}; > -} > - > -#endif // ANDROID_DRM_COMPOSITOR_H_ > diff --git a/drmcompositorworker.h b/drmcompositorworker.h > deleted file mode 100644 > index 731bc65..0000000 > --- a/drmcompositorworker.h > +++ /dev/null > @@ -1,41 +0,0 @@ > -/* > - * Copyright (C) 2015 The Android Open Source Project > - * > - * Licensed under the Apache License, Version 2.0 (the "License"); > - * you may not use this file except in compliance with the License. > - * You may obtain a copy of the License at > - * > - * http://www.apache.org/licenses/LICENSE-2.0 > - * > - * Unless required by applicable law or agreed to in writing, software > - * distributed under the License is distributed on an "AS IS" BASIS, > - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > - * See the License for the specific language governing permissions and > - * limitations under the License. > - */ > - > -#ifndef ANDROID_DRM_COMPOSITOR_WORKER_H_ > -#define ANDROID_DRM_COMPOSITOR_WORKER_H_ > - > -#include "worker.h" > - > -namespace android { > - > -class DrmDisplayCompositor; > - > -class DrmCompositorWorker : public Worker { > - public: > - DrmCompositorWorker(DrmDisplayCompositor *compositor); > - ~DrmCompositorWorker() override; > - > - int Init(); > - > - protected: > - void Routine() override; > - > - DrmDisplayCompositor *compositor_; > - bool did_squash_all_ = false; > -}; > -} > - > -#endif > diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp > index 0f8084b..66e67a4 100644 > --- a/drmdisplaycomposition.cpp > +++ b/drmdisplaycomposition.cpp > @@ -17,6 +17,7 @@ > #define LOG_TAG "hwc-drm-display-composition" > > #include "drmdisplaycomposition.h" > +#include "drmdisplaycompositor.h" > #include "drmcrtc.h" > #include "drmplane.h" > #include "drmresources.h" > diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h > index 13da19d..b165adc 100644 > --- a/drmdisplaycomposition.h > +++ b/drmdisplaycomposition.h > @@ -42,6 +42,16 @@ enum DrmCompositionType { > DRM_COMPOSITION_TYPE_MODESET, > }; > > +struct DrmCompositionDisplayLayersMap { > + int display; > + bool geometry_changed = true; > + std::vector<DrmHwcLayer> layers; > + > + DrmCompositionDisplayLayersMap() = default; > + DrmCompositionDisplayLayersMap(DrmCompositionDisplayLayersMap &&rhs) = > + default; > +}; > + > struct DrmCompositionRegion { > DrmHwcRect<int> frame; > std::vector<size_t> source_layers; > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp > index a1baed1..bd670cf 100644 > --- a/drmdisplaycompositor.cpp > +++ b/drmdisplaycompositor.cpp > @@ -37,8 +37,6 @@ > #include "drmresources.h" > #include "glworker.h" > > -#define DRM_DISPLAY_COMPOSITOR_MAX_QUEUE_DEPTH 2 > - > namespace android { > > void SquashState::Init(DrmHwcLayer *layers, size_t num_layers) { > @@ -176,58 +174,9 @@ static bool UsesSquash(const std::vector<DrmCompositionPlane> &comp_planes) { > }); > } > > -DrmDisplayCompositor::FrameWorker::FrameWorker(DrmDisplayCompositor *compositor) > - : Worker("frame-worker", HAL_PRIORITY_URGENT_DISPLAY), > - compositor_(compositor) { > -} > - > -DrmDisplayCompositor::FrameWorker::~FrameWorker() { > -} > - > -int DrmDisplayCompositor::FrameWorker::Init() { > - return InitWorker(); > -} > - > -void DrmDisplayCompositor::FrameWorker::QueueFrame( > - std::unique_ptr<DrmDisplayComposition> composition, int status) { > - Lock(); > - FrameState frame; > - frame.composition = std::move(composition); > - frame.status = status; > - frame_queue_.push(std::move(frame)); > - Unlock(); > - Signal(); > -} > - > -void DrmDisplayCompositor::FrameWorker::Routine() { > - int wait_ret = 0; > - > - Lock(); > - if (frame_queue_.empty()) { > - wait_ret = WaitForSignalOrExitLocked(); > - } > - > - FrameState frame; > - if (!frame_queue_.empty()) { > - frame = std::move(frame_queue_.front()); > - frame_queue_.pop(); > - } > - Unlock(); > - > - if (wait_ret == -EINTR) { > - return; > - } else if (wait_ret) { > - ALOGE("Failed to wait for signal, %d", wait_ret); > - return; > - } > - compositor_->ApplyFrame(std::move(frame.composition), frame.status); > -} > - > DrmDisplayCompositor::DrmDisplayCompositor() > : drm_(NULL), > display_(-1), > - worker_(this), > - frame_worker_(this), > initialized_(false), > active_(false), > use_hw_overlays_(true), > @@ -245,9 +194,6 @@ DrmDisplayCompositor::~DrmDisplayCompositor() { > if (!initialized_) > return; > > - worker_.Exit(); > - frame_worker_.Exit(); > - > int ret = pthread_mutex_lock(&lock_); > if (ret) > ALOGE("Failed to acquire compositor lock %d", ret); > @@ -257,10 +203,6 @@ DrmDisplayCompositor::~DrmDisplayCompositor() { > if (mode_.old_blob_id) > drm_->DestroyPropertyBlob(mode_.old_blob_id); > > - while (!composite_queue_.empty()) { > - composite_queue_.front().reset(); > - composite_queue_.pop(); > - } > active_composition_.reset(); > > ret = pthread_mutex_unlock(&lock_); > @@ -279,18 +221,6 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) { > ALOGE("Failed to initialize drm compositor lock %d\n", ret); > return ret; > } > - ret = worker_.Init(); > - if (ret) { > - pthread_mutex_destroy(&lock_); > - ALOGE("Failed to initialize compositor worker %d\n", ret); > - return ret; > - } > - ret = frame_worker_.Init(); > - if (ret) { > - pthread_mutex_destroy(&lock_); > - ALOGE("Failed to initialize frame worker %d\n", ret); > - return ret; > - } > > initialized_ = true; > return 0; > @@ -301,55 +231,6 @@ std::unique_ptr<DrmDisplayComposition> DrmDisplayCompositor::CreateComposition() > return std::unique_ptr<DrmDisplayComposition>(new DrmDisplayComposition()); > } > > -int DrmDisplayCompositor::QueueComposition( > - std::unique_ptr<DrmDisplayComposition> composition) { > - switch (composition->type()) { > - case DRM_COMPOSITION_TYPE_FRAME: > - if (!active_) > - return -ENODEV; > - break; > - case DRM_COMPOSITION_TYPE_DPMS: > - /* > - * Update the state as soon as we get it so we can start/stop queuing > - * frames asap. > - */ > - active_ = (composition->dpms_mode() == DRM_MODE_DPMS_ON); > - break; > - case DRM_COMPOSITION_TYPE_MODESET: > - break; > - case DRM_COMPOSITION_TYPE_EMPTY: > - return 0; > - default: > - ALOGE("Unknown composition type %d/%d", composition->type(), display_); > - return -ENOENT; > - } > - > - int ret = pthread_mutex_lock(&lock_); > - if (ret) { > - ALOGE("Failed to acquire compositor lock %d", ret); > - return ret; > - } > - > - // Block the queue if it gets too large. Otherwise, SurfaceFlinger will start > - // to eat our buffer handles when we get about 1 second behind. > - while (composite_queue_.size() >= DRM_DISPLAY_COMPOSITOR_MAX_QUEUE_DEPTH) { > - pthread_mutex_unlock(&lock_); > - sched_yield(); > - pthread_mutex_lock(&lock_); > - } > - > - composite_queue_.push(std::move(composition)); > - > - ret = pthread_mutex_unlock(&lock_); > - if (ret) { > - ALOGE("Failed to release compositor lock %d", ret); > - return ret; > - } > - > - worker_.Signal(); > - return 0; > -} > - > std::tuple<uint32_t, uint32_t, int> > DrmDisplayCompositor::GetActiveModeResolution() { > DrmConnector *connector = drm_->GetConnectorForDisplay(display_); > @@ -514,6 +395,15 @@ int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) { > std::vector<DrmCompositionRegion> &pre_comp_regions = > display_comp->pre_comp_regions(); > > + if (!pre_compositor_) { > + pre_compositor_.reset(new GLWorkerCompositor()); > + int ret = pre_compositor_->Init(); > + if (ret) { > + ALOGE("Failed to initialize OpenGL compositor %d", ret); > + return ret; > + } > + } > + > int squash_layer_index = -1; > if (squash_regions.size() > 0) { > squash_framebuffer_index_ = (squash_framebuffer_index_ + 1) % 2; > @@ -906,41 +796,9 @@ void DrmDisplayCompositor::ApplyFrame( > ALOGE("Failed to release lock for active_composition swap"); > } > > -int DrmDisplayCompositor::Composite() { > - ATRACE_CALL(); > - > - if (!pre_compositor_) { > - pre_compositor_.reset(new GLWorkerCompositor()); > - int ret = pre_compositor_->Init(); > - if (ret) { > - ALOGE("Failed to initialize OpenGL compositor %d", ret); > - return ret; > - } > - } > - > - int ret = pthread_mutex_lock(&lock_); > - if (ret) { > - ALOGE("Failed to acquire compositor lock %d", ret); > - return ret; > - } > - if (composite_queue_.empty()) { > - ret = pthread_mutex_unlock(&lock_); > - if (ret) > - ALOGE("Failed to release compositor lock %d", ret); > - return ret; > - } > - > - std::unique_ptr<DrmDisplayComposition> composition( > - std::move(composite_queue_.front())); > - > - composite_queue_.pop(); > - > - ret = pthread_mutex_unlock(&lock_); > - if (ret) { > - ALOGE("Failed to release compositor lock %d", ret); > - return ret; > - } > - > +int DrmDisplayCompositor::ApplyComposition( > + std::unique_ptr<DrmDisplayComposition> composition) { > + int ret = 0; > switch (composition->type()) { > case DRM_COMPOSITION_TYPE_FRAME: > ret = PrepareFrame(composition.get()); > @@ -959,7 +817,7 @@ int DrmDisplayCompositor::Composite() { > } > > // If use_hw_overlays_ is false, we can't use hardware to composite the > - // frame. So squash all layers into a single composition and queue that > + // frame. So squash all layers into a single composition and apply that > // instead. > if (!use_hw_overlays_) { > std::unique_ptr<DrmDisplayComposition> squashed = CreateComposition(); > @@ -975,9 +833,10 @@ int DrmDisplayCompositor::Composite() { > return ret; > } > } > - frame_worker_.QueueFrame(std::move(composition), ret); > + ApplyFrame(std::move(composition), ret); > break; > case DRM_COMPOSITION_TYPE_DPMS: > + active_ = (composition->dpms_mode() == DRM_MODE_DPMS_ON); > ret = ApplyDpms(composition.get()); > if (ret) > ALOGE("Failed to apply dpms for display %d", display_); > @@ -1001,24 +860,6 @@ int DrmDisplayCompositor::Composite() { > return ret; > } > > -bool DrmDisplayCompositor::HaveQueuedComposites() const { > - int ret = pthread_mutex_lock(&lock_); > - if (ret) { > - ALOGE("Failed to acquire compositor lock %d", ret); > - return false; > - } > - > - bool empty_ret = !composite_queue_.empty(); > - > - ret = pthread_mutex_unlock(&lock_); > - if (ret) { > - ALOGE("Failed to release compositor lock %d", ret); > - return false; > - } > - > - return empty_ret; > -} > - > int DrmDisplayCompositor::SquashAll() { > AutoLock lock(&lock_, "compositor"); > int ret = lock.Lock(); > diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h > index 9487cac..f1965fb 100644 > --- a/drmdisplaycompositor.h > +++ b/drmdisplaycompositor.h > @@ -18,14 +18,12 @@ > #define ANDROID_DRM_DISPLAY_COMPOSITOR_H_ > > #include "drmhwcomposer.h" > -#include "drmcomposition.h" > -#include "drmcompositorworker.h" > +#include "drmdisplaycomposition.h" > #include "drmframebuffer.h" > #include "separate_rects.h" > > #include <pthread.h> > #include <memory> > -#include <queue> > #include <sstream> > #include <tuple> > > @@ -89,42 +87,18 @@ class DrmDisplayCompositor { > int Init(DrmResources *drm, int display); > > std::unique_ptr<DrmDisplayComposition> CreateComposition() const; > - int QueueComposition(std::unique_ptr<DrmDisplayComposition> composition); > + int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition); > int Composite(); > int SquashAll(); > void Dump(std::ostringstream *out) const; > > std::tuple<uint32_t, uint32_t, int> GetActiveModeResolution(); > > - bool HaveQueuedComposites() const; > - > SquashState *squash_state() { > return &squash_state_; > } > > private: > - struct FrameState { > - std::unique_ptr<DrmDisplayComposition> composition; > - int status = 0; > - }; > - > - class FrameWorker : public Worker { > - public: > - FrameWorker(DrmDisplayCompositor *compositor); > - ~FrameWorker() override; > - > - int Init(); > - void QueueFrame(std::unique_ptr<DrmDisplayComposition> composition, > - int status); > - > - protected: > - void Routine() override; > - > - private: > - DrmDisplayCompositor *compositor_; > - std::queue<FrameState> frame_queue_; > - }; > - > struct ModeState { > bool needs_modeset = false; > DrmMode mode; > @@ -158,10 +132,6 @@ class DrmDisplayCompositor { > DrmResources *drm_; > int display_; > > - DrmCompositorWorker worker_; > - FrameWorker frame_worker_; > - > - std::queue<std::unique_ptr<DrmDisplayComposition>> composite_queue_; > std::unique_ptr<DrmDisplayComposition> active_composition_; > > bool initialized_; > @@ -178,7 +148,7 @@ class DrmDisplayCompositor { > int squash_framebuffer_index_; > DrmFramebuffer squash_framebuffers_[2]; > > - // mutable since we need to acquire in HaveQueuedComposites > + // mutable since we need to acquire in Dump() > mutable pthread_mutex_t lock_; > > // State tracking progress since our last Dump(). These are mutable since > diff --git a/drmeventlistener.cpp b/drmeventlistener.cpp > index 0514aa6..984d1dd 100644 > --- a/drmeventlistener.cpp > +++ b/drmeventlistener.cpp > @@ -20,10 +20,13 @@ > #include "drmresources.h" > > #include <assert.h> > +#include <errno.h> > #include <linux/netlink.h> > #include <sys/socket.h> > > #include <cutils/log.h> > +#include <hardware/hardware.h> > +#include <hardware/hwcomposer.h> > #include <xf86drm.h> > > namespace android { > diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp > index 8c853f4..762ee8c 100644 > --- a/drmhwctwo.cpp > +++ b/drmhwctwo.cpp > @@ -557,7 +557,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { > i = overlay_planes.erase(i); > } > > - ret = compositor_.QueueComposition(std::move(composition)); > + ret = compositor_.ApplyComposition(std::move(composition)); > if (ret) { > ALOGE("Failed to apply the frame composition ret=%d", ret); > return HWC2::Error::BadParameter; > @@ -593,7 +593,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetActiveConfig(hwc2_config_t config) { > compositor_.CreateComposition(); > composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); > int ret = composition->SetDisplayMode(*mode); > - ret = compositor_.QueueComposition(std::move(composition)); > + ret = compositor_.ApplyComposition(std::move(composition)); > if (ret) { > ALOGE("Failed to queue dpms composition on %d", ret); > return HWC2::Error::BadConfig; > @@ -673,7 +673,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetPowerMode(int32_t mode_in) { > compositor_.CreateComposition(); > composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); > composition->SetDpmsMode(dpms_value); > - int ret = compositor_.QueueComposition(std::move(composition)); > + int ret = compositor_.ApplyComposition(std::move(composition)); > if (ret) { > ALOGE("Failed to apply the dpms composition ret=%d", ret); > return HWC2::Error::BadParameter; > diff --git a/drmresources.cpp b/drmresources.cpp > index 6b8ed03..762f5ef 100644 > --- a/drmresources.cpp > +++ b/drmresources.cpp > @@ -35,7 +35,7 @@ > > namespace android { > > -DrmResources::DrmResources() : compositor_(this), event_listener_(this) { > +DrmResources::DrmResources() : event_listener_(this) { > } > > DrmResources::~DrmResources() { > @@ -201,10 +201,6 @@ int DrmResources::Init() { > if (ret) > return ret; > > - ret = compositor_.Init(); > - if (ret) > - return ret; > - > ret = event_listener_.Init(); > if (ret) { > ALOGE("Can't initialize event listener %d", ret); > @@ -333,54 +329,6 @@ int DrmResources::DestroyPropertyBlob(uint32_t blob_id) { > return 0; > } > > -int DrmResources::SetDisplayActiveMode(int display, const DrmMode &mode) { > - std::unique_ptr<DrmComposition> comp(compositor_.CreateComposition(NULL)); > - if (!comp) { > - ALOGE("Failed to create composition for dpms on %d", display); > - return -ENOMEM; > - } > - int ret = comp->SetDisplayMode(display, mode); > - if (ret) { > - ALOGE("Failed to add mode to composition on %d %d", display, ret); > - return ret; > - } > - ret = compositor_.QueueComposition(std::move(comp)); > - if (ret) { > - ALOGE("Failed to queue dpms composition on %d %d", display, ret); > - return ret; > - } > - return 0; > -} > - > -int DrmResources::SetDpmsMode(int display, uint64_t mode) { > - if (mode != DRM_MODE_DPMS_ON && mode != DRM_MODE_DPMS_OFF) { > - ALOGE("Invalid dpms mode %" PRIu64, mode); > - return -EINVAL; > - } > - > - std::unique_ptr<DrmComposition> comp(compositor_.CreateComposition(NULL)); > - if (!comp) { > - ALOGE("Failed to create composition for dpms on %d", display); > - return -ENOMEM; > - } > - int ret = comp->SetDpmsMode(display, mode); > - if (ret) { > - ALOGE("Failed to add dpms %" PRIu64 " to composition on %d %d", mode, > - display, ret); > - return ret; > - } > - ret = compositor_.QueueComposition(std::move(comp)); > - if (ret) { > - ALOGE("Failed to queue dpms composition on %d %d", display, ret); > - return ret; > - } > - return 0; > -} > - > -DrmCompositor *DrmResources::compositor() { > - return &compositor_; > -} > - > DrmEventListener *DrmResources::event_listener() { > return &event_listener_; > } > diff --git a/drmresources.h b/drmresources.h > index 011f87e..a2d8d16 100644 > --- a/drmresources.h > +++ b/drmresources.h > @@ -17,7 +17,6 @@ > #ifndef ANDROID_DRM_H_ > #define ANDROID_DRM_H_ > > -#include "drmcompositor.h" > #include "drmconnector.h" > #include "drmcrtc.h" > #include "drmencoder.h" > @@ -58,7 +57,6 @@ class DrmResources { > DrmConnector *GetConnectorForDisplay(int display) const; > DrmCrtc *GetCrtcForDisplay(int display) const; > DrmPlane *GetPlane(uint32_t id) const; > - DrmCompositor *compositor(); > DrmEventListener *event_listener(); > > int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, > @@ -69,8 +67,6 @@ class DrmResources { > DrmProperty *property); > > uint32_t next_mode_id(); > - int SetDisplayActiveMode(int display, const DrmMode &mode); > - int SetDpmsMode(int display, uint64_t mode); > > int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); > int DestroyPropertyBlob(uint32_t blob_id); > @@ -89,7 +85,6 @@ class DrmResources { > std::vector<std::unique_ptr<DrmEncoder>> encoders_; > std::vector<std::unique_ptr<DrmCrtc>> crtcs_; > std::vector<std::unique_ptr<DrmPlane>> planes_; > - DrmCompositor compositor_; > DrmEventListener event_listener_; > > std::pair<uint32_t, uint32_t> min_resolution_; > diff --git a/glworker.cpp b/glworker.cpp > index e12995e..e90576a 100644 > --- a/glworker.cpp > +++ b/glworker.cpp > @@ -143,6 +143,38 @@ static bool HasExtension(const char *extension, const char *extensions) { > return false; > } > > +int GLWorkerCompositor::BeginContext() { > + private_.saved_egl_display = eglGetCurrentDisplay(); > + private_.saved_egl_ctx = eglGetCurrentContext(); > + > + if (private_.saved_egl_display != egl_display_ || > + private_.saved_egl_ctx != egl_ctx_) { > + private_.saved_egl_read = eglGetCurrentSurface(EGL_READ); > + private_.saved_egl_draw = eglGetCurrentSurface(EGL_DRAW); > + } else { > + return 0; > + } > + > + if (!eglMakeCurrent(egl_display_, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_ctx_)) { > + ALOGE("BeginContext failed: %s", GetEGLError()); > + return 1; > + } > + return 0; > +} > + > +int GLWorkerCompositor::EndContext() { > + if (private_.saved_egl_display != eglGetCurrentDisplay() || > + private_.saved_egl_ctx != eglGetCurrentContext()) { > + if (!eglMakeCurrent(private_.saved_egl_display, private_.saved_egl_read, > + private_.saved_egl_draw, private_.saved_egl_ctx)) { > + ALOGE("EndContext failed: %s", GetEGLError()); > + return 1; > + } > + } > + > + return 0; > +} > + > static AutoGLShader CompileAndCheckShader(GLenum type, unsigned source_count, > const GLchar **sources, > std::ostringstream *shader_log) { > @@ -508,10 +540,9 @@ int GLWorkerCompositor::Init() { > return 1; > } > > - if (!eglMakeCurrent(egl_display_, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_ctx_)) { > - ALOGE("Failed to make the OpenGL ES Context current: %s", GetEGLError()); > - return 1; > - } > + ret = BeginContext(); > + if (ret) > + return ret; > > gl_extensions = (const char *)glGetString(GL_EXTENSIONS); > > @@ -530,6 +561,9 @@ int GLWorkerCompositor::Init() { > > std::ostringstream shader_log; > blend_programs_.emplace_back(GenerateProgram(1, &shader_log)); > + > + EndContext(); > + > if (blend_programs_.back().get() == 0) { > ALOGE("%s", shader_log.str().c_str()); > return 1; > @@ -558,12 +592,17 @@ int GLWorkerCompositor::Composite(DrmHwcLayer *layers, > return -EALREADY; > } > > + ret = BeginContext(); > + if (ret) > + return -1; > + > GLint frame_width = framebuffer->getWidth(); > GLint frame_height = framebuffer->getHeight(); > CachedFramebuffer *cached_framebuffer = > PrepareAndCacheFramebuffer(framebuffer); > if (cached_framebuffer == NULL) { > ALOGE("Composite failed because of failed framebuffer"); > + EndContext(); > return -EINVAL; > } > > @@ -597,8 +636,10 @@ int GLWorkerCompositor::Composite(DrmHwcLayer *layers, > } > } > > - if (ret) > + if (ret) { > + EndContext(); > return ret; > + } > > glViewport(0, 0, frame_width, frame_height); > > @@ -676,6 +717,7 @@ int GLWorkerCompositor::Composite(DrmHwcLayer *layers, > > glBindFramebuffer(GL_FRAMEBUFFER, 0); > > + EndContext(); > return ret; > } > > diff --git a/glworker.h b/glworker.h > index 158490c..26de55d 100644 > --- a/glworker.h > +++ b/glworker.h > @@ -64,6 +64,16 @@ class GLWorkerCompositor { > bool Promote(); > }; > > + struct { > + EGLDisplay saved_egl_display = EGL_NO_DISPLAY; > + EGLContext saved_egl_ctx = EGL_NO_CONTEXT; > + EGLSurface saved_egl_read = EGL_NO_SURFACE; > + EGLSurface saved_egl_draw = EGL_NO_SURFACE; > + } private_; > + > + int BeginContext(); > + int EndContext(); > + > CachedFramebuffer *FindCachedFramebuffer( > const sp<GraphicBuffer> &framebuffer); > CachedFramebuffer *PrepareAndCacheFramebuffer( > -- > 2.11.0 >
On Wed, 2017-09-27 at 15:14 -0400, Sean Paul wrote: > On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.c > om> wrote: > > From: Sean Paul <seanpaul@chromium.org> > > > > Since HWC2 doesn't require the use of threads to implement correct > > synchronization, remove some of these threads. > > > > My SoB seems to have been dropped (or perhaps I just forgot to add it > in the original thread). At any rate, here you go: > > Signed-off-by: Sean Paul <seanpaul@chromium.org> Fixed in v3. Rob.
2017-09-27 19:58 GMT+08:00 Robert Foss <robert.foss@collabora.com>: > From: Sean Paul <seanpaul@chromium.org> > > Since HWC2 doesn't require the use of threads to implement correct > synchronization, remove some of these threads. May I ask to avoid HWC2 only implementation? The main reason is not all GPUs support drm_hwcompser (as discussed in another thread). To continue supporting these GPUs we need to keep using HWC1 version of SurfaceFlinger. So it's better to keep the code compatible with HWC1. At least make it be a compile-time option. Personally I have a patch to make HWC1 vs HWC2 a compile-time choice of drm_hwcomposer. I can submit it if it's acceptable.
On Thu, Sep 28, 2017 at 11:43 AM, Chih-Wei Huang <cwhuang@android-x86.org> wrote: > 2017-09-27 19:58 GMT+08:00 Robert Foss <robert.foss@collabora.com>: >> From: Sean Paul <seanpaul@chromium.org> >> >> Since HWC2 doesn't require the use of threads to implement correct >> synchronization, remove some of these threads. > > May I ask to avoid HWC2 only implementation? > The main reason is not all GPUs support > drm_hwcompser (as discussed in another thread). Which thread? Is that because they don't support explicit fences? Or something else? > To continue supporting these GPUs we need to > keep using HWC1 version of SurfaceFlinger. I think that is a lot of complexity to keep which will impact future changes as well. For example, is keeping it going to make removing sw_sync dependency (A non-stable debugfs interface) more difficult? drm_hwcomposer is already complex enough IMO with the GL compositing that removing some complexity would be a good thing. > So it's better to keep the code compatible with > HWC1. At least make it be a compile-time option. > > Personally I have a patch to make > HWC1 vs HWC2 a compile-time choice > of drm_hwcomposer. Perhaps just leave the current state as a separate branch. > I can submit it if it's acceptable. > > > -- > Chih-Wei > Android-x86 project > http://www.android-x86.org
2017-09-29 5:29 GMT+08:00 Rob Herring <robh@kernel.org>: > On Thu, Sep 28, 2017 at 11:43 AM, Chih-Wei Huang > <cwhuang@android-x86.org> wrote: >> 2017-09-27 19:58 GMT+08:00 Robert Foss <robert.foss@collabora.com>: >>> From: Sean Paul <seanpaul@chromium.org> >>> >>> Since HWC2 doesn't require the use of threads to implement correct >>> synchronization, remove some of these threads. >> >> May I ask to avoid HWC2 only implementation? >> The main reason is not all GPUs support >> drm_hwcompser (as discussed in another thread). > > Which thread? Is that because they don't support explicit fences? Or > something else? The "drm_hwcomposer moving to fd.o" thread. For example, see https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html >> To continue supporting these GPUs we need to >> keep using HWC1 version of SurfaceFlinger. > > I think that is a lot of complexity to keep which will impact future > changes as well. For example, is keeping it going to make removing > sw_sync dependency (A non-stable debugfs interface) more difficult? > drm_hwcomposer is already complex enough IMO with the GL compositing > that removing some complexity would be a good thing. > >> So it's better to keep the code compatible with >> HWC1. At least make it be a compile-time option. >> >> Personally I have a patch to make >> HWC1 vs HWC2 a compile-time choice >> of drm_hwcomposer. FYI, the patch is https://osdn.net/projects/android-x86/scm/git/external-drm_hwcomposer/commits/7acc332019d211cb2747fd4068cf41aaa62753fb > Perhaps just leave the current state as a separate branch. Did you mean we maintain the branch in our repo? (that's what we do now, but I hope to avoid that) Or fd.o could help to maintain the two branches (HWC1 and HWC2)?
On Fri, 2017-09-29 at 13:49 +0800, Chih-Wei Huang wrote: > 2017-09-29 5:29 GMT+08:00 Rob Herring <robh@kernel.org>: > > On Thu, Sep 28, 2017 at 11:43 AM, Chih-Wei Huang > > <cwhuang@android-x86.org> wrote: > > > 2017-09-27 19:58 GMT+08:00 Robert Foss <robert.foss@collabora.com > > > >: > > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > > > Since HWC2 doesn't require the use of threads to implement > > > > correct > > > > synchronization, remove some of these threads. > > > > > > May I ask to avoid HWC2 only implementation? > > > The main reason is not all GPUs support > > > drm_hwcompser (as discussed in another thread). > > > > Which thread? Is that because they don't support explicit fences? > > Or > > something else? > > The "drm_hwcomposer moving to fd.o" thread. > For example, see > https://lists.freedesktop.org/archives/dri-devel/2017-September/15358 > 0.html > > > > To continue supporting these GPUs we need to > > > keep using HWC1 version of SurfaceFlinger. > > > > I think that is a lot of complexity to keep which will impact > > future > > changes as well. For example, is keeping it going to make removing > > sw_sync dependency (A non-stable debugfs interface) more difficult? > > drm_hwcomposer is already complex enough IMO with the GL > > compositing > > that removing some complexity would be a good thing. > > > > > So it's better to keep the code compatible with > > > HWC1. At least make it be a compile-time option. > > > > > > Personally I have a patch to make > > > HWC1 vs HWC2 a compile-time choice > > > of drm_hwcomposer. > > FYI, the patch is > https://osdn.net/projects/android-x86/scm/git/external-drm_hwcomposer > /commits/7acc332019d211cb2747fd4068cf41aaa62753fb > > > Perhaps just leave the current state as a separate branch. > > Did you mean we maintain the branch in our repo? > (that's what we do now, but I hope to avoid that) > > Or fd.o could help to maintain the two branches (HWC1 and HWC2)? > Android later than O will not support HWC1 (as far as I understand it), so HWC2 is the way forward. Furthermore I think targeting aosp/master at all time is the right thing to do for drm_hwcomposer. I for one am less than keen on maintaining branch that is incompatible with aosp/master upstream. Ideally we wouldn't maintain a compile time switch either, not on principle but because of the development overhead it causes. We have very finite resources contributing to drm_hwcomposer. If it was cheap&&easy to support old Android versions we should, but I don't think it is. I would suggest maintaining a HWC1 fork downstream as the way forward. But any input is welcome. Rob.
2017-09-29 16:44 GMT+08:00 Robert Foss <robert.foss@collabora.com>: > On Fri, 2017-09-29 at 13:49 +0800, Chih-Wei Huang wrote: >> 2017-09-29 5:29 GMT+08:00 Rob Herring <robh@kernel.org>: >> > Perhaps just leave the current state as a separate branch. >> >> Did you mean we maintain the branch in our repo? >> (that's what we do now, but I hope to avoid that) >> >> Or fd.o could help to maintain the two branches (HWC1 and HWC2)? > > Android later than O will not support HWC1 (as far as I understand it), > so HWC2 is the way forward. If all x86 GPUs we want to support can work with drm_hwcomposer, I'm happy to switch to HWC2. However it seems impossible at this moment. :( > Furthermore I think targeting aosp/master at all time is the right > thing to do for drm_hwcomposer. > > I for one am less than keen on maintaining branch that is incompatible > with aosp/master upstream. > > Ideally we wouldn't maintain a compile time switch either, not on > principle but because of the development overhead it causes. > We have very finite resources contributing to drm_hwcomposer. > If it was cheap&&easy to support old Android versions we should, but I > don't think it is. My point is not to support old Android versions, but to support old(?) GPUs that can't work with drm_hwcomposer. > I would suggest maintaining a HWC1 fork downstream as the way forward. > But any input is welcome.
On Fri, 2017-09-29 at 17:07 +0800, Chih-Wei Huang wrote: > 2017-09-29 16:44 GMT+08:00 Robert Foss <robert.foss@collabora.com>: > > On Fri, 2017-09-29 at 13:49 +0800, Chih-Wei Huang wrote: > > > 2017-09-29 5:29 GMT+08:00 Rob Herring <robh@kernel.org>: > > > > Perhaps just leave the current state as a separate branch. > > > > > > Did you mean we maintain the branch in our repo? > > > (that's what we do now, but I hope to avoid that) > > > > > > Or fd.o could help to maintain the two branches (HWC1 and HWC2)? > > > > Android later than O will not support HWC1 (as far as I understand > > it), > > so HWC2 is the way forward. > > If all x86 GPUs we want to support > can work with drm_hwcomposer, > I'm happy to switch to HWC2. > However it seems impossible at this moment. :( > > > Furthermore I think targeting aosp/master at all time is the right > > thing to do for drm_hwcomposer. > > > > I for one am less than keen on maintaining branch that is > > incompatible > > with aosp/master upstream. > > > > Ideally we wouldn't maintain a compile time switch either, not on > > principle but because of the development overhead it causes. > > We have very finite resources contributing to drm_hwcomposer. > > If it was cheap&&easy to support old Android versions we should, > > but I > > don't think it is. > > My point is not to support old Android versions, > but to support old(?) GPUs that can't work with drm_hwcomposer. > If some GPU (which supports fences and atomic) does not work on drm_hwcomposer, I would consider that a bug. So the idea would be to fix it, and fixing it would prevent you from having any issues with HWC2 being the only supported API as far as I understand our IRC chat. Rob.
diff --git a/Android.mk b/Android.mk index aa95b44..5d16c2f 100644 --- a/Android.mk +++ b/Android.mk @@ -57,9 +57,6 @@ LOCAL_C_INCLUDES := \ LOCAL_SRC_FILES := \ autolock.cpp \ drmresources.cpp \ - drmcomposition.cpp \ - drmcompositor.cpp \ - drmcompositorworker.cpp \ drmconnector.cpp \ drmcrtc.cpp \ drmdisplaycomposition.cpp \ diff --git a/drmcomposition.cpp b/drmcomposition.cpp deleted file mode 100644 index 1aaf920..0000000 --- a/drmcomposition.cpp +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "hwc-drm-composition" - -#include "drmcomposition.h" -#include "drmcrtc.h" -#include "drmplane.h" -#include "drmresources.h" -#include "platform.h" - -#include <stdlib.h> - -#include <cutils/log.h> -#include <cutils/properties.h> -#include <sw_sync.h> -#include <sync/sync.h> - -namespace android { - -DrmComposition::DrmComposition(DrmResources *drm, Importer *importer, - Planner *planner) - : drm_(drm), importer_(importer), planner_(planner) { - char use_overlay_planes_prop[PROPERTY_VALUE_MAX]; - property_get("hwc.drm.use_overlay_planes", use_overlay_planes_prop, "1"); - bool use_overlay_planes = atoi(use_overlay_planes_prop); - - for (auto &plane : drm->planes()) { - if (plane->type() == DRM_PLANE_TYPE_PRIMARY) - primary_planes_.push_back(plane.get()); - else if (use_overlay_planes && plane->type() == DRM_PLANE_TYPE_OVERLAY) - overlay_planes_.push_back(plane.get()); - } -} - -int DrmComposition::Init(uint64_t frame_no) { - for (auto &conn : drm_->connectors()) { - int display = conn->display(); - composition_map_[display].reset(new DrmDisplayComposition()); - if (!composition_map_[display]) { - ALOGE("Failed to allocate new display composition\n"); - return -ENOMEM; - } - - // If the display hasn't been modeset yet, this will be NULL - DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); - - int ret = composition_map_[display]->Init(drm_, crtc, importer_, planner_, - frame_no); - if (ret) { - ALOGE("Failed to init display composition for %d", display); - return ret; - } - } - return 0; -} - -int DrmComposition::SetLayers(size_t num_displays, - DrmCompositionDisplayLayersMap *maps) { - int ret = 0; - for (size_t display_index = 0; display_index < num_displays; - display_index++) { - DrmCompositionDisplayLayersMap &map = maps[display_index]; - int display = map.display; - - if (!drm_->GetConnectorForDisplay(display)) { - ALOGE("Invalid display given to SetLayers %d", display); - continue; - } - - ret = composition_map_[display]->SetLayers( - map.layers.data(), map.layers.size(), map.geometry_changed); - if (ret) - return ret; - } - - return 0; -} - -int DrmComposition::SetDpmsMode(int display, uint32_t dpms_mode) { - return composition_map_[display]->SetDpmsMode(dpms_mode); -} - -int DrmComposition::SetDisplayMode(int display, const DrmMode &display_mode) { - return composition_map_[display]->SetDisplayMode(display_mode); -} - -std::unique_ptr<DrmDisplayComposition> DrmComposition::TakeDisplayComposition( - int display) { - return std::move(composition_map_[display]); -} - -int DrmComposition::Plan(std::map<int, DrmDisplayCompositor> &compositor_map) { - int ret = 0; - for (auto &conn : drm_->connectors()) { - int display = conn->display(); - DrmDisplayComposition *comp = GetDisplayComposition(display); - ret = comp->Plan(compositor_map[display].squash_state(), &primary_planes_, - &overlay_planes_); - if (ret) { - ALOGE("Failed to plan composition for dislay %d", display); - return ret; - } - } - - return 0; -} - -int DrmComposition::DisableUnusedPlanes() { - for (auto &conn : drm_->connectors()) { - int display = conn->display(); - DrmDisplayComposition *comp = GetDisplayComposition(display); - - /* - * Leave empty compositions alone - * TODO: re-visit this and potentially disable leftover planes after the - * active compositions have gobbled up all they can - */ - if (comp->type() == DRM_COMPOSITION_TYPE_EMPTY || - comp->type() == DRM_COMPOSITION_TYPE_MODESET) - continue; - - DrmCrtc *crtc = drm_->GetCrtcForDisplay(display); - if (!crtc) { - ALOGE("Failed to find crtc for display %d", display); - continue; - } - - for (std::vector<DrmPlane *>::iterator iter = primary_planes_.begin(); - iter != primary_planes_.end(); ++iter) { - if ((*iter)->GetCrtcSupported(*crtc)) { - comp->AddPlaneDisable(*iter); - primary_planes_.erase(iter); - break; - } - } - for (std::vector<DrmPlane *>::iterator iter = overlay_planes_.begin(); - iter != overlay_planes_.end();) { - if ((*iter)->GetCrtcSupported(*crtc)) { - comp->AddPlaneDisable(*iter); - iter = overlay_planes_.erase(iter); - } else { - iter++; - } - } - } - return 0; -} - -DrmDisplayComposition *DrmComposition::GetDisplayComposition(int display) { - return composition_map_[display].get(); -} -} diff --git a/drmcomposition.h b/drmcomposition.h deleted file mode 100644 index eae8cde..0000000 --- a/drmcomposition.h +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_DRM_COMPOSITION_H_ -#define ANDROID_DRM_COMPOSITION_H_ - -#include "drmhwcomposer.h" -#include "drmdisplaycomposition.h" -#include "drmplane.h" -#include "platform.h" - -#include <map> -#include <vector> - -#include <hardware/hardware.h> -#include <hardware/hwcomposer.h> - -namespace android { - -class DrmDisplayCompositor; - -struct DrmCompositionDisplayLayersMap { - int display; - bool geometry_changed = true; - std::vector<DrmHwcLayer> layers; - - DrmCompositionDisplayLayersMap() = default; - DrmCompositionDisplayLayersMap(DrmCompositionDisplayLayersMap &&rhs) = - default; -}; - -class DrmComposition { - public: - DrmComposition(DrmResources *drm, Importer *importer, Planner *planner); - - int Init(uint64_t frame_no); - - int SetLayers(size_t num_displays, DrmCompositionDisplayLayersMap *maps); - int SetDpmsMode(int display, uint32_t dpms_mode); - int SetDisplayMode(int display, const DrmMode &display_mode); - - std::unique_ptr<DrmDisplayComposition> TakeDisplayComposition(int display); - DrmDisplayComposition *GetDisplayComposition(int display); - - int Plan(std::map<int, DrmDisplayCompositor> &compositor_map); - int DisableUnusedPlanes(); - - private: - DrmComposition(const DrmComposition &) = delete; - - DrmResources *drm_; - Importer *importer_; - Planner *planner_; - - std::vector<DrmPlane *> primary_planes_; - std::vector<DrmPlane *> overlay_planes_; - - /* - * This _must_ be read-only after it's passed to QueueComposition. Otherwise - * locking is required to maintain consistency across the compositor threads. - */ - std::map<int, std::unique_ptr<DrmDisplayComposition>> composition_map_; -}; -} - -#endif // ANDROID_DRM_COMPOSITION_H_ diff --git a/drmcompositor.cpp b/drmcompositor.cpp deleted file mode 100644 index c1f3ed8..0000000 --- a/drmcompositor.cpp +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "hwc-drm-compositor" - -#include "drmcompositor.h" -#include "drmdisplaycompositor.h" -#include "drmresources.h" -#include "platform.h" - -#include <sstream> -#include <stdlib.h> - -#include <cutils/log.h> - -namespace android { - -DrmCompositor::DrmCompositor(DrmResources *drm) : drm_(drm), frame_no_(0) { -} - -DrmCompositor::~DrmCompositor() { -} - -int DrmCompositor::Init() { - for (auto &conn : drm_->connectors()) { - int display = conn->display(); - int ret = compositor_map_[display].Init(drm_, display); - if (ret) { - ALOGE("Failed to initialize display compositor for %d", display); - return ret; - } - } - planner_ = Planner::CreateInstance(drm_); - if (!planner_) { - ALOGE("Failed to create planner instance for composition"); - return -ENOMEM; - } - - return 0; -} - -std::unique_ptr<DrmComposition> DrmCompositor::CreateComposition( - Importer *importer) { - std::unique_ptr<DrmComposition> composition( - new DrmComposition(drm_, importer, planner_.get())); - int ret = composition->Init(++frame_no_); - if (ret) { - ALOGE("Failed to initialize drm composition %d", ret); - return nullptr; - } - return composition; -} - -int DrmCompositor::QueueComposition( - std::unique_ptr<DrmComposition> composition) { - int ret; - - ret = composition->Plan(compositor_map_); - if (ret) - return ret; - - ret = composition->DisableUnusedPlanes(); - if (ret) - return ret; - - for (auto &conn : drm_->connectors()) { - int display = conn->display(); - int ret = compositor_map_[display].QueueComposition( - composition->TakeDisplayComposition(display)); - if (ret) { - ALOGE("Failed to queue composition for display %d (%d)", display, ret); - return ret; - } - } - - return 0; -} - -int DrmCompositor::Composite() { - /* - * This shouldn't be called, we should be calling Composite() on the display - * compositors directly. - */ - ALOGE("Calling base drm compositor Composite() function"); - return -EINVAL; -} - -void DrmCompositor::Dump(std::ostringstream *out) const { - *out << "DrmCompositor stats:\n"; - for (auto &conn : drm_->connectors()) - compositor_map_[conn->display()].Dump(out); -} -} diff --git a/drmcompositor.h b/drmcompositor.h deleted file mode 100644 index 19271b5..0000000 --- a/drmcompositor.h +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_DRM_COMPOSITOR_H_ -#define ANDROID_DRM_COMPOSITOR_H_ - -#include "drmcomposition.h" -#include "drmdisplaycompositor.h" -#include "platform.h" - -#include <map> -#include <memory> -#include <sstream> - -namespace android { - -class DrmCompositor { - public: - DrmCompositor(DrmResources *drm); - ~DrmCompositor(); - - int Init(); - - std::unique_ptr<DrmComposition> CreateComposition(Importer *importer); - - int QueueComposition(std::unique_ptr<DrmComposition> composition); - int Composite(); - void Dump(std::ostringstream *out) const; - - private: - DrmCompositor(const DrmCompositor &) = delete; - - DrmResources *drm_; - std::unique_ptr<Planner> planner_; - - uint64_t frame_no_; - - // mutable for Dump() propagation - mutable std::map<int, DrmDisplayCompositor> compositor_map_; -}; -} - -#endif // ANDROID_DRM_COMPOSITOR_H_ diff --git a/drmcompositorworker.h b/drmcompositorworker.h deleted file mode 100644 index 731bc65..0000000 --- a/drmcompositorworker.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_DRM_COMPOSITOR_WORKER_H_ -#define ANDROID_DRM_COMPOSITOR_WORKER_H_ - -#include "worker.h" - -namespace android { - -class DrmDisplayCompositor; - -class DrmCompositorWorker : public Worker { - public: - DrmCompositorWorker(DrmDisplayCompositor *compositor); - ~DrmCompositorWorker() override; - - int Init(); - - protected: - void Routine() override; - - DrmDisplayCompositor *compositor_; - bool did_squash_all_ = false; -}; -} - -#endif diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp index 0f8084b..66e67a4 100644 --- a/drmdisplaycomposition.cpp +++ b/drmdisplaycomposition.cpp @@ -17,6 +17,7 @@ #define LOG_TAG "hwc-drm-display-composition" #include "drmdisplaycomposition.h" +#include "drmdisplaycompositor.h" #include "drmcrtc.h" #include "drmplane.h" #include "drmresources.h" diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h index 13da19d..b165adc 100644 --- a/drmdisplaycomposition.h +++ b/drmdisplaycomposition.h @@ -42,6 +42,16 @@ enum DrmCompositionType { DRM_COMPOSITION_TYPE_MODESET, }; +struct DrmCompositionDisplayLayersMap { + int display; + bool geometry_changed = true; + std::vector<DrmHwcLayer> layers; + + DrmCompositionDisplayLayersMap() = default; + DrmCompositionDisplayLayersMap(DrmCompositionDisplayLayersMap &&rhs) = + default; +}; + struct DrmCompositionRegion { DrmHwcRect<int> frame; std::vector<size_t> source_layers; diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index a1baed1..bd670cf 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -37,8 +37,6 @@ #include "drmresources.h" #include "glworker.h" -#define DRM_DISPLAY_COMPOSITOR_MAX_QUEUE_DEPTH 2 - namespace android { void SquashState::Init(DrmHwcLayer *layers, size_t num_layers) { @@ -176,58 +174,9 @@ static bool UsesSquash(const std::vector<DrmCompositionPlane> &comp_planes) { }); } -DrmDisplayCompositor::FrameWorker::FrameWorker(DrmDisplayCompositor *compositor) - : Worker("frame-worker", HAL_PRIORITY_URGENT_DISPLAY), - compositor_(compositor) { -} - -DrmDisplayCompositor::FrameWorker::~FrameWorker() { -} - -int DrmDisplayCompositor::FrameWorker::Init() { - return InitWorker(); -} - -void DrmDisplayCompositor::FrameWorker::QueueFrame( - std::unique_ptr<DrmDisplayComposition> composition, int status) { - Lock(); - FrameState frame; - frame.composition = std::move(composition); - frame.status = status; - frame_queue_.push(std::move(frame)); - Unlock(); - Signal(); -} - -void DrmDisplayCompositor::FrameWorker::Routine() { - int wait_ret = 0; - - Lock(); - if (frame_queue_.empty()) { - wait_ret = WaitForSignalOrExitLocked(); - } - - FrameState frame; - if (!frame_queue_.empty()) { - frame = std::move(frame_queue_.front()); - frame_queue_.pop(); - } - Unlock(); - - if (wait_ret == -EINTR) { - return; - } else if (wait_ret) { - ALOGE("Failed to wait for signal, %d", wait_ret); - return; - } - compositor_->ApplyFrame(std::move(frame.composition), frame.status); -} - DrmDisplayCompositor::DrmDisplayCompositor() : drm_(NULL), display_(-1), - worker_(this), - frame_worker_(this), initialized_(false), active_(false), use_hw_overlays_(true), @@ -245,9 +194,6 @@ DrmDisplayCompositor::~DrmDisplayCompositor() { if (!initialized_) return; - worker_.Exit(); - frame_worker_.Exit(); - int ret = pthread_mutex_lock(&lock_); if (ret) ALOGE("Failed to acquire compositor lock %d", ret); @@ -257,10 +203,6 @@ DrmDisplayCompositor::~DrmDisplayCompositor() { if (mode_.old_blob_id) drm_->DestroyPropertyBlob(mode_.old_blob_id); - while (!composite_queue_.empty()) { - composite_queue_.front().reset(); - composite_queue_.pop(); - } active_composition_.reset(); ret = pthread_mutex_unlock(&lock_); @@ -279,18 +221,6 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int display) { ALOGE("Failed to initialize drm compositor lock %d\n", ret); return ret; } - ret = worker_.Init(); - if (ret) { - pthread_mutex_destroy(&lock_); - ALOGE("Failed to initialize compositor worker %d\n", ret); - return ret; - } - ret = frame_worker_.Init(); - if (ret) { - pthread_mutex_destroy(&lock_); - ALOGE("Failed to initialize frame worker %d\n", ret); - return ret; - } initialized_ = true; return 0; @@ -301,55 +231,6 @@ std::unique_ptr<DrmDisplayComposition> DrmDisplayCompositor::CreateComposition() return std::unique_ptr<DrmDisplayComposition>(new DrmDisplayComposition()); } -int DrmDisplayCompositor::QueueComposition( - std::unique_ptr<DrmDisplayComposition> composition) { - switch (composition->type()) { - case DRM_COMPOSITION_TYPE_FRAME: - if (!active_) - return -ENODEV; - break; - case DRM_COMPOSITION_TYPE_DPMS: - /* - * Update the state as soon as we get it so we can start/stop queuing - * frames asap. - */ - active_ = (composition->dpms_mode() == DRM_MODE_DPMS_ON); - break; - case DRM_COMPOSITION_TYPE_MODESET: - break; - case DRM_COMPOSITION_TYPE_EMPTY: - return 0; - default: - ALOGE("Unknown composition type %d/%d", composition->type(), display_); - return -ENOENT; - } - - int ret = pthread_mutex_lock(&lock_); - if (ret) { - ALOGE("Failed to acquire compositor lock %d", ret); - return ret; - } - - // Block the queue if it gets too large. Otherwise, SurfaceFlinger will start - // to eat our buffer handles when we get about 1 second behind. - while (composite_queue_.size() >= DRM_DISPLAY_COMPOSITOR_MAX_QUEUE_DEPTH) { - pthread_mutex_unlock(&lock_); - sched_yield(); - pthread_mutex_lock(&lock_); - } - - composite_queue_.push(std::move(composition)); - - ret = pthread_mutex_unlock(&lock_); - if (ret) { - ALOGE("Failed to release compositor lock %d", ret); - return ret; - } - - worker_.Signal(); - return 0; -} - std::tuple<uint32_t, uint32_t, int> DrmDisplayCompositor::GetActiveModeResolution() { DrmConnector *connector = drm_->GetConnectorForDisplay(display_); @@ -514,6 +395,15 @@ int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) { std::vector<DrmCompositionRegion> &pre_comp_regions = display_comp->pre_comp_regions(); + if (!pre_compositor_) { + pre_compositor_.reset(new GLWorkerCompositor()); + int ret = pre_compositor_->Init(); + if (ret) { + ALOGE("Failed to initialize OpenGL compositor %d", ret); + return ret; + } + } + int squash_layer_index = -1; if (squash_regions.size() > 0) { squash_framebuffer_index_ = (squash_framebuffer_index_ + 1) % 2; @@ -906,41 +796,9 @@ void DrmDisplayCompositor::ApplyFrame( ALOGE("Failed to release lock for active_composition swap"); } -int DrmDisplayCompositor::Composite() { - ATRACE_CALL(); - - if (!pre_compositor_) { - pre_compositor_.reset(new GLWorkerCompositor()); - int ret = pre_compositor_->Init(); - if (ret) { - ALOGE("Failed to initialize OpenGL compositor %d", ret); - return ret; - } - } - - int ret = pthread_mutex_lock(&lock_); - if (ret) { - ALOGE("Failed to acquire compositor lock %d", ret); - return ret; - } - if (composite_queue_.empty()) { - ret = pthread_mutex_unlock(&lock_); - if (ret) - ALOGE("Failed to release compositor lock %d", ret); - return ret; - } - - std::unique_ptr<DrmDisplayComposition> composition( - std::move(composite_queue_.front())); - - composite_queue_.pop(); - - ret = pthread_mutex_unlock(&lock_); - if (ret) { - ALOGE("Failed to release compositor lock %d", ret); - return ret; - } - +int DrmDisplayCompositor::ApplyComposition( + std::unique_ptr<DrmDisplayComposition> composition) { + int ret = 0; switch (composition->type()) { case DRM_COMPOSITION_TYPE_FRAME: ret = PrepareFrame(composition.get()); @@ -959,7 +817,7 @@ int DrmDisplayCompositor::Composite() { } // If use_hw_overlays_ is false, we can't use hardware to composite the - // frame. So squash all layers into a single composition and queue that + // frame. So squash all layers into a single composition and apply that // instead. if (!use_hw_overlays_) { std::unique_ptr<DrmDisplayComposition> squashed = CreateComposition(); @@ -975,9 +833,10 @@ int DrmDisplayCompositor::Composite() { return ret; } } - frame_worker_.QueueFrame(std::move(composition), ret); + ApplyFrame(std::move(composition), ret); break; case DRM_COMPOSITION_TYPE_DPMS: + active_ = (composition->dpms_mode() == DRM_MODE_DPMS_ON); ret = ApplyDpms(composition.get()); if (ret) ALOGE("Failed to apply dpms for display %d", display_); @@ -1001,24 +860,6 @@ int DrmDisplayCompositor::Composite() { return ret; } -bool DrmDisplayCompositor::HaveQueuedComposites() const { - int ret = pthread_mutex_lock(&lock_); - if (ret) { - ALOGE("Failed to acquire compositor lock %d", ret); - return false; - } - - bool empty_ret = !composite_queue_.empty(); - - ret = pthread_mutex_unlock(&lock_); - if (ret) { - ALOGE("Failed to release compositor lock %d", ret); - return false; - } - - return empty_ret; -} - int DrmDisplayCompositor::SquashAll() { AutoLock lock(&lock_, "compositor"); int ret = lock.Lock(); diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h index 9487cac..f1965fb 100644 --- a/drmdisplaycompositor.h +++ b/drmdisplaycompositor.h @@ -18,14 +18,12 @@ #define ANDROID_DRM_DISPLAY_COMPOSITOR_H_ #include "drmhwcomposer.h" -#include "drmcomposition.h" -#include "drmcompositorworker.h" +#include "drmdisplaycomposition.h" #include "drmframebuffer.h" #include "separate_rects.h" #include <pthread.h> #include <memory> -#include <queue> #include <sstream> #include <tuple> @@ -89,42 +87,18 @@ class DrmDisplayCompositor { int Init(DrmResources *drm, int display); std::unique_ptr<DrmDisplayComposition> CreateComposition() const; - int QueueComposition(std::unique_ptr<DrmDisplayComposition> composition); + int ApplyComposition(std::unique_ptr<DrmDisplayComposition> composition); int Composite(); int SquashAll(); void Dump(std::ostringstream *out) const; std::tuple<uint32_t, uint32_t, int> GetActiveModeResolution(); - bool HaveQueuedComposites() const; - SquashState *squash_state() { return &squash_state_; } private: - struct FrameState { - std::unique_ptr<DrmDisplayComposition> composition; - int status = 0; - }; - - class FrameWorker : public Worker { - public: - FrameWorker(DrmDisplayCompositor *compositor); - ~FrameWorker() override; - - int Init(); - void QueueFrame(std::unique_ptr<DrmDisplayComposition> composition, - int status); - - protected: - void Routine() override; - - private: - DrmDisplayCompositor *compositor_; - std::queue<FrameState> frame_queue_; - }; - struct ModeState { bool needs_modeset = false; DrmMode mode; @@ -158,10 +132,6 @@ class DrmDisplayCompositor { DrmResources *drm_; int display_; - DrmCompositorWorker worker_; - FrameWorker frame_worker_; - - std::queue<std::unique_ptr<DrmDisplayComposition>> composite_queue_; std::unique_ptr<DrmDisplayComposition> active_composition_; bool initialized_; @@ -178,7 +148,7 @@ class DrmDisplayCompositor { int squash_framebuffer_index_; DrmFramebuffer squash_framebuffers_[2]; - // mutable since we need to acquire in HaveQueuedComposites + // mutable since we need to acquire in Dump() mutable pthread_mutex_t lock_; // State tracking progress since our last Dump(). These are mutable since diff --git a/drmeventlistener.cpp b/drmeventlistener.cpp index 0514aa6..984d1dd 100644 --- a/drmeventlistener.cpp +++ b/drmeventlistener.cpp @@ -20,10 +20,13 @@ #include "drmresources.h" #include <assert.h> +#include <errno.h> #include <linux/netlink.h> #include <sys/socket.h> #include <cutils/log.h> +#include <hardware/hardware.h> +#include <hardware/hwcomposer.h> #include <xf86drm.h> namespace android { diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index 8c853f4..762ee8c 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -557,7 +557,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) { i = overlay_planes.erase(i); } - ret = compositor_.QueueComposition(std::move(composition)); + ret = compositor_.ApplyComposition(std::move(composition)); if (ret) { ALOGE("Failed to apply the frame composition ret=%d", ret); return HWC2::Error::BadParameter; @@ -593,7 +593,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetActiveConfig(hwc2_config_t config) { compositor_.CreateComposition(); composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); int ret = composition->SetDisplayMode(*mode); - ret = compositor_.QueueComposition(std::move(composition)); + ret = compositor_.ApplyComposition(std::move(composition)); if (ret) { ALOGE("Failed to queue dpms composition on %d", ret); return HWC2::Error::BadConfig; @@ -673,7 +673,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetPowerMode(int32_t mode_in) { compositor_.CreateComposition(); composition->Init(drm_, crtc_, importer_.get(), planner_.get(), frame_no_); composition->SetDpmsMode(dpms_value); - int ret = compositor_.QueueComposition(std::move(composition)); + int ret = compositor_.ApplyComposition(std::move(composition)); if (ret) { ALOGE("Failed to apply the dpms composition ret=%d", ret); return HWC2::Error::BadParameter; diff --git a/drmresources.cpp b/drmresources.cpp index 6b8ed03..762f5ef 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -35,7 +35,7 @@ namespace android { -DrmResources::DrmResources() : compositor_(this), event_listener_(this) { +DrmResources::DrmResources() : event_listener_(this) { } DrmResources::~DrmResources() { @@ -201,10 +201,6 @@ int DrmResources::Init() { if (ret) return ret; - ret = compositor_.Init(); - if (ret) - return ret; - ret = event_listener_.Init(); if (ret) { ALOGE("Can't initialize event listener %d", ret); @@ -333,54 +329,6 @@ int DrmResources::DestroyPropertyBlob(uint32_t blob_id) { return 0; } -int DrmResources::SetDisplayActiveMode(int display, const DrmMode &mode) { - std::unique_ptr<DrmComposition> comp(compositor_.CreateComposition(NULL)); - if (!comp) { - ALOGE("Failed to create composition for dpms on %d", display); - return -ENOMEM; - } - int ret = comp->SetDisplayMode(display, mode); - if (ret) { - ALOGE("Failed to add mode to composition on %d %d", display, ret); - return ret; - } - ret = compositor_.QueueComposition(std::move(comp)); - if (ret) { - ALOGE("Failed to queue dpms composition on %d %d", display, ret); - return ret; - } - return 0; -} - -int DrmResources::SetDpmsMode(int display, uint64_t mode) { - if (mode != DRM_MODE_DPMS_ON && mode != DRM_MODE_DPMS_OFF) { - ALOGE("Invalid dpms mode %" PRIu64, mode); - return -EINVAL; - } - - std::unique_ptr<DrmComposition> comp(compositor_.CreateComposition(NULL)); - if (!comp) { - ALOGE("Failed to create composition for dpms on %d", display); - return -ENOMEM; - } - int ret = comp->SetDpmsMode(display, mode); - if (ret) { - ALOGE("Failed to add dpms %" PRIu64 " to composition on %d %d", mode, - display, ret); - return ret; - } - ret = compositor_.QueueComposition(std::move(comp)); - if (ret) { - ALOGE("Failed to queue dpms composition on %d %d", display, ret); - return ret; - } - return 0; -} - -DrmCompositor *DrmResources::compositor() { - return &compositor_; -} - DrmEventListener *DrmResources::event_listener() { return &event_listener_; } diff --git a/drmresources.h b/drmresources.h index 011f87e..a2d8d16 100644 --- a/drmresources.h +++ b/drmresources.h @@ -17,7 +17,6 @@ #ifndef ANDROID_DRM_H_ #define ANDROID_DRM_H_ -#include "drmcompositor.h" #include "drmconnector.h" #include "drmcrtc.h" #include "drmencoder.h" @@ -58,7 +57,6 @@ class DrmResources { DrmConnector *GetConnectorForDisplay(int display) const; DrmCrtc *GetCrtcForDisplay(int display) const; DrmPlane *GetPlane(uint32_t id) const; - DrmCompositor *compositor(); DrmEventListener *event_listener(); int GetPlaneProperty(const DrmPlane &plane, const char *prop_name, @@ -69,8 +67,6 @@ class DrmResources { DrmProperty *property); uint32_t next_mode_id(); - int SetDisplayActiveMode(int display, const DrmMode &mode); - int SetDpmsMode(int display, uint64_t mode); int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id); int DestroyPropertyBlob(uint32_t blob_id); @@ -89,7 +85,6 @@ class DrmResources { std::vector<std::unique_ptr<DrmEncoder>> encoders_; std::vector<std::unique_ptr<DrmCrtc>> crtcs_; std::vector<std::unique_ptr<DrmPlane>> planes_; - DrmCompositor compositor_; DrmEventListener event_listener_; std::pair<uint32_t, uint32_t> min_resolution_; diff --git a/glworker.cpp b/glworker.cpp index e12995e..e90576a 100644 --- a/glworker.cpp +++ b/glworker.cpp @@ -143,6 +143,38 @@ static bool HasExtension(const char *extension, const char *extensions) { return false; } +int GLWorkerCompositor::BeginContext() { + private_.saved_egl_display = eglGetCurrentDisplay(); + private_.saved_egl_ctx = eglGetCurrentContext(); + + if (private_.saved_egl_display != egl_display_ || + private_.saved_egl_ctx != egl_ctx_) { + private_.saved_egl_read = eglGetCurrentSurface(EGL_READ); + private_.saved_egl_draw = eglGetCurrentSurface(EGL_DRAW); + } else { + return 0; + } + + if (!eglMakeCurrent(egl_display_, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_ctx_)) { + ALOGE("BeginContext failed: %s", GetEGLError()); + return 1; + } + return 0; +} + +int GLWorkerCompositor::EndContext() { + if (private_.saved_egl_display != eglGetCurrentDisplay() || + private_.saved_egl_ctx != eglGetCurrentContext()) { + if (!eglMakeCurrent(private_.saved_egl_display, private_.saved_egl_read, + private_.saved_egl_draw, private_.saved_egl_ctx)) { + ALOGE("EndContext failed: %s", GetEGLError()); + return 1; + } + } + + return 0; +} + static AutoGLShader CompileAndCheckShader(GLenum type, unsigned source_count, const GLchar **sources, std::ostringstream *shader_log) { @@ -508,10 +540,9 @@ int GLWorkerCompositor::Init() { return 1; } - if (!eglMakeCurrent(egl_display_, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_ctx_)) { - ALOGE("Failed to make the OpenGL ES Context current: %s", GetEGLError()); - return 1; - } + ret = BeginContext(); + if (ret) + return ret; gl_extensions = (const char *)glGetString(GL_EXTENSIONS); @@ -530,6 +561,9 @@ int GLWorkerCompositor::Init() { std::ostringstream shader_log; blend_programs_.emplace_back(GenerateProgram(1, &shader_log)); + + EndContext(); + if (blend_programs_.back().get() == 0) { ALOGE("%s", shader_log.str().c_str()); return 1; @@ -558,12 +592,17 @@ int GLWorkerCompositor::Composite(DrmHwcLayer *layers, return -EALREADY; } + ret = BeginContext(); + if (ret) + return -1; + GLint frame_width = framebuffer->getWidth(); GLint frame_height = framebuffer->getHeight(); CachedFramebuffer *cached_framebuffer = PrepareAndCacheFramebuffer(framebuffer); if (cached_framebuffer == NULL) { ALOGE("Composite failed because of failed framebuffer"); + EndContext(); return -EINVAL; } @@ -597,8 +636,10 @@ int GLWorkerCompositor::Composite(DrmHwcLayer *layers, } } - if (ret) + if (ret) { + EndContext(); return ret; + } glViewport(0, 0, frame_width, frame_height); @@ -676,6 +717,7 @@ int GLWorkerCompositor::Composite(DrmHwcLayer *layers, glBindFramebuffer(GL_FRAMEBUFFER, 0); + EndContext(); return ret; } diff --git a/glworker.h b/glworker.h index 158490c..26de55d 100644 --- a/glworker.h +++ b/glworker.h @@ -64,6 +64,16 @@ class GLWorkerCompositor { bool Promote(); }; + struct { + EGLDisplay saved_egl_display = EGL_NO_DISPLAY; + EGLContext saved_egl_ctx = EGL_NO_CONTEXT; + EGLSurface saved_egl_read = EGL_NO_SURFACE; + EGLSurface saved_egl_draw = EGL_NO_SURFACE; + } private_; + + int BeginContext(); + int EndContext(); + CachedFramebuffer *FindCachedFramebuffer( const sp<GraphicBuffer> &framebuffer); CachedFramebuffer *PrepareAndCacheFramebuffer(