Mode validation & pixel clock

[Benjamin Herrenschmidt]

Hi !

I've having a problem where X is happily pickup up modes with pixel
clocks way too high for the analog monitor.

The problem is that if you look at common/xf86Mode.c, in
xf86CheckModeForMonitor(), it does:

    if (monitor->DDC) {
	xf86MonPtr DDC = (xf86MonPtr)(monitor->DDC);
	struct detailed_monitor_section* detMon;
	struct monitor_ranges *mon_range;
	int i;

	mon_range = NULL;
	for (i = 0; i < 4; i++) {
	    detMon = &DDC->det_mon[i];
	    if(detMon->type == DS_RANGES) {
		mon_range = &detMon->section.ranges;
	    }
	}
	if (mon_range) {
	    /* mode->Clock in kHz, DDC in MHz */
	    if (mon_range->max_clock < 2550 &&
		 mode->Clock / 1000.0 > mon_range->max_clock) {
		xf86Msg(X_WARNING,
		   "(%s,%s) mode clock %gMHz exceeds DDC maximum %dMHz\n",
		   mode->name, monitor->id,
		   mode->Clock/1000.0, mon_range->max_clock);
	    }
	}
    }

That is, it does detect that the mode is wrong, but doesn't do anything
about it except printing a warning (not very useful).

By simply adding a:

 		return MODE_CLOCK_HIGH;

After the warning, I get a sane behaviour where if I don' specify any
mode in the config file, it will discard automatically modes that are
grossly out of range.

I suppose the problem is that if we do that and end up with a broken
DDC, we might refuse valid modes ... The problem i see here is that the
max_clock isn't part of the monitor infos, unlike max V&H refresh, and
thus cannot be overriden by the config file.

So I suppose we should add the max pixel clock to the Monitor section,
have it default to the max in DDC or some arbitrary value (0 == not
specified maybe) when no DDC is available, and make the above test
actually do more than just print a warning. Is that ok ?

Also, I still have a problem with analog panels. We do tend to ignore
the modes in the EDID block completely, we only use the range infos
there, which means that we tend to pick a mode that is higher than the
panel native one. While this might be ok with CRTs, that doesn't do any
good on LCDs.

Is there any reason why we don't follow the supported mode list in
there ? It does, from experience, contains some crazy modes, but the
highest ones tend to be ok, or at least the second or third from the
top. Crazy heuristic but might work well enough. Users with crazy
displays can always override the mode (maybe we could make a
"IgnoreEDID" option generic in the Monitor section for that).

What do you guys think ? That is probably not enough in the long run, we
need to deal with calculating modes via CVT for flat panels etc... but
that need bigger API changes, since there are lots of cases where
drivers can "probe" the panel size from the firmware or chip state but
not all of the detailed timings (and from experience, CVT gives you a
generally useable mode from that).

Ben.