On Tue, 17 Mar 2026, Ville Syrjälä <[email protected]> wrote:
> 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 <[email protected]> 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 <[email protected]>
>> >>>> ---
>> >>>> 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.

I've been meaning to nuke edid_filter_invalid_blocks()...

It's a real problem that users report issues with EDID, attach the EDID
from sysfs, but it's not the actual EDID because the kernel modified it.

BR,
Jani.


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

-- 
Jani Nikula, Intel

Reply via email to