Make EDID handling more robust

Detail:
  c/edidsupport:
  - Make readedid() check that the EDID has a valid header. Previously only the checksum was checked, which is wholly insufficient (e.g. an EDID block full of null bytes would be considered valid). loadedid() also updated for consistency, although with the current code the header will have already been detected by loadmodefile().
  - Add some extra checks to readedidblock() to cope with more situations in which the GraphicsV call could fail. Fix confusing comment/code ordering.
  h/errors, Resources/Germany/Messages, Resources/UK/Messages - Add new "Invalid EDID block" error, for use when no valid header is found
Admin:
  Tested on Raspberry Pi 3
  Booting an EDID-enabled Pi without a monitor (and with the broken BCMVideo 0.47) no longer results in an (unintelligible) error box on entry to the desktop + system stuck in mode 0
  However, note that other issues in the boot sequence mean that booting with EDID enabled and no monitor still isn't 100% working (PreDesk Repeat will terminate early, potentially skipping some directories)


Version 0.67. Tagged as 'ScrModes-0_67'

Resources/Germany/Messages

@@ -28,4 +28,5 @@ E25:EDID checksum is incorrect (block %0 of %1 failed) - cannot read monitor set
 E26:Unable to read EDID - does this hardware support it?
 E27:EDID channel communications error.
 E28:Bad ScreenModes_EnumerateAudioFormats flags
+E29:Invalid EDID block
 NoName:Unidentified

Resources/UK/Messages

@@ -28,4 +28,5 @@ E25:EDID checksum is incorrect (block %0 of %1 failed) - cannot read monitor set
 E26:Unable to read EDID - does this hardware support it?
 E27:EDID channel communications error
 E28:Bad ScreenModes_EnumerateAudioFormats flags
+E29:Invalid EDID block
 NoName:Unidentified

VersionNum

-/* (0.66)
+/* (0.67)
  *
  * This file is automatically maintained by srccommit, do not edit manually.
  * Last processed by srccommit version: 1.1.
  *
  */
-#define Module_MajorVersion_CMHG        0.66
+#define Module_MajorVersion_CMHG        0.67
 #define Module_MinorVersion_CMHG        
-#define Module_Date_CMHG                17 Jun 2017
+#define Module_Date_CMHG                11 Jul 2017
 
-#define Module_MajorVersion             "0.66"
-#define Module_Version                  66
+#define Module_MajorVersion             "0.67"
+#define Module_Version                  67
 #define Module_MinorVersion             ""
-#define Module_Date                     "17 Jun 2017"
+#define Module_Date                     "11 Jul 2017"
 
-#define Module_ApplicationDate          "17-Jun-17"
+#define Module_ApplicationDate          "11-Jul-17"
 
 #define Module_ComponentName            "ScrModes"
 #define Module_ComponentPath            "castle/RiscOS/Sources/Video/UserI/ScrModes"
 
-#define Module_FullVersion              "0.66"
-#define Module_HelpVersion              "0.66 (17 Jun 2017)"
-#define Module_LibraryVersionInfo       "0:66"
+#define Module_FullVersion              "0.67"
+#define Module_HelpVersion              "0.67 (11 Jul 2017)"
+#define Module_LibraryVersionInfo       "0:67"

c/edidsupport

@@ -183,6 +183,40 @@ static _kernel_oserror *check_edid_checksum(EDIDBlockRef edidblock, bool checkal
     return res;
 }
 
+static _kernel_oserror *check_edid_header(EDIDBlockRef edidblock)
+{
+    /* Look for the EDID header 0x00ffffffffffff00 */
+    int is_edid = 1;
+    for (int i = 0; i<8;i++)
+    {
+        uint8_t c = edidblock->header[i];
+        if (((i == 0) || (i == 7)) && (c != 0x00))
+        {
+            is_edid = 0;
+        }
+        if (((i > 0) && (i < 7)) && (c != 0xff))
+        {
+            is_edid = 0;
+        }
+    }
+    if (!is_edid)
+    {
+        return error(ERR_INVALIDEDID, 0, 0, 0);
+    }
+    return NULL;
+}
+
+static _kernel_oserror *check_edid_valid(EDIDBlockRef edidblock, bool checkall)
+{
+    /* Perform sanity checks on the EDID prior to the full parsing */
+    _kernel_oserror *res = check_edid_header(edidblock);
+    if (!res)
+    {
+        res = check_edid_checksum(edidblock, checkall);
+    }
+    return res;
+}
+
 /* Takes a block of text from an 18-byte EDID data block
  * And fills a supplied character buffer with a usable 0-terminated string
  * Buffer must be at least 14 bytes in length.
@@ -1707,23 +1741,38 @@ static _kernel_oserror *readedidblock(int displaynum, EDIDBlockRef edidblock, in
     _kernel_oserror *res;
     int iic_code = (0xa1 << 16) | (0x80*offset << 0);
     int op_code = (displaynum << 24) | (0 << 16) | (GraphicsV_IICOp << 0);
+    int remain;
 
-    res = _swix(OS_CallAVector, _INR(0,2) | _IN(4) | _IN(9) | _OUT(0) | _OUT(4), iic_code, edidblock+offset, 0x80*count, op_code, GraphicsV, &iic_code, &op_code);
+    res = _swix(OS_CallAVector, _INR(0,2) | _IN(4) | _IN(9) | _OUT(0) | _OUT(2) | _OUT(4), iic_code, edidblock+offset, 0x80*count, op_code, GraphicsV, &iic_code, &remain, &op_code);
+    /* OS_CallAVector / GraphicsV shouldn't return a RISC OS error,
+     * but deal with it just in case
+     */
+    if (res)
+    {
+        return res;
+    }
 
     /* If GraphicsV 14 was not claimed, R4 (op_code) should return
      * unchanged in which case we need to alert the user that the
      * hardware doesn't like EDID :-(
      */
-    if (iic_code != 0)
-    {
-        res = error(ERR_IICOPFAIL, 0, 0, 0);
-    }
-
-    /* An 'EDID read not supported' error will trump an IIC failure. */
     if (op_code != 0)
     {
         res = error(ERR_CANTREADEDID, 0, 0, 0);
     }
+    else
+    {
+        /* Call was claimed, check for any IIC-related errors.
+         * Most of the time a non-zero iic_code will be the indicator that
+         * something has gone wrong, but for safety we also check that there
+         * are no remaining bytes left to transfer (could be a buggy driver)
+         */
+        if ((iic_code != 0) || (remain != 0))
+        {
+            res = error(ERR_IICOPFAIL, 0, 0, 0);
+        }
+    }
+
     return res;
 }
 
@@ -1802,7 +1851,7 @@ _kernel_oserror *loadedid(const char *file)
         res = _swix(OS_File, _INR(0,3), OSFile_Load, file, edidblock, 0);
         if (!res)
         {
-            res = check_edid_checksum(edidblock, true);
+            res = check_edid_valid(edidblock, true);
             if (!res)
             {
                 /* All block checksum OK, parse */
@@ -1834,7 +1883,7 @@ _kernel_oserror *readedid(int displaynum, bool parse)
     res = readedidblock(displaynum, edidblock, 0, 1);
     if (!res)
     {
-        res = check_edid_checksum(edidblock, false);
+        res = check_edid_valid(edidblock, false);
     }
 
     if (!res)
@@ -1858,7 +1907,7 @@ _kernel_oserror *readedid(int displaynum, bool parse)
                 res = readedidblock(displaynum, edidblock, 1, edidblock->extension_block_count);
                 if (!res)
                 {
-                    res = check_edid_checksum(edidblock, true);
+                    res = check_edid_valid(edidblock, true);
                 }
             }
         }

h/errors

@@ -50,6 +50,7 @@
 #define ERR_CANTREADEDID  26    /* Cannot read EDID */
 #define ERR_IICOPFAIL     27    /* IICStatus reported an error reading EDID */
 #define ERR_BADENUMAUDIO  28    /* Bad ScreenModes_EnumerateAudioFormats flags */
+#define ERR_INVALIDEDID   29    /* Invalid EDID block */
 
 /* EOF errors.h */