SF: Store DeviceProductInfo in DisplayDevice

When a new display is connected we query the display
identification data, parse the EDID and store it in
DisplayDevice for future use. This saves an unnecessary IPC
to HWC when SurfaceFlinger::getDisplayInfo() is called.

Bug: 149075047
Test: 1. flash a device
      2. boot a device connected to a display
      3. Check for sensible DeviceProductInfo:
         adb shell dumpsys SurfaceFlinger | grep deviceProductInfo
         adb shell dumpsys display | grep deviceProductInfo
      4. reboot without a display
      5. Check that  DeviceProductInfo is empty / null in
         adb shell dumpsys SurfaceFlinger | grep deviceProductInfo
         adb shell dumpsys display | grep deviceProductInfo
Change-Id: I35b6b3869eb740dd90913c9d3095fdc591b5ba3c

7 files changed, 61 insertions, 13 deletions

diff --git a/libs/ui/DeviceProductInfo.cpp b/libs/ui/DeviceProductInfo.cpp
index 7bced9b76a..4d6ce4306a 100644
--- a/libs/ui/DeviceProductInfo.cpp
+++ b/libs/ui/DeviceProductInfo.cpp
@@ -16,13 +16,17 @@
 
 #include <ui/DeviceProductInfo.h>
 
+#include <android-base/stringprintf.h>
 #include <ui/FlattenableHelpers.h>
+#include <utils/Log.h>
 
 #define RETURN_IF_ERROR(op) \
     if (const status_t status = (op); status != OK) return status;
 
 namespace android {
 
+using base::StringAppendF;
+
 size_t DeviceProductInfo::getFlattenedSize() const {
     return FlattenableHelpers::getFlattenedSize(name) +
             FlattenableHelpers::getFlattenedSize(manufacturerPnpId) +
@@ -52,4 +56,32 @@ status_t DeviceProductInfo::unflatten(void const* buffer, size_t size) {
     return OK;
 }
 
+void DeviceProductInfo::dump(std::string& result) const {
+    StringAppendF(&result, "{name=%s, ", name.c_str());
+    StringAppendF(&result, "manufacturerPnpId=%s, ", manufacturerPnpId.data());
+    StringAppendF(&result, "productId=%s, ", productId.c_str());
+
+    if (const auto* model = std::get_if<ModelYear>(&manufactureOrModelDate)) {
+        StringAppendF(&result, "modelYear=%u, ", model->year);
+    } else if (const auto* manufactureWeekAndYear =
+                       std::get_if<ManufactureWeekAndYear>(&manufactureOrModelDate)) {
+        StringAppendF(&result, "manufactureWeek=%u, ", manufactureWeekAndYear->week);
+        StringAppendF(&result, "manufactureYear=%d, ", manufactureWeekAndYear->year);
+    } else if (const auto* manufactureYear =
+                       std::get_if<ManufactureYear>(&manufactureOrModelDate)) {
+        StringAppendF(&result, "manufactureYear=%d, ", manufactureYear->year);
+    } else {
+        ALOGE("Unknown alternative for variant DeviceProductInfo::ManufactureOrModelDate");
+    }
+
+    result.append("relativeAddress=[");
+    for (size_t i = 0; i < relativeAddress.size(); i++) {
+        if (i != 0) {
+            result.append(", ");
+        }
+        StringAppendF(&result, "%u", relativeAddress[i]);
+    }
+    result.append("]}");
+}
+
 } // namespace android
diff --git a/libs/ui/include/ui/DeviceProductInfo.h b/libs/ui/include/ui/DeviceProductInfo.h
index 0bc1e5c4f3..807a5d96a3 100644
--- a/libs/ui/include/ui/DeviceProductInfo.h
+++ b/libs/ui/include/ui/DeviceProductInfo.h
@@ -68,6 +68,8 @@ struct DeviceProductInfo : LightFlattenable<DeviceProductInfo> {
     size_t getFlattenedSize() const;
     status_t flatten(void* buffer, size_t size) const;
     status_t unflatten(void const* buffer, size_t size);
+
+    void dump(std::string& result) const;
 };
 
 } // namespace android
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 9af9cad099..2cebecb8d3 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -116,6 +116,10 @@ void DisplayDevice::setDisplayName(const std::string& displayName) {
     }
 }
 
+void DisplayDevice::setDeviceProductInfo(std::optional<DeviceProductInfo> info) {
+    mDeviceProductInfo = std::move(info);
+}
+
 uint32_t DisplayDevice::getPageFlipCount() const {
     return mCompositionDisplay->getRenderSurface()->getPageFlipCount();
 }
@@ -267,6 +271,12 @@ void DisplayDevice::dump(std::string& result) const {
     StringAppendF(&result, "powerMode=%s (%d), ", to_string(mPowerMode).c_str(),
                   static_cast<int32_t>(mPowerMode));
     StringAppendF(&result, "activeConfig=%d, ", mActiveConfig.value());
+    StringAppendF(&result, "deviceProductInfo=");
+    if (mDeviceProductInfo) {
+        mDeviceProductInfo->dump(result);
+    } else {
+        result.append("{}");
+    }
     getCompositionDisplay()->dump(result);
 }
 
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 8c8615363a..5a5bbe62cc 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -132,6 +132,11 @@ public:
     void setDisplayName(const std::string& displayName);
     const std::string& getDisplayName() const { return mDisplayName; }
 
+    void setDeviceProductInfo(std::optional<DeviceProductInfo> info);
+    const std::optional<DeviceProductInfo>& getDeviceProductInfo() const {
+        return mDeviceProductInfo;
+    }
+
     /* ------------------------------------------------------------------------
      * Display power mode management.
      */
@@ -178,6 +183,8 @@ private:
 
     // TODO(b/74619554): Remove special cases for primary display.
     const bool mIsPrimary;
+
+    std::optional<DeviceProductInfo> mDeviceProductInfo;
 };
 
 struct DisplayDeviceState {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1b9ddb1886..e191c954a5 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -836,7 +836,7 @@ status_t SurfaceFlinger::getDisplayInfo(const sp<IBinder>& displayToken, Display
     }
 
     info->secure = display->isSecure();
-    info->deviceProductInfo = getDeviceProductInfoLocked(*display);
+    info->deviceProductInfo = display->getDeviceProductInfo();
 
     return NO_ERROR;
 }
@@ -1310,13 +1310,8 @@ status_t SurfaceFlinger::getHdrCapabilities(const sp<IBinder>& displayToken,
 }
 
 std::optional<DeviceProductInfo> SurfaceFlinger::getDeviceProductInfoLocked(
-        const DisplayDevice& display) const {
-    // TODO(b/149075047): Populate DeviceProductInfo on hotplug and store it in DisplayDevice to
-    // avoid repetitive HAL IPC and EDID parsing.
-    const auto displayId = display.getId();
-    LOG_FATAL_IF(!displayId);
-
-    const auto hwcDisplayId = getHwComposer().fromPhysicalDisplayId(*displayId);
+        DisplayId displayId) const {
+    const auto hwcDisplayId = getHwComposer().fromPhysicalDisplayId(displayId);
     LOG_FATAL_IF(!hwcDisplayId);
 
     uint8_t port;
@@ -2544,6 +2539,7 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal(
         LOG_ALWAYS_FATAL_IF(!displayId);
         auto activeConfigId = HwcConfigIndexType(getHwComposer().getActiveConfigIndex(*displayId));
         display->setActiveConfig(activeConfigId);
+        display->setDeviceProductInfo(getDeviceProductInfoLocked(*displayId));
     }
 
     display->setLayerStack(state.layerStack);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 4b5a843589..ad0d9d46fc 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -774,7 +774,7 @@ private:
         return nullptr;
     }
 
-    std::optional<DeviceProductInfo> getDeviceProductInfoLocked(const DisplayDevice&) const;
+    std::optional<DeviceProductInfo> getDeviceProductInfoLocked(DisplayId) const;
 
     // mark a region of a layer stack dirty. this updates the dirty
     // region of all screens presenting this layer stack.
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index f41b10e67c..a554b1b603 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -609,12 +609,13 @@ struct HwcDisplayVariant {
 
         if (PhysicalDisplay::HAS_IDENTIFICATION_DATA) {
             EXPECT_CALL(*test->mComposer, getDisplayIdentificationData(HWC_DISPLAY_ID, _, _))
-                    .WillOnce(DoAll(SetArgPointee<1>(PhysicalDisplay::PORT),
-                                    SetArgPointee<2>(PhysicalDisplay::GET_IDENTIFICATION_DATA()),
-                                    Return(Error::NONE)));
+                    .WillRepeatedly(
+                            DoAll(SetArgPointee<1>(PhysicalDisplay::PORT),
+                                  SetArgPointee<2>(PhysicalDisplay::GET_IDENTIFICATION_DATA()),
+                                  Return(Error::NONE)));
         } else {
             EXPECT_CALL(*test->mComposer, getDisplayIdentificationData(HWC_DISPLAY_ID, _, _))
-                    .WillOnce(Return(Error::UNSUPPORTED));
+                    .WillRepeatedly(Return(Error::UNSUPPORTED));
         }
     }