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
diff --git a/libs/ui/DisplayIdentification.cpp b/libs/ui/DisplayIdentification.cpp
index e5af740..8b13d78 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 @@
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::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 @@
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 @@
}
}
- 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 @@
}
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 8bc2017..648e024 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 @@
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 @@
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 721b466..76e3f66 100644
--- a/libs/ui/tests/DisplayIdentification_test.cpp
+++ b/libs/ui/tests/DisplayIdentification_test.cpp
@@ -194,6 +194,10 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 839fb7d..e910c72 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -51,7 +51,8 @@
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 8e78af4..f1fa938 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -675,6 +675,11 @@
}
});
}
+
+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 d426bca..8e2aeaf 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -105,6 +105,7 @@
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 @@
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 @@
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 c83e0da..bd093f5 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -76,6 +76,7 @@
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 @@
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 @@
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 @@
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 @@
.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 @@
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 @@
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 @@
}
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 dec4bfe..b95c619 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -134,7 +134,8 @@
// 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 @@
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 @@
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 aeca824..4bfbde5 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3817,7 +3817,8 @@
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 82aa557..08412cb 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -148,6 +148,7 @@
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(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 6619975..ab0ea3b 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -86,6 +86,7 @@
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 0ff846e7..fcfeacc 100644
--- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
@@ -4,10 +4,10 @@
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 @@
}
} # 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 @@
}
} # deprecate_vsync_sf
- flag {
+flag {
name: "detached_mirror"
namespace: "window_surfaces"
description: "Ignore local transform when mirroring a partial hierarchy"
@@ -96,11 +107,11 @@
} # 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 @@
} # 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 @@
} # 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 6030427..e0753a3 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -166,6 +166,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 933d03d..352000e 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -239,7 +239,7 @@
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 2cc1987..5edd2cd 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
@@ -35,6 +35,7 @@
(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, (),