Re: [PATCH v3] drm/ast: DisplayPort edid supports 256 bytes

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 19.08.26 um 07:58 schrieb Jammy Huang:
> DisplayPort supports EDID up to 256 bytes (blocks 0 and 1). Update the
> block check to allow these two blocks. For any block beyond that
> (i.e. the monitor's EDID is larger than the 256 bytes ASPEED DP can
> hold), report it as an all-zero block instead of a read failure.
>
> Also remove the manual checksum/extension-count patch of block 0's
> last four bytes. That code was already dead, so the condition can never
> be true.
>
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>

I know it's controversial, but there appears to be no better solution 
now. Therefore

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> ---
> ASPEED DisplayPort's EDID size can be 256 bytes at most. Thus, EDID
> blocks fetched can be 0 and 1.
> ---
> Changes in v3:
> - For block > 1, write a zeroed buffer. Returning a zeroed block
>    instead makes drm_edid's block checker classify it as
>    EDID_BLOCK_ZERO, which is treated as a non-fatal invalid block.
> - Link to v2: https://lore.kernel.org/r/20260317-upstream_ast_dp_edid-v2-1-31166703c491@aspeedtech.com
>
> 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 | 24 ++++++++----------------
>   1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 9d07dad358c..38530cd126c 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -88,8 +88,14 @@ 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) {
> +		/* ASPEED DP's EDID buffer holds blocks 0 and 1 only (256
> +		 * bytes). Report anything beyond that as an all-zero block
> +		 * rather than a read failure.
> +		 */
> +		memset(buf, 0, len);
> +		return 0;
> +	}
>   
>   	/*
>   	 * Protect access to I/O registers from concurrent modesetting
> @@ -154,20 +160,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, (HRB 36809, AG Nürnberg)