(cc Villa, Jani)
Hi
Am 17.03.26 um 13:19 schrieb Jammy Huang:
DisplayPort supports EDID up to 256 bytes (blocks 0 and 1). Update the
block check to allow these two blocks while returning 0 for any
additional extension blocks.
Furthermore, remove the manual EDID byte manipulation logic. The DRM
core (drm_edid) already handles error correction and checksum
validation.
Does that really work? AFAICT
- The allocations in _drm_do_get_edid() [1] do not guarantee that the
memory is cleared to zero.
- If the helper detects an error, it will conditionally print an error [2].
- As ast is polling the EDID, it'll fill the kernel log over time with
these warnings.
I think we'd still need some sort of 'valid error state' that signals an
EOF that is not an error, but still counts as an invalid header.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.19.8/source/drivers/gpu/drm/drm_edid.c#L2366
[2]
https://elixir.bootlin.com/linux/v6.19.8/source/drivers/gpu/drm/drm_edid.c#L1919
Signed-off-by: Jammy Huang <[email protected]>
---
ASPEED DisplayPort's EDID size can be 256 bytes at most. Thus, EDID
blocks fetched can be 0 and 1.
---
Changes in v2:
Becasue drm-edid will handle invalid EDID if happen, we have 2 changes
below.
- Return 0 for the number of block more than 1.
- Remove modification of EDID
- Link to v1:
https://lore.kernel.org/r/[email protected]
---
drivers/gpu/drm/ast/ast_dp.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 9d07dad358c..282c694218c 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -88,8 +88,8 @@ static int ast_astdp_read_edid_block(void *data, u8 *buf,
unsigned int block, si
int ret = 0;
unsigned int i;
- if (block > 0)
- return -EIO; /* extension headers not supported */
+ if (block > 1)
+ return 0;
/*
* Protect access to I/O registers from concurrent modesetting
@@ -154,20 +154,6 @@ static int ast_astdp_read_edid_block(void *data, u8 *buf,
unsigned int block, si
ediddata[2] = ast_get_index_reg(ast, AST_IO_VGACRI, 0xda);
ediddata[3] = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdb);
- if (i == 31) {
- /*
- * For 128-bytes EDID_1.3,
- * 1. Add the value of Bytes-126 to Bytes-127.
- * The Bytes-127 is Checksum. Sum of all
128bytes should
- * equal 0 (mod 256).
- * 2. Modify Bytes-126 to be 0.
- * The Bytes-126 indicates the Number of
extensions to
- * follow. 0 represents noextensions.
- */
- ediddata[3] = ediddata[3] + ediddata[2];
- ediddata[2] = 0;
- }
-
memcpy(buf, ediddata, min((len - i), 4));
buf += 4;
}
---
base-commit: 5ee8dbf54602dc340d6235b1d6aa17c0f283f48c
change-id: 20260313-upstream_ast_dp_edid-5fe6adf7ad36
Best regards,
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
