Skip to content

Commit 8d0a0c4

Browse files
author
Lucas Berthou
committed
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
1 parent 5b0bdd2 commit 8d0a0c4

15 files changed

Lines changed: 265 additions & 64 deletions

File tree

‎libs/ui/DisplayIdentification.cpp‎

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <ftl/hash.h>
2727
#include <log/log.h>
2828
#include <ui/DisplayIdentification.h>
29+
#include <ui/Size.h>
2930

3031
namespace android {
3132
namespace {
@@ -46,6 +47,10 @@ std::optional<uint8_t> getEdidDescriptorType(const byte_view& view) {
4647
return view[3];
4748
}
4849

50+
bool isDetailedTimingDescriptor(const byte_view& view) {
51+
return view[0] != 0 && view[1] != 0;
52+
}
53+
4954
std::string_view parseEdidText(const byte_view& view) {
5055
std::string_view text(reinterpret_cast<const char*>(view.data()), view.size());
5156
text = text.substr(0, text.find('\n'));
@@ -219,6 +224,8 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
219224
std::string_view displayName;
220225
std::string_view serialNumber;
221226
std::string_view asciiText;
227+
ui::Size preferredDTDPixelSize;
228+
ui::Size preferredDTDPhysicalSize;
222229

223230
constexpr size_t kDescriptorCount = 4;
224231
constexpr size_t kDescriptorLength = 18;
@@ -243,6 +250,35 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
243250
serialNumber = parseEdidText(descriptor);
244251
break;
245252
}
253+
} else if (isDetailedTimingDescriptor(view)) {
254+
static constexpr size_t kHorizontalPhysicalLsbOffset = 12;
255+
static constexpr size_t kHorizontalPhysicalMsbOffset = 14;
256+
static constexpr size_t kVerticalPhysicalLsbOffset = 13;
257+
static constexpr size_t kVerticalPhysicalMsbOffset = 14;
258+
const uint32_t hSize =
259+
static_cast<uint32_t>(view[kHorizontalPhysicalLsbOffset] |
260+
((view[kHorizontalPhysicalMsbOffset] >> 4) << 8));
261+
const uint32_t vSize =
262+
static_cast<uint32_t>(view[kVerticalPhysicalLsbOffset] |
263+
((view[kVerticalPhysicalMsbOffset] & 0b1111) << 8));
264+
265+
static constexpr size_t kHorizontalPixelLsbOffset = 2;
266+
static constexpr size_t kHorizontalPixelMsbOffset = 4;
267+
static constexpr size_t kVerticalPixelLsbOffset = 5;
268+
static constexpr size_t kVerticalPixelMsbOffset = 7;
269+
270+
const uint8_t hLsb = view[kHorizontalPixelLsbOffset];
271+
const uint8_t hMsb = view[kHorizontalPixelMsbOffset];
272+
const int32_t hPixel = hLsb + ((hMsb & 0xF0) << 4);
273+
274+
const uint8_t vLsb = view[kVerticalPixelLsbOffset];
275+
const uint8_t vMsb = view[kVerticalPixelMsbOffset];
276+
const int32_t vPixel = vLsb + ((vMsb & 0xF0) << 4);
277+
278+
preferredDTDPixelSize.setWidth(hPixel);
279+
preferredDTDPixelSize.setHeight(vPixel);
280+
preferredDTDPhysicalSize.setWidth(hSize);
281+
preferredDTDPhysicalSize.setHeight(vSize);
246282
}
247283

248284
view = view.subspan(kDescriptorLength);
@@ -297,14 +333,22 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
297333
}
298334
}
299335

300-
return Edid{.manufacturerId = manufacturerId,
301-
.productId = productId,
302-
.pnpId = *pnpId,
303-
.modelHash = modelHash,
304-
.displayName = displayName,
305-
.manufactureOrModelYear = manufactureOrModelYear,
306-
.manufactureWeek = manufactureWeek,
307-
.cea861Block = cea861Block};
336+
DetailedTimingDescriptor preferredDetailedTimingDescriptor{
337+
.pixelSizeCount = preferredDTDPixelSize,
338+
.physicalSizeInMm = preferredDTDPhysicalSize,
339+
};
340+
341+
return Edid{
342+
.manufacturerId = manufacturerId,
343+
.productId = productId,
344+
.pnpId = *pnpId,
345+
.modelHash = modelHash,
346+
.displayName = displayName,
347+
.manufactureOrModelYear = manufactureOrModelYear,
348+
.manufactureWeek = manufactureWeek,
349+
.cea861Block = cea861Block,
350+
.preferredDetailedTimingDescriptor = preferredDetailedTimingDescriptor,
351+
};
308352
}
309353

310354
std::optional<PnpId> getPnpId(uint16_t manufacturerId) {
@@ -336,9 +380,12 @@ std::optional<DisplayIdentificationInfo> parseDisplayIdentificationData(
336380
}
337381

338382
const auto displayId = PhysicalDisplayId::fromEdid(port, edid->manufacturerId, edid->modelHash);
339-
return DisplayIdentificationInfo{.id = displayId,
340-
.name = std::string(edid->displayName),
341-
.deviceProductInfo = buildDeviceProductInfo(*edid)};
383+
return DisplayIdentificationInfo{
384+
.id = displayId,
385+
.name = std::string(edid->displayName),
386+
.deviceProductInfo = buildDeviceProductInfo(*edid),
387+
.preferredDetailedTimingDescriptor = edid->preferredDetailedTimingDescriptor,
388+
};
342389
}
343390

344391
PhysicalDisplayId getVirtualDisplayId(uint32_t id) {

‎libs/ui/include/ui/DisplayIdentification.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include <ui/DeviceProductInfo.h>
2727
#include <ui/DisplayId.h>
28+
#include <ui/Size.h>
2829

2930
#define LEGACY_DISPLAY_TYPE_PRIMARY 0
3031
#define LEGACY_DISPLAY_TYPE_EXTERNAL 1
@@ -33,10 +34,16 @@ namespace android {
3334

3435
using DisplayIdentificationData = std::vector<uint8_t>;
3536

37+
struct DetailedTimingDescriptor {
38+
ui::Size pixelSizeCount;
39+
ui::Size physicalSizeInMm;
40+
};
41+
3642
struct DisplayIdentificationInfo {
3743
PhysicalDisplayId id;
3844
std::string name;
3945
std::optional<DeviceProductInfo> deviceProductInfo;
46+
std::optional<DetailedTimingDescriptor> preferredDetailedTimingDescriptor;
4047
};
4148

4249
struct ExtensionBlock {
@@ -68,6 +75,7 @@ struct Edid {
6875
uint8_t manufactureOrModelYear;
6976
uint8_t manufactureWeek;
7077
std::optional<Cea861ExtensionBlock> cea861Block;
78+
std::optional<DetailedTimingDescriptor> preferredDetailedTimingDescriptor;
7179
};
7280

7381
bool isEdid(const DisplayIdentificationData&);

‎libs/ui/tests/DisplayIdentification_test.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
194194
EXPECT_EQ(21, edid->manufactureOrModelYear);
195195
EXPECT_EQ(0, edid->manufactureWeek);
196196
EXPECT_FALSE(edid->cea861Block);
197+
EXPECT_EQ(1280, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
198+
EXPECT_EQ(800, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
199+
EXPECT_EQ(261, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
200+
EXPECT_EQ(163, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
197201

198202
edid = parseEdid(getExternalEdid());
199203
ASSERT_TRUE(edid);
@@ -206,6 +210,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
206210
EXPECT_EQ(22, edid->manufactureOrModelYear);
207211
EXPECT_EQ(2, edid->manufactureWeek);
208212
EXPECT_FALSE(edid->cea861Block);
213+
EXPECT_EQ(1280, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
214+
EXPECT_EQ(800, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
215+
EXPECT_EQ(641, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
216+
EXPECT_EQ(400, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
209217

210218
edid = parseEdid(getExternalEedid());
211219
ASSERT_TRUE(edid);
@@ -224,6 +232,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
224232
EXPECT_EQ(0, physicalAddress.b);
225233
EXPECT_EQ(0, physicalAddress.c);
226234
EXPECT_EQ(0, physicalAddress.d);
235+
EXPECT_EQ(1366, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
236+
EXPECT_EQ(768, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
237+
EXPECT_EQ(160, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
238+
EXPECT_EQ(90, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
227239

228240
edid = parseEdid(getPanasonicTvEdid());
229241
ASSERT_TRUE(edid);
@@ -242,6 +254,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
242254
EXPECT_EQ(0, physicalAddress.b);
243255
EXPECT_EQ(0, physicalAddress.c);
244256
EXPECT_EQ(0, physicalAddress.d);
257+
EXPECT_EQ(1920, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
258+
EXPECT_EQ(1080, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
259+
EXPECT_EQ(698, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
260+
EXPECT_EQ(392, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
245261

246262
edid = parseEdid(getHisenseTvEdid());
247263
ASSERT_TRUE(edid);
@@ -260,6 +276,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
260276
EXPECT_EQ(2, physicalAddress.b);
261277
EXPECT_EQ(3, physicalAddress.c);
262278
EXPECT_EQ(4, physicalAddress.d);
279+
EXPECT_EQ(1920, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
280+
EXPECT_EQ(1080, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
281+
EXPECT_EQ(575, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
282+
EXPECT_EQ(323, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
263283

264284
edid = parseEdid(getCtlDisplayEdid());
265285
ASSERT_TRUE(edid);
@@ -273,6 +293,10 @@ TEST(DisplayIdentificationTest, parseEdid) {
273293
EXPECT_EQ(0xff, edid->manufactureWeek);
274294
ASSERT_TRUE(edid->cea861Block);
275295
EXPECT_FALSE(edid->cea861Block->hdmiVendorDataBlock);
296+
EXPECT_EQ(1360, edid->preferredDetailedTimingDescriptor->pixelSizeCount.width);
297+
EXPECT_EQ(768, edid->preferredDetailedTimingDescriptor->pixelSizeCount.height);
298+
EXPECT_EQ(521, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.width);
299+
EXPECT_EQ(293, edid->preferredDetailedTimingDescriptor->physicalSizeInMm.height);
276300
}
277301

278302
TEST(DisplayIdentificationTest, parseInvalidEdid) {

‎services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ class HWComposer : public android::HWComposer {
5151
MOCK_CONST_METHOD0(getMaxVirtualDisplayCount, size_t());
5252
MOCK_CONST_METHOD0(getMaxVirtualDisplayDimension, size_t());
5353
MOCK_METHOD3(allocateVirtualDisplay, bool(HalVirtualDisplayId, ui::Size, ui::PixelFormat*));
54-
MOCK_METHOD2(allocatePhysicalDisplay, void(hal::HWDisplayId, PhysicalDisplayId));
54+
MOCK_METHOD3(allocatePhysicalDisplay,
55+
void(hal::HWDisplayId, PhysicalDisplayId, std::optional<ui::Size>));
5556

5657
MOCK_METHOD1(createLayer, std::shared_ptr<HWC2::Layer>(HalDisplayId));
5758
MOCK_METHOD(status_t, getDeviceCompositionChanges,

‎services/surfaceflinger/DisplayHardware/HWC2.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,11 @@ void Display::loadDisplayCapabilities() {
675675
}
676676
});
677677
}
678+
679+
void Display::setPhysicalSizeInMm(std::optional<ui::Size> size) {
680+
mPhysicalSize = size;
681+
}
682+
678683
} // namespace impl
679684

680685
// Layer methods

‎services/surfaceflinger/DisplayHardware/HWC2.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ class Display {
105105
virtual bool isVsyncPeriodSwitchSupported() const = 0;
106106
virtual bool hasDisplayIdleTimerCapability() const = 0;
107107
virtual void onLayerDestroyed(hal::HWLayerId layerId) = 0;
108+
virtual std::optional<ui::Size> getPhysicalSizeInMm() const = 0;
108109

109110
[[nodiscard]] virtual hal::Error acceptChanges() = 0;
110111
[[nodiscard]] virtual base::expected<std::shared_ptr<HWC2::Layer>, hal::Error>
@@ -285,6 +286,8 @@ class Display : public HWC2::Display {
285286
bool hasDisplayIdleTimerCapability() const override;
286287
void onLayerDestroyed(hal::HWLayerId layerId) override;
287288
hal::Error getPhysicalDisplayOrientation(Hwc2::AidlTransform* outTransform) const override;
289+
void setPhysicalSizeInMm(std::optional<ui::Size> size);
290+
std::optional<ui::Size> getPhysicalSizeInMm() const override { return mPhysicalSize; }
288291

289292
private:
290293
void loadDisplayCapabilities();
@@ -318,6 +321,8 @@ class Display : public HWC2::Display {
318321
std::optional<
319322
std::unordered_set<aidl::android::hardware::graphics::composer3::DisplayCapability>>
320323
mDisplayCapabilities GUARDED_BY(mDisplayCapabilitiesMutex);
324+
// Physical size in mm.
325+
std::optional<ui::Size> mPhysicalSize;
321326
};
322327

323328
} // namespace impl

‎services/surfaceflinger/DisplayHardware/HWComposer.cpp‎

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ using aidl::android::hardware::graphics::common::HdrConversionCapability;
7676
using aidl::android::hardware::graphics::common::HdrConversionStrategy;
7777
using aidl::android::hardware::graphics::composer3::Capability;
7878
using aidl::android::hardware::graphics::composer3::DisplayCapability;
79+
using aidl::android::hardware::graphics::composer3::DisplayConfiguration;
7980
using namespace std::string_literals;
8081

8182
namespace android {
@@ -222,8 +223,8 @@ bool HWComposer::allocateVirtualDisplay(HalVirtualDisplayId displayId, ui::Size
222223
return true;
223224
}
224225

225-
void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId,
226-
PhysicalDisplayId displayId) {
226+
void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId, PhysicalDisplayId displayId,
227+
std::optional<ui::Size> physicalSize) {
227228
mPhysicalDisplayIdMap[hwcDisplayId] = displayId;
228229

229230
if (!mPrimaryHwcDisplayId) {
@@ -235,6 +236,7 @@ void HWComposer::allocatePhysicalDisplay(hal::HWDisplayId hwcDisplayId,
235236
std::make_unique<HWC2::impl::Display>(*mComposer.get(), mCapabilities, hwcDisplayId,
236237
hal::DisplayType::PHYSICAL);
237238
newDisplay->setConnected(true);
239+
newDisplay->setPhysicalSizeInMm(physicalSize);
238240
displayData.hwcDisplay = std::move(newDisplay);
239241
}
240242

@@ -276,6 +278,47 @@ std::vector<HWComposer::HWCDisplayMode> HWComposer::getModes(PhysicalDisplayId d
276278
return getModesFromLegacyDisplayConfigs(hwcDisplayId);
277279
}
278280

281+
DisplayConfiguration::Dpi HWComposer::getEstimatedDotsPerInchFromSize(
282+
uint64_t hwcDisplayId, const HWCDisplayMode& hwcMode) const {
283+
if (!FlagManager::getInstance().correct_dpi_with_display_size()) {
284+
return {-1, -1};
285+
}
286+
if (const auto displayId = toPhysicalDisplayId(hwcDisplayId)) {
287+
if (const auto it = mDisplayData.find(displayId.value());
288+
it != mDisplayData.end() && it->second.hwcDisplay->getPhysicalSizeInMm()) {
289+
ui::Size size = it->second.hwcDisplay->getPhysicalSizeInMm().value();
290+
if (hwcMode.width > 0 && hwcMode.height > 0 && size.width > 0 && size.height > 0) {
291+
static constexpr float kMmPerInch = 25.4f;
292+
return {hwcMode.width * kMmPerInch / size.width,
293+
hwcMode.height * kMmPerInch / size.height};
294+
}
295+
}
296+
}
297+
return {-1, -1};
298+
}
299+
300+
DisplayConfiguration::Dpi HWComposer::correctedDpiIfneeded(
301+
DisplayConfiguration::Dpi dpi, DisplayConfiguration::Dpi estimatedDpi) const {
302+
// hwc can be unreliable when it comes to dpi. A rough estimated dpi may yield better
303+
// results. For instance, libdrm and bad edid may result in a dpi of {350, 290} for a
304+
// 16:9 3840x2160 display, which would match a 4:3 aspect ratio.
305+
// The logic here checks if hwc was able to provide some dpi, and if so if the dpi
306+
// disparity between the axes is more reasonable than a rough estimate, otherwise use
307+
// the estimated dpi as a corrected value.
308+
if (estimatedDpi.x == -1 || estimatedDpi.x == -1) {
309+
return dpi;
310+
}
311+
if (dpi.x == -1 || dpi.y == -1) {
312+
return estimatedDpi;
313+
}
314+
if (std::min(dpi.x, dpi.y) != 0 && std::min(estimatedDpi.x, estimatedDpi.y) != 0 &&
315+
abs(dpi.x - dpi.y) / std::min(dpi.x, dpi.y) >
316+
abs(estimatedDpi.x - estimatedDpi.y) / std::min(estimatedDpi.x, estimatedDpi.y)) {
317+
return estimatedDpi;
318+
}
319+
return dpi;
320+
}
321+
279322
std::vector<HWComposer::HWCDisplayMode> HWComposer::getModesFromDisplayConfigurations(
280323
uint64_t hwcDisplayId, int32_t maxFrameIntervalNs) const {
281324
std::vector<hal::DisplayConfiguration> configs;
@@ -294,9 +337,16 @@ std::vector<HWComposer::HWCDisplayMode> HWComposer::getModesFromDisplayConfigura
294337
.configGroup = config.configGroup,
295338
.vrrConfig = config.vrrConfig};
296339

340+
const DisplayConfiguration::Dpi estimatedDPI =
341+
getEstimatedDotsPerInchFromSize(hwcDisplayId, hwcMode);
297342
if (config.dpi) {
298-
hwcMode.dpiX = config.dpi->x;
299-
hwcMode.dpiY = config.dpi->y;
343+
const DisplayConfiguration::Dpi dpi =
344+
correctedDpiIfneeded(config.dpi.value(), estimatedDPI);
345+
hwcMode.dpiX = dpi.x;
346+
hwcMode.dpiY = dpi.y;
347+
} else if (estimatedDPI.x != -1 && estimatedDPI.y != -1) {
348+
hwcMode.dpiX = estimatedDPI.x;
349+
hwcMode.dpiY = estimatedDPI.y;
300350
}
301351

302352
if (!mEnableVrrTimeout) {
@@ -328,12 +378,14 @@ std::vector<HWComposer::HWCDisplayMode> HWComposer::getModesFromLegacyDisplayCon
328378

329379
const int32_t dpiX = getAttribute(hwcDisplayId, configId, hal::Attribute::DPI_X);
330380
const int32_t dpiY = getAttribute(hwcDisplayId, configId, hal::Attribute::DPI_Y);
331-
if (dpiX != -1) {
332-
hwcMode.dpiX = static_cast<float>(dpiX) / 1000.f;
333-
}
334-
if (dpiY != -1) {
335-
hwcMode.dpiY = static_cast<float>(dpiY) / 1000.f;
336-
}
381+
const DisplayConfiguration::Dpi hwcDpi =
382+
DisplayConfiguration::Dpi{dpiX == -1 ? dpiY : dpiX / 1000.f,
383+
dpiY == -1 ? dpiY : dpiY / 1000.f};
384+
const DisplayConfiguration::Dpi estimatedDPI =
385+
getEstimatedDotsPerInchFromSize(hwcDisplayId, hwcMode);
386+
const DisplayConfiguration::Dpi dpi = correctedDpiIfneeded(hwcDpi, estimatedDPI);
387+
hwcMode.dpiX = dpi.x;
388+
hwcMode.dpiY = dpi.y;
337389

338390
modes.push_back(hwcMode);
339391
}
@@ -1072,6 +1124,8 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugConnect(
10721124
getDisplayIdentificationData(hwcDisplayId, &port, &data);
10731125
if (auto newInfo = parseDisplayIdentificationData(port, data)) {
10741126
info->deviceProductInfo = std::move(newInfo->deviceProductInfo);
1127+
info->preferredDetailedTimingDescriptor =
1128+
std::move(newInfo->preferredDetailedTimingDescriptor);
10751129
} else {
10761130
ALOGE("Failed to parse identification data for display %" PRIu64, hwcDisplayId);
10771131
}
@@ -1114,7 +1168,11 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugConnect(
11141168
}
11151169

11161170
if (!isConnected(info->id)) {
1117-
allocatePhysicalDisplay(hwcDisplayId, info->id);
1171+
std::optional<ui::Size> size = std::nullopt;
1172+
if (info->preferredDetailedTimingDescriptor) {
1173+
size = info->preferredDetailedTimingDescriptor->physicalSizeInMm;
1174+
}
1175+
allocatePhysicalDisplay(hwcDisplayId, info->id, size);
11181176
}
11191177
return info;
11201178
}

0 commit comments

Comments
 (0)