drm_hwcomposer: Tidy-up DrmPlane class

This allow to throw away few lines from DrmDevice::Init() making it less
complicated.

Signed-off-by: Roman Stratiienko <roman.o.stratiienko@globallogic.com>

5 files changed, 55 insertions, 52 deletions

diff --git a/compositor/Planner.cpp b/compositor/Planner.cpp
index f43e314..24b43ba 100644
--- a/compositor/Planner.cpp
+++ b/compositor/Planner.cpp
@@ -31,10 +31,10 @@ std::vector<DrmPlane *> Planner::GetUsablePlanes(
   std::vector<DrmPlane *> usable_planes;
   std::copy_if(primary_planes->begin(), primary_planes->end(),
                std::back_inserter(usable_planes),
-               [=](DrmPlane *plane) { return plane->GetCrtcSupported(*crtc); });
+               [=](DrmPlane *plane) { return plane->IsCrtcSupported(*crtc); });
   std::copy_if(overlay_planes->begin(), overlay_planes->end(),
                std::back_inserter(usable_planes),
-               [=](DrmPlane *plane) { return plane->GetCrtcSupported(*crtc); });
+               [=](DrmPlane *plane) { return plane->IsCrtcSupported(*crtc); });
   return usable_planes;
 }
 
diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index 8245b78..1c8427c 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -305,25 +305,13 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
   }
 
   for (uint32_t i = 0; i < plane_res->count_planes; ++i) {
-    auto p = MakeDrmModePlaneUnique(fd(), plane_res->planes[i]);
-    if (!p) {
-      ALOGE("Failed to get plane %d", plane_res->planes[i]);
-      ret = -ENODEV;
-      break;
-    }
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+    auto plane = DrmPlane::CreateInstance(*this, plane_res->planes[i]);
 
-    std::unique_ptr<DrmPlane> plane(new DrmPlane(this, p.get()));
-
-    ret = plane->Init();
-    if (ret) {
-      ALOGE("Init plane %d failed", plane_res->planes[i]);
-      break;
+    if (plane) {
+      planes_.emplace_back(std::move(plane));
     }
-
-    planes_.emplace_back(std::move(plane));
   }
-  if (ret)
-    return std::make_tuple(ret, 0);
 
   for (auto &conn : connectors_) {
     ret = CreateDisplayPipe(conn.get());
diff --git a/drm/DrmPlane.cpp b/drm/DrmPlane.cpp
index 25a4902..5001457 100644
--- a/drm/DrmPlane.cpp
+++ b/drm/DrmPlane.cpp
@@ -29,15 +29,28 @@
 
 namespace android {
 
-DrmPlane::DrmPlane(DrmDevice *drm, drmModePlanePtr p)
-    : drm_(drm),
-      id_(p->plane_id),
-      possible_crtc_mask_(p->possible_crtcs),
-      // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
-      formats_(p->formats, p->formats + p->count_formats) {
+auto DrmPlane::CreateInstance(DrmDevice &dev, uint32_t plane_id)
+    -> std::unique_ptr<DrmPlane> {
+  auto p = MakeDrmModePlaneUnique(dev.fd(), plane_id);
+  if (!p) {
+    ALOGE("Failed to get plane %d", plane_id);
+    return {};
+  }
+
+  auto plane = std::unique_ptr<DrmPlane>(new DrmPlane(dev, std::move(p)));
+
+  if (plane->Init() != 0) {
+    ALOGE("Failed to init plane %d", plane_id);
+    return {};
+  }
+
+  return plane;
 }
 
 int DrmPlane::Init() {
+  // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+  formats_ = {plane_->formats, plane_->formats + plane_->count_formats};
+
   DrmProperty p;
 
   if (!GetPlaneProperty("type", p)) {
@@ -134,38 +147,38 @@ int DrmPlane::Init() {
   return 0;
 }
 
-bool DrmPlane::GetCrtcSupported(const DrmCrtc &crtc) const {
-  return ((1 << crtc.pipe()) & possible_crtc_mask_) != 0;
+bool DrmPlane::IsCrtcSupported(const DrmCrtc &crtc) const {
+  return ((1 << crtc.pipe()) & plane_->possible_crtcs) != 0;
 }
 
 bool DrmPlane::IsValidForLayer(DrmHwcLayer *layer) {
   if (!rotation_property_) {
     if (layer->transform != DrmHwcTransform::kIdentity) {
-      ALOGV("No rotation property on plane %d", id_);
+      ALOGV("No rotation property on plane %d", GetId());
       return false;
     }
   } else {
     if (transform_enum_map_.count(layer->transform) == 0) {
-      ALOGV("Transform is not supported on plane %d", id_);
+      ALOGV("Transform is not supported on plane %d", GetId());
       return false;
     }
   }
 
   if (alpha_property_.id() == 0 && layer->alpha != UINT16_MAX) {
-    ALOGV("Alpha is not supported on plane %d", id_);
+    ALOGV("Alpha is not supported on plane %d", GetId());
     return false;
   }
 
   if (blending_enum_map_.count(layer->blending) == 0 &&
       layer->blending != DrmHwcBlending::kNone &&
       layer->blending != DrmHwcBlending::kPreMult) {
-    ALOGV("Blending is not supported on plane %d", id_);
+    ALOGV("Blending is not supported on plane %d", GetId());
     return false;
   }
 
   uint32_t format = layer->buffer_info.format;
   if (!IsFormatSupported(format)) {
-    ALOGV("Plane %d does not supports %c%c%c%c format", id_, format,
+    ALOGV("Plane %d does not supports %c%c%c%c format", GetId(), format,
           format >> 8, format >> 16, format >> 24);
     return false;
   }
@@ -173,10 +186,6 @@ bool DrmPlane::IsValidForLayer(DrmHwcLayer *layer) {
   return true;
 }
 
-uint32_t DrmPlane::GetType() const {
-  return type_;
-}
-
 bool DrmPlane::IsFormatSupported(uint32_t format) const {
   return std::find(std::begin(formats_), std::end(formats_), format) !=
          std::end(formats_);
@@ -290,20 +299,17 @@ auto DrmPlane::AtomicDisablePlane(drmModeAtomicReq &pset) -> int {
   return 0;
 }
 
-const DrmProperty &DrmPlane::GetZPosProperty() const {
-  return zpos_property_;
-}
-
 auto DrmPlane::GetPlaneProperty(const char *prop_name, DrmProperty &property,
                                 Presence presence) -> bool {
-  int err = drm_->GetProperty(id_, DRM_MODE_OBJECT_PLANE, prop_name, &property);
+  int err = drm_->GetProperty(GetId(), DRM_MODE_OBJECT_PLANE, prop_name,
+                              &property);
   if (err != 0) {
     if (presence == Presence::kMandatory) {
       ALOGE("Could not get mandatory property \"%s\" from plane %d", prop_name,
-            id_);
+            GetId());
     } else {
       ALOGV("Could not get optional property \"%s\" from plane %d", prop_name,
-            id_);
+            GetId());
     }
     return false;
   }
diff --git a/drm/DrmPlane.h b/drm/DrmPlane.h
index e1ee920..9826f67 100644
--- a/drm/DrmPlane.h
+++ b/drm/DrmPlane.h
@@ -33,16 +33,18 @@ struct DrmHwcLayer;
 
 class DrmPlane {
  public:
-  DrmPlane(DrmDevice *drm, drmModePlanePtr p);
   DrmPlane(const DrmPlane &) = delete;
   DrmPlane &operator=(const DrmPlane &) = delete;
 
-  int Init();
+  static auto CreateInstance(DrmDevice &dev, uint32_t plane_id)
+      -> std::unique_ptr<DrmPlane>;
 
-  bool GetCrtcSupported(const DrmCrtc &crtc) const;
+  bool IsCrtcSupported(const DrmCrtc &crtc) const;
   bool IsValidForLayer(DrmHwcLayer *layer);
 
-  uint32_t GetType() const;
+  auto GetType() const {
+    return type_;
+  }
 
   bool IsFormatSupported(uint32_t format) const;
   bool HasNonRgbFormat() const;
@@ -50,19 +52,26 @@ class DrmPlane {
   auto AtomicSetState(drmModeAtomicReq &pset, DrmHwcLayer &layer, uint32_t zpos,
                       uint32_t crtc_id) -> int;
   auto AtomicDisablePlane(drmModeAtomicReq &pset) -> int;
-  const DrmProperty &GetZPosProperty() const;
+  auto &GetZPosProperty() const {
+    return zpos_property_;
+  }
+
+  auto GetId() const {
+    return plane_->plane_id;
+  }
 
  private:
-  DrmDevice *drm_;
-  uint32_t id_;
+  DrmPlane(DrmDevice &dev, DrmModePlaneUnique plane)
+      : drm_(&dev), plane_(std::move(plane)){};
+  DrmDevice *const drm_;
+  DrmModePlaneUnique plane_;
 
   enum class Presence { kOptional, kMandatory };
 
+  auto Init() -> int;
   auto GetPlaneProperty(const char *prop_name, DrmProperty &property,
                         Presence presence = Presence::kMandatory) -> bool;
 
-  uint32_t possible_crtc_mask_;
-
   uint32_t type_{};
 
   std::vector<uint32_t> formats_;
diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp
index a2a093f..87cc238 100644
--- a/hwc2_device/DrmHwcTwo.cpp
+++ b/hwc2_device/DrmHwcTwo.cpp
@@ -43,7 +43,7 @@ HWC2::Error DrmHwcTwo::CreateDisplay(hwc2_display_t displ,
   }
   auto display_planes = std::vector<DrmPlane *>();
   for (const auto &plane : drm->planes()) {
-    if (plane->GetCrtcSupported(*crtc))
+    if (plane->IsCrtcSupported(*crtc))
       display_planes.push_back(plane.get());
   }
   displays_.at(displ).Init(&display_planes);