commit 907a51ab1b0f26b691ae3c6050bfdc904b9ee11a
author Gil Dekel <gildekel@google.com> Fri Feb 14 22:44:01 2025 -0500
committer Gil Dekel <gildekel@google.com> Sat Feb 15 23:23:52 2025 -0500
tree 77ed7b5e61777709f1dda0b6d352e1a51d911e49
parent c88294c89db5bdc96f2e465b9f5325667861d11d

drm_hwcomposer: Produce stable port IDs

Currently, port IDs are generated by returning a monotonically
increasing value (uint64_t hwc2_display_t). This is problematic for
two reasons:

hwc2_display_t is a 64bit value, and the returned port is 8bit.
clients of drm-hwc cannot rely on port ID consistency between
re-plugs to the same connector.

This patch provides a more stable approach to producing port IDs. We
combine the index of the DRM device in the device list, with the
index of the connector within a DRM device in a 3/5 bit split. This will
allow us to support up to 8 DRM devices, each with 32 independent
connectors (ports). If more support is required in the future, we will
have to extend the API to return uint16_t port values instead.

Signed-off-by: Gil Dekel <gildekel@google.com>

drm/DrmDevice.cpp

diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index 787d77c..6fd5c0c 100644
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -35,13 +35,13 @@
 namespace android {
 
 auto DrmDevice::CreateInstance(std::string const &path,
-                               ResourceManager *res_man)
+                               ResourceManager *res_man, uint32_t index)
     -> std::unique_ptr<DrmDevice> {
   if (!IsKMSDev(path.c_str())) {
     return {};
   }
 
-  auto device = std::unique_ptr<DrmDevice>(new DrmDevice(res_man));
+  auto device = std::unique_ptr<DrmDevice>(new DrmDevice(res_man, index));
 
   if (device->Init(path.c_str()) != 0) {
     return {};
@@ -50,7 +50,8 @@
   return device;
 }
 
-DrmDevice::DrmDevice(ResourceManager *res_man) : res_man_(res_man) {
+DrmDevice::DrmDevice(ResourceManager *res_man, uint32_t index)
+    : index_in_dev_array_(index), res_man_(res_man) {
   drm_fb_importer_ = std::make_unique<DrmFbImporter>(*this);
 }
 

drm/DrmDevice.h

diff --git a/drm/DrmDevice.h b/drm/DrmDevice.h
index 7ee7d10..baa719d 100644
--- a/drm/DrmDevice.h
+++ b/drm/DrmDevice.h
@@ -37,13 +37,17 @@
  public:
   ~DrmDevice() = default;
 
-  static auto CreateInstance(std::string const &path, ResourceManager *res_man)
-      -> std::unique_ptr<DrmDevice>;
+  static auto CreateInstance(std::string const &path, ResourceManager *res_man,
+                             uint32_t index) -> std::unique_ptr<DrmDevice>;
 
   auto &GetFd() const {
     return fd_;
   }
 
+  auto GetIndexInDevArray() const {
+    return index_in_dev_array_;
+  }
+
   auto &GetResMan() {
     return *res_man_;
   }
@@ -103,12 +107,13 @@
                   DrmProperty *property) const;
 
  private:
-  explicit DrmDevice(ResourceManager *res_man);
+  explicit DrmDevice(ResourceManager *res_man, uint32_t index);
   auto Init(const char *path) -> int;
 
   static auto IsKMSDev(const char *path) -> bool;
 
   SharedFd fd_;
+  const uint32_t index_in_dev_array_;
 
   std::vector<std::unique_ptr<DrmConnector>> connectors_;
   std::vector<std::unique_ptr<DrmConnector>> writeback_connectors_;

drm/ResourceManager.cpp

diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp
index c866263..fee251e 100644
--- a/drm/ResourceManager.cpp
+++ b/drm/ResourceManager.cpp
@@ -55,7 +55,7 @@
   auto path_len = property_get("vendor.hwc.drm.device", path_pattern,
                                "/dev/dri/card%");
   if (path_pattern[path_len - 1] != '%') {
-    auto dev = DrmDevice::CreateInstance(path_pattern, this);
+    auto dev = DrmDevice::CreateInstance(path_pattern, this, 0);
     if (dev) {
       drms_.emplace_back(std::move(dev));
     }
@@ -69,7 +69,7 @@
       if (stat(path.str().c_str(), &buf) != 0)
         break;
 
-      auto dev = DrmDevice::CreateInstance(path.str(), this);
+      auto dev = DrmDevice::CreateInstance(path.str(), this, idx);
       if (dev) {
         drms_.emplace_back(std::move(dev));
       }

hwc2_device/HwcDisplay.cpp

diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 016998f..42ebd63 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -1158,12 +1158,21 @@
     return HWC2::Error::Unsupported;
   }
 
-  auto blob = GetPipe().connector->Get()->GetEdidBlob();
+  auto *connector = GetPipe().connector->Get();
+  auto blob = connector->GetEdidBlob();
   if (!blob) {
     return HWC2::Error::Unsupported;
   }
 
-  *outPort = handle_; /* TDOD(nobody): What should be here? */
+  constexpr uint8_t kDrmDeviceBitShift = 5U;
+  constexpr uint8_t kDrmDeviceBitMask = 0xE0;
+  constexpr uint8_t kConnectorBitMask = 0x1F;
+  const auto kDrmIdx = static_cast<uint8_t>(
+      connector->GetDev().GetIndexInDevArray());
+  const auto kConnectorIdx = static_cast<uint8_t>(
+      connector->GetIndexInResArray());
+  *outPort = (((kDrmIdx << kDrmDeviceBitShift) & kDrmDeviceBitMask) |
+              (kConnectorIdx & kConnectorBitMask));
 
   if (outData) {
     *outDataSize = std::min(*outDataSize, blob->length);