* [CI v2 00/12] drm/edid: low level EDID block read refactoring etc.
@ 2022-04-11 9:47 Jani Nikula
2022-04-11 9:47 ` [CI v2 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks Jani Nikula
` (11 more replies)
0 siblings, 12 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Submitting [1] again for CI. Just a slight change in patch 3 to address
review.
BR,
Jani.
[1] https://patchwork.freedesktop.org/series/102329/
Jani Nikula (12):
drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks
drm/edid: have edid_block_check() detect blocks that are all zero
drm/edid: refactor EDID block status printing
drm/edid: add a helper to log dump an EDID block
drm/edid: pass struct edid to connector_bad_edid()
drm/edid: add typedef for block read function
drm/edid: abstract an EDID block read helper
drm/edid: use EDID block read helper in drm_do_get_edid()
drm/edid: convert extension block read to EDID block read helper
drm/edid: drop extra local var
drm/edid: add single point of return to drm_do_get_edid()
drm/edid: add EDID block count and size helpers
drivers/gpu/drm/drm_edid.c | 350 ++++++++++++++++++++++++-------------
1 file changed, 225 insertions(+), 125 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [CI v2 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 02/12] drm/edid: have edid_block_check() detect blocks that are all zero Jani Nikula
` (10 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
As edid_is_zero() is only ever used on EDID blocks, convert it to
edid_block_is_zero() with implicit block size.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 04e818ecd662..f4da3f92f41b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1632,9 +1632,9 @@ static int edid_block_tag(const void *_block)
return block[0];
}
-static bool edid_is_zero(const void *edid, int length)
+static bool edid_block_is_zero(const void *edid)
{
- return !memchr_inv(edid, 0, length);
+ return !memchr_inv(edid, 0, EDID_LENGTH);
}
/**
@@ -1785,7 +1785,7 @@ bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
}
if (!valid && print_bad_edid) {
- if (edid_is_zero(block, EDID_LENGTH)) {
+ if (edid_block_is_zero(block)) {
pr_notice("EDID block is all zeroes\n");
} else {
pr_notice("Raw EDID:\n");
@@ -1942,7 +1942,7 @@ static void connector_bad_edid(struct drm_connector *connector,
u8 *block = edid + i * EDID_LENGTH;
char prefix[20];
- if (edid_is_zero(block, EDID_LENGTH))
+ if (edid_block_is_zero(block))
sprintf(prefix, "\t[%02x] ZERO ", i);
else if (!drm_edid_block_valid(block, i, false, NULL))
sprintf(prefix, "\t[%02x] BAD ", i);
@@ -2019,7 +2019,7 @@ static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
goto out;
if (drm_edid_block_valid(edid, 0, false, edid_corrupt))
break;
- if (try == 0 && edid_is_zero(edid, EDID_LENGTH)) {
+ if (try == 0 && edid_block_is_zero(edid)) {
if (null_edid_counter)
(*null_edid_counter)++;
goto carp;
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 02/12] drm/edid: have edid_block_check() detect blocks that are all zero
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
2022-04-11 9:47 ` [CI v2 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 03/12] drm/edid: refactor EDID block status printing Jani Nikula
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
We have the check function, have it also detect blocks that are all zero
instead of leaving that to callers.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f4da3f92f41b..f062d1715ec3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1671,6 +1671,7 @@ EXPORT_SYMBOL(drm_edid_are_equal);
enum edid_block_status {
EDID_BLOCK_OK = 0,
EDID_BLOCK_NULL,
+ EDID_BLOCK_ZERO,
EDID_BLOCK_HEADER_CORRUPT,
EDID_BLOCK_HEADER_REPAIR,
EDID_BLOCK_HEADER_FIXED,
@@ -1689,15 +1690,23 @@ static enum edid_block_status edid_block_check(const void *_block,
if (is_base_block) {
int score = drm_edid_header_is_valid(block);
- if (score < clamp(edid_fixup, 0, 8))
- return EDID_BLOCK_HEADER_CORRUPT;
+ if (score < clamp(edid_fixup, 0, 8)) {
+ if (edid_block_is_zero(block))
+ return EDID_BLOCK_ZERO;
+ else
+ 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 (edid_block_compute_checksum(block) != edid_block_get_checksum(block)) {
+ if (edid_block_is_zero(block))
+ return EDID_BLOCK_ZERO;
+ else
+ return EDID_BLOCK_CHECKSUM;
+ }
if (is_base_block) {
if (block->version != 1)
@@ -1785,7 +1794,7 @@ bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
}
if (!valid && print_bad_edid) {
- if (edid_block_is_zero(block)) {
+ if (status == EDID_BLOCK_ZERO) {
pr_notice("EDID block is all zeroes\n");
} else {
pr_notice("Raw EDID:\n");
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 03/12] drm/edid: refactor EDID block status printing
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
2022-04-11 9:47 ` [CI v2 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks Jani Nikula
2022-04-11 9:47 ` [CI v2 02/12] drm/edid: have edid_block_check() detect blocks that are all zero Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 04/12] drm/edid: add a helper to log dump an EDID block Jani Nikula
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
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.
v2: Complain more loudly about unknown status codes (Ville)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.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..88e8f906a229 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:
+ WARN(1, "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;
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 04/12] drm/edid: add a helper to log dump an EDID block
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (2 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 03/12] drm/edid: refactor EDID block status printing Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 05/12] drm/edid: pass struct edid to connector_bad_edid() Jani Nikula
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Unify debug log dumping. There's duplication in the error paths for EDID
block validity checks, but this should be neglible.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 88e8f906a229..0b8098e34236 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1773,6 +1773,23 @@ static void edid_block_status_print(enum edid_block_status status,
}
}
+static void edid_block_dump(const char *level, const void *block, int block_num)
+{
+ enum edid_block_status status;
+ char prefix[20];
+
+ status = edid_block_check(block, block_num == 0);
+ if (status == EDID_BLOCK_ZERO)
+ sprintf(prefix, "\t[%02x] ZERO ", block_num);
+ else if (!edid_block_status_valid(status, edid_block_tag(block)))
+ sprintf(prefix, "\t[%02x] BAD ", block_num);
+ else
+ sprintf(prefix, "\t[%02x] GOOD ", block_num);
+
+ print_hex_dump(level, prefix, DUMP_PREFIX_NONE, 16, 1,
+ block, EDID_LENGTH, false);
+}
+
/**
* drm_edid_block_valid - Sanity check the EDID block (base or extension)
* @raw_edid: pointer to raw EDID block
@@ -1826,9 +1843,7 @@ bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
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);
+ edid_block_dump(KERN_NOTICE, block, block_num);
}
return valid;
@@ -1976,18 +1991,8 @@ static void connector_bad_edid(struct drm_connector *connector,
drm_dbg_kms(connector->dev, "%s: EDID is invalid:\n", connector->name);
for (i = 0; i < num_blocks; i++) {
u8 *block = edid + i * EDID_LENGTH;
- char prefix[20];
-
- if (edid_block_is_zero(block))
- sprintf(prefix, "\t[%02x] ZERO ", i);
- else if (!drm_edid_block_valid(block, i, false, NULL))
- sprintf(prefix, "\t[%02x] BAD ", i);
- else
- sprintf(prefix, "\t[%02x] GOOD ", i);
- print_hex_dump(KERN_DEBUG,
- prefix, DUMP_PREFIX_NONE, 16, 1,
- block, EDID_LENGTH, false);
+ edid_block_dump(KERN_DEBUG, block, i);
}
}
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 05/12] drm/edid: pass struct edid to connector_bad_edid()
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (3 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 04/12] drm/edid: add a helper to log dump an EDID block Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 06/12] drm/edid: add typedef for block read function Jani Nikula
` (6 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Avoid casting here and there, and make it const.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0b8098e34236..e1afd6a55a8c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1967,7 +1967,7 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
}
static void connector_bad_edid(struct drm_connector *connector,
- u8 *edid, int num_blocks)
+ const struct edid *edid, int num_blocks)
{
int i;
u8 last_block;
@@ -1978,22 +1978,19 @@ static void connector_bad_edid(struct drm_connector *connector,
* of 0x7e in the EDID of the _index_ of the last block in the
* combined chunk of memory.
*/
- last_block = edid[0x7e];
+ last_block = edid->extensions;
/* Calculate real checksum for the last edid extension block data */
if (last_block < num_blocks)
connector->real_edid_checksum =
- edid_block_compute_checksum(edid + last_block * EDID_LENGTH);
+ edid_block_compute_checksum(edid + last_block);
if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
return;
drm_dbg_kms(connector->dev, "%s: EDID is invalid:\n", connector->name);
- for (i = 0; i < num_blocks; i++) {
- u8 *block = edid + i * EDID_LENGTH;
-
- edid_block_dump(KERN_DEBUG, block, i);
- }
+ for (i = 0; i < num_blocks; i++)
+ edid_block_dump(KERN_DEBUG, edid + i, i);
}
/* Get override or firmware EDID */
@@ -2139,7 +2136,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
}
if (invalid_blocks) {
- connector_bad_edid(connector, (u8 *)edid, edid->extensions + 1);
+ connector_bad_edid(connector, edid, edid->extensions + 1);
edid = edid_filter_invalid_blocks(edid, invalid_blocks);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 06/12] drm/edid: add typedef for block read function
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (4 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 05/12] drm/edid: pass struct edid to connector_bad_edid() Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 07/12] drm/edid: abstract an EDID block read helper Jani Nikula
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Make the callback a bit easier on the eye.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e1afd6a55a8c..5b450bad018e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2037,10 +2037,11 @@ int drm_add_override_edid_modes(struct drm_connector *connector)
}
EXPORT_SYMBOL(drm_add_override_edid_modes);
+typedef int read_block_fn(void *context, u8 *buf, unsigned int block, size_t len);
+
static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
- int (*get_edid_block)(void *data, u8 *buf, unsigned int block,
- size_t len),
- void *data)
+ read_block_fn read_block,
+ void *context)
{
int *null_edid_counter = connector ? &connector->null_edid_counter : NULL;
bool *edid_corrupt = connector ? &connector->edid_corrupt : NULL;
@@ -2053,7 +2054,7 @@ static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
/* base block fetch */
for (try = 0; try < 4; try++) {
- if (get_edid_block(data, edid, 0, EDID_LENGTH))
+ if (read_block(context, edid, 0, EDID_LENGTH))
goto out;
if (drm_edid_block_valid(edid, 0, false, edid_corrupt))
break;
@@ -2097,9 +2098,8 @@ static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
* Return: Pointer to valid EDID or NULL if we couldn't find any.
*/
struct edid *drm_do_get_edid(struct drm_connector *connector,
- int (*get_edid_block)(void *data, u8 *buf, unsigned int block,
- size_t len),
- void *data)
+ read_block_fn read_block,
+ void *context)
{
int j, invalid_blocks = 0;
struct edid *edid, *new, *override;
@@ -2108,7 +2108,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
if (override)
return override;
- edid = drm_do_get_edid_base_block(connector, get_edid_block, data);
+ edid = drm_do_get_edid_base_block(connector, read_block, context);
if (!edid)
return NULL;
@@ -2125,7 +2125,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
int try;
for (try = 0; try < 4; try++) {
- if (get_edid_block(data, block, j, EDID_LENGTH))
+ if (read_block(context, block, j, EDID_LENGTH))
goto out;
if (drm_edid_block_valid(block, j, false, NULL))
break;
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 07/12] drm/edid: abstract an EDID block read helper
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (5 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 06/12] drm/edid: add typedef for block read function Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 08/12] drm/edid: use EDID block read helper in drm_do_get_edid() Jani Nikula
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
We have an abstraction for the EDID base block read, yet duplicating the
retries and error handling for extension block reads. Introduce a more
generic EDID block read helper.
Switch to the helper piecemeal, starting with drm_edid_get_panel_id(),
which doesn't need or have access to the connector anyway.
The subtle change is switching from drm_edid_block_valid() to
edid_block_check(). We also status print once, not for every attempt.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 58 +++++++++++++++++++++++++++++++++-----
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5b450bad018e..58be9be72dde 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1670,6 +1670,7 @@ EXPORT_SYMBOL(drm_edid_are_equal);
enum edid_block_status {
EDID_BLOCK_OK = 0,
+ EDID_BLOCK_READ_FAIL,
EDID_BLOCK_NULL,
EDID_BLOCK_ZERO,
EDID_BLOCK_HEADER_CORRUPT,
@@ -1736,6 +1737,9 @@ static void edid_block_status_print(enum edid_block_status status,
switch (status) {
case EDID_BLOCK_OK:
break;
+ case EDID_BLOCK_READ_FAIL:
+ pr_debug("EDID block %d read failed\n", block_num);
+ break;
case EDID_BLOCK_NULL:
pr_debug("EDID block %d pointer is NULL\n", block_num);
break;
@@ -2039,6 +2043,39 @@ EXPORT_SYMBOL(drm_add_override_edid_modes);
typedef int read_block_fn(void *context, u8 *buf, unsigned int block, size_t len);
+static enum edid_block_status edid_block_read(void *block, unsigned int block_num,
+ read_block_fn read_block,
+ void *context)
+{
+ enum edid_block_status status;
+ bool is_base_block = block_num == 0;
+ int try;
+
+ for (try = 0; try < 4; try++) {
+ if (read_block(context, block, block_num, EDID_LENGTH))
+ return EDID_BLOCK_READ_FAIL;
+
+ status = edid_block_check(block, is_base_block);
+ if (status == EDID_BLOCK_HEADER_REPAIR) {
+ edid_header_fix(block);
+
+ /* Retry with fixed header, update status if that worked. */
+ status = edid_block_check(block, is_base_block);
+ if (status == EDID_BLOCK_OK)
+ status = EDID_BLOCK_HEADER_FIXED;
+ }
+
+ if (edid_block_status_valid(status, edid_block_tag(block)))
+ break;
+
+ /* Fail early for unrepairable base block all zeros. */
+ if (try == 0 && is_base_block && status == EDID_BLOCK_ZERO)
+ break;
+ }
+
+ return status;
+}
+
static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
read_block_fn read_block,
void *context)
@@ -2237,20 +2274,27 @@ static u32 edid_extract_panel_id(const struct edid *edid)
u32 drm_edid_get_panel_id(struct i2c_adapter *adapter)
{
- const struct edid *edid;
- u32 panel_id;
-
- edid = drm_do_get_edid_base_block(NULL, drm_do_probe_ddc_edid, adapter);
+ enum edid_block_status status;
+ void *base_block;
+ u32 panel_id = 0;
/*
* There are no manufacturer IDs of 0, so if there is a problem reading
* the EDID then we'll just return 0.
*/
- if (!edid)
+
+ base_block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+ if (!base_block)
return 0;
- panel_id = edid_extract_panel_id(edid);
- kfree(edid);
+ status = edid_block_read(base_block, 0, drm_do_probe_ddc_edid, adapter);
+
+ edid_block_status_print(status, base_block, 0);
+
+ if (edid_block_status_valid(status, edid_block_tag(base_block)))
+ panel_id = edid_extract_panel_id(base_block);
+
+ kfree(base_block);
return panel_id;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 08/12] drm/edid: use EDID block read helper in drm_do_get_edid()
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (6 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 07/12] drm/edid: abstract an EDID block read helper Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 09/12] drm/edid: convert extension block read to EDID block read helper Jani Nikula
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Convert drm_do_get_edit() from the base block read helper to the generic
block read helper. There's quite a bit going on here, as the corrupt and
null EDID information is moved back to the caller. As we see, they were
not all that clear to begin with, and this change underlines that.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 62 ++++++++++++++------------------------
1 file changed, 23 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 58be9be72dde..359d3d6f216e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2076,44 +2076,6 @@ static enum edid_block_status edid_block_read(void *block, unsigned int block_nu
return status;
}
-static struct edid *drm_do_get_edid_base_block(struct drm_connector *connector,
- read_block_fn read_block,
- void *context)
-{
- int *null_edid_counter = connector ? &connector->null_edid_counter : NULL;
- bool *edid_corrupt = connector ? &connector->edid_corrupt : NULL;
- void *edid;
- int try;
-
- edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
- if (edid == NULL)
- return NULL;
-
- /* base block fetch */
- for (try = 0; try < 4; try++) {
- if (read_block(context, edid, 0, EDID_LENGTH))
- goto out;
- if (drm_edid_block_valid(edid, 0, false, edid_corrupt))
- break;
- if (try == 0 && edid_block_is_zero(edid)) {
- if (null_edid_counter)
- (*null_edid_counter)++;
- goto carp;
- }
- }
- if (try == 4)
- goto carp;
-
- return edid;
-
-carp:
- if (connector)
- connector_bad_edid(connector, edid, 1);
-out:
- kfree(edid);
- return NULL;
-}
-
/**
* drm_do_get_edid - get EDID data using a custom EDID block read function
* @connector: connector we're probing
@@ -2138,6 +2100,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
read_block_fn read_block,
void *context)
{
+ enum edid_block_status status;
int j, invalid_blocks = 0;
struct edid *edid, *new, *override;
@@ -2145,10 +2108,31 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
if (override)
return override;
- edid = drm_do_get_edid_base_block(connector, read_block, context);
+ edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
if (!edid)
return NULL;
+ status = edid_block_read(edid, 0, read_block, context);
+
+ edid_block_status_print(status, edid, 0);
+
+ if (status == EDID_BLOCK_READ_FAIL)
+ goto out;
+
+ /* FIXME: Clarify what a corrupt EDID actually means. */
+ if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
+ connector->edid_corrupt = false;
+ else
+ connector->edid_corrupt = true;
+
+ if (!edid_block_status_valid(status, edid_block_tag(edid))) {
+ if (status == EDID_BLOCK_ZERO)
+ connector->null_edid_counter++;
+
+ connector_bad_edid(connector, edid, 1);
+ goto out;
+ }
+
if (edid->extensions == 0)
return edid;
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 09/12] drm/edid: convert extension block read to EDID block read helper
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (7 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 08/12] drm/edid: use EDID block read helper in drm_do_get_edid() Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 10/12] drm/edid: drop extra local var Jani Nikula
` (2 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Use the EDID block read helper also for extension block reads, making
edid_block_read() the only place with the read retry logic.
Note: We observe that drm_do_get_edid() does not use invalid extension
blocks to flag the EDID as corrupt.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 359d3d6f216e..8a050b65c06a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2143,17 +2143,16 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
for (j = 1; j <= edid->extensions; j++) {
void *block = edid + j;
- int try;
- for (try = 0; try < 4; try++) {
- if (read_block(context, block, j, EDID_LENGTH))
- goto out;
- if (drm_edid_block_valid(block, j, false, NULL))
- break;
- }
+ status = edid_block_read(block, j, read_block, context);
- if (try == 4)
+ edid_block_status_print(status, block, j);
+
+ if (!edid_block_status_valid(status, edid_block_tag(block))) {
+ if (status == EDID_BLOCK_READ_FAIL)
+ goto out;
invalid_blocks++;
+ }
}
if (invalid_blocks) {
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 10/12] drm/edid: drop extra local var
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (8 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 09/12] drm/edid: convert extension block read to EDID block read helper Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 11/12] drm/edid: add single point of return to drm_do_get_edid() Jani Nikula
2022-04-11 9:47 ` [CI v2 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
We don't need override as a variable for anything.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 8a050b65c06a..5a1906a5c523 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2102,11 +2102,11 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
{
enum edid_block_status status;
int j, invalid_blocks = 0;
- struct edid *edid, *new, *override;
+ struct edid *edid, *new;
- override = drm_get_override_edid(connector);
- if (override)
- return override;
+ edid = drm_get_override_edid(connector);
+ if (edid)
+ return edid;
edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
if (!edid)
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 11/12] drm/edid: add single point of return to drm_do_get_edid()
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (9 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 10/12] drm/edid: drop extra local var Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 9:47 ` [CI v2 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
11 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
This will be useful in the future. Use fail label for fail exit.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5a1906a5c523..0933a5c44998 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2106,7 +2106,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
edid = drm_get_override_edid(connector);
if (edid)
- return edid;
+ goto ok;
edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
if (!edid)
@@ -2117,7 +2117,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
edid_block_status_print(status, edid, 0);
if (status == EDID_BLOCK_READ_FAIL)
- goto out;
+ goto fail;
/* FIXME: Clarify what a corrupt EDID actually means. */
if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
@@ -2130,15 +2130,15 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
connector->null_edid_counter++;
connector_bad_edid(connector, edid, 1);
- goto out;
+ goto fail;
}
if (edid->extensions == 0)
- return edid;
+ goto ok;
new = krealloc(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL);
if (!new)
- goto out;
+ goto fail;
edid = new;
for (j = 1; j <= edid->extensions; j++) {
@@ -2150,7 +2150,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
if (!edid_block_status_valid(status, edid_block_tag(block))) {
if (status == EDID_BLOCK_READ_FAIL)
- goto out;
+ goto fail;
invalid_blocks++;
}
}
@@ -2161,9 +2161,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
edid = edid_filter_invalid_blocks(edid, invalid_blocks);
}
+ok:
return edid;
-out:
+fail:
kfree(edid);
return NULL;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [CI v2 12/12] drm/edid: add EDID block count and size helpers
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
` (10 preceding siblings ...)
2022-04-11 9:47 ` [CI v2 11/12] drm/edid: add single point of return to drm_do_get_edid() Jani Nikula
@ 2022-04-11 9:47 ` Jani Nikula
2022-04-11 12:14 ` [Intel-gfx] " kernel test robot
2022-04-11 12:24 ` kernel test robot
11 siblings, 2 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 9:47 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx
Add some helpers to figure out the EDID extension block count, block
count, size, pointers to blocks.
Unfortunately, we'll need to cast away the const in a few places where
we actually need to access the data.
v2: fix s/j/i/ introduced in a rebase
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_edid.c | 80 +++++++++++++++++++++++++++-----------
1 file changed, 57 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0933a5c44998..c8148e5b9969 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1568,6 +1568,38 @@ static const struct drm_display_mode edid_4k_modes[] = {
/*** DDC fetch and block validation ***/
+static int edid_extension_block_count(const struct edid *edid)
+{
+ return edid->extensions;
+}
+
+static int edid_block_count(const struct edid *edid)
+{
+ return edid_extension_block_count(edid) + 1;
+}
+
+static int edid_size_by_blocks(int num_blocks)
+{
+ return num_blocks * EDID_LENGTH;
+}
+
+static int edid_size(const struct edid *edid)
+{
+ return edid_size_by_blocks(edid_block_count(edid));
+}
+
+static const void *edid_block_data(const struct edid *edid, int index)
+{
+ BUILD_BUG_ON(sizeof(*edid) != EDID_LENGTH);
+
+ return edid + index;
+}
+
+static const void *edid_extension_block_data(const struct edid *edid, int index)
+{
+ return edid_block_data(edid, index + 1);
+}
+
static const u8 edid_header[] = {
0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
};
@@ -1654,8 +1686,8 @@ bool drm_edid_are_equal(const struct edid *edid1, const struct edid *edid2)
return false;
if (edid1) {
- edid1_len = EDID_LENGTH * (1 + edid1->extensions);
- edid2_len = EDID_LENGTH * (1 + edid2->extensions);
+ edid1_len = edid_size(edid1);
+ edid2_len = edid_size(edid2);
if (edid1_len != edid2_len)
return false;
@@ -1865,14 +1897,16 @@ EXPORT_SYMBOL(drm_edid_block_valid);
bool drm_edid_is_valid(struct edid *edid)
{
int i;
- u8 *raw = (u8 *)edid;
if (!edid)
return false;
- for (i = 0; i <= edid->extensions; i++)
- if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true, NULL))
+ for (i = 0; i < edid_block_count(edid); i++) {
+ void *block = (void *)edid_block_data(edid, i);
+
+ if (!drm_edid_block_valid(block, i, true, NULL))
return false;
+ }
return true;
}
@@ -1885,13 +1919,13 @@ static struct edid *edid_filter_invalid_blocks(const struct edid *edid,
int valid_extensions = edid->extensions - invalid_blocks;
int i;
- new = kmalloc_array(valid_extensions + 1, EDID_LENGTH, GFP_KERNEL);
+ new = kmalloc(edid_size_by_blocks(valid_extensions + 1), GFP_KERNEL);
if (!new)
goto out;
dest_block = new;
- for (i = 0; i <= edid->extensions; i++) {
- const void *block = edid + i;
+ for (i = 0; i < edid_block_count(edid); i++) {
+ const void *block = edid_block_data(edid, i);
if (edid_block_valid(block, i == 0))
memcpy(dest_block++, block, EDID_LENGTH);
@@ -2101,7 +2135,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
void *context)
{
enum edid_block_status status;
- int j, invalid_blocks = 0;
+ int i, invalid_blocks = 0;
struct edid *edid, *new;
edid = drm_get_override_edid(connector);
@@ -2133,20 +2167,20 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
goto fail;
}
- if (edid->extensions == 0)
+ if (!edid_extension_block_count(edid) == 0)
goto ok;
- new = krealloc(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL);
+ new = krealloc(edid, edid_size(edid), GFP_KERNEL);
if (!new)
goto fail;
edid = new;
- for (j = 1; j <= edid->extensions; j++) {
- void *block = edid + j;
+ for (i = 1; i < edid_block_count(edid); i++) {
+ void *block = (void *)edid_block_data(edid, i);
- status = edid_block_read(block, j, read_block, context);
+ status = edid_block_read(block, i, read_block, context);
- edid_block_status_print(status, block, j);
+ edid_block_status_print(status, block, i);
if (!edid_block_status_valid(status, edid_block_tag(block))) {
if (status == EDID_BLOCK_READ_FAIL)
@@ -2156,7 +2190,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
}
if (invalid_blocks) {
- connector_bad_edid(connector, edid, edid->extensions + 1);
+ connector_bad_edid(connector, edid, edid_block_count(edid));
edid = edid_filter_invalid_blocks(edid, invalid_blocks);
}
@@ -2321,7 +2355,7 @@ EXPORT_SYMBOL(drm_get_edid_switcheroo);
*/
struct edid *drm_edid_duplicate(const struct edid *edid)
{
- return kmemdup(edid, (edid->extensions + 1) * EDID_LENGTH, GFP_KERNEL);
+ return kmemdup(edid, edid_size(edid), GFP_KERNEL);
}
EXPORT_SYMBOL(drm_edid_duplicate);
@@ -2505,8 +2539,8 @@ drm_for_each_detailed_block(const struct edid *edid, detailed_cb *cb, void *clos
for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
cb(&(edid->detailed_timings[i]), closure);
- for (i = 1; i <= edid->extensions; i++) {
- const u8 *ext = (const u8 *)edid + (i * EDID_LENGTH);
+ for (i = 0; i < edid_extension_block_count(edid); i++) {
+ const u8 *ext = edid_extension_block_data(edid, i);
switch (*ext) {
case CEA_EXT:
@@ -3476,17 +3510,17 @@ const u8 *drm_find_edid_extension(const struct edid *edid,
int i;
/* No EDID or EDID extensions */
- if (edid == NULL || edid->extensions == 0)
+ if (!edid || !edid_extension_block_count(edid))
return NULL;
/* Find CEA extension */
- for (i = *ext_index; i < edid->extensions; i++) {
- edid_ext = (const u8 *)edid + EDID_LENGTH * (i + 1);
+ for (i = *ext_index; i < edid_extension_block_count(edid); i++) {
+ edid_ext = edid_extension_block_data(edid, i);
if (edid_block_tag(edid_ext) == ext_id)
break;
}
- if (i >= edid->extensions)
+ if (i >= edid_extension_block_count(edid))
return NULL;
*ext_index = i + 1;
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers
2022-04-11 9:47 ` [CI v2 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
@ 2022-04-11 12:14 ` kernel test robot
2022-04-11 13:57 ` Jani Nikula
2022-04-11 12:24 ` kernel test robot
1 sibling, 1 reply; 17+ messages in thread
From: kernel test robot @ 2022-04-11 12:14 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: jani.nikula, intel-gfx, kbuild-all
Hi Jani,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220411]
[cannot apply to drm/drm-next drm-intel/for-linux-next v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220411/202204112055.cUmakJdJ-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
git checkout ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_edid.c: In function 'drm_do_get_edid':
>> drivers/gpu/drm/drm_edid.c:1664:21: warning: array subscript [128, 32640] is outside array bounds of 'struct edid[1]' [-Warray-bounds]
1664 | return block[0];
| ~~~~~^~~
drivers/gpu/drm/drm_edid.c:2173:15: note: referencing an object of size 128 allocated by 'krealloc'
2173 | new = krealloc(edid, edid_size(edid), GFP_KERNEL);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +1664 drivers/gpu/drm/drm_edid.c
c465bbc87ce372 Stefan Brüns 2014-11-30 1659
4ba0f53ce685b0 Jani Nikula 2022-03-31 1660 static int edid_block_tag(const void *_block)
4ba0f53ce685b0 Jani Nikula 2022-03-31 1661 {
4ba0f53ce685b0 Jani Nikula 2022-03-31 1662 const u8 *block = _block;
4ba0f53ce685b0 Jani Nikula 2022-03-31 1663
4ba0f53ce685b0 Jani Nikula 2022-03-31 @1664 return block[0];
4ba0f53ce685b0 Jani Nikula 2022-03-31 1665 }
4ba0f53ce685b0 Jani Nikula 2022-03-31 1666
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers
2022-04-11 9:47 ` [CI v2 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
2022-04-11 12:14 ` [Intel-gfx] " kernel test robot
@ 2022-04-11 12:24 ` kernel test robot
2022-04-11 13:54 ` Jani Nikula
1 sibling, 1 reply; 17+ messages in thread
From: kernel test robot @ 2022-04-11 12:24 UTC (permalink / raw)
To: Jani Nikula, dri-devel; +Cc: jani.nikula, intel-gfx, llvm, kbuild-all
Hi Jani,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220411]
[cannot apply to drm/drm-next drm-intel/for-linux-next v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-a001-20220411 (https://download.01.org/0day-ci/archive/20220411/202204112019.U9iIZWqP-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c6e83f560f06cdfe8aa47b248d8bdc58f947274b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
git checkout ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_edid.c:2170:6: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
if (!edid_extension_block_count(edid) == 0)
^ ~~
drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses after the '!' to evaluate the comparison first
if (!edid_extension_block_count(edid) == 0)
^
( )
drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses around left hand side expression to silence this warning
if (!edid_extension_block_count(edid) == 0)
^
( )
1 warning generated.
vim +2170 drivers/gpu/drm/drm_edid.c
2112
2113 /**
2114 * drm_do_get_edid - get EDID data using a custom EDID block read function
2115 * @connector: connector we're probing
2116 * @get_edid_block: EDID block read function
2117 * @data: private data passed to the block read function
2118 *
2119 * When the I2C adapter connected to the DDC bus is hidden behind a device that
2120 * exposes a different interface to read EDID blocks this function can be used
2121 * to get EDID data using a custom block read function.
2122 *
2123 * As in the general case the DDC bus is accessible by the kernel at the I2C
2124 * level, drivers must make all reasonable efforts to expose it as an I2C
2125 * adapter and use drm_get_edid() instead of abusing this function.
2126 *
2127 * The EDID may be overridden using debugfs override_edid or firmware EDID
2128 * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
2129 * order. Having either of them bypasses actual EDID reads.
2130 *
2131 * Return: Pointer to valid EDID or NULL if we couldn't find any.
2132 */
2133 struct edid *drm_do_get_edid(struct drm_connector *connector,
2134 read_block_fn read_block,
2135 void *context)
2136 {
2137 enum edid_block_status status;
2138 int i, invalid_blocks = 0;
2139 struct edid *edid, *new;
2140
2141 edid = drm_get_override_edid(connector);
2142 if (edid)
2143 goto ok;
2144
2145 edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
2146 if (!edid)
2147 return NULL;
2148
2149 status = edid_block_read(edid, 0, read_block, context);
2150
2151 edid_block_status_print(status, edid, 0);
2152
2153 if (status == EDID_BLOCK_READ_FAIL)
2154 goto fail;
2155
2156 /* FIXME: Clarify what a corrupt EDID actually means. */
2157 if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
2158 connector->edid_corrupt = false;
2159 else
2160 connector->edid_corrupt = true;
2161
2162 if (!edid_block_status_valid(status, edid_block_tag(edid))) {
2163 if (status == EDID_BLOCK_ZERO)
2164 connector->null_edid_counter++;
2165
2166 connector_bad_edid(connector, edid, 1);
2167 goto fail;
2168 }
2169
> 2170 if (!edid_extension_block_count(edid) == 0)
2171 goto ok;
2172
2173 new = krealloc(edid, edid_size(edid), GFP_KERNEL);
2174 if (!new)
2175 goto fail;
2176 edid = new;
2177
2178 for (i = 1; i < edid_block_count(edid); i++) {
2179 void *block = (void *)edid_block_data(edid, i);
2180
2181 status = edid_block_read(block, i, read_block, context);
2182
2183 edid_block_status_print(status, block, i);
2184
2185 if (!edid_block_status_valid(status, edid_block_tag(block))) {
2186 if (status == EDID_BLOCK_READ_FAIL)
2187 goto fail;
2188 invalid_blocks++;
2189 }
2190 }
2191
2192 if (invalid_blocks) {
2193 connector_bad_edid(connector, edid, edid_block_count(edid));
2194
2195 edid = edid_filter_invalid_blocks(edid, invalid_blocks);
2196 }
2197
2198 ok:
2199 return edid;
2200
2201 fail:
2202 kfree(edid);
2203 return NULL;
2204 }
2205 EXPORT_SYMBOL_GPL(drm_do_get_edid);
2206
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers
2022-04-11 12:24 ` kernel test robot
@ 2022-04-11 13:54 ` Jani Nikula
0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 13:54 UTC (permalink / raw)
To: kernel test robot, dri-devel; +Cc: intel-gfx, llvm, kbuild-all
On Mon, 11 Apr 2022, kernel test robot <lkp@intel.com> wrote:
> Hi Jani,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on drm-tip/drm-tip]
> [also build test WARNING on next-20220411]
> [cannot apply to drm/drm-next drm-intel/for-linux-next v5.18-rc2]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
> base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
> config: i386-randconfig-a001-20220411 (https://download.01.org/0day-ci/archive/20220411/202204112019.U9iIZWqP-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c6e83f560f06cdfe8aa47b248d8bdc58f947274b)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
> git checkout ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
> # save the config file to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/gpu/drm/drm_edid.c:2170:6: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
> if (!edid_extension_block_count(edid) == 0)
> ^ ~~
> drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses after the '!' to evaluate the comparison first
> if (!edid_extension_block_count(edid) == 0)
> ^
> ( )
> drivers/gpu/drm/drm_edid.c:2170:6: note: add parentheses around left hand side expression to silence this warning
> if (!edid_extension_block_count(edid) == 0)
> ^
> ( )
> 1 warning generated.
Whoops, thanks for the report!
BR,
Jani.
>
>
> vim +2170 drivers/gpu/drm/drm_edid.c
>
> 2112
> 2113 /**
> 2114 * drm_do_get_edid - get EDID data using a custom EDID block read function
> 2115 * @connector: connector we're probing
> 2116 * @get_edid_block: EDID block read function
> 2117 * @data: private data passed to the block read function
> 2118 *
> 2119 * When the I2C adapter connected to the DDC bus is hidden behind a device that
> 2120 * exposes a different interface to read EDID blocks this function can be used
> 2121 * to get EDID data using a custom block read function.
> 2122 *
> 2123 * As in the general case the DDC bus is accessible by the kernel at the I2C
> 2124 * level, drivers must make all reasonable efforts to expose it as an I2C
> 2125 * adapter and use drm_get_edid() instead of abusing this function.
> 2126 *
> 2127 * The EDID may be overridden using debugfs override_edid or firmware EDID
> 2128 * (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
> 2129 * order. Having either of them bypasses actual EDID reads.
> 2130 *
> 2131 * Return: Pointer to valid EDID or NULL if we couldn't find any.
> 2132 */
> 2133 struct edid *drm_do_get_edid(struct drm_connector *connector,
> 2134 read_block_fn read_block,
> 2135 void *context)
> 2136 {
> 2137 enum edid_block_status status;
> 2138 int i, invalid_blocks = 0;
> 2139 struct edid *edid, *new;
> 2140
> 2141 edid = drm_get_override_edid(connector);
> 2142 if (edid)
> 2143 goto ok;
> 2144
> 2145 edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
> 2146 if (!edid)
> 2147 return NULL;
> 2148
> 2149 status = edid_block_read(edid, 0, read_block, context);
> 2150
> 2151 edid_block_status_print(status, edid, 0);
> 2152
> 2153 if (status == EDID_BLOCK_READ_FAIL)
> 2154 goto fail;
> 2155
> 2156 /* FIXME: Clarify what a corrupt EDID actually means. */
> 2157 if (status == EDID_BLOCK_OK || status == EDID_BLOCK_VERSION)
> 2158 connector->edid_corrupt = false;
> 2159 else
> 2160 connector->edid_corrupt = true;
> 2161
> 2162 if (!edid_block_status_valid(status, edid_block_tag(edid))) {
> 2163 if (status == EDID_BLOCK_ZERO)
> 2164 connector->null_edid_counter++;
> 2165
> 2166 connector_bad_edid(connector, edid, 1);
> 2167 goto fail;
> 2168 }
> 2169
>> 2170 if (!edid_extension_block_count(edid) == 0)
> 2171 goto ok;
> 2172
> 2173 new = krealloc(edid, edid_size(edid), GFP_KERNEL);
> 2174 if (!new)
> 2175 goto fail;
> 2176 edid = new;
> 2177
> 2178 for (i = 1; i < edid_block_count(edid); i++) {
> 2179 void *block = (void *)edid_block_data(edid, i);
> 2180
> 2181 status = edid_block_read(block, i, read_block, context);
> 2182
> 2183 edid_block_status_print(status, block, i);
> 2184
> 2185 if (!edid_block_status_valid(status, edid_block_tag(block))) {
> 2186 if (status == EDID_BLOCK_READ_FAIL)
> 2187 goto fail;
> 2188 invalid_blocks++;
> 2189 }
> 2190 }
> 2191
> 2192 if (invalid_blocks) {
> 2193 connector_bad_edid(connector, edid, edid_block_count(edid));
> 2194
> 2195 edid = edid_filter_invalid_blocks(edid, invalid_blocks);
> 2196 }
> 2197
> 2198 ok:
> 2199 return edid;
> 2200
> 2201 fail:
> 2202 kfree(edid);
> 2203 return NULL;
> 2204 }
> 2205 EXPORT_SYMBOL_GPL(drm_do_get_edid);
> 2206
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [CI v2 12/12] drm/edid: add EDID block count and size helpers
2022-04-11 12:14 ` [Intel-gfx] " kernel test robot
@ 2022-04-11 13:57 ` Jani Nikula
0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2022-04-11 13:57 UTC (permalink / raw)
To: kernel test robot, dri-devel; +Cc: intel-gfx, kbuild-all
On Mon, 11 Apr 2022, kernel test robot <lkp@intel.com> wrote:
> Hi Jani,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on drm-tip/drm-tip]
> [also build test WARNING on next-20220411]
> [cannot apply to drm/drm-next drm-intel/for-linux-next v5.18-rc2]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
> base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
> config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220411/202204112055.cUmakJdJ-lkp@intel.com/config)
> compiler: arceb-elf-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jani-Nikula/drm-edid-low-level-EDID-block-read-refactoring-etc/20220411-175027
> git checkout ba74d3cc8cc1b6ba4c34a039e797994ddbc77567
> # save the config file to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/gpu/drm/drm_edid.c: In function 'drm_do_get_edid':
>>> drivers/gpu/drm/drm_edid.c:1664:21: warning: array subscript [128, 32640] is outside array bounds of 'struct edid[1]' [-Warray-bounds]
> 1664 | return block[0];
> | ~~~~~^~~
> drivers/gpu/drm/drm_edid.c:2173:15: note: referencing an object of size 128 allocated by 'krealloc'
> 2173 | new = krealloc(edid, edid_size(edid), GFP_KERNEL);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I presume this is caused by the other blunder.
BR,
Jani.
>
>
> vim +1664 drivers/gpu/drm/drm_edid.c
>
> c465bbc87ce372 Stefan Brüns 2014-11-30 1659
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 1660 static int edid_block_tag(const void *_block)
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 1661 {
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 1662 const u8 *block = _block;
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 1663
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 @1664 return block[0];
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 1665 }
> 4ba0f53ce685b0 Jani Nikula 2022-03-31 1666
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-04-11 13:58 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 9:47 [CI v2 00/12] drm/edid: low level EDID block read refactoring etc Jani Nikula
2022-04-11 9:47 ` [CI v2 01/12] drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks Jani Nikula
2022-04-11 9:47 ` [CI v2 02/12] drm/edid: have edid_block_check() detect blocks that are all zero Jani Nikula
2022-04-11 9:47 ` [CI v2 03/12] drm/edid: refactor EDID block status printing Jani Nikula
2022-04-11 9:47 ` [CI v2 04/12] drm/edid: add a helper to log dump an EDID block Jani Nikula
2022-04-11 9:47 ` [CI v2 05/12] drm/edid: pass struct edid to connector_bad_edid() Jani Nikula
2022-04-11 9:47 ` [CI v2 06/12] drm/edid: add typedef for block read function Jani Nikula
2022-04-11 9:47 ` [CI v2 07/12] drm/edid: abstract an EDID block read helper Jani Nikula
2022-04-11 9:47 ` [CI v2 08/12] drm/edid: use EDID block read helper in drm_do_get_edid() Jani Nikula
2022-04-11 9:47 ` [CI v2 09/12] drm/edid: convert extension block read to EDID block read helper Jani Nikula
2022-04-11 9:47 ` [CI v2 10/12] drm/edid: drop extra local var Jani Nikula
2022-04-11 9:47 ` [CI v2 11/12] drm/edid: add single point of return to drm_do_get_edid() Jani Nikula
2022-04-11 9:47 ` [CI v2 12/12] drm/edid: add EDID block count and size helpers Jani Nikula
2022-04-11 12:14 ` [Intel-gfx] " kernel test robot
2022-04-11 13:57 ` Jani Nikula
2022-04-11 12:24 ` kernel test robot
2022-04-11 13:54 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).