Flat panel monitors that return EDID V1.3 don't get the h/v sync they need. The data in the monitor's EDID PROM for the Standard Timing Identificaiton slot #0 (byte 26) is (interpretation based on page 17 of EDID spec): byte 0: 0xd1: 0xd1 = 209 => (209 + 31) * 8 = 1920 (horizontal size) byte 1: 0x00: 00b => 16:10 aspect ratio => 1920 * 10 / 16 = 1200 (vert size) 0000000b => refresh rate of 0 + 60 => 60Hz Which is correct. The sticking point, I think, is how to interpret the 00b in byte 1. In EDID versions prior to 1.3, it indicated an aspect ratio of 1:1. In EDID 1.3 and higher, it indicates an aspect ratio of 16:10. ----- I'll take this bug, since I think I know how to fix it.
Egbert is the original author of the code in question.
Created attachment 1122 [details] [review] [FIXED_X11R68x] Fix Please test this (I haven't gotten around to it yet) and report your findings. I will commit it to freedesktop. It's on my TODO list.
New code does not produce better results on nv driver. I will now go to NV and see what it up there.
New code does not produce better results on nv driver. I will now go to NV and see what it up there. (II) NV(0): Probing for EDID on I2C bus A... (II) NV(0): I2C device "DDC:ddc2" registered at address 0xA0. (II) NV(0): I2C device "DDC:ddc2" removed. (--) NV(0): DDC detected a DFP: (II) NV(0): Manufacturer: SUN Model: 586 Serial#: 808464432 (II) NV(0): Year: 2004 Week: 26 (II) NV(0): EDID Version: 1.3 (II) NV(0): Digital Display Input (II) NV(0): Max H-Image Size [cm]: horiz.: 51 vert.: 32 (II) NV(0): Gamma: 2.50 (II) NV(0): DPMS capabilities: Off; RGB/Color Display (II) NV(0): First detailed timing is preferred mode (II) NV(0): redX: 0.633 redY: 0.353 greenX: 0.295 greenY: 0.596 (II) NV(0): blueX: 0.141 blueY: 0.082 whiteX: 0.318 whiteY: 0.348 (II) NV(0): Supported VESA Video Modes: (II) NV(0): 720x400@70Hz (II) NV(0): 640x480@60Hz (II) NV(0): 640x480@67Hz (II) NV(0): 640x480@72Hz (II) NV(0): 640x480@75Hz (II) NV(0): 800x600@56Hz (II) NV(0): 800x600@60Hz (II) NV(0): 800x600@72Hz (II) NV(0): 800x600@75Hz (II) NV(0): 832x624@75Hz (II) NV(0): 1024x768@60Hz (II) NV(0): 1024x768@70Hz (II) NV(0): 1024x768@75Hz (II) NV(0): 1280x1024@75Hz (II) NV(0): 1152x870@75Hz (II) NV(0): Manufacturer's mask: 0 (II) NV(0): Supported Future Video Modes: (II) NV(0): #0: hsize: 1920 vsize 1920 refresh: 60 vid: 209 -----------------------------------^^^^ this needs to be 1200. (II) NV(0): #1: hsize: 1920 vsize 1080 refresh: 60 vid: 49361 (II) NV(0): #2: hsize: 1280 vsize 1024 refresh: 60 vid: 32897 (II) NV(0): #3: hsize: 1280 vsize 1024 refresh: 76 vid: 36993 (II) NV(0): #4: hsize: 1600 vsize 1200 refresh: 60 vid: 16553 (II) NV(0): #5: hsize: 1152 vsize 921 refresh: 66 vid: 34417
+ case RATIO1_1: y = _HSIZE1(x) * ((v->version>1||v->revision>2) ? 10/16 : 1); \ + break; \ Will that work in integers? Won't it precompute 10/16 as 0? Would this work better? + case RATIO1_1: y = ((v->version>1||v->revision>2) ? (_HSIZE1(x) * 10)/16 : _HSIZE1(x)); \ + break; \
Alans fix gets us much closer. Looks like we lost mode line #0 (II) NV(0): Supported Future Video Modes: (II) NV(0): #1: hsize: 1920 vsize 1080 refresh: 60 vid: 49361 (II) NV(0): #2: hsize: 1280 vsize 1024 refresh: 60 vid: 32897 (II) NV(0): #3: hsize: 1280 vsize 1024 refresh: 76 vid: 36993 (II) NV(0): #4: hsize: 1600 vsize 1200 refresh: 60 vid: 16553 (II) NV(0): #5: hsize: 1152 vsize 921 refresh: 66 vid: 34417
With Alans fix and removeing the VALID_TIMING macro, I can get the correct 'Supported Future Video Modes' info. So, #define _VALID_TIMING(x) ((x[0] != 0x01 && x[1] != 0x01) \ && (x[0] != 0x00 && x[1] != 0x00) \ && (x[0] != 0x20 && x[1] != 0x20) ) is catching the 209 dec. value. Still drilling.
O.k. WIth the following 2 changes to edid.h, I was able to get the correct output. #define _VALID_TIMING(x) (((x[0] != 0x01) && (x[1] != 0x01)) \ || ((x[0] != 0x00) && (x[1] != 0x00)) \ || ((x[0] != 0x20) && (x[1] != 0x20)) ) and #define _VSIZE1(x,y,r) switch(RATIO(x)){ \ case RATIO1_1: y = ((v->version>1||v->revision>2) ? ((_HSIZE1(x)*10)/16) : _HSIZE1(x)); \ break; \ With these fixes in, the code works correctly, more testing is needed.
Yes, you're right. The other lines which do '* 3 / 4' do the multiplication first, then the division. I didn't pay any attention to the type of the variables. Stupid me.
#define _VALID_TIMING(x) (((x[0] != 0x01) && (x[1] != 0x01)) \ || ((x[0] != 0x00) && (x[1] != 0x00)) \ || ((x[0] != 0x20) && (x[1] != 0x20)) ) is still wrong. It sould be valid unless (x0==1 and x1==1) or (x0==0 and x1==0) or (x0==0x20 and x1==0x20): #define _VALID_TIMING(x) !(((x[0] == 0x01) && (x[1] == 0x01)) \ || ((x[0] == 0x00) && (x[1] == 0x00)) \ || ((x[0] == 0x20) && (x[1] == 0x20)) ) or #define _VALID_TIMING(x) (!((x[0] == 0x01) && (x[1] == 0x01)) \ && !((x[0] == 0x00) && (x[1] == 0x00)) \ && !((x[0] == 0x20) && (x[1] == 0x20)) ) or #define _VALID_TIMING(x) (((x[0] != 0x01) || (x[1] != 0x01)) \ && ((x[0] != 0x00) || (x[1] != 0x00)) \ && ((x[0] != 0x20) || (x[1] != 0x20)) ) Could you please try this?
#define _VALID_TIMING(x) !(((x[0] == 0x01) && (x[1] == 0x01)) \ || ((x[0] == 0x00) && (x[1] == 0x00)) \ || ((x[0] == 0x20) && (x[1] == 0x20)) ) This one works, and is easyest to read....
Jay, thanks for your help and all the testing. I've just committed the code. Please update from CVS head. If problems arise please reopen.
Comment on attachment 1122 [details] [review] [FIXED_X11R68x] Fix Thomas Winischhofer suggested limit support of DFP to EDID >=v1.2 instead of > v1.2. However the 'VESA ENHANCED EXTENDED DISPLAY IDENTIFICATION DATA Implementation Guide' suggests to limit this the way it is currently implemented.
The issue in the last comment got resolved in a conversation on release-wranglers.
Comment on attachment 1122 [details] [review] [FIXED_X11R68x] Fix Approval for X11R6.8.x stable branch granted in the 2004-12-13 release-wranglers phone call.
Comment on attachment 1122 [details] [review] [FIXED_X11R68x] Fix Patch checked-in into X11R6.8.x stable branch: /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.365.2.105; previous revision: 1.365.2.104 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/programs/Xserver/hw/xfree86/ddc/edid.h,v <-- edid.h new revision: 1.2.4.1; previous revision: 1.2 /cvs/xorg/xc/programs/Xserver/hw/xfree86/ddc/interpret_edid.c,v <-- interpret_edid.c new revision: 1.3.2.1; previous revision: 1.3 /cvs/xorg/xc/programs/Xserver/hw/xfree86/ddc/print_edid.c,v <-- print_edid.c new revision: 1.2.4.1; previous revision: 1.2 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. Mailing the commit message to xorg-commit@lists.freedesktop.org...
Created attachment 1576 [details] [review] [FIXED_X11R68x] Patch to bring 6.8.2 branch in line with CVS head Sorry - I did not realize the patch as attached in bugzilla did not include the final two changes discussed in the comments. This additional patch, when applied on top of the current 6.8.2 branch brings it in line with the correct code as integrated into HEAD branch.
Comment on attachment 1576 [details] [review] [FIXED_X11R68x] Patch to bring 6.8.2 branch in line with CVS head Requesting approval for 6.8.2 branch since incomplete patch was provided in bugzilla missing two small changes that went into the commit to CVS HEAD.
Comment on attachment 1576 [details] [review] [FIXED_X11R68x] Patch to bring 6.8.2 branch in line with CVS head Approved for commit into X11R6.8.x stable branch in the 2005-01-10 Xorg release-wranglers phone call.
Comment on attachment 1576 [details] [review] [FIXED_X11R68x] Patch to bring 6.8.2 branch in line with CVS head Patch checked-in into X11R6.8.x stable branch: /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.365.2.117; previous revision: 1.365.2.116 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. /cvs/xorg/xc/programs/Xserver/hw/xfree86/ddc/edid.h,v <-- edid.h new revision: 1.2.4.2; previous revision: 1.2.4.1 cvs commit: Using deprecated info format strings. Convert your scripts to use the new argument format and remove '1's from your info file format strings. Mailing the commit message to xorg-commit@lists.freedesktop.org...
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.