drm_hwcomposer: uses edid DTD to get better estimate of dpi
Edid are not the most reliable when it comes to panel size, however it
seems like preferred DTD are slightly more reliable. This change moves
from the drm parse edid display size to the first preferred DTD to try
improve dpi accuracy.
If no preferred DTD is available we still fallback to the display size.
Change-Id: Ibcc95da9f38ba74e9b95b9d68587b38efbed8de2
signed-off-by: Lucas Berthou <berlu@google.com>
diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 42ebd63..9db384b 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -316,6 +316,19 @@
return changed_layers;
}
+auto HwcDisplay::GetDisplayBoundsMm() -> std::pair<int32_t, int32_t> {
+
+ const auto bounds = GetEdid()->GetBoundsMm();
+ if (bounds.first > 0 || bounds.second > 0) {
+ return bounds;
+ }
+
+ ALOGE("Failed to get display bounds for d=%d\n", int(handle_));
+ // mm_width and mm_height are unreliable. so only provide mm_width to avoid
+ // wrong dpi computations or other use of the values.
+ return {configs_.mm_width, -1};
+}
+
auto HwcDisplay::AcceptValidatedComposition() -> void {
for (std::pair<const hwc2_layer_t, HwcLayer> &l : layers_) {
l.second.AcceptTypeChange();
@@ -549,15 +562,25 @@
*value = hwc_config.mode.GetVSyncPeriodNs();
break;
case HWC2::Attribute::DpiY:
- // ideally this should be vdisplay/mm_heigth, however mm_height
- // comes from edid parsing and is highly unreliable. Viewing the
- // rarity of anisotropic displays, falling back to a single value
- // for dpi yield more correct output.
+ *value = GetEdid()->GetDpiY();
+ if (*value < 0) {
+ // default to raw mode DpiX for both x and y when no good value
+ // can be provided from edid.
+ *value = mm_width ? int(hwc_config.mode.GetRawMode().hdisplay *
+ kUmPerInch / mm_width)
+ : -1;
+ }
+ break;
case HWC2::Attribute::DpiX:
// Dots per 1000 inches
- *value = mm_width ? int(hwc_config.mode.GetRawMode().hdisplay *
- kUmPerInch / mm_width)
- : -1;
+ *value = GetEdid()->GetDpiX();
+ if (*value < 0) {
+ // default to raw mode DpiX for both x and y when no good value
+ // can be provided from edid.
+ *value = mm_width ? int(hwc_config.mode.GetRawMode().hdisplay *
+ kUmPerInch / mm_width)
+ : -1;
+ }
break;
#if __ANDROID_API__ > 29
case HWC2::Attribute::ConfigGroup:
diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index 7391785..c3a5f1a 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -96,6 +96,8 @@
// Get the HwcDisplayConfig, or nullptor if none.
auto GetConfig(hwc2_config_t config_id) const -> const HwcDisplayConfig *;
+ auto GetDisplayBoundsMm() -> std::pair<int32_t, int32_t>;
+
// To be called after SetDisplayProperties. Returns an empty vector if the
// requested layers have been validated, otherwise the vector describes
// the requested composition type changes.
diff --git a/hwc3/ComposerClient.cpp b/hwc3/ComposerClient.cpp
index 50d8ce6..7f3bc00 100644
--- a/hwc3/ComposerClient.cpp
+++ b/hwc3/ComposerClient.cpp
@@ -271,7 +271,7 @@
#endif
DisplayConfiguration HwcDisplayConfigToAidlConfiguration(
- const HwcDisplayConfigs& configs, const HwcDisplayConfig& config) {
+ int32_t width, int32_t height, const HwcDisplayConfig& config) {
DisplayConfiguration aidl_configuration =
{.configId = static_cast<int32_t>(config.id),
.width = config.mode.GetRawMode().hdisplay,
@@ -279,15 +279,14 @@
.configGroup = static_cast<int32_t>(config.group_id),
.vsyncPeriod = config.mode.GetVSyncPeriodNs()};
- if (configs.mm_width != 0) {
- // ideally this should be vdisplay/mm_heigth, however mm_height
- // comes from edid parsing and is highly unreliable. Viewing the
- // rarity of anisotropic displays, falling back to a single value
- // for dpi yield more correct output.
+ if (width > 0) {
static const float kMmPerInch = 25.4;
- float dpi = float(config.mode.GetRawMode().hdisplay) * kMmPerInch /
- float(configs.mm_width);
- aidl_configuration.dpi = {.x = dpi, .y = dpi};
+ float dpi_x = float(config.mode.GetRawMode().hdisplay) * kMmPerInch /
+ float(width);
+ float dpi_y = height <= 0 ? dpi_x :
+ float(config.mode.GetRawMode().vdisplay) * kMmPerInch /
+ float(height);
+ aidl_configuration.dpi = {.x = dpi_x, .y = dpi_y};
}
// TODO: Populate vrrConfig.
return aidl_configuration;
@@ -838,9 +837,11 @@
return ToBinderStatus(hwc3::Error::kBadConfig);
}
- DisplayConfiguration
- aidl_configuration = HwcDisplayConfigToAidlConfiguration(configs,
- config->second);
+ const auto bounds = display->GetDisplayBoundsMm();
+ DisplayConfiguration aidl_configuration =
+ HwcDisplayConfigToAidlConfiguration(/*width =*/ bounds.first,
+ /*height =*/bounds.second,
+ config->second);
// Legacy API for querying DPI uses units of dots per 1000 inches.
static const int kLegacyDpiUnit = 1000;
switch (attribute) {
@@ -1419,9 +1420,12 @@
}
const HwcDisplayConfigs& configs = display->GetDisplayConfigs();
+ const auto bounds = display->GetDisplayBoundsMm();
for (const auto& [id, config] : configs.hwc_configs) {
configurations->push_back(
- HwcDisplayConfigToAidlConfiguration(configs, config));
+ HwcDisplayConfigToAidlConfiguration(/*width =*/ bounds.first,
+ /*height =*/ bounds.second,
+ config));
}
return ndk::ScopedAStatus::ok();
}
diff --git a/utils/EdidWrapper.h b/utils/EdidWrapper.h
index 30124d7..137e8be 100644
--- a/utils/EdidWrapper.h
+++ b/utils/EdidWrapper.h
@@ -18,6 +18,7 @@
#if HAS_LIBDISPLAY_INFO
extern "C" {
+#include <libdisplay-info/edid.h>
#include <libdisplay-info/info.h>
}
#endif
@@ -48,6 +49,16 @@
virtual void GetColorModes(std::vector<Colormode> &color_modes) {
color_modes.clear();
};
+ virtual int GetDpiX() {
+ return -1;
+ }
+ virtual int GetDpiY() {
+ return -1;
+ }
+
+ virtual auto GetBoundsMm() -> std::pair<int32_t, int32_t> {
+ return {-1, -1};
+ }
};
#if HAS_LIBDISPLAY_INFO
@@ -70,10 +81,17 @@
void GetColorModes(std::vector<Colormode> &color_modes) override;
+ auto GetDpiX() -> int override;
+ auto GetDpiY() -> int override;
+
+ auto GetBoundsMm() -> std::pair<int32_t, int32_t> override;
+
private:
LibdisplayEdidWrapper(di_info *info) : info_(std::move(info)) {
}
+ std::pair<int32_t, int32_t> GetDpi();
+
di_info *info_{};
};
#endif
diff --git a/utils/LibdisplayEdidWrapper.cpp b/utils/LibdisplayEdidWrapper.cpp
index d2d2a1c..e35b461 100644
--- a/utils/LibdisplayEdidWrapper.cpp
+++ b/utils/LibdisplayEdidWrapper.cpp
@@ -95,5 +95,51 @@
}
}
+auto LibdisplayEdidWrapper::GetDpiX() -> int {
+ return GetDpi().first;
+}
+
+auto LibdisplayEdidWrapper::GetDpiY() -> int {
+ return GetDpi().second;
+}
+
+auto LibdisplayEdidWrapper::GetBoundsMm() -> std::pair<int32_t, int32_t> {
+ const auto edid = di_info_get_edid(info_);
+ const auto detailed_timing_defs = di_edid_get_detailed_timing_defs(edid);
+ const auto dtd = detailed_timing_defs[0];
+ if (dtd == nullptr || dtd->horiz_image_mm == 0 || dtd->vert_image_mm == 0) {
+ // try to fallback on display size if no dtd.
+ // However since edid screen size are vastly unreliable only provide a valid
+ // width to avoid invalid dpi computation.
+ const auto screen_size = di_edid_get_screen_size(edid);
+ return {screen_size->width_cm * 10, -1};
+ }
+
+ return {dtd->horiz_image_mm, dtd->vert_image_mm};
+}
+
+auto LibdisplayEdidWrapper::GetDpi() -> std::pair<int32_t, int32_t> {
+ static const int32_t kUmPerInch = 25400;
+ const auto edid = di_info_get_edid(info_);
+ const auto detailed_timing_defs = di_edid_get_detailed_timing_defs(edid);
+ const auto dtd = detailed_timing_defs[0];
+ if (dtd == nullptr || dtd->horiz_image_mm == 0 || dtd->vert_image_mm == 0) {
+ // try to fallback on display size if no dtd.
+ const auto screen_size = di_edid_get_screen_size(edid);
+ const auto standard_timings = di_edid_get_standard_timings(edid);
+ if (screen_size->width_cm <= 0 || standard_timings == nullptr) {
+ return {-1, -1};
+ }
+
+ // display size is more unreliable so use only horizontal dpi.
+ int32_t horiz_video = standard_timings[0]->horiz_video;
+ int32_t dpi = horiz_video * kUmPerInch / (screen_size->width_cm * 10);
+ return {dpi, dpi};
+ }
+
+ return {dtd->horiz_video * kUmPerInch / dtd->horiz_image_mm,
+ dtd->vert_video * kUmPerInch / dtd->vert_image_mm};
+}
+
} // namespace android
#endif