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

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 17.03.26 um 09:12 schrieb Ville Syrjälä:
> On Tue, Mar 17, 2026 at 08:41:35AM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 17.03.26 um 08:09 schrieb Ville Syrjälä:
>>> On Mon, Mar 16, 2026 at 01:38:22PM +0200, Jani Nikula wrote:
>>>> On Mon, 16 Mar 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>>> Hi Jammy
>>>>>
>>>>> Am 13.03.26 um 11:04 schrieb Jammy Huang:
>>>>>> DisplayPort supports edid at most 256 bytes. Thus, allow it to fetch
>>>>>> edid block 0 and 1.
>>>>>>
>>>>>> 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.
>>>>>> ---
>>>>>>     drivers/gpu/drm/ast/ast_dp.c | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
>>>>>> index 9d07dad358c..c938e1d6b1d 100644
>>>>>> --- a/drivers/gpu/drm/ast/ast_dp.c
>>>>>> +++ b/drivers/gpu/drm/ast/ast_dp.c
>>>>>> @@ -88,7 +88,7 @@ static int ast_astdp_read_edid_block(void *data, u8 *buf, unsigned int block, si
>>>>>>     	int ret = 0;
>>>>>>     	unsigned int i;
>>>>>>     
>>>>>> -	if (block > 0)
>>>>>> +	if (block > 1)
>>>>>>     		return -EIO; /* extension headers not supported */
>>>>> But see the code at [1]. It clears the number of extensions to zero and
>>>>> updates the checksum accordingly. This is required to make the shortened
>>>>> EDID work with DRM. If you leave this as-is, it will still clear support
>>>>> for any extension in block 1.
>>>>>
>>>>> See the table 2.4 in the VESA EDID 1.4 standard for the semantics. For
>>>>> 1.3, if the number of blocks is >2, the first extensions is a 'block map
>>>>> of the extensions'. This is useless, as it's not a data extension in
>>>>> itself.  In 1.4, the block map is optional. That code should clear the
>>>>> EDID's number of extensions to 0 or 1, depending on whether there is a
>>>>> block map to be expected.
>>>> I think long-term the goal should be for the kernel to not modify the
>>>> EDID, at all.
>>> I think as a short term goal it would be much better if all EDID
>>> mangling would be done by drm_edid.c rather than individual drivers.
>> We don't fix the EDID here, but work around a hardware limitation. I
>> guess we could fix this automatically near edid_block_read() [1] if the
>> driver clearly communicates this issue.  The read loop could then fix
>> the header's checksum by itself.
> edid_filter_invalid_blocks() already has the code to update
> the extension block count and fix up the checksum.

Great.

>
> So seems to me all we'd need is some way for the driver to
> indicate it simply cannot read the requested block (based on
> which edid_read_block() would return some other value than
> EDID_BLOCK_READ_FAIL), and then the fixup happens naturally.

I was thinking of something like: return -ENXIO from the driver's helper 
if the read is beyond the available blocks, and then return something 
like EDID_BLOCK_READ_EOF, which triggers the header fix-up.  Note that 
returning -EIO would still signal a real I/O error.

Best regards
Thomas

>
> Hmm. What happens if the driver returns "success" for the
> read, but just leaves the entire block zeroed? Looks to me
> like we should then end up with status==EDID_BLOCK_ZERO
> and the block will get treated as invalid and filtered out.
>
>> Best regards
>> Thomas
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.19.8/source/drivers/gpu/drm/drm_edid.c#L2424
>>
>> -- 
>> --
>> 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)
>>

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