RE: [PATCH v2] drm/ast: DisplayPort edid supports 256 bytes

[Jammy Huang]

Hi Thomas,

Good catch - returning 0 without writing to buf wasn't safe. But I also found that returning -EIO isn't right either: _drm_do_get_edid() treats EDID_BLOCK_READ_FAIL as fatal for any block index, not just block 0, so that would have discarded the whole EDID for any monitor reporting more than one extension.

In v3, for block > 1 I now zero the buffer explicitly and return 0. That makes drm_edid's block checker classify it as EDID_BLOCK_ZERO instead, which is non-fatal - it gets trimmed via edid_filter_invalid_blocks() (extensions count reduced, checksum recomputed), and blocks 0 and 1 still come through fine. This also directly addresses your point about the buffer not being guaranteed zeroed, since we zero it ourselves now.

Best regards,
Jammy
> 
> (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#L
> 2366
> [2]
> https://elixir.bootlin.com/linux/v6.19.8/source/drivers/gpu/drm/drm_edid.c#L
> 1919
> 
> >
> > Signed-off-by: Jammy Huang <jammy_huang@xxxxxxxxxxxxxx>
> > ---
> > 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/20260313-upstream_ast_dp_edid-v1-1-2a75b7c091b2
> @aspeedtech.com
> > ---
> >   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)
>