SF: parsing Detailed Timing Descriptor in the framework

The first DTD in an edid is the preferred timing which allows to get
a more precise and more often correct physical size for a display. This
can be used to estimate or correct the dpi in case the edid reports the
wrong size and hwc is not providing the right value.

See edid spec: https://glenwing.github.io/docs/VESA-EEDID-A2.pdf

Flag: com.android.graphics.surfaceflinger.flags.correct_dpi_with_display_size
Bug: 361413340
Test: DisplayIdentification_test
Test: HWComposerTest
Test: manual - see bug and doc

Change-Id: I0bb85dcf8039f923f1ac892c4a1d6bda771dbf4f

15 files changed, 265 insertions, 64 deletions

diff --git a/libs/ui/DisplayIdentification.cpp b/libs/ui/DisplayIdentification.cpp
index e5af7406ed..8b13d78840 100644
--- a/libs/ui/DisplayIdentification.cpp
+++ b/libs/ui/DisplayIdentification.cpp
@@ -26,6 +26,7 @@
 #include <ftl/hash.h>
 #include <log/log.h>
 #include <ui/DisplayIdentification.h>
+#include <ui/Size.h>
 
 namespace android {
 namespace {
@@ -46,6 +47,10 @@ std::optional<uint8_t> getEdidDescriptorType(const byte_view& view) {
     return view[3];
 }
 
+bool isDetailedTimingDescriptor(const byte_view& view) {
+    return view[0] != 0 && view[1] != 0;
+}
+
 std::string_view parseEdidText(const byte_view& view) {
     std::string_view text(reinterpret_cast<const char*>(view.data()), view.size());
     text = text.substr(0, text.find('\n'));
@@ -219,6 +224,8 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
     std::string_view displayName;
     std::string_view serialNumber;
     std::string_view asciiText;
+    ui::Size preferredDTDPixelSize;
+    ui::Size preferredDTDPhysicalSize;
 
     constexpr size_t kDescriptorCount = 4;
     constexpr size_t kDescriptorLength = 18;
@@ -243,6 +250,35 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
                     serialNumber = parseEdidText(descriptor);
                     break;
             }
+        } else if (isDetailedTimingDescriptor(view)) {
+            static constexpr size_t kHorizontalPhysicalLsbOffset = 12;
+            static constexpr size_t kHorizontalPhysicalMsbOffset = 14;
+            static constexpr size_t kVerticalPhysicalLsbOffset = 13;
+            static constexpr size_t kVerticalPhysicalMsbOffset = 14;
+            const uint32_t hSize =
+                    static_cast<uint32_t>(view[kHorizontalPhysicalLsbOffset] |
+                                          ((view[kHorizontalPhysicalMsbOffset] >> 4) << 8));
+            const uint32_t vSize =
+                    static_cast<uint32_t>(view[kVerticalPhysicalLsbOffset] |
+                                          ((view[kVerticalPhysicalMsbOffset] & 0b1111) << 8));
+
+            static constexpr size_t kHorizontalPixelLsbOffset = 2;
+            static constexpr size_t kHorizontalPixelMsbOffset = 4;
+            static constexpr size_t kVerticalPixelLsbOffset = 5;
+            static constexpr size_t kVerticalPixelMsbOffset = 7;
+
+            const uint8_t hLsb = view[kHorizontalPixelLsbOffset];
+            const uint8_t hMsb = view[kHorizontalPixelMsbOffset];
+            const int32_t hPixel = hLsb + ((hMsb & 0xF0) << 4);
+
+            const uint8_t vLsb = view[kVerticalPixelLsbOffset];
+            const uint8_t vMsb = view[kVerticalPixelMsbOffset];
+            const int32_t vPixel = vLsb + ((vMsb & 0xF0) << 4);
+
+            preferredDTDPixelSize.setWidth(hPixel);
+            preferredDTDPixelSize.setHeight(vPixel);
+            preferredDTDPhysicalSize.setWidth(hSize);
+            preferredDTDPhysicalSize.setHeight(vSize);
         }
 
         view = view.subspan(kDescriptorLength);
@@ -297,14 +333,22 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
         }
     }
 
-    return Edid{.manufacturerId = manufacturerId,
-                .productId = productId,
-                .pnpId = *pnpId,
-                .modelHash = modelHash,
-                .displayName = displayName,
-                .manufactureOrModelYear = manufactureOrModelYear,
-                .manufactureWeek = manufactureWeek,
-                .cea861Block = cea861Block};
+    DetailedTimingDescriptor preferredDetailedTimingDescriptor{
+            .pixelSizeCount = preferredDTDPixelSize,
+            .physicalSizeInMm = preferredDTDPhysicalSize,
+    };
+
+    return Edid{
+            .manufacturerId = manufacturerId,
+            .productId = productId,
+            .pnpId = *pnpId,
+            .modelHash = modelHash,
+            .displayName = displayName,
+            .manufactureOrModelYear = manufactureOrModelYear,
+            .manufactureWeek = manufactureWeek,
+            .cea861Block = cea861Block,
+            .preferredDetailedTimingDescriptor = preferredDetailedTimingDescriptor,
+    };
 }
 
 std::optional<PnpId> getPnpId(uint16_t manufacturerId) {
@@ -336,9 +380,12 @@ std::optional<DisplayIdentificationInfo> parseDisplayIdentificationData(
     }
 
     const auto displayId = PhysicalDisplayId::fromEdid(port, edid->manufacturerId, edid->modelHash);
-    return DisplayIdentificationInfo{.id = displayId,
-                                     .name = std::string(edid->displayName),
-                                     .deviceProductInfo = buildDeviceProductInfo(*edid)};
+    return DisplayIdentificationInfo{
+            .id = displayId,
+            .name = std::string(edid->displayName),
+            .deviceProductInfo = buildDeviceProductInfo(*edid),
+            .preferredDetailedTimingDescriptor = edid->preferredDetailedTimingDescriptor,
+    };
 }
 
 PhysicalDisplayId getVirtualDisplayId(uint32_t id) {
diff --git a/libs/ui/include/ui/DisplayIdentification.h b/libs/ui/include/ui/DisplayIdentification.h
index 8bc2017b55..648e024d86 100644
--- a/libs/ui/include/ui/DisplayIdentification.h
+++ b/libs/ui/include/ui/DisplayIdentification.h
@@ -25,6 +25,7 @@
 
 #include <ui/DeviceProductInfo.h>
 #include <ui/DisplayId.h>
+#include <ui/Size.h>
 
 #define LEGACY_DISPLAY_TYPE_PRIMARY 0
 #define LEGACY_DISPLAY_TYPE_EXTERNAL 1
@@ -33,10 +34,16 @@ namespace android {
 
 using DisplayIdentificationData = std::vector<uint8_t>;
 
+struct DetailedTimingDescriptor {
+    ui::Size pixelSizeCount;
+    ui::Size physicalSizeInMm;
+};
+
 struct DisplayIdentificationInfo {
     PhysicalDisplayId id;
     std::string name;
     std::optional<DeviceProductInfo> deviceProductInfo;
+    std::optional<DetailedTimingDescriptor> preferredDetailedTimingDescriptor;
 };
 
 struct ExtensionBlock {
@@ -68,6 +75,7 @@ struct Edid {
     uint8_t manufactureOrModelYear;
     uint8_t manufactureWeek;
     std::optional<Cea861ExtensionBlock> cea861Block;
+    std::optional<DetailedTimingDescriptor> preferredDetailedTimingDescriptor;
 };
 
 bool isEdid(const DisplayIdentificationData&);
diff --git a/libs/ui/tests/DisplayIdentification_test.cpp b/libs/ui/tests/DisplayIdentification_test.cpp
index 721b46688e..76e3f66e1b 100644
--- a/libs/ui/tests/DisplayIdentification_test.cpp
+++ b/libs/ui/tests/DisplayIdentification_test.cpp
@@ -194,6 +194,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
     EXPECT_EQ(21, edid->manufactureOrModelYear);
     EXPECT_EQ(0, edid->manufactureWeek);
     EXPECT_FALSE(edid->cea861Block);
+    EXPECT_EQ(1280, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
+    EXPECT_EQ(800, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
+    EXPECT_EQ(261, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
+    EXPECT_EQ(163, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
 
     edid = parseEdid(getExternalEdid());
     ASSERT_TRUE(edid);
@@ -206,6 +210,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
     EXPECT_EQ(22, edid->manufactureOrModelYear);
     EXPECT_EQ(2, edid->manufactureWeek);
     EXPECT_FALSE(edid->cea861Block);
+    EXPECT_EQ(1280, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
+    EXPECT_EQ(800, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
+    EXPECT_EQ(641, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
+    EXPECT_EQ(400, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
 
     edid = parseEdid(getExternalEedid());
     ASSERT_TRUE(edid);
@@ -224,6 +232,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
     EXPECT_EQ(0, physicalAddress.b);
     EXPECT_EQ(0, physicalAddress.c);
     EXPECT_EQ(0, physicalAddress.d);
+    EXPECT_EQ(1366, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
+    EXPECT_EQ(768, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
+    EXPECT_EQ(160, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
+    EXPECT_EQ(90, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
 
     edid = parseEdid(getPanasonicTvEdid());
     ASSERT_TRUE(edid);
@@ -242,6 +254,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
     EXPECT_EQ(0, physicalAddress.b);
     EXPECT_EQ(0, physicalAddress.c);
     EXPECT_EQ(0, physicalAddress.d);
+    EXPECT_EQ(1920, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
+    EXPECT_EQ(1080, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
+    EXPECT_EQ(698, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
+    EXPECT_EQ(392, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
 
     edid = parseEdid(getHisenseTvEdid());
     ASSERT_TRUE(edid);
@@ -260,6 +276,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
     EXPECT_EQ(2, physicalAddress.b);
     EXPECT_EQ(3, physicalAddress.c);
     EXPECT_EQ(4, physicalAddress.d);
+    EXPECT_EQ(1920, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
+    EXPECT_EQ(1080, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
+    EXPECT_EQ(575, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
+    EXPECT_EQ(323, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
 
     edid = parseEdid(getCtlDisplayEdid());
     ASSERT_TRUE(edid);
@@ -273,6 +293,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
     EXPECT_EQ(0xff, edid->manufactureWeek);
     ASSERT_TRUE(edid->cea861Block);
     EXPECT_FALSE(edid->cea861Block->hdmiVendorDataBlock);
+    EXPECT_EQ(1360, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
+    EXPECT_EQ(768, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
+    EXPECT_EQ(521, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
+    EXPECT_EQ(293, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
 }
 
 TEST(DisplayIdentificationTest, parseInvalidEdid) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 839fb7d7a3..e910c72e2e 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -51,7 +51,8 @@ public:
     MOCK_CONST_METHOD0(getMaxVirtualDisplayCount, size_t());
     MOCK_CONST_METHOD0(getMaxVirtualDisplayDimension, size_t());
     MOCK_METHOD3(allocateVirtualDisplay, bool(HalVirtualDisplayId, ui::Size, ui::PixelFormat*));
-    MOCK_METHOD2(allocatePhysicalDisplay, void(hal::HWDisplayId, PhysicalDisplayId));
+    MOCK_METHOD3(allocatePhysicalDisplay,
+                 void(hal::HWDisplayId, PhysicalDisplayId, std::optional<ui::Size>));
 
     MOCK_METHOD1(createLayer, std::shared_ptr<HWC2::Layer>(HalDisplayId));
     MOCK_METHOD(status_t, getDeviceCompositionChanges,
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index 8e78af4a09..f1fa9389eb 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -675,6 +675,11 @@ void Display::loadDisplayCapabilities() {
         }
     });
 }
+
+void Display::setPhysicalSizeInMm(std::optional<ui::Size> size) {
+    mPhysicalSize = size;
+}
+
 } // namespace impl
 
 // Layer methods
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index d426bca437..8e2aeaf234 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -105,6 +105,7 @@ public:
     virtual bool isVsyncPeriodSwitchSupported() const = 0;
     virtual bool hasDisplayIdleTimerCapability() const = 0;
     virtual void onLayerDestroyed(hal::HWLayerId layerId) = 0;
+    virtual std::optional<ui::Size> getPhysicalSizeInMm() const = 0;
 
     [[nodiscard]] virtual hal::Error acceptChanges() = 0;
     [[nodiscard]] virtual base::expected<std::shared_ptr<HWC2::Layer>, hal::Error>
@@ -285,6 +286,8 @@ public:
     bool hasDisplayIdleTimerCapability() const override;
     void onLayerDestroyed(hal::HWLayerId layerId) override;
     hal::Error getPhysicalDisplayOrientation(Hwc2::AidlTransform* outTransform) const override;
+    void setPhysicalSizeInMm(std::optional<ui::Size> size);
+    std::optional<ui::Size> getPhysicalSizeInMm() const override { return mPhysicalSize; }
 
 private:
     void loadDisplayCapabilities();
@@ -318,6 +321,8 @@ private:
     std::optional<
             std::unordered_set<aidl::android::hardware::graphics::composer3::DisplayCapability>>
             mDisplayCapabilities GUARDED_BY(mDisplayCapabilitiesMutex);
+    // Physical size in mm.
+    std::optional<ui::Size> mPhysicalSize;
 };
 
 } // namespace impl
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index c83e0da99a..bd093f52cf 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -76,6 +76,7 @@ using aidl::android::hardware::graphics::common::HdrConversionCapability;
 using aidl::android::hardware::graphics::common::HdrConversionStrategy;
 using aidl::android::hardware::graphics::composer3::Capability;
 using aidl::android::hardware::graphics::composer3::DisplayCapability;
+using aidl::android::hardware::graphics::composer3::DisplayConfiguration;
 using namespace std::string_literals;
 
 namespace android {
@@ -222,8 +223,8 @@ bool HWComposer::allocateVirtualDisplay(HalVirtualDisplayId displayId, ui::Size
     return true;
 }
 
-void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId,
-                                         PhysicalDisplayId displayId) {
+void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId, PhysicalDisplayId displayId,
+                                         std::optional<ui::Size> physicalSize) {
     mPhysicalDisplayIdMap[hwcDisplayId] = displayId;
 
     if (!mPrimaryHwcDisplayId) {
@@ -235,6 +236,7 @@ void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId,
             std::make_unique<HWC2::impl::Display>(*mComposer.get(), mCapabilities, hwcDisplayId,
                                                   hal::DisplayType::PHYSICAL);
     newDisplay->setConnected(true);
+    newDisplay->setPhysicalSizeInMm(physicalSize);
     displayData.hwcDisplay = std::move(newDisplay);
 }
 
@@ -276,6 +278,47 @@ std::vector<HWComposer::HWCDisplayMode> HWComposer::getModes(PhysicalDisplayId d
     return getModesFromLegacyDisplayConfigs(hwcDisplayId);
 }
 
+DisplayConfiguration::Dpi HWComposer::getEstimatedDotsPerInchFromSize(
+        uint64_t hwcDisplayId, const HWCDisplayMode& hwcMode) const {
+    if (!FlagManager::getInstance().correct_dpi_with_display_size()) {
+        return {-1, -1};
+    }
+    if (const auto displayId = toPhysicalDisplayId(hwcDisplayId)) {
+        if (const auto it = mDisplayData.find(displayId.value());
+            it != mDisplayData.end() && it->second.hwcDisplay->getPhysicalSizeInMm()) {
+            ui::Size size = it->second.hwcDisplay->getPhysicalSizeInMm().value();
+            if (hwcMode.width > 0 && hwcMode.height > 0 && size.width > 0 && size.height > 0) {
+                static constexpr float kMmPerInch = 25.4f;
+                return {hwcMode.width * kMmPerInch / size.width,
+                        hwcMode.height * kMmPerInch / size.height};
+            }
+        }
+    }
+    return {-1, -1};
+}
+
+DisplayConfiguration::Dpi HWComposer::correctedDpiIfneeded(
+        DisplayConfiguration::Dpi dpi, DisplayConfiguration::Dpi estimatedDpi) const {
+    // hwc can be unreliable when it comes to dpi. A rough estimated dpi may yield better
+    // results. For instance, libdrm and bad edid may result in a dpi of {350, 290} for a
+    // 16:9 3840x2160 display, which would match a 4:3 aspect ratio.
+    // The logic here checks if hwc was able to provide some dpi, and if so if the dpi
+    // disparity between the axes is more reasonable than a rough estimate, otherwise use
+    // the estimated dpi as a corrected value.
+    if (estimatedDpi.x == -1 || estimatedDpi.x == -1) {
+        return dpi;
+    }
+    if (dpi.x == -1 || dpi.y == -1) {
+        return estimatedDpi;
+    }
+    if (std::min(dpi.x, dpi.y) != 0 && std::min(estimatedDpi.x, estimatedDpi.y) != 0 &&
+        abs(dpi.x - dpi.y) / std::min(dpi.x, dpi.y) >
+                abs(estimatedDpi.x - estimatedDpi.y) / std::min(estimatedDpi.x, estimatedDpi.y)) {
+        return estimatedDpi;
+    }
+    return dpi;
+}
+
 std::vector<HWComposer::HWCDisplayMode> HWComposer::getModesFromDisplayConfigurations(
         uint64_t hwcDisplayId, int32_t maxFrameIntervalNs) const {
     std::vector<hal::DisplayConfiguration> configs;
@@ -294,9 +337,16 @@ std::vector<HWComposer::HWCDisplayMode> HWComposer::getModesFromDisplayConfigura
                                       .configGroup = config.configGroup,
                                       .vrrConfig = config.vrrConfig};
 
+        const DisplayConfiguration::Dpi estimatedDPI =
+                getEstimatedDotsPerInchFromSize(hwcDisplayId, hwcMode);
         if (config.dpi) {
-            hwcMode.dpiX = config.dpi->x;
-            hwcMode.dpiY = config.dpi->y;
+            const DisplayConfiguration::Dpi dpi =
+                    correctedDpiIfneeded(config.dpi.value(), estimatedDPI);
+            hwcMode.dpiX = dpi.x;
+            hwcMode.dpiY = dpi.y;
+        } else if (estimatedDPI.x != -1 && estimatedDPI.y != -1) {
+            hwcMode.dpiX = estimatedDPI.x;
+            hwcMode.dpiY = estimatedDPI.y;
         }
 
         if (!mEnableVrrTimeout) {
@@ -328,12 +378,14 @@ std::vector<HWComposer::HWCDisplayMode> HWComposer::getModesFromLegacyDisplayCon
 
         const int32_t dpiX = getAttribute(hwcDisplayId, configId, hal::Attribute::DPI_X);
         const int32_t dpiY = getAttribute(hwcDisplayId, configId, hal::Attribute::DPI_Y);
-        if (dpiX != -1) {
-            hwcMode.dpiX = static_cast<float>(dpiX) / 1000.f;
-        }
-        if (dpiY != -1) {
-            hwcMode.dpiY = static_cast<float>(dpiY) / 1000.f;
-        }
+        const DisplayConfiguration::Dpi hwcDpi =
+                DisplayConfiguration::Dpi{dpiX == -1 ? dpiY : dpiX / 1000.f,
+                                          dpiY == -1 ? dpiY : dpiY / 1000.f};
+        const DisplayConfiguration::Dpi estimatedDPI =
+                getEstimatedDotsPerInchFromSize(hwcDisplayId, hwcMode);
+        const DisplayConfiguration::Dpi dpi = correctedDpiIfneeded(hwcDpi, estimatedDPI);
+        hwcMode.dpiX = dpi.x;
+        hwcMode.dpiY = dpi.y;
 
         modes.push_back(hwcMode);
     }
@@ -1072,6 +1124,8 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugConnect(
             getDisplayIdentificationData(hwcDisplayId, &port, &data);
             if (auto newInfo = parseDisplayIdentificationData(port, data)) {
                 info->deviceProductInfo = std::move(newInfo->deviceProductInfo);
+                info->preferredDetailedTimingDescriptor =
+                        std::move(newInfo->preferredDetailedTimingDescriptor);
             } else {
                 ALOGE("Failed to parse identification data for display %" PRIu64, hwcDisplayId);
             }
@@ -1114,7 +1168,11 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugConnect(
     }
 
     if (!isConnected(info->id)) {
-        allocatePhysicalDisplay(hwcDisplayId, info->id);
+        std::optional<ui::Size> size = std::nullopt;
+        if (info->preferredDetailedTimingDescriptor) {
+            size = info->preferredDetailedTimingDescriptor->physicalSizeInMm;
+        }
+        allocatePhysicalDisplay(hwcDisplayId, info->id, size);
     }
     return info;
 }
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index dec4bfed41..b95c619f75 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -134,7 +134,8 @@ public:
     // supported by the HWC can be queried in advance, but allocation may fail for other reasons.
     virtual bool allocateVirtualDisplay(HalVirtualDisplayId, ui::Size, ui::PixelFormat*) = 0;
 
-    virtual void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId) = 0;
+    virtual void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId,
+                                         std::optional<ui::Size> physicalSize) = 0;
 
     // Attempts to create a new layer on this display
     virtual std::shared_ptr<HWC2::Layer> createLayer(HalDisplayId) = 0;
@@ -349,7 +350,8 @@ public:
     bool allocateVirtualDisplay(HalVirtualDisplayId, ui::Size, ui::PixelFormat*) override;
 
     // Called from SurfaceFlinger, when the state for a new physical display needs to be recreated.
-    void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId) override;
+    void allocatePhysicalDisplay(hal::HWDisplayId, PhysicalDisplayId,
+                                 std::optional<ui::Size> physicalSize) override;
 
     // Attempts to create a new layer on this display
     std::shared_ptr<HWC2::Layer> createLayer(HalDisplayId) override;
@@ -532,6 +534,13 @@ private:
     std::optional<DisplayIdentificationInfo> onHotplugDisconnect(hal::HWDisplayId);
     bool shouldIgnoreHotplugConnect(hal::HWDisplayId, bool hasDisplayIdentificationData) const;
 
+    aidl::android::hardware::graphics::composer3::DisplayConfiguration::Dpi
+    getEstimatedDotsPerInchFromSize(uint64_t hwcDisplayId, const HWCDisplayMode& hwcMode) const;
+
+    aidl::android::hardware::graphics::composer3::DisplayConfiguration::Dpi correctedDpiIfneeded(
+            aidl::android::hardware::graphics::composer3::DisplayConfiguration::Dpi dpi,
+            aidl::android::hardware::graphics::composer3::DisplayConfiguration::Dpi estimatedDpi)
+            const;
     std::vector<HWCDisplayMode> getModesFromDisplayConfigurations(uint64_t hwcDisplayId,
                                                                   int32_t maxFrameIntervalNs) const;
     std::vector<HWCDisplayMode> getModesFromLegacyDisplayConfigs(uint64_t hwcDisplayId) const;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index aeca824427..4bfbde5f28 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3817,7 +3817,8 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken,
         mDisplays.erase(displayToken);
 
         if (const auto& physical = currentState.physical) {
-            getHwComposer().allocatePhysicalDisplay(physical->hwcDisplayId, physical->id);
+            getHwComposer().allocatePhysicalDisplay(physical->hwcDisplayId, physical->id,
+                                                    /*physicalSize=*/std::nullopt);
         }
 
         processDisplayAdded(displayToken, currentState);
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index 82aa5572c4..08412cb8e4 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -148,6 +148,7 @@ void FlagManager::dump(std::string& result) const {
     DUMP_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter);
     DUMP_READ_ONLY_FLAG(detached_mirror);
     DUMP_READ_ONLY_FLAG(commit_not_composited);
+    DUMP_READ_ONLY_FLAG(correct_dpi_with_display_size);
     DUMP_READ_ONLY_FLAG(local_tonemap_screenshots);
     DUMP_READ_ONLY_FLAG(override_trusted_overlay);
     DUMP_READ_ONLY_FLAG(flush_buffer_slots_to_uncache);
@@ -252,6 +253,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(deprecate_vsync_sf, "");
 FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, "");
 FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, "");
 FLAG_MANAGER_READ_ONLY_FLAG(commit_not_composited, "");
+FLAG_MANAGER_READ_ONLY_FLAG(correct_dpi_with_display_size, "");
 FLAG_MANAGER_READ_ONLY_FLAG(local_tonemap_screenshots, "debug.sf.local_tonemap_screenshots");
 FLAG_MANAGER_READ_ONLY_FLAG(override_trusted_overlay, "");
 FLAG_MANAGER_READ_ONLY_FLAG(flush_buffer_slots_to_uncache, "");
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index 6619975cd8..ab0ea3bdaa 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -86,6 +86,7 @@ public:
     bool allow_n_vsyncs_in_targeter() const;
     bool detached_mirror() const;
     bool commit_not_composited() const;
+    bool correct_dpi_with_display_size() const;
     bool local_tonemap_screenshots() const;
     bool override_trusted_overlay() const;
     bool flush_buffer_slots_to_uncache() const;
diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
index 0ff846e744..fcfeaccee7 100644
--- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
@@ -4,10 +4,10 @@ package: "com.android.graphics.surfaceflinger.flags"
 container: "system"
 
 flag {
-    name: "adpf_gpu_sf"
-    namespace: "game"
-    description: "Guards use of the sending ADPF GPU duration hint and load hints from SurfaceFlinger to Power HAL"
-    bug: "284324521"
+  name: "adpf_gpu_sf"
+  namespace: "game"
+  description: "Guards use of the sending ADPF GPU duration hint and load hints from SurfaceFlinger to Power HAL"
+  bug: "284324521"
 } # adpf_gpu_sf
 
 flag {
@@ -21,18 +21,29 @@ flag {
   }
  } # ce_fence_promise
 
- flag {
-   name: "commit_not_composited"
-   namespace: "core_graphics"
-   description: "mark frames as non janky if the transaction resulted in no composition"
-   bug: "340633280"
-   is_fixed_read_only: true
-   metadata {
-     purpose: PURPOSE_BUGFIX
-   }
-  } # commit_not_composited
+flag {
+  name: "commit_not_composited"
+  namespace: "core_graphics"
+  description: "mark frames as non janky if the transaction resulted in no composition"
+  bug: "340633280"
+  is_fixed_read_only: true
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
+} # commit_not_composited
 
- flag {
+flag {
+  name: "correct_dpi_with_display_size"
+  namespace: "core_graphics"
+  description: "indicate whether missing or likely incorrect dpi should be corrected using the display size."
+  bug: "328425848"
+  is_fixed_read_only: true
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
+} # correct_dpi_with_display_size
+
+flag {
   name: "deprecate_vsync_sf"
   namespace: "core_graphics"
   description: "Depracate eVsyncSourceSurfaceFlinger and use vsync_app everywhere"
@@ -43,7 +54,7 @@ flag {
   }
 } # deprecate_vsync_sf
 
- flag {
+flag {
   name: "detached_mirror"
   namespace: "window_surfaces"
   description: "Ignore local transform when mirroring a partial hierarchy"
@@ -96,11 +107,11 @@ flag {
 } # latch_unsignaled_with_auto_refresh_changed
 
 flag {
-    name: "local_tonemap_screenshots"
-    namespace: "core_graphics"
-    description: "Enables local tonemapping when capturing screenshots"
-    bug: "329464641"
-    is_fixed_read_only: true
+  name: "local_tonemap_screenshots"
+  namespace: "core_graphics"
+  description: "Enables local tonemapping when capturing screenshots"
+  bug: "329464641"
+  is_fixed_read_only: true
 } # local_tonemap_screenshots
 
 flag {
@@ -115,11 +126,11 @@ flag {
  } # single_hop_screenshot
 
 flag {
-    name: "true_hdr_screenshots"
-    namespace: "core_graphics"
-    description: "Enables screenshotting display content in HDR, sans tone mapping"
-    bug: "329470026"
-    is_fixed_read_only: true
+  name: "true_hdr_screenshots"
+  namespace: "core_graphics"
+  description: "Enables screenshotting display content in HDR, sans tone mapping"
+  bug: "329470026"
+  is_fixed_read_only: true
 } # true_hdr_screenshots
 
  flag {
@@ -134,11 +145,11 @@ flag {
 } # override_trusted_overlay
 
 flag {
-    name: "view_set_requested_frame_rate_mrr"
-    namespace: "core_graphics"
-    description: "Enable to use frame rate category NoPreference with fixed frame rate vote on MRR devices"
-    bug: "352206100"
-    is_fixed_read_only: true
+  name: "view_set_requested_frame_rate_mrr"
+  namespace: "core_graphics"
+  description: "Enable to use frame rate category NoPreference with fixed frame rate vote on MRR devices"
+  bug: "352206100"
+  is_fixed_read_only: true
 } # view_set_requested_frame_rate_mrr
 
 flag {
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 6030427d3e..e0753a3cfb 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -166,6 +166,7 @@ TEST_F(HWComposerTest, getModesWithLegacyDisplayConfigs) {
     expectHotplugConnect(kHwcDisplayId);
     const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
     ASSERT_TRUE(info);
+    ASSERT_TRUE(info->preferredDetailedTimingDescriptor.has_value());
 
     EXPECT_CALL(*mHal, isVrrSupported()).WillRepeatedly(Return(false));
 
@@ -179,6 +180,10 @@ TEST_F(HWComposerTest, getModesWithLegacyDisplayConfigs) {
         constexpr int32_t kHeight = 720;
         constexpr int32_t kConfigGroup = 1;
         constexpr int32_t kVsyncPeriod = 16666667;
+        constexpr float kMmPerInch = 25.4f;
+        const ui::Size size = info->preferredDetailedTimingDescriptor->physicalSizeInMm;
+        const float expectedDpiX = (kWidth * kMmPerInch / size.width);
+        const float expectedDpiY = (kHeight * kMmPerInch / size.height);
 
         EXPECT_CALL(*mHal,
                     getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::WIDTH,
@@ -218,8 +223,13 @@ TEST_F(HWComposerTest, getModesWithLegacyDisplayConfigs) {
         EXPECT_EQ(modes.front().height, kHeight);
         EXPECT_EQ(modes.front().configGroup, kConfigGroup);
         EXPECT_EQ(modes.front().vsyncPeriod, kVsyncPeriod);
-        EXPECT_EQ(modes.front().dpiX, -1);
-        EXPECT_EQ(modes.front().dpiY, -1);
+        if (!FlagManager::getInstance().correct_dpi_with_display_size()) {
+            EXPECT_EQ(modes.front().dpiX, -1);
+            EXPECT_EQ(modes.front().dpiY, -1);
+        } else {
+            EXPECT_EQ(modes.front().dpiX, expectedDpiX);
+            EXPECT_EQ(modes.front().dpiY, expectedDpiY);
+        }
 
         // Optional parameters are supported
         constexpr int32_t kDpi = 320;
@@ -271,6 +281,10 @@ TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_OFF) {
         constexpr int32_t kHeight = 720;
         constexpr int32_t kConfigGroup = 1;
         constexpr int32_t kVsyncPeriod = 16666667;
+        constexpr float kMmPerInch = 25.4f;
+        const ui::Size size = info->preferredDetailedTimingDescriptor->physicalSizeInMm;
+        const float expectedDpiX = (kWidth * kMmPerInch / size.width);
+        const float expectedDpiY = (kHeight * kMmPerInch / size.height);
 
         EXPECT_CALL(*mHal,
                     getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::WIDTH,
@@ -310,8 +324,13 @@ TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_OFF) {
         EXPECT_EQ(modes.front().height, kHeight);
         EXPECT_EQ(modes.front().configGroup, kConfigGroup);
         EXPECT_EQ(modes.front().vsyncPeriod, kVsyncPeriod);
-        EXPECT_EQ(modes.front().dpiX, -1);
-        EXPECT_EQ(modes.front().dpiY, -1);
+        if (!FlagManager::getInstance().correct_dpi_with_display_size()) {
+            EXPECT_EQ(modes.front().dpiX, -1);
+            EXPECT_EQ(modes.front().dpiY, -1);
+        } else {
+            EXPECT_EQ(modes.front().dpiX, expectedDpiX);
+            EXPECT_EQ(modes.front().dpiY, expectedDpiY);
+        }
 
         // Optional parameters are supported
         constexpr int32_t kDpi = 320;
@@ -361,6 +380,10 @@ TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_ON) {
         constexpr int32_t kHeight = 720;
         constexpr int32_t kConfigGroup = 1;
         constexpr int32_t kVsyncPeriod = 16666667;
+        constexpr float kMmPerInch = 25.4f;
+        const ui::Size size = info->preferredDetailedTimingDescriptor->physicalSizeInMm;
+        const float expectedDpiX = (kWidth * kMmPerInch / size.width);
+        const float expectedDpiY = (kHeight * kMmPerInch / size.height);
         const hal::VrrConfig vrrConfig =
                 hal::VrrConfig{.minFrameIntervalNs = static_cast<Fps>(120_Hz).getPeriodNsecs(),
                                .notifyExpectedPresentConfig = hal::VrrConfig::
@@ -387,8 +410,13 @@ TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_ON) {
         EXPECT_EQ(modes.front().configGroup, kConfigGroup);
         EXPECT_EQ(modes.front().vsyncPeriod, kVsyncPeriod);
         EXPECT_EQ(modes.front().vrrConfig, vrrConfig);
-        EXPECT_EQ(modes.front().dpiX, -1);
-        EXPECT_EQ(modes.front().dpiY, -1);
+        if (!FlagManager::getInstance().correct_dpi_with_display_size()) {
+            EXPECT_EQ(modes.front().dpiX, -1);
+            EXPECT_EQ(modes.front().dpiY, -1);
+        } else {
+            EXPECT_EQ(modes.front().dpiX, expectedDpiX);
+            EXPECT_EQ(modes.front().dpiY, expectedDpiY);
+        }
 
         // Supports optional dpi parameter
         constexpr int32_t kDpi = 320;
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
index 933d03dac1..352000ef9a 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -239,7 +239,7 @@ void SetupNewDisplayDeviceInternalTest::setupNewDisplayDeviceInternalTest() {
         ASSERT_TRUE(displayId);
         const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
         ASSERT_TRUE(hwcDisplayId);
-        mFlinger.getHwComposer().allocatePhysicalDisplay(*hwcDisplayId, *displayId);
+        mFlinger.getHwComposer().allocatePhysicalDisplay(*hwcDisplayId, *displayId, std::nullopt);
         DisplayModePtr activeMode = DisplayMode::Builder(Case::Display::HWC_ACTIVE_CONFIG_ID)
                                             .setResolution(Case::Display::RESOLUTION)
                                             .setVsyncPeriod(DEFAULT_VSYNC_PERIOD)
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
index 2cc19873f5..5edd2cd9e3 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
@@ -35,6 +35,7 @@ public:
                 (const, override));
     MOCK_METHOD(bool, isVsyncPeriodSwitchSupported, (), (const, override));
     MOCK_METHOD(void, onLayerDestroyed, (hal::HWLayerId), (override));
+    MOCK_METHOD(std::optional<ui::Size>, getPhysicalSizeInMm, (), (const override));
 
     MOCK_METHOD(hal::Error, acceptChanges, (), (override));
     MOCK_METHOD((base::expected<std::shared_ptr<HWC2::Layer>, hal::Error>), createLayer, (),