| Message ID | 012d5b007a6a3771540499fb1273882d631887b3.1649322799.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/edid: low level EDID block read refactoring etc. | expand |
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
On Thu, 07 Apr 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > 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? Sent v2 with WARN(). Thanks, Jani > > 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
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); + 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;
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(-)