diff mbox series

[07/12] drm/edid: split drm_edid_block_valid() to check and act parts

Message ID 296443a99ce907b11d08ddc88407aa35d9bdc5a3.1648578814.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/edid: cleanup and refactoring around validity checks | expand

Commit Message

Jani Nikula March 29, 2022, 6:42 p.m. UTC 
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.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 150 ++++++++++++++++++++++---------------
 1 file changed, 88 insertions(+), 62 deletions(-)

Comments

Ville Syrjala March 31, 2022, 2:54 p.m. UTC | #1 
On Tue, Mar 29, 2022 at 09:42:14PM +0300, Jani Nikula wrote:
> 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.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@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)

s/base/is_base_block/ or something?

> +{
> +	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))

That should clamp to 0-8?

Might be nicer to just define a .set() op for the modparam
and check it there, but that's clearly material for a separate patch.

> +			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");

This debug message seems to disappear. Intentional?

> -		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
Jani Nikula March 31, 2022, 3:54 p.m. UTC | #2 
On Thu, 31 Mar 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Mar 29, 2022 at 09:42:14PM +0300, Jani Nikula wrote:
>> 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.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@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)
>
> s/base/is_base_block/ or something?

Okay.

>
>> +{
>> +	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))
>
> That should clamp to 0-8?

Indeed, thanks!

> Might be nicer to just define a .set() op for the modparam
> and check it there, but that's clearly material for a separate patch.

Yes.

BR,
Jani.

>
>> +			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");
>
> This debug message seems to disappear. Intentional?
>
>> -		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
Jani Nikula March 31, 2022, 4:49 p.m. UTC | #3 
On Thu, 31 Mar 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> -
>> -		if (edid->revision > 4)
>> -			DRM_DEBUG("EDID minor > 4, assuming backward compatibility\n");
>
> This debug message seems to disappear. Intentional?

Intentional, but failed to mention it in the commit message.

Do we want to keep it? With my new approach, it basically means another
valid return value that's distinct from just ok.


BR,
Jani.
Ville Syrjala March 31, 2022, 4:58 p.m. UTC | #4 
On Thu, Mar 31, 2022 at 07:49:10PM +0300, Jani Nikula wrote:
> On Thu, 31 Mar 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> -
> >> -		if (edid->revision > 4)
> >> -			DRM_DEBUG("EDID minor > 4, assuming backward compatibility\n");
> >
> > This debug message seems to disappear. Intentional?
> 
> Intentional, but failed to mention it in the commit message.
> 
> Do we want to keep it? With my new approach, it basically means another
> valid return value that's distinct from just ok.

Seems pretty pointless to me. Especially with DisplayID on the
scene it seems rather unlikely that there would ever be EDID 1.5+.
diff mbox series

Patch

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);