[PATCH 0/2] edid-decode: corrections

[PATCH 0/2] edid-decode: corrections

[joevt]

These two changes are in response to the last two commits:
- report length for unknown data blocks
- fix reporting PNP as a proper PNP

joevt (2):
  edid-decode: fix pnp and oui
  edid-decode: output correct unknown block length

 edid-decode.cpp           |  4 +++-
 oui.h                     |  2 +-
 parse-displayid-block.cpp | 10 +++++-----
 3 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.24.3 (Apple Git-128)

[PATCH 1/2] edid-decode: fix pnp and oui

[joevt]

Changes:

- In oui.h, PNP-IDs are 32-bit numbers representing 3 ASCII characters followed by a null character. The most significant byte contains the first character, for example: hex identifier 0x41505000 ('APP\0'). This way, it is not possible to match an OUI (24-bit number, bits 31-24 are zero) when we're trying to match a PNP and vice versa. The fact that an OUI might look like a PNP is a coincidence - only one of the interpretations is correct. oui.h currently only includes unambiguous OUIs/PNPs. It is extremely unlikely that an EDID has an OUI/PNP that matches one of those when a different vendor is meant because the other three possible interpretations don't exist in the list of known OUIs and PNPs.

- For DisplayID blocks, OUIs and PNPs are assumed to be big-endian to match the DisplayID specs 1.3 and 2.0. This change causes the "Endian-ness (le) of OUI is different than expected (be)." message to appear for Apple VSDB because Apple includes their OUI as little endian even though the same EDID may have a VESA VSDB in the correct big endian format. Maybe this message shouldn't happen for DisplayID tags from the 1.3 spec since they are expected to use PNP and we already output the message "Expected PNP ID but found OUI." We don't check reverse byte order interpretation of PNPs so the bigendian parameter could be ignored in the 1.3 spec cases.

- In data_block_oui, matched_ascii can only be true if we find the name using a PNP. Currently in oui.h, we have no conflicts between the PNP list and the OUI list so an OUI match and a PNP match cannot both occur and therefore it doesn't matter that we try to match OUI first if we're actually expecting to match a PNP.

Notes:

- data_block_oui tries to match 3 variations (3 interpretations of the bytes): OUI in expected order, OUI in unexpected order, and PNP (big endian).
- Expected order is big-endian for DisplayID blocks and little-endian for CTA blocks.
- I don't think any EDIDs use PNP, no matter what DisplayID version or tag is used. The PNP code exists only because the DisplayID v1.3 spec says it uses PNPs.
- There are 15 known OUIs that could be interpreted as a PNP (none of those are included in oui.h). Of those 15 only one of them, or 6 if you reverse the bytes, also exists as a known PNP (but none of those are included in oui.h). None of the four interpretations of those 6 have the same vendor name.
    DYC=Dycam Inc CYD=Cyclades Corporation 445943=zte corporation 435944=
    TNE= ENT=Enterprise Comm. & Computing Inc 544E45=Private 454E54=
    PFJ= JFP= 50464A=HUAWEI TECHNOLOGIES CO.,LTD 4A4650=
    PCH= HCP=Hitachi Computer Products Inc 504348=ThingsMatrix Inc. 484350=
    XEL= LEX=Lexical Ltd 58454C=Ericsson AB 4C4558=
    TGA= AGT=Agilent Technologies 544741=XCHENG HOLDING 414754=
- There are 169 known OUIs that also have a reverse match (but none are in oui.h). 25 pairs of those do not match the same oui (their bytes are not the same in reverse). The two vendor names of a pair are never the same.
    000010=SYTEK INC. 100000=Private
    0000AA=XEROX CORPORATION AA0000=DIGITAL EQUIPMENT CORPORATION
    0002BC=LVL 7 Systems, Inc. BC0200=Stewart Audio
    ...

Signed-off-by: Joe van Tunen <joevt@shaw.ca>
---
 edid-decode.cpp           | 4 +++-
 oui.h                     | 2 +-
 parse-displayid-block.cpp | 8 ++++----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/edid-decode.cpp b/edid-decode.cpp
index 1fc7fce..6df328b 100644
--- a/edid-decode.cpp
+++ b/edid-decode.cpp
@@ -706,9 +706,11 @@ void edid_state::data_block_oui(std::string block_name, const unsigned char *x,
 			} else if (do_ascii && valid_ascii) {
 				unsigned asciioui = (x[0] << 24) + (x[1] << 16) + (x[2] << 8);
 				ouiname = oui_name(asciioui, ouinum);
+				if (ouiname) {
+					matched_ascii = true;
+				}
 			}
 		}
-		matched_ascii = do_ascii && valid_ascii && ouiname != NULL;
 	}
 
 	std::string name;
diff --git a/oui.h b/oui.h
index 77342ac..d4fea3c 100644
--- a/oui.h
+++ b/oui.h
@@ -15,6 +15,6 @@ oneoui(0xca125c, Microsoft,    "Microsoft"   )
 oneoui(0x3a0292, VESA,         "VESA"        )
 
 // https://uefi.org/pnp_id_list
-oneoui(0x415050, asciiApple,   "Apple"       )  // 'APP\0'
+oneoui(0x41505000, asciiApple,   "Apple"     )  // 'APP\0'
 
 #undef oneoui
diff --git a/parse-displayid-block.cpp b/parse-displayid-block.cpp
index d527bf2..b318766 100644
--- a/parse-displayid-block.cpp
+++ b/parse-displayid-block.cpp
@@ -1678,7 +1678,7 @@ unsigned edid_state::displayid_block(const unsigned version, const unsigned char
 	// DisplayID 2.0
 	case 0x20:
 		data_block_oui("Product Identification Data Block (" + utohex(tag) + ")",
-			       x + 3, len, &ouinum, false, false, false);
+			       x + 3, len, &ouinum, false, false, true);
 		dooutputname = false;
 		hasoui = true;
 		break;
@@ -1694,16 +1694,16 @@ unsigned edid_state::displayid_block(const unsigned version, const unsigned char
 	case 0x2b: data_block = "Adaptive Sync Data Block"; break;
 	case 0x32: data_block = "Video Timing Modes Type 10 - Formula-based Timings Data Block"; break;
 	// 0x2a .. 0x7d RESERVED for Additional VESA-defined Data Blocks
-	case 0x7e:
+	case 0x7e: // DisplayID 2.0
 		data_block_oui("Vendor-Specific Data Block (" + utohex(tag) + ")",
 			       x + 3, len, &ouinum, false, false, true);
 		dooutputname = false;
 		hasoui = true;
 		tag |= ouinum;
 		break;
-	case 0x7f:
+	case 0x7f: // DisplayID 1.3
 		data_block_oui("Vendor-Specific Data Block (" + utohex(tag) + ")",
-			       x + 3, len, &ouinum, false, true, false);
+			       x + 3, len, &ouinum, false, true, true);
 		dooutputname = false;
 		hasoui = true;
 		tag |= ouinum;
-- 
2.24.3 (Apple Git-128)

[PATCH 2/2] edid-decode: output correct unknown block length

[joevt]

The correct length of a DisplayID block is len. It is the number of bytes in the payload bytes (doesn't include the tag or length or revision). It may include the OUI but we don't output the OUI in hex_block, and an unknown block can't have an OUI unless there's a new DisplayID spec with a new type of block that we don't know about.

The length parameter passed to displayid_block is the number of bytes that remain in a DisplayID extension block (maybe it needs to be renamed?). We pass it to displayid_block so it can handle length related errors. displayid_block handles all interpretation of the data block. The only byte we know that can be interpreted on entry is the tag byte because it is the first byte and length is at least one (according to the for loop in parse_displayid_block).
The payload length byte occurs after the tag byte - it might exist beyond the limit of length. If that's the case, we can at least report the tag block type with the error, but we choose not to. Instead, we output the rest of the bytes (including the tag) as hex because it probably isn't a real block.

Signed-off-by: Joe van Tunen <joevt@shaw.ca>
---
 parse-displayid-block.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-displayid-block.cpp b/parse-displayid-block.cpp
index b318766..c8cea53 100644
--- a/parse-displayid-block.cpp
+++ b/parse-displayid-block.cpp
@@ -1711,7 +1711,7 @@ unsigned edid_state::displayid_block(const unsigned version, const unsigned char
 	// 0x80 RESERVED
 	case 0x81: data_block = "CTA-861 DisplayID Data Block"; break;
 	// 0x82 .. 0xff RESERVED
-	default:   data_block = "Unknown DisplayID Data Block (" + utohex(tag) + ", length " + std::to_string(length) + ")"; break;
+	default:   data_block = "Unknown DisplayID Data Block (" + utohex(tag) + ", length " + std::to_string(len) + ")"; break;
 	}
 
 	if (length < 3) {
-- 
2.24.3 (Apple Git-128)