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

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

[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.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
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,
-- 
Jammy Huang <jammy_huang@aspeedtech.com>

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

[Thomas Zimmermann]

(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 <jammy_huang@aspeedtech.com>
> ---
> 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)

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@aspeedtech.com>
> > ---
> > 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)
> 

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

[Jani Nikula]

On Tue, 21 Jul 2026, Jammy Huang <jammy_huang@aspeedtech.com> wrote:
> 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.

There's a long-term goal to *not* modify the EDID in kernel, but rather
return even the broken EDID extensions to userspace, if that's what the
display has.

There are probably a bunch of hurdles in making that happen, but
intentionally returning zeroed out EDID blocks is just adding another
unnecessary hurdle.


BR,
Jani.


-- 
Jani Nikula, Intel

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

[Jammy Huang]

Hi Jani,

Thanks for your input. So, your suggestion is go back to return -EIO for block > 1,
and stop trying to modify the EDID.
In this way, we can handle EDID whose blocks is less than 2.

BR,
Jammy

> > 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.
> 
> There's a long-term goal to *not* modify the EDID in kernel, but rather return
> even the broken EDID extensions to userspace, if that's what the display has.
> 
> There are probably a bunch of hurdles in making that happen, but intentionally
> returning zeroed out EDID blocks is just adding another unnecessary hurdle.
> 
> 
> BR,
> Jani.
> 
> 
> --
> Jani Nikula, Intel

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

[Jammy Huang]

Hi Thomas,

I'd like to get your thoughts on Jani's point that the kernel should avoid modifying EDID and instead expose the data as-is to userspace.

In practice, most monitors I have encountered only contain two EDID blocks, so simply returning block 0 and block 1 already works for the majority of cases.

Given that, would you agree that it may be acceptable to stop modifying the EDID in the driver and only support the blocks the hardware can actually read now?

Best regards,
Jammy

> 
> Hi Jani,
> 
> Thanks for your input. So, your suggestion is go back to return -EIO for block >
> 1, and stop trying to modify the EDID.
> In this way, we can handle EDID whose blocks is less than 2.
> 
> BR,
> Jammy
> 
> > > 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.
> >
> > There's a long-term goal to *not* modify the EDID in kernel, but
> > rather return even the broken EDID extensions to userspace, if that's what
> the display has.
> >
> > There are probably a bunch of hurdles in making that happen, but
> > intentionally returning zeroed out EDID blocks is just adding another
> unnecessary hurdle.
> >
> >
> > BR,
> > Jani.
> >
> >
> > --
> > Jani Nikula, Intel

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

[Thomas Zimmermann]

Hi Jammy

Am 18.08.26 um 08:43 schrieb Jammy Huang:
> Hi Thomas,
>
> I'd like to get your thoughts on Jani's point that the kernel should avoid modifying EDID and instead expose the data as-is to userspace.

I agree with Jani that the EDID should best be unmodified. But I've 
found that returning unprocessed EDID makes other components fail. In 
some cases, such as ast, at least the checksum needs to be corrected.

>
> In practice, most monitors I have encountered only contain two EDID blocks, so simply returning block 0 and block 1 already works for the majority of cases.
>
> Given that, would you agree that it may be acceptable to stop modifying the EDID in the driver and only support the blocks the hardware can actually read now?

AFAIK there's currently no clear way to signal an EOF to the EDID 
parser.  Returning any errno code will make it fail hard.   IMHO that 
next best thing is to zero-out the EDID buffer, so that the parser stops 
silently. (Your v3, I think.) That's also easy enough to revert if/when 
the EDID parser gets a real EOF state.

Best regards
Thomas


>
> Best regards,
> Jammy
>
>> Hi Jani,
>>
>> Thanks for your input. So, your suggestion is go back to return -EIO for block >
>> 1, and stop trying to modify the EDID.
>> In this way, we can handle EDID whose blocks is less than 2.
>>
>> BR,
>> Jammy
>>
>>>> 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.
>>> There's a long-term goal to *not* modify the EDID in kernel, but
>>> rather return even the broken EDID extensions to userspace, if that's what
>> the display has.
>>> There are probably a bunch of hurdles in making that happen, but
>>> intentionally returning zeroed out EDID blocks is just adding another
>> unnecessary hurdle.
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>> --
>>> Jani Nikula, Intel

-- 
--
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)

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

[Jammy Huang]

Hi Thomas,

Many thanks for your input. I will provide patch v3 in which zero-out the EDID buffers > 1.

Best regards,
Jammy

> 
> Hi Jammy
> 
> Am 18.08.26 um 08:43 schrieb Jammy Huang:
> > Hi Thomas,
> >
> > I'd like to get your thoughts on Jani's point that the kernel should avoid
> modifying EDID and instead expose the data as-is to userspace.
> 
> I agree with Jani that the EDID should best be unmodified. But I've found that
> returning unprocessed EDID makes other components fail. In some cases, such
> as ast, at least the checksum needs to be corrected.
> 
> >
> > In practice, most monitors I have encountered only contain two EDID blocks,
> so simply returning block 0 and block 1 already works for the majority of cases.
> >
> > Given that, would you agree that it may be acceptable to stop modifying the
> EDID in the driver and only support the blocks the hardware can actually read
> now?
> 
> AFAIK there's currently no clear way to signal an EOF to the EDID
> parser.  Returning any errno code will make it fail hard.   IMHO that
> next best thing is to zero-out the EDID buffer, so that the parser stops
> silently. (Your v3, I think.) That's also easy enough to revert if/when
> the EDID parser gets a real EOF state.
> 
> Best regards
> Thomas
> 
> 
> >
> > Best regards,
> > Jammy
> >
> >> Hi Jani,
> >>
> >> Thanks for your input. So, your suggestion is go back to return -EIO for block
> >
> >> 1, and stop trying to modify the EDID.
> >> In this way, we can handle EDID whose blocks is less than 2.
> >>
> >> BR,
> >> Jammy
> >>
> >>>> 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.
> >>> There's a long-term goal to *not* modify the EDID in kernel, but
> >>> rather return even the broken EDID extensions to userspace, if that's what
> >> the display has.
> >>> There are probably a bunch of hurdles in making that happen, but
> >>> intentionally returning zeroed out EDID blocks is just adding another
> >> unnecessary hurdle.
> >>>
> >>> BR,
> >>> Jani.
> >>>
> >>>
> >>> --
> >>> Jani Nikula, Intel
> 
> --
> --
> 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)
>