Add edid_block_check() that only checks the EDID block validity, without
any actions. Turns out it's simple and crystal clear.

Rewrite drm_edid_block_valid() around it, keeping all the functionality
fairly closely the same, warts and all. Turns out it's incredibly
complicated for a function you'd expect to be simple, with all the
fixing and printing and special casing. (Maybe we'll want to simplify it
in the future.)

Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nik...@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 150 ++++++++++++++++++++++---------------
 1 file changed, 88 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 481643751d10..04eb6949c9c8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1668,10 +1668,55 @@ bool drm_edid_are_equal(const struct edid *edid1, const 
struct edid *edid2)
 }
 EXPORT_SYMBOL(drm_edid_are_equal);
 
+enum edid_block_status {
+       EDID_BLOCK_OK = 0,
+       EDID_BLOCK_NULL,
+       EDID_BLOCK_HEADER_CORRUPT,
+       EDID_BLOCK_HEADER_REPAIR,
+       EDID_BLOCK_HEADER_FIXED,
+       EDID_BLOCK_CHECKSUM,
+       EDID_BLOCK_VERSION,
+};
+
+static enum edid_block_status edid_block_check(const void *_block, bool base)
+{
+       const struct edid *block = _block;
+
+       if (!block)
+               return EDID_BLOCK_NULL;
+
+       if (base) {
+               int score = drm_edid_header_is_valid(block);
+
+               if (score < clamp(edid_fixup, 6, 8))
+                       return EDID_BLOCK_HEADER_CORRUPT;
+
+               if (score < 8)
+                       return EDID_BLOCK_HEADER_REPAIR;
+       }
+
+       if (edid_block_compute_checksum(block) != 
edid_block_get_checksum(block))
+               return EDID_BLOCK_CHECKSUM;
+
+       if (base) {
+               if (block->version != 1)
+                       return EDID_BLOCK_VERSION;
+       }
+
+       return EDID_BLOCK_OK;
+}
+
+static bool edid_block_status_valid(enum edid_block_status status, int tag)
+{
+       return status == EDID_BLOCK_OK ||
+               status == EDID_BLOCK_HEADER_FIXED ||
+               (status == EDID_BLOCK_CHECKSUM && tag == CEA_EXT);
+}
+
 /**
  * drm_edid_block_valid - Sanity check the EDID block (base or extension)
  * @raw_edid: pointer to raw EDID block
- * @block: type of block to validate (0 for base, extension otherwise)
+ * @block_num: type of block to validate (0 for base, extension otherwise)
  * @print_bad_edid: if true, dump bad EDID blocks to the console
  * @edid_corrupt: if true, the header or checksum is invalid
  *
@@ -1680,88 +1725,69 @@ EXPORT_SYMBOL(drm_edid_are_equal);
  *
  * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
                          bool *edid_corrupt)
 {
-       u8 csum;
-       struct edid *edid = (struct edid *)raw_edid;
+       struct edid *block = (struct edid *)_block;
+       enum edid_block_status status;
+       bool base = block_num == 0;
+       bool valid;
 
-       if (WARN_ON(!raw_edid))
+       if (WARN_ON(!block))
                return false;
 
-       if (edid_fixup > 8 || edid_fixup < 0)
-               edid_fixup = 6;
-
-       if (block == 0) {
-               int score = drm_edid_header_is_valid(raw_edid);
+       status = edid_block_check(block, base);
+       if (status == EDID_BLOCK_HEADER_REPAIR) {
+               DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
+               edid_header_fix(block);
 
-               if (score == 8) {
-                       if (edid_corrupt)
-                               *edid_corrupt = false;
-               } else if (score >= edid_fixup) {
-                       /* Displayport Link CTS Core 1.2 rev1.1 test 4.2.2.6
-                        * The corrupt flag needs to be set here otherwise, the
-                        * fix-up code here will correct the problem, the
-                        * checksum is correct and the test fails
-                        */
-                       if (edid_corrupt)
-                               *edid_corrupt = true;
-                       DRM_DEBUG("Fixing EDID header, your hardware may be 
failing\n");
-                       edid_header_fix(raw_edid);
-               } else {
-                       if (edid_corrupt)
-                               *edid_corrupt = true;
-                       goto bad;
-               }
+               /* Retry with fixed header, update status if that worked. */
+               status = edid_block_check(block, base);
+               if (status == EDID_BLOCK_OK)
+                       status = EDID_BLOCK_HEADER_FIXED;
        }
 
-       csum = edid_block_compute_checksum(raw_edid);
-       if (csum != edid_block_get_checksum(raw_edid)) {
-               if (edid_corrupt)
+       if (edid_corrupt) {
+               /*
+                * Unknown major version isn't corrupt but we can't use it. Only
+                * the base block can reset edid_corrupt to false.
+                */
+               if (base && (status == EDID_BLOCK_OK || status == 
EDID_BLOCK_VERSION))
+                       *edid_corrupt = false;
+               else if (status != EDID_BLOCK_OK)
                        *edid_corrupt = true;
-
-               /* allow CEA to slide through, switches mangle this */
-               if (edid_block_tag(raw_edid) == CEA_EXT) {
-                       DRM_DEBUG("EDID checksum is invalid, remainder is 
%d\n", csum);
-                       DRM_DEBUG("Assuming a KVM switch modified the CEA block 
but left the original checksum\n");
-               } else {
-                       if (print_bad_edid)
-                               DRM_NOTE("EDID checksum is invalid, remainder 
is %d\n", csum);
-
-                       goto bad;
-               }
        }
 
-       /* per-block-type checks */
-       switch (edid_block_tag(raw_edid)) {
-       case 0: /* base */
-               if (edid->version != 1) {
-                       DRM_NOTE("EDID has major version %d, instead of 1\n", 
edid->version);
-                       goto bad;
+       /* Determine whether we can use this block with this status. */
+       valid = edid_block_status_valid(status, edid_block_tag(block));
+
+       /* Some fairly random status printouts. */
+       if (status == EDID_BLOCK_CHECKSUM) {
+               if (valid) {
+                       DRM_DEBUG("EDID block checksum is invalid, remainder is 
%d\n",
+                                 edid_block_compute_checksum(block));
+                       DRM_DEBUG("Assuming a KVM switch modified the block but 
left the original checksum\n");
+               } else if (print_bad_edid) {
+                       DRM_NOTE("EDID block checksum is invalid, remainder is 
%d\n",
+                                edid_block_compute_checksum(block));
                }
-
-               if (edid->revision > 4)
-                       DRM_DEBUG("EDID minor > 4, assuming backward 
compatibility\n");
-               break;
-
-       default:
-               break;
+       } else if (status == EDID_BLOCK_VERSION) {
+               DRM_NOTE("EDID has major version %d, instead of 1\n",
+                        block->version);
        }
 
-       return true;
-
-bad:
-       if (print_bad_edid) {
-               if (edid_is_zero(raw_edid, EDID_LENGTH)) {
+       if (!valid && print_bad_edid) {
+               if (edid_is_zero(block, EDID_LENGTH)) {
                        pr_notice("EDID block is all zeroes\n");
                } else {
                        pr_notice("Raw EDID:\n");
                        print_hex_dump(KERN_NOTICE,
                                       " \t", DUMP_PREFIX_NONE, 16, 1,
-                                      raw_edid, EDID_LENGTH, false);
+                                      block, EDID_LENGTH, false);
                }
        }
-       return false;
+
+       return valid;
 }
 EXPORT_SYMBOL(drm_edid_block_valid);
 
-- 
2.30.2

Reply via email to