[PATCH edid-decode v2] Fix manufacture/model week/year parsing

[Tormod Volden]

On Mon, Dec 19, 2011 at 1:45 AM, Jeremy Huddleston <jeremyhu at apple.com> wrote:
> <lazy>Can you please link the EDID specification, I just looked on wikipedia which is not quite authoritative</lazy>

Yes, I even edited Wikipedia the last time to correct the year
encoding :) BTW,
http://en.wikipedia.org/wiki/Extended_display_identification_data only
explains 1.3 in detail. I found the 1.3 spec through that wiki page
and the 1.4 verification guide by googling, but I am not sure the
locations are meant/allowed to be public so I will not link to them.
The file names are E-EDID%20Standard.pdf and EEDIDverifGuideRa.pdf

>
> The changes look logical to me assuming the fields represent the values I think they do (based on wikipedia).  Still, I'm confused about this > 2006 issue.  Is that somewhere in the spec?

Yes, see section 3.2.4 of the latter document (table 3-4). Values
0-0x0F (1990-2005) are reserved (do not use).

> What if firmware updates cause the display to support EDID v1.4 but it was manufactured in 2005?

I think the authors of the spec did not think about that possibility
:) But in practice, are there any monitors manufactured before 2006
that receive firmware updates?

>
> To check for <1.4, you should change "edid[0x13] < 4" to "(edid[0x12] == 1 && edid[0x13] < 4)" ... your existing check would not work for EDID version 2.0.

Yes, this was sloppy coding. Anyway, to do this properly I think there
is no way around moving the EDID version parsing above the year/week
parsing, and then use the claims_one_point_four that will be defined.
I think it is logic to evaluate the EDID version as soon as possible,
and certainly before any fields which might depend on the version. I
will submit a patch for the rearrangement and hope nobody will
protest, and rebase this patch as a follow-up.

Cheers,
Tormod


>
> If you change the 1.4 check (and that matches the spec), or if you remove the 1.4 check,
>
> Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>
>
> On Dec 17, 2011, at 15:24, Tormod Volden wrote:
>
>> From: Tormod Volden <debian.tormod at gmail.com>
>>
>> For instance, the handling of week=0xff or week=0 was not working,
>> as could be seen from the data/sharp-* examples.
>>
>> Only EDID version 1.4 requires the year to be 2006 or later.
>>
>> Signed-off-by: Tormod Volden <debian.tormod at gmail.com>
>> ---
>>
>> v2: We should not require year >= 2006 if version < 1.4.
>>
>> This can be made prettier if we do the EDID version
>> parsing before the week and year parsing, but I will leave it like
>> this for now.
>>
>> Tormod
>>
>>
>> edid-decode.c |   22 +++++++++++-----------
>> 1 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/edid-decode.c b/edid-decode.c
>> index ab4bde8..9c89528 100644
>> --- a/edid-decode.c
>> +++ b/edid-decode.c
>> @@ -953,18 +953,18 @@ int main(int argc, char **argv)
>>
>>     time(&the_time);
>>     ptm = localtime(&the_time);
>> -    if (edid[0x10] < 55 || edid[0x10] == 0xff) {
>> +    if (edid[0x10] < 0x37 || edid[0x10] == 0xff)
>>       has_valid_week = 1;
>> -     if (edid[0x11] > 0x0f) {
>> -         if (edid[0x10] == 0xff) {
>> -             has_valid_year = 1;
>> -             printf("Made week %hd of model year %hd\n", edid[0x10],
>> -                    edid[0x11]);
>> -         } else if (edid[0x11] + 90 <= ptm->tm_year) {
>> -             has_valid_year = 1;
>> -             printf("Made week %hd of %hd\n", edid[0x10], edid[0x11] + 1990);
>> -         }
>> -     }
>> +    if (edid[0x11] + 90 <= ptm->tm_year) {
>> +     /* Year must be 2006 or later from EDID version 1.4 */
>> +     if (edid[0x11] > 0x0f || edid[0x13] < 4)
>> +         has_valid_year = 1;
>> +     if (edid[0x10] == 0xff)
>> +         printf("Model year %hd\n", edid[0x11] + 1990);
>> +     else if (edid[0x10] == 0)
>> +         printf("Manufactured year %hd\n", edid[0x11] + 1990);
>> +     else
>> +         printf("Manufactured week %hd of %hd\n", edid[0x10], edid[0x11] + 1990);
>>     }
>>
>>     printf("EDID version: %hd.%hd\n", edid[0x12], edid[0x13]);
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>
>