[PATCH hwc v2 0/6] Implement fencing

[PATCH hwc v2 0/6] Implement fencing

[Robert Foss]

This series removes the old thread-based synchronization utilities and
replaces them with fences.

It has been tested on various platforms, including etnaviv/freedreno/virgl.

Robert Foss (5):
  drm_hwcomposer: Add support for IN_FENCE_FD property to DrmPlane
  drm_hwcomposer: Submit in-fence to DRM
  drm_hwcomposer: Add FENCE_OUT_PTR property to DrmCrtc
  drm_hwcomposer: Add GetCrtcCount function
  drm_hwcomposer: Add out-fence support

Sean Paul (1):
  drm_hwcomposer: Remove threading

 Android.mk                |   3 -
 drmcomposition.cpp        | 166 --------------------------------
 drmcomposition.h          |  79 ---------------
 drmcompositor.cpp         | 106 --------------------
 drmcompositor.h           |  56 -----------
 drmcompositorworker.h     |  41 --------
 drmcrtc.cpp               |  10 ++
 drmcrtc.h                 |   2 +
 drmdisplaycomposition.cpp |   1 +
 drmdisplaycomposition.h   |  19 ++++
 drmdisplaycompositor.cpp  | 240 +++++++++-------------------------------------
 drmdisplaycompositor.h    |  36 +------
 drmeventlistener.cpp      |   3 +
 drmhwctwo.cpp             |  15 +--
 drmplane.cpp              |   8 ++
 drmplane.h                |   2 +
 drmresources.cpp          |  58 +----------
 drmresources.h            |   6 +-
 glworker.cpp              |  52 +++++++++-
 glworker.h                |  10 ++
 20 files changed, 163 insertions(+), 750 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

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Robert Foss]

From: Sean Paul <seanpaul@chromium.org>

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

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Emil Velikov]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Robert Foss]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Sean Paul]

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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Robert Foss]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Chih-Wei Huang]

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.


-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Rob Herring]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Chih-Wei Huang]

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


-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Robert Foss]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Chih-Wei Huang]

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.



-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 1/6] drm_hwcomposer: Remove threading

[Robert Foss]

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.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH hwc v2 2/6] drm_hwcomposer: Add support for IN_FENCE_FD property to DrmPlane

[Robert Foss]

Add support for the IN_FENCE_FD property to DrmPlane.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drmplane.cpp | 8 ++++++++
 drmplane.h   | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drmplane.cpp b/drmplane.cpp
index c4ea722..1f739ae 100644
--- a/drmplane.cpp
+++ b/drmplane.cpp
@@ -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_;
+}
 }
diff --git a/drmplane.h b/drmplane.h
index 2e06986..5b73b08 100644
--- a/drmplane.h
+++ b/drmplane.h
@@ -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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 2/6] drm_hwcomposer: Add support for IN_FENCE_FD property to DrmPlane

[Sean Paul]

On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
> Add support for the IN_FENCE_FD property to DrmPlane.
>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drmplane.cpp | 8 ++++++++
>  drmplane.h   | 2 ++
>  2 files changed, 10 insertions(+)
>
> diff --git a/drmplane.cpp b/drmplane.cpp
> index c4ea722..1f739ae 100644
> --- a/drmplane.cpp
> +++ b/drmplane.cpp
> @@ -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_;
> +}
>  }
> diff --git a/drmplane.h b/drmplane.h
> index 2e06986..5b73b08 100644
> --- a/drmplane.h
> +++ b/drmplane.h
> @@ -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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM

[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 <robert.foss@collabora.com>
---
 drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index bd670cf..71c0451 100644
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -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)
@@ -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) {
+        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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM

[Sean Paul]

On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
> 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 <robert.foss@collabora.com>
> ---
>  drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index bd670cf..71c0451 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -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)
> @@ -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)

With that fixed:

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> +        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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM

[Robert Foss]

Hey Sean,

On Wed, 2017-09-27 at 14:55 -0400, Sean Paul wrote:
> On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.c
> om> wrote:
> > 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 <robert.foss@collabora.com>
> > ---
> >  drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
> >  1 file changed, 16 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index bd670cf..71c0451 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -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)
> > @@ -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)
> 
> With that fixed:
> 
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

Ack.
Submitting a fix in v3 tomorrow.

> 
> > +        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
> > 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 3/6] drm_hwcomposer: Submit in-fence to DRM

[Rob Herring]

On Wed, Sep 27, 2017 at 1:55 PM, Sean Paul <seanpaul@chromium.org> wrote:
> On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
>> 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 <robert.foss@collabora.com>
>> ---
>>  drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
>>  1 file changed, 16 insertions(+), 19 deletions(-)
>>
>> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
>> index bd670cf..71c0451 100644
>> --- a/drmdisplaycompositor.cpp
>> +++ b/drmdisplaycompositor.cpp
>> @@ -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)
>> @@ -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)

Your nit is a bug. :) We're checking for if a valid fd. I think this
may be John's issue with needing a glFinish().

A fix is coming.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH hwc v2 4/6] drm_hwcomposer: Add FENCE_OUT_PTR property to DrmCrtc

[Robert Foss]

Add support for handling the FENCE_OUT_PTR property to DrmCrtc

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drmcrtc.cpp | 10 ++++++++++
 drmcrtc.h   |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drmcrtc.cpp b/drmcrtc.cpp
index 1fbdc12..c139869 100644
--- a/drmcrtc.cpp
+++ b/drmcrtc.cpp
@@ -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_;
+}
 }
diff --git a/drmcrtc.h b/drmcrtc.h
index ad95352..2e8c811 100644
--- a/drmcrtc.h
+++ b/drmcrtc.h
@@ -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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 4/6] drm_hwcomposer: Add FENCE_OUT_PTR property to DrmCrtc

[Sean Paul]

On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
> Add support for handling the FENCE_OUT_PTR property to DrmCrtc
>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drmcrtc.cpp | 10 ++++++++++
>  drmcrtc.h   |  2 ++
>  2 files changed, 12 insertions(+)
>
> diff --git a/drmcrtc.cpp b/drmcrtc.cpp
> index 1fbdc12..c139869 100644
> --- a/drmcrtc.cpp
> +++ b/drmcrtc.cpp
> @@ -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_;
> +}
>  }
> diff --git a/drmcrtc.h b/drmcrtc.h
> index ad95352..2e8c811 100644
> --- a/drmcrtc.h
> +++ b/drmcrtc.h
> @@ -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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH hwc v2 5/6] drm_hwcomposer: Add GetCrtcCount function

[Robert Foss]

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

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drmresources.cpp | 4 ++++
 drmresources.h   | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drmresources.cpp b/drmresources.cpp
index 762f5ef..0578cc6 100644
--- a/drmresources.cpp
+++ b/drmresources.cpp
@@ -241,6 +241,10 @@ DrmPlane *DrmResources::GetPlane(uint32_t id) const {
   return NULL;
 }
 
+uint32_t DrmResources::GetCrtcCount() const {
+  return (uint32_t) crtcs_.size();
+}
+
 uint32_t DrmResources::next_mode_id() {
   return ++mode_id_;
 }
diff --git a/drmresources.h b/drmresources.h
index a2d8d16..0cc2456 100644
--- a/drmresources.h
+++ b/drmresources.h
@@ -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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 5/6] drm_hwcomposer: Add GetCrtcCount function

[Sean Paul]

On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
> This GetCrtrcCount helper functions enables convenient
> fetching of the number of Crtcs from DrmResources.
>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
>  drmresources.cpp | 4 ++++
>  drmresources.h   | 1 +
>  2 files changed, 5 insertions(+)
>
> diff --git a/drmresources.cpp b/drmresources.cpp
> index 762f5ef..0578cc6 100644
> --- a/drmresources.cpp
> +++ b/drmresources.cpp
> @@ -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.



> +
>  uint32_t DrmResources::next_mode_id() {
>    return ++mode_id_;
>  }
> diff --git a/drmresources.h b/drmresources.h
> index a2d8d16..0cc2456 100644
> --- a/drmresources.h
> +++ b/drmresources.h
> @@ -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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 5/6] drm_hwcomposer: Add GetCrtcCount function

[Robert Foss]

On Wed, 2017-09-27 at 15:12 -0400, Sean Paul wrote:
> On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.c
> om> wrote:
> > This GetCrtrcCount helper functions enables convenient
> > fetching of the number of Crtcs from DrmResources.
> > 
> > Signed-off-by: Robert Foss <robert.foss@collabora.com>
> > ---
> >  drmresources.cpp | 4 ++++
> >  drmresources.h   | 1 +
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/drmresources.cpp b/drmresources.cpp
> > index 762f5ef..0578cc6 100644
> > --- a/drmresources.cpp
> > +++ b/drmresources.cpp
> > @@ -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.
> 
> 

Yeah, that is nicer and more versatile.



Rob.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH hwc v2 6/6] drm_hwcomposer: Add out-fence support

[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 <robert.foss@collabora.com>
---

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

diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h
index b165adc..0586d58 100644
--- a/drmdisplaycomposition.h
+++ b/drmdisplaycomposition.h
@@ -189,6 +189,14 @@ class DrmDisplayComposition {
     return planner_;
   }
 
+  int take_out_fence() {
+    return out_fence_.Release();
+  }
+
+  void set_out_fence(int out_fence) {
+    out_fence_.Set(dup(out_fence));
+  }
+
   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_;
diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index 71c0451..a1427d3 100644
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -492,6 +492,7 @@ int 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()];
 
   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;
 }
 
diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp
index 762ee8c..89399bf 100644
--- a/drmhwctwo.cpp
+++ b/drmhwctwo.cpp
@@ -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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 6/6] drm_hwcomposer: Add out-fence support

[Sean Paul]

On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
> 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 <robert.foss@collabora.com>
> ---
>
> 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(-)
>
> diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h
> index b165adc..0586d58 100644
> --- a/drmdisplaycomposition.h
> +++ b/drmdisplaycomposition.h
> @@ -189,6 +189,14 @@ class DrmDisplayComposition {
>      return planner_;
>    }
>
> +  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).

> +  }
> +
>    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_;
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index 71c0451..a1427d3 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -492,6 +492,7 @@ int 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.

>
>    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;
>  }
>
> diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp
> index 762ee8c..89399bf 100644
> --- a/drmhwctwo.cpp
> +++ b/drmhwctwo.cpp
> @@ -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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH hwc v2 6/6] drm_hwcomposer: Add out-fence support

[Robert Foss]

On Wed, 2017-09-27 at 15:11 -0400, Sean Paul wrote:
> On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.c
> om> wrote:
> > 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 <robert.foss@collabora.com>
> > ---
> > 
> > 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(-)
> > 
> > diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h
> > index b165adc..0586d58 100644
> > --- a/drmdisplaycomposition.h
> > +++ b/drmdisplaycomposition.h
> > @@ -189,6 +189,14 @@ class DrmDisplayComposition {
> >      return planner_;
> >    }
> > 
> > +  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).

Yeah, that makes a lot of sense, and is a lot easier to read too.

> 
> > +  }
> > +
> >    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_;
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index 71c0451..a1427d3 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -492,6 +492,7 @@ int
> > 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.

C99 and variable length arrays man.
Progress at a glacial pace is still happening in C-land.


Rob.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel