Implement fencing

 16 files changed, 93 insertions(+), 724 deletions(-)

On 27 September 2017 at 12:58, Robert Foss <[email protected]

Quoted 3 lines

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

From: Sean Paul <[email protected]>

Since HWC2 doesn't require the use of threads to implement correct synchronization, remove some of these threads.

Signed-off-by Robert Foss <[email protected]>

---
 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

Android.mk +0 -3

index aa95b44..5d16c2f 100644
@@ -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 \

drmcomposition.cpp deleted +0 -166

deleted file mode 100644
index 1aaf920..0000000
@@ -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();
-}
-}

drmcomposition.h deleted +0 -79

deleted file mode 100644
index eae8cde..0000000
@@ -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_

drmcompositor.cpp deleted +0 -106

deleted file mode 100644
index c1f3ed8..0000000
@@ -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);
-}
-}

drmcompositor.h deleted +0 -56

deleted file mode 100644
index 19271b5..0000000
@@ -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_

drmcompositorworker.h deleted +0 -41

deleted file mode 100644
index 731bc65..0000000
@@ -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

drmdisplaycomposition.cpp +1 -0

index 0f8084b..66e67a4 100644
@@ -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"

drmdisplaycomposition.h +10 -0

index 13da19d..b165adc 100644
@@ -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;

drmdisplaycompositor.cpp +15 -174

index a1baed1..bd670cf 100644
@@ -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();

drmdisplaycompositor.h +3 -33

index 9487cac..f1965fb 100644
@@ -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

drmeventlistener.cpp +3 -0

index 0514aa6..984d1dd 100644
@@ -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 {

drmhwctwo.cpp +3 -3

index 8c853f4..762ee8c 100644
@@ -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;

drmresources.cpp +1 -53

index 6b8ed03..762f5ef 100644
@@ -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_;
 }

drmresources.h +0 -5

index 011f87e..a2d8d16 100644
@@ -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_;

glworker.cpp +47 -5

index e12995e..e90576a 100644
@@ -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;
 }
 

glworker.h +10 -1

index 158490c..26de55d 100644
@@ -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

Quoted 2 lines · Robert Foss

From: Sean Paul <[email protected]>

Quoted 3 lines

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 <[email protected]>

From: Sean Paul <[email protected]>

Since HWC2 doesn't require the use of threads to implement correct synchronization, remove some of these threads.

2017-09-27 19:58 GMT+08:00 Robert Foss <[email protected]>:

Quoted 4 lines

From: Sean Paul <[email protected]>

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.

--
Chih-Wei
Android-x86 project
http://www.android-x86.org

Quoted 6 lines · Chih-Wei Huang

2017-09-27 19:58 GMT+08:00 Robert Foss <[email protected]>:

Quoted 4 lines

From: Sean Paul <[email protected]>

Since HWC2 doesn't require the use of threads to implement correct synchronization, remove some of these threads.

Quoted 3 lines

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?

Quoted 2 lines

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.

Quoted 6 lines

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.

2017-09-29 5:29 GMT+08:00 Rob Herring <[email protected]>:

Quoted 10 lines

Quoted 8 lines · Chih-Wei Huang

2017-09-27 19:58 GMT+08:00 Robert Foss <[email protected]

Quoted 6 lines

:
From: Sean Paul <[email protected]>

Since HWC2 doesn't require the use of threads to implement correct
synchronization, remove some of these threads.

Quoted 7 lines

Quoted 3 lines

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

Quoted 17 lines

Quoted 2 lines

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.

Quoted 6 lines

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

Quoted 1 line

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)?

Quoted 7 lines

2017-09-29 5:29 GMT+08:00 Rob Herring <[email protected]>:

Quoted 1 line

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.

2017-09-29 16:44 GMT+08:00 Robert Foss <[email protected]>:

Quoted 12 lines · Chih-Wei Huang

Quoted 7 lines

2017-09-29 5:29 GMT+08:00 Rob Herring <[email protected]>:

Quoted 1 line

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. :(

Quoted 7 lines

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.

Quoted 6 lines

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.

Add support for the IN_FENCE_FD property to DrmPlane.

Signed-off-by Robert Foss <[email protected]>

---
 drmplane.cpp | 8 ++++++++
 drmplane.h   | 2 ++
 2 files changed, 10 insertions(+)

drmplane.cpp +8 -0

index c4ea722..1f739ae 100644
@@ -126,6 +126,10 @@ int DrmPlane::Init() {
   if (ret)
     ALOGI("Could not get alpha property");
 
+  ret = drm_->GetPlaneProperty(*this, "IN_FENCE_FD", &in_fence_fd_property_);
+  if (ret)
+    ALOGI("Could not get IN_FENCE_FD property");
+
   return 0;
 }
 

@@ -188,4 +192,8 @@ const DrmProperty &DrmPlane::rotation_property() const {
 const DrmProperty &DrmPlane::alpha_property() const {
   return alpha_property_;
 }
+
+const DrmProperty &DrmPlane::in_fence_fd_property() const {
+  return in_fence_fd_property_;
+}
 }

drmplane.h +2 -1

index 2e06986..5b73b08 100644
@@ -54,6 +54,7 @@ class DrmPlane {
   const DrmProperty &src_h_property() const;
   const DrmProperty &rotation_property() const;
   const DrmProperty &alpha_property() const;
+  const DrmProperty &in_fence_fd_property() const;
 
  private:
   DrmResources *drm_;

@@ -75,6 +76,7 @@ class DrmPlane {
   DrmProperty src_h_property_;
   DrmProperty rotation_property_;
   DrmProperty alpha_property_;
+  DrmProperty in_fence_fd_property_;
 };
 }
 
--

2.11.0

Add support for in-fences through the IN_FENCE_FD property. In-fences signal when their associated buffer may be read by DRM/KMS.

Signed-off-by Robert Foss <[email protected]>

---
 drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

drmdisplaycompositor.cpp +2 -19

index bd670cf..71c0451 100644
@@ -529,6 +529,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     std::vector<size_t> &source_layers = comp_plane.source_layers();
 
     int fb_id = -1;
+    int fence_fd = -1;
     DrmHwcRect<int> display_frame;
     DrmHwcRect<float> source_crop;
     uint64_t rotation = 0;

@@ -547,30 +548,12 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         break;
       }
       DrmHwcLayer &layer = layers[source_layers.front()];
-      if (!test_only && layer.acquire_fence.get() >= 0) {
-        int acquire_fence = layer.acquire_fence.get();
-        int total_fence_timeout = 0;
-        for (int i = 0; i < kAcquireWaitTries; ++i) {
-          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i);
-          total_fence_timeout += fence_timeout;
-          ret = sync_wait(acquire_fence, fence_timeout);
-          if (ret)
-            ALOGW("Acquire fence %d wait %d failed (%d). Total time %d",
-                  acquire_fence, i, ret, total_fence_timeout);
-          else
-            break;
-        }
-        if (ret) {
-          ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
-          break;
-        }
-        layer.acquire_fence.Close();
-      }
       if (!layer.buffer) {
         ALOGE("Expected a valid framebuffer for pset");
         break;
       }
       fb_id = layer.buffer->fb_id;
+      fence_fd = layer.acquire_fence.get();
       display_frame = layer.display_frame;
       source_crop = layer.source_crop;
       if (layer.blending == DrmHwcBlending::kPreMult)

drmdisplaycompositor.cpp +12 -1

+        int prop_id = plane->in_fence_fd_property().id();
+        if (prop_id == 0) {
+                ALOGE("Failed to get IN_FENCE_FD property id");
+                break;
+        }
+        ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd);
+        if (ret < 0) {
+          ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret);
+          break;
+        }
+      }
     }
+
     // Disable the plane if there's no framebuffer
     if (fb_id < 0) {
       ret = drmModeAtomicAddProperty(pset, plane->id(),
--

2.11.0

Quoted 61 lines · Robert Foss

Add support for in-fences through the IN_FENCE_FD property. In- fences signal
when their associated buffer may be read by DRM/KMS.

Signed-off-by Robert Foss <[email protected]>

---
 drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------

 1 file changed, 16 insertions(+), 19 deletions(-)

drmdisplaycompositor.cpp +0 -0

index bd670cf..71c0451 100644
@@ -529,6 +529,7 @@ int

DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     std::vector<size_t> &source_layers = comp_plane.source_layers();

     int fb_id = -1;
+    int fence_fd = -1;
     DrmHwcRect<int> display_frame;
     DrmHwcRect<float> source_crop;
     uint64_t rotation = 0;

drmdisplaycompositor.cpp +0 -0

@@ -547,30 +548,12 @@ int

DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         break;
       }
       DrmHwcLayer &layer = layers[source_layers.front()]; -      if (!test_only && layer.acquire_fence.get() >= 0) { -        int acquire_fence = layer.acquire_fence.get(); -        int total_fence_timeout = 0;
-        for (int i = 0; i < kAcquireWaitTries; ++i) {
-          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i); -          total_fence_timeout += fence_timeout;
-          ret = sync_wait(acquire_fence, fence_timeout); -          if (ret)
-            ALOGW("Acquire fence %d wait %d failed (%d). Total time %d",
-                  acquire_fence, i, ret, total_fence_timeout); -          else
-            break;
-        }
-        if (ret) {
-          ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
-          break;
-        }
-        layer.acquire_fence.Close();
-      }
       if (!layer.buffer) {
         ALOGE("Expected a valid framebuffer for pset"); break;
       }
       fb_id = layer.buffer->fb_id;
+      fence_fd = layer.acquire_fence.get(); display_frame = layer.display_frame;
       source_crop = layer.source_crop;
       if (layer.blending == DrmHwcBlending::kPreMult)

drmdisplaycompositor.cpp +0 -0

@@ -587,7 +570,21 @@ int

Quoted 7 lines

DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         rotation |= 1 << DRM_ROTATE_180;
       else if (layer.transform & DrmHwcTransform::kRotate270) rotation |= 1 << DRM_ROTATE_270;
+
+      if (fence_fd != -1) {

nit: if (fence_fd < 0)

With that fixed:

Reviewed-by Sean Paul <[email protected]>

Quoted 14 lines

+        int prop_id = plane->in_fence_fd_property().id(); +        if (prop_id == 0) {
+                ALOGE("Failed to get IN_FENCE_FD property id"); +                break;
+        }
+        ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd);
+        if (ret < 0) {
+          ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret);
+          break;
+        }
+      }

Quoted 8 lines

     }
+
     // Disable the plane if there's no framebuffer if (fb_id < 0) {
       ret = drmModeAtomicAddProperty(pset, plane->id(), --
2.11.0

Quoted 52 lines · Robert Foss

Add support for in-fences through the IN_FENCE_FD property. In-fences signal when their associated buffer may be read by DRM/KMS.

Signed-off-by Robert Foss <[email protected]>

---
 drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

drmdisplaycompositor.cpp +2 -19

index bd670cf..71c0451 100644
@@ -529,6 +529,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     std::vector<size_t> &source_layers = comp_plane.source_layers();
 
     int fb_id = -1;
+    int fence_fd = -1;
     DrmHwcRect<int> display_frame;
     DrmHwcRect<float> source_crop;
     uint64_t rotation = 0;

@@ -547,30 +548,12 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         break;
       }
       DrmHwcLayer &layer = layers[source_layers.front()];
-      if (!test_only && layer.acquire_fence.get() >= 0) {
-        int acquire_fence = layer.acquire_fence.get();
-        int total_fence_timeout = 0;
-        for (int i = 0; i < kAcquireWaitTries; ++i) {
-          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i);
-          total_fence_timeout += fence_timeout;
-          ret = sync_wait(acquire_fence, fence_timeout);
-          if (ret)
-            ALOGW("Acquire fence %d wait %d failed (%d). Total time %d",
-                  acquire_fence, i, ret, total_fence_timeout);
-          else
-            break;
-        }
-        if (ret) {
-          ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
-          break;
-        }
-        layer.acquire_fence.Close();
-      }
       if (!layer.buffer) {
         ALOGE("Expected a valid framebuffer for pset");
         break;
       }
       fb_id = layer.buffer->fb_id;
+      fence_fd = layer.acquire_fence.get();
       display_frame = layer.display_frame;
       source_crop = layer.source_crop;
       if (layer.blending == DrmHwcBlending::kPreMult)

drmdisplaycompositor.cpp int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, · line 574

Quoted 6 lines

drmdisplaycompositor.cpp +2 -0

@@ -587,7 +570,21 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         rotation |= 1 << DRM_ROTATE_180;
       else if (layer.transform & DrmHwcTransform::kRotate270)
         rotation |= 1 << DRM_ROTATE_270;
+
+      if (fence_fd != -1) {

nit: if (fence_fd < 0)

Add support for handling the FENCE_OUT_PTR property to DrmCrtc

Signed-off-by Robert Foss <[email protected]>

---
 drmcrtc.cpp | 10 ++++++++++
 drmcrtc.h   |  2 ++
 2 files changed, 12 insertions(+)

drmcrtc.cpp +10 -0

index 1fbdc12..c139869 100644
@@ -51,6 +51,12 @@ int DrmCrtc::Init() {
     ALOGE("Failed to get MODE_ID property");
     return ret;
   }
+
+  ret = drm_->GetCrtcProperty(*this, "OUT_FENCE_PTR", &out_fence_ptr_property_);
+  if (ret) {
+    ALOGE("Failed to get OUT_FENCE_PTR property");
+    return ret;
+  }
   return 0;
 }
 

@@ -81,4 +87,8 @@ const DrmProperty &DrmCrtc::active_property() const {
 const DrmProperty &DrmCrtc::mode_property() const {
   return mode_property_;
 }
+
+const DrmProperty &DrmCrtc::out_fence_ptr_property() const {
+  return out_fence_ptr_property_;
+}
 }

drmcrtc.h +2 -1

index ad95352..2e8c811 100644
@@ -45,6 +45,7 @@ class DrmCrtc {
 
   const DrmProperty &active_property() const;
   const DrmProperty &mode_property() const;
+  const DrmProperty &out_fence_ptr_property() const;
 
  private:
   DrmResources *drm_;

@@ -63,6 +64,7 @@ class DrmCrtc {
 
   DrmProperty active_property_;
   DrmProperty mode_property_;
+  DrmProperty out_fence_ptr_property_;
 };
 }
 
--

2.11.0

This GetCrtrcCount helper functions enables convenient fetching of the number of Crtcs from DrmResources.

Signed-off-by Robert Foss <[email protected]>

---
 drmresources.cpp | 4 ++++
 drmresources.h   | 1 +
 2 files changed, 5 insertions(+)

drmresources.cpp +0 -0

index 762f5ef..0578cc6 100644

drmresources.cpp +1 -0

+
 uint32_t DrmResources::next_mode_id() {
   return ++mode_id_;
 }

drmresources.h +1 -1

index a2d8d16..0cc2456 100644
@@ -66,6 +66,7 @@ class DrmResources {
   int GetConnectorProperty(const DrmConnector &connector, const char *prop_name,
                            DrmProperty *property);
 
+  uint32_t GetCrtcCount() const;
   uint32_t next_mode_id();
 
   int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id);
--

2.11.0

Quoted 14 lines · Robert Foss

This GetCrtrcCount helper functions enables convenient fetching of the number of Crtcs from DrmResources.

Signed-off-by Robert Foss <[email protected]>

---
 drmresources.cpp | 4 ++++
 drmresources.h   | 1 +

 2 files changed, 5 insertions(+)

drmresources.cpp +0 -0

index 762f5ef..0578cc6 100644

drmresources.cpp DrmPlane *DrmResources::GetPlane(uint32_t id) · line 247

Quoted 7 lines

drmresources.cpp +3 -0

@@ -241,6 +241,10 @@ DrmPlane *DrmResources::GetPlane(uint32_t id)
 const {
    return NULL;
  }
 
+uint32_t DrmResources::GetCrtcCount() const {
+  return (uint32_t) crtcs_.size();
+}

The "blessed" way of doing this would be to add a new function

const std::vector<std::unique_ptr<DrmCrtc>> &crtcs() const { return crtcs_;
}

and then use crtcs()->size() wherever needed.

Add support for out-fences through the OUT_FENCE_PTR property. Out-fences signal when their associated buffer may be read by a device.

Signed-off-by Robert Foss <[email protected]>

---

Changes since v1:
Sergi Granell
- Set atomic property to be out_fences[crtc->pipe()] not out_fences[0]

 drmdisplaycomposition.h  |  9 +++++++++
 drmdisplaycompositor.cpp | 16 ++++++++++++++++
 drmhwctwo.cpp            |  9 ++-------
 3 files changed, 27 insertions(+), 7 deletions(-)

drmdisplaycomposition.h +0 -0

index b165adc..0586d58 100644

drmdisplaycomposition.h +3 -0

+  }
+
   void Dump(std::ostringstream *out) const;
 
  private:

@@ -215,6 +223,7 @@ class DrmDisplayComposition {
   int timeline_current_ = 0;
   int timeline_squash_done_ = 0;
   int timeline_pre_comp_done_ = 0;
+  UniqueFd out_fence_ = -1;
 
   bool geometry_changed_;
   std::vector<DrmHwcLayer> layers_;

drmdisplaycompositor.cpp +0 -0

index 71c0451..a1427d3 100644

drmdisplaycompositor.cpp +15 -0

   DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
   if (!connector) {

@@ -510,6 +511,16 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     return -ENOMEM;
   }
 
+  if (crtc->out_fence_ptr_property().id() != 0) {
+    ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->out_fence_ptr_property().id(),
+                                   (uint64_t) &out_fences[crtc->pipe()]);
+    if (ret < 0) {
+      ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret);
+      drmModeAtomicFree(pset);
+      return ret;
+    }
+  }
+
   if (mode_.needs_modeset) {
     ret = drmModeAtomicAddProperty(pset, crtc->id(), crtc->mode_property().id(),
                                    mode_.blob_id) < 0 ||

@@ -708,6 +719,11 @@ out:
     mode_.needs_modeset = false;
   }
 
+  if (crtc->out_fence_ptr_property().id()) {
+    display_comp->set_out_fence((int) out_fences[crtc->pipe()]);
+    close((int) out_fences[crtc->pipe()]);
+  }
+
   return ret;
 }
 

drmhwctwo.cpp +2 -8

index 762ee8c..89399bf 100644
@@ -557,19 +557,14 @@ HWC2::Error DrmHwcTwo::HwcDisplay::PresentDisplay(int32_t *retire_fence) {
     i = overlay_planes.erase(i);
   }
 
+  AddFenceToRetireFence(composition->take_out_fence());
+
   ret = compositor_.ApplyComposition(std::move(composition));
   if (ret) {
     ALOGE("Failed to apply the frame composition ret=%d", ret);
     return HWC2::Error::BadParameter;
   }
 
-  // Now that the release fences have been generated by the compositor, make
-  // sure they're managed properly
-  for (std::pair<const uint32_t, DrmHwcTwo::HwcLayer *> &l : z_map) {
-    l.second->manage_release_fence();
-    AddFenceToRetireFence(l.second->release_fence());
-  }
-
   // The retire fence returned here is for the last frame, so return it and
   // promote the next retire fence
   *retire_fence = retire_fence_.Release();
--

2.11.0

Quoted 25 lines · Robert Foss

Add support for out-fences through the OUT_FENCE_PTR property. Out-fences signal when their associated buffer may be read by a device.

Signed-off-by Robert Foss <[email protected]>

---

Changes since v1:
  Sergi Granell
    - Set atomic property to be out_fences[crtc->pipe()] not out_fences[0]

 drmdisplaycomposition.h  |  9 +++++++++
 drmdisplaycompositor.cpp | 16 ++++++++++++++++
 drmhwctwo.cpp            |  9 ++-------

 3 files changed, 27 insertions(+), 7 deletions(-)

drmdisplaycomposition.h +0 -0

index b165adc..0586d58 100644
@@ -189,6 +189,14 @@ class DrmDisplayComposition {
      return planner_;
    }
 

drmdisplaycomposition.h class DrmDisplayComposition { · line 197

Quoted 6 lines

drmdisplaycomposition.h +6 -0

+  int take_out_fence() {
+    return out_fence_.Release();
+  }
+
+  void set_out_fence(int out_fence) {
+    out_fence_.Set(dup(out_fence));

Why dup if you're just going to close the original? I think the helper
functions actually hurt you here. It would be easier to understand what was going on if you just manipulated out_fence_ directly in CommitFrame (then you wouldn't need the dup/close).

Quoted 12 lines

drmdisplaycomposition.h +2 -0

+  }
+
    void Dump(std::ostringstream *out) const;
 

  private:

drmdisplaycomposition.h +1 -0

@@ -215,6 +223,7 @@ class DrmDisplayComposition {
    int timeline_current_ = 0;
    int timeline_squash_done_ = 0;
    int timeline_pre_comp_done_ = 0;
+  UniqueFd out_fence_ = -1;
 

Quoted 7 lines

drmdisplaycomposition.h +0 -0

    bool geometry_changed_;
    std::vector<DrmHwcLayer> layers_;

drmdisplaycompositor.cpp +0 -0

index 71c0451..a1427d3 100644
@@ -492,6 +492,7 @@ int

Quoted 6 lines

DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp, display_comp->composition_planes(); std::vector<DrmCompositionRegion> &pre_comp_regions = display_comp->pre_comp_regions();
+  uint64_t out_fences[drm_->GetCrtcCount()];

Huh. I didn't know you could do this.