From 907a51ab1b0f26b691ae3c6050bfdc904b9ee11a Mon Sep 17 00:00:00 2001
From: Gil Dekel <gildekel@google.com>
Date: Fri, 14 Feb 2025 22:44:01 -0500
Subject: [PATCH] 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          |  7 ++++---
 drm/DrmDevice.h            | 11 ++++++++---
 drm/ResourceManager.cpp    |  4 ++--
 hwc2_device/HwcDisplay.cpp | 13 +++++++++++--
 4 files changed, 25 insertions(+), 10 deletions(-)

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 @@ auto DrmDevice::CreateInstance(std::string const &path,
   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);
 }
 
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 @@ class DrmDevice {
  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 @@ class DrmDevice {
                   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_;
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 @@ void ResourceManager::Init() {
   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 @@ void ResourceManager::Init() {
       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));
       }
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 @@ HWC2::Error HwcDisplay::GetDisplayIdentificationData(uint8_t *outPort,
     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);