Re: [PATCH v2] edid: add support for DisplayID extension (5k resolution)

[address@hidden]

The change to edid_checksum is needed because the DisplayID section has another 
checksum inside for the actually used part of its 128-byte extension block. The 
checksum in this case uses the same algorithm, but for a shorter block. Thus I 
added a parameter to specify the size of the block.

I'll address the rest of your comments in a 3-rd version of this patch.

> On 15 Mar 2021, at 10:44, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>> +typedef struct Timings {
> 
>> +static void generate_timings(Timings *timings, uint32_t refresh_rate,
>> +                             uint32_t xres, uint32_t yres)
> 
> Adding these should be splitted to a separate patch.
> 
>> -static void edid_checksum(uint8_t *edid)
>> +static void edid_checksum(uint8_t *edid, size_t len)
> 
> Why this change?  Also a good candidate for a separate patch.
> 
>> +    if (size >= 384) {
>> +        did = edid + 256;
> 
> "if (size >= 384 && large_screen)" ?
> Also setting did should be next to setting dta.
> 
> 
>       if (did) {
>> +        dummy_displayid(did);
> 
> init_displayid() ?
> 
> Especially if we generate that only in case we actually have a large
> screen so we never have an empty extension section?
> 
> take care,
>  Gerd
>