On Thu, Feb 17, 2005 at 10:52:33AM -0800, Mark Vojkovich wrote:
>On Wed, 16 Feb 2005, David Dawes wrote:
>
>> On Wed, Feb 16, 2005 at 06:07:43PM -0800, Mark Vojkovich wrote:
>> >   It used to be that if you specified a modeline, say "1600x1200" in
>> >the XF86Config file, that modeline would take preference over any
>> >internal modelines of the same name.  This no longer appears to be
>> >the case.  If I have a "1600x1200" modeline in the XF86Config file,
>> >it no longer gets used, but another mode instead (I presume the
>> >internal mode).  I have to name my mode to something else in order
>> >to use it.
>> >
>> >   It seems like the server was changed to ignore modes placed
>> >in the monitor section if they conflict with internal modes.  Was
>> >this change intentional?
>>
>> It works correctly for me.  If explicitly provided modes are not
>> overriding the default modes then it is a bug.  Can you send your
>> log file?
>
>   It appears that what's happening is modes from the monitor's
>edid take precedence over Section "Monitor" overrides.  I specified
>mode "1600x1200" in my SubSection "Display" Modes.  I provided a custom
>modeline in the Section "Monitor":
>
># 1600x1200 @ 79.1 Hz, 98.9 kHz
>   Modeline  "1600x1200" 213.6 1600 1664 1856 2160 1200 1201 1204 1250
>
>but the monitor is reporting 86 Hz, 106 kHz.
>
>(**) NV(0): *Preferred EDID mode "1600x1200": 230.0 MHz, 106.5 kHz, 85.2 Hz

Adding the EDID modes to the set is part of the recent autoconfig enhancements.

>  ...
>
>(**) NV(0):  Mode "1600x1200": 213.6 MHz, 98.9 kHz, 79.1 Hz
>(II) NV(0): Modeline "1600x1200"  213.60  1600 1664 1856 2160  1200 1201 1204 
>1250
>
>   I think the priority should be:  Section "Monitor", EDID, builtin.
>But it appears that it's EDID, Section "Monitor", builtin.

Yes, I agree that the modes specified explicitly in the Monitor section
should have first priority.  The attached patch prevents EDID modes matching
Monitor section modes from being added to the pool, much the same way as
happens already for the built-in default/VESA modes.

Let me know how it goes.

David
Index: xf86Mode.c
===================================================================
RCS file: /home/x-cvs/xc/programs/Xserver/hw/xfree86/common/xf86Mode.c,v
retrieving revision 1.75
diff -u -r1.75 xf86Mode.c
--- xf86Mode.c  17 Feb 2005 03:46:48 -0000      1.75
+++ xf86Mode.c  17 Feb 2005 21:47:19 -0000
@@ -1241,10 +1241,6 @@
     return MODE_OK;
 }
 
-#ifndef MODENAMESIZE
-#define MODENAMESIZE (4 + 1 + 4 + 1)
-#endif
-
 /*
  * xf86ValidateModes
  *
@@ -1494,29 +1490,45 @@
                        hmax = h;
 
                    if (!haveEDIDModes) {
-                       new = xnfcalloc(1, sizeof(DisplayModeRec));
-                       new->type = M_T_DEFAULT | M_T_EDID;
-                       new->Clock = dt->clock / 1000;
-                       new->HDisplay = dt->h_active;
-                       new->HSyncStart = new->HDisplay + dt->h_sync_off;
-                       new->HSyncEnd = new->HSyncStart + dt->h_sync_width;
-                       new->HTotal = new->HDisplay + dt->h_blanking;
-                       new->VDisplay = dt->v_active;
-                       new->VSyncStart = new->VDisplay + dt->v_sync_off;
-                       new->VSyncEnd = new->VSyncStart + dt->v_sync_width;
-                       new->VTotal = new->VDisplay + dt->v_blanking;
-                       new->name = xnfalloc(MODENAMESIZE);
-                       snprintf(new->name, MODENAMESIZE, "%dx%d",
-                                new->HDisplay, new->VDisplay);
-                       if (firstPreferred && firstDetailed) {
-                           new->type |= M_T_PREFER;
-                           preferredName = new->name;
+                       char *newName = NULL;
+
+                       xasprintf(&newName, "%dx%d",
+                                 dt->h_active, dt->v_active);
+                       if (newName && !xf86ModeIsPresent(newName,
+                                                         monitor->Modes,
+                                                         0, M_T_DEFAULT)) {
+                           new = xcalloc(1, sizeof(DisplayModeRec));
+                           if (new) {
+                               new->type = M_T_DEFAULT | M_T_EDID;
+                               new->Clock = dt->clock / 1000;
+                               new->HDisplay = dt->h_active;
+                               new->HSyncStart = new->HDisplay +
+                                                       dt->h_sync_off;
+                               new->HSyncEnd = new->HSyncStart +
+                                                       dt->h_sync_width;
+                               new->HTotal = new->HDisplay + dt->h_blanking;
+                               new->VDisplay = dt->v_active;
+                               new->VSyncStart = new->VDisplay +
+                                                       dt->v_sync_off;
+                               new->VSyncEnd = new->VSyncStart +
+                                                       dt->v_sync_width;
+                               new->VTotal = new->VDisplay + dt->v_blanking;
+                               new->name = newName;
+                               newName = NULL;
+                               if (firstPreferred && firstDetailed) {
+                                   new->type |= M_T_PREFER;
+                                   preferredName = new->name;
+                               }
+                               xf86DrvMsgVerb(scrp->scrnIndex, X_INFO, 4,
+                                   "Adding detailed EDID mode %s to monitor "
+                                   "(preferred: %s).\n",
+                                   new->name,
+                                   new->type & M_T_PREFER ? "yes" : "no");
+                               xf86AddModeToMonitor(monitor, new);
+                           }
                        }
-                       xf86DrvMsgVerb(scrp->scrnIndex, X_INFO, 4,
-                           "Adding detailed EDID mode %s to monitor "
-                           "(preferred: %s).\n",
-                           new->name, new->type & M_T_PREFER ? "yes" : "no");
-                       xf86AddModeToMonitor(monitor, new);
+                       if (newName)
+                           xfree(newName);
                    }
 
                    if (digital || (firstPreferred && firstDetailed)) {

Reply via email to