Merge "SF: Confine display serial number to EDID parser" into rvc-dev (3c04ff26) · Commits · e / os / platform_frameworks_native

services/surfaceflinger/DisplayHardware/DisplayIdentification.cpp

+17 −13

Original line number	Diff line number	Diff line
		@@ -104,9 +104,8 @@ uint16_t DisplayId::manufacturerId() const {
		return static_cast<uint16_t>(value >> 40);
		}

		DisplayId DisplayId::fromEdid(uint8_t port, uint16_t manufacturerId, uint32_t displayNameHash) {
		return {(static_cast<Type>(manufacturerId) << 40) \| (static_cast<Type>(displayNameHash) << 8) \|
		port};
		DisplayId DisplayId::fromEdid(uint8_t port, uint16_t manufacturerId, uint32_t modelHash) {
		return {(static_cast<Type>(manufacturerId) << 40) \| (static_cast<Type>(modelHash) << 8) \| port};
		}

		bool isEdid(const DisplayIdentificationData& data) {
		@@ -209,23 +208,30 @@ std::optional<Edid> parseEdid(const DisplayIdentificationData& edid) {
		view.remove_prefix(kDescriptorLength);
		}

		if (displayName.empty()) {
		std::string_view modelString = displayName;

		if (modelString.empty()) {
		ALOGW("Invalid EDID: falling back to serial number due to missing display name.");
		displayName = serialNumber;
		modelString = serialNumber;
		}
		if (displayName.empty()) {
		if (modelString.empty()) {
		ALOGW("Invalid EDID: falling back to ASCII text due to missing serial number.");
		displayName = asciiText;
		modelString = asciiText;
		}
		if (displayName.empty()) {
		if (modelString.empty()) {
		ALOGE("Invalid EDID: display name and fallback descriptors are missing.");
		return {};
		}

		// Hash model string instead of using product code or (integer) serial number, since the latter
		// have been observed to change on some displays with multiple inputs.
		const auto modelHash = static_cast<uint32_t>(std::hash<std::string_view>()(modelString));

		return Edid{.manufacturerId = manufacturerId,
		.productId = productId,
		.pnpId = *pnpId,
		.modelHash = modelHash,
		.displayName = displayName,
		.productId = productId,
		.manufactureWeek = manufactureWeek,
		.manufactureOrModelYear = manufactureOrModelYear};
		}
		@@ -253,10 +259,8 @@ std::optional<DisplayIdentificationInfo> parseDisplayIdentificationData(
		return {};
		}

		// Hash display name instead of using product code or serial number, since the latter have been
		// observed to change on some displays with multiple inputs.
		const auto hash = static_cast<uint32_t>(std::hash<std::string_view>()(edid->displayName));
		return DisplayIdentificationInfo{.id = DisplayId::fromEdid(port, edid->manufacturerId, hash),
		const auto displayId = DisplayId::fromEdid(port, edid->manufacturerId, edid->modelHash);
		return DisplayIdentificationInfo{.id = displayId,
		.name = std::string(edid->displayName),
		.deviceProductInfo = buildDeviceProductInfo(*edid)};
		}

services/surfaceflinger/DisplayHardware/DisplayIdentification.h

+2 −1

Original line number	Diff line number	Diff line
		@@ -34,7 +34,7 @@ struct DisplayId {

		uint16_t manufacturerId() const;

		static DisplayId fromEdid(uint8_t port, uint16_t manufacturerId, uint32_t displayNameHash);
		static DisplayId fromEdid(uint8_t port, uint16_t manufacturerId, uint32_t modelHash);
		};

		inline bool operator==(DisplayId lhs, DisplayId rhs) {
		@@ -61,6 +61,7 @@ struct Edid {
		uint16_t manufacturerId;
		uint16_t productId;
		PnpId pnpId;
		uint32_t modelHash;
		std::string_view displayName;
		uint8_t manufactureOrModelYear;
		uint8_t manufactureWeek;

services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp

+19 −4

Original line number	Diff line number	Diff line
		@@ -14,6 +14,9 @@
		* limitations under the License.
		*/

		#include <functional>
		#include <string_view>

		#include <gmock/gmock.h>
		#include <gtest/gtest.h>

		@@ -124,6 +127,10 @@ DisplayIdentificationData asDisplayIdentificationData(const unsigned char (&byte
		return DisplayIdentificationData(bytes, bytes + N - 1);
		}

		uint32_t hash(const char* str) {
		return static_cast<uint32_t>(std::hash<std::string_view>()(str));
		}

		} // namespace

		const DisplayIdentificationData& getInternalEdid() {
		@@ -173,7 +180,8 @@ TEST(DisplayIdentificationTest, parseEdid) {
		EXPECT_EQ(0x4ca3u, edid->manufacturerId);
		EXPECT_STREQ("SEC", edid->pnpId.data());
		// ASCII text should be used as fallback if display name and serial number are missing.
		EXPECT_EQ("121AT11-801", edid->displayName);
		EXPECT_EQ(hash("121AT11-801"), edid->modelHash);
		EXPECT_TRUE(edid->displayName.empty());
		EXPECT_EQ(12610, edid->productId);
		EXPECT_EQ(21, edid->manufactureOrModelYear);
		EXPECT_EQ(0, edid->manufactureWeek);
		@@ -182,6 +190,7 @@ TEST(DisplayIdentificationTest, parseEdid) {
		ASSERT_TRUE(edid);
		EXPECT_EQ(0x22f0u, edid->manufacturerId);
		EXPECT_STREQ("HWP", edid->pnpId.data());
		EXPECT_EQ(hash("HP ZR30w"), edid->modelHash);
		EXPECT_EQ("HP ZR30w", edid->displayName);
		EXPECT_EQ(10348, edid->productId);
		EXPECT_EQ(22, edid->manufactureOrModelYear);
		@@ -191,6 +200,7 @@ TEST(DisplayIdentificationTest, parseEdid) {
		ASSERT_TRUE(edid);
		EXPECT_EQ(0x4c2du, edid->manufacturerId);
		EXPECT_STREQ("SAM", edid->pnpId.data());
		EXPECT_EQ(hash("SAMSUNG"), edid->modelHash);
		EXPECT_EQ("SAMSUNG", edid->displayName);
		EXPECT_EQ(2302, edid->productId);
		EXPECT_EQ(21, edid->manufactureOrModelYear);
		@@ -200,6 +210,7 @@ TEST(DisplayIdentificationTest, parseEdid) {
		ASSERT_TRUE(edid);
		EXPECT_EQ(13481, edid->manufacturerId);
		EXPECT_STREQ("MEI", edid->pnpId.data());
		EXPECT_EQ(hash("Panasonic-TV"), edid->modelHash);
		EXPECT_EQ("Panasonic-TV", edid->displayName);
		EXPECT_EQ(41622, edid->productId);
		EXPECT_EQ(29, edid->manufactureOrModelYear);
		@@ -209,6 +220,7 @@ TEST(DisplayIdentificationTest, parseEdid) {
		ASSERT_TRUE(edid);
		EXPECT_EQ(8355, edid->manufacturerId);
		EXPECT_STREQ("HEC", edid->pnpId.data());
		EXPECT_EQ(hash("Hisense"), edid->modelHash);
		EXPECT_EQ("Hisense", edid->displayName);
		EXPECT_EQ(0, edid->productId);
		EXPECT_EQ(29, edid->manufactureOrModelYear);
		@@ -218,6 +230,7 @@ TEST(DisplayIdentificationTest, parseEdid) {
		ASSERT_TRUE(edid);
		EXPECT_EQ(3724, edid->manufacturerId);
		EXPECT_STREQ("CTL", edid->pnpId.data());
		EXPECT_EQ(hash("LP2361"), edid->modelHash);
		EXPECT_EQ("LP2361", edid->displayName);
		EXPECT_EQ(9373, edid->productId);
		EXPECT_EQ(23, edid->manufactureOrModelYear);
		@@ -234,13 +247,15 @@ TEST(DisplayIdentificationTest, parseInvalidEdid) {
		auto edid = parseEdid(data);
		ASSERT_TRUE(edid);
		// Serial number should be used as fallback if display name is invalid.
		EXPECT_EQ("CN4202137Q", edid->displayName);
		const auto modelHash = hash("CN4202137Q");
		EXPECT_EQ(modelHash, edid->modelHash);
		EXPECT_TRUE(edid->displayName.empty());

		// Parsing should succeed even if EDID is truncated.
		data.pop_back();
		edid = parseEdid(data);
		ASSERT_TRUE(edid);
		EXPECT_EQ("CN4202137Q", edid->displayName);
		EXPECT_EQ(modelHash, edid->modelHash);
		}

		TEST(DisplayIdentificationTest, getPnpId) {
		@@ -278,7 +293,7 @@ TEST(DisplayIdentificationTest, deviceProductInfo) {
		ASSERT_TRUE(displayIdInfo);
		ASSERT_TRUE(displayIdInfo->deviceProductInfo);
		const auto& info = *displayIdInfo->deviceProductInfo;
		EXPECT_STREQ("121AT11-801", info.name.data());
		EXPECT_STREQ("", info.name.data());
		EXPECT_STREQ("SEC", info.manufacturerPnpId.data());
		EXPECT_STREQ("12610", info.productId.data());
		ASSERT_TRUE(std::holds_alternative<ManufactureYear>(info.manufactureOrModelDate));