Re: [Intel-gfx] [PATCH 03/12] drm/edid: refactor EDID block status printing

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Apr 07, 2022 at 12:14:29PM +0300, Jani Nikula wrote:
> Split out a function to log EDID block status. The printouts get changed
> slightly.
> 
> Unfortunately, not all users will have struct drm_device available, so
> we convert to pr_* debug logging instead of drm device based logging.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 75 ++++++++++++++++++++++++++------------
>  1 file changed, 51 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f062d1715ec3..3d04d63464ba 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1729,6 +1729,50 @@ static bool edid_block_valid(const void *block, bool base)
>  				       edid_block_tag(block));
>  }
>  
> +static void edid_block_status_print(enum edid_block_status status,
> +				    const struct edid *block,
> +				    int block_num)
> +{
> +	switch (status) {
> +	case EDID_BLOCK_OK:
> +		break;
> +	case EDID_BLOCK_NULL:
> +		pr_debug("EDID block %d pointer is NULL\n", block_num);
> +		break;
> +	case EDID_BLOCK_ZERO:
> +		pr_notice("EDID block %d is all zeroes\n", block_num);
> +		break;
> +	case EDID_BLOCK_HEADER_CORRUPT:
> +		pr_notice("EDID has corrupt header\n");
> +		break;
> +	case EDID_BLOCK_HEADER_REPAIR:
> +		pr_debug("EDID corrupt header needs repair\n");
> +		break;
> +	case EDID_BLOCK_HEADER_FIXED:
> +		pr_debug("EDID corrupt header fixed\n");
> +		break;
> +	case EDID_BLOCK_CHECKSUM:
> +		if (edid_block_status_valid(status, edid_block_tag(block))) {
> +			pr_debug("EDID block %d (tag 0x%02x) checksum is invalid, remainder is %d, ignoring\n",
> +				 block_num, edid_block_tag(block),
> +				 edid_block_compute_checksum(block));
> +		} else {
> +			pr_notice("EDID block %d (tag 0x%02x) checksum is invalid, remainder is %d\n",
> +				  block_num, edid_block_tag(block),
> +				  edid_block_compute_checksum(block));
> +		}
> +		break;
> +	case EDID_BLOCK_VERSION:
> +		pr_notice("EDID has major version %d, instead of 1\n",
> +			  block->version);
> +		break;
> +	default:
> +		pr_debug("EDID block %d unknown edid block status code %d\n",
> +			 block_num, status);

Maybe this should complaing a bit more loudly. Indicates a bug in the
code no?

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		break;
> +	}
> +}
> +
>  /**
>   * drm_edid_block_valid - Sanity check the EDID block (base or extension)
>   * @raw_edid: pointer to raw EDID block
> @@ -1775,33 +1819,16 @@ bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
>  			*edid_corrupt = true;
>  	}
>  
> +	edid_block_status_print(status, block, block_num);
> +
>  	/* 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));
> -		}
> -	} else if (status == EDID_BLOCK_VERSION) {
> -		DRM_NOTE("EDID has major version %d, instead of 1\n",
> -			 block->version);
> -	}
> -
> -	if (!valid && print_bad_edid) {
> -		if (status == EDID_BLOCK_ZERO) {
> -			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,
> -				       block, EDID_LENGTH, false);
> -		}
> +	if (!valid && print_bad_edid && status != EDID_BLOCK_ZERO) {
> +		pr_notice("Raw EDID:\n");
> +		print_hex_dump(KERN_NOTICE,
> +			       " \t", DUMP_PREFIX_NONE, 16, 1,
> +			       block, EDID_LENGTH, false);
>  	}
>  
>  	return valid;
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel