Bug 5386

Created attachment 4156 [details] [review]
edid-inject-2.patch

this doesn't work, but it's a start.

*** Bug 1129 has been marked as a duplicate of this bug. ***

*** Bug 1438 has been marked as a duplicate of this bug. ***

*** Bug 4918 has been marked as a duplicate of this bug. ***

*** Bug 2677 has been marked as a duplicate of this bug. ***

*** Bug 2989 has been marked as a duplicate of this bug. ***

Prerequisites:
- dumping edid.
- loading dumped edid.
This way we can have testcases quite easily. We wait until something breaks for
someone, and have them hand us their dump.

What is this modeline "generation" going to be based on, and in what order?
1) the prefered mode provided by recent monitors
2) the information found in vesas VTB-EXT
(http://www.vesa.org/public/EDID%20EXTENSIONS/VTBextRa.pdf)
3) the vesa modes provided (edid speak: "established timing" and "Manufacturer's
Reserved Timing")
4/fallback) cvt generation on the basis of the ranges detected. The gtf weights
can apparently also be provided, so cvt generation should take that into account
too.

Also: Unichrome driver would be really helped if the EDID block was parsed to
return a max provided resolution. This is how the panel size is found on some
rare unichrome laptops (acer aspire 135x again). When you're parsing everything
else then this is just a side-effect.

Also: Some quick yet waterproof way to know that a gathered EDID block is new,
so that a monitor change can be detected. Some sort of hash perhaps, or are
bytes 08-17 (aka complete serial number) deemed sufficient?

Oh, another thought: the generated modelist should become part of the Modes list
provided as part of MonRec/Ptr.

We might also want to use MonRec more dynamically: for instance:
When a DDC capable monitor is removed and another one attached, there is no real
"need" to toss away the old info. There's a good chance that the same monitor
resurfaces soon, be it on the exact same device and head, be it somewhere else.

We might want to carry a scrnIndex around, so that we know which pScrn owns a
given monitor at any given time. When that scrnIndex is -1, then we know that
this monitor is not currently present.

Whatever you do, don't change the ABI, ie don't add modes in "ValidateModes"
like XFree86 4.5+ does.

(In reply to comment #9)
> Whatever you do, don't change the ABI, ie don't add modes in "ValidateModes"
> like XFree86 4.5+ does.

the easiest way to do this for all drivers at once, afaict, is to add it to the
end of xf86DoEDID_DDC{1,2}; does this count as breaking the ABI for you?

Created attachment 4233 [details] [review]
DumpDDC/LoadDDC: Probably a horrible bit of code, but loads of fun :)

Not sure about portability of this. But apart from the path (/tmp/ so #ifndef
WIN32 it?) it doesn't seem too unacceptable compared to what's being done
throughout X.

Basically:
Option "DumpDDC" "True" will dump to /tmp/???-????.edid with the first 3 the
manufacturer name bytes, and the last 4 the model number in hex.
Option "LoadDDC" "/somewhere/something.edid" will then forgo trying to gather
the EDID block from the monitor itself, and just try to load 128bytes from
something.edid.

(In reply to comment #10)
> (In reply to comment #9)
> > Whatever you do, don't change the ABI, ie don't add modes in "ValidateModes"
> > like XFree86 4.5+ does.
> 
> the easiest way to do this for all drivers at once, afaict, is to add it to the
> end of xf86DoEDID_DDC{1,2}; does this count as breaking the ABI for you?
Yeah, we can pull all of this off without really needing to break the ABI.

Imho we need to move the DDC code to have xf86DoEDID_DDC? to plainly get the
block and do some basic stuff with it.

Since most drivers (next comment) call xf86SetDDCproperties immediately after
getting the edid block, we need to hook off of that, and it's pretty much
perfect for what we need to do.

xf86SetDDCproperties currently only hooks the xf86MonPtr to pScrn->monitor->DDC,
and then makes the rawData available as a root window property. I don't know
anything using this as a root window property, so this might be ditchable.

What it should do is not only hook pScrn->monitor->DDC to the provided
xf86MonPtr, but also try to fill pScrn->monitor as best as it can from the DDC
info (when config hasn't already provided this). This means that the DDC
handling code in xf86Mode.c (in xf86CheckModeForMonitor and xf86ValidateModes)
can be dropped in favour of this too (a good thing for the monstrously big
xf86ValidateModes).

ddcProperty.c is where it's at :) I'm pretty much testing/playing/writing now,
nothing much worth showing. The reducedblanking bit is done decently already
(what enormous progress :p), i'm unable to check the ranges code (bloody crt
doesn't provide that) and i'm trying to generate DisplayModes, as that's a
prerequisite to trying to guess ranges. Will post as soon as there's something
worth posting again :)

drivers needing work:
apm: doesn't call xf86SetDDCproperties
cyrix: PROBE_DETECT only.
glint: doesn't call xf86SetDDCproperties, DDC after validation.
i128: sets pScrn->monitor->DDC directly, doesn't call xf86SetDDCproperties
i740: Should ditch the #defines surrounding DDC code and DDC handling should be
moved to before validation
i810: sets pScrn->monitor->DDC directly, doesn't call xf86SetDDCproperties
i830: calls xf86SetDDCproperties properly, but pokes at MonRec from the edid
block itself. Is probably ok, as it probably just overwrites values (check after
implementation has taken on some shape).
nsc: PROBE_DETECT only.
rendition: ok, but should get a once-over (have rv2100 somewhere myself so np).
sis: pokes at MonRec from the edid block itself.
sisusb: probably does have the DDC bus implemented.
sunffb: i2c bus implemented, but no real DDC code.
tdfx: DDC handling after validation. (have voodoo 3k)
voodoo: includes xf86DDC.h but not even an i2c bus.

All in all, very doable and not much work. Verification is going to be most of
the work. i830 and sis might want to ignore xf86SetDDCproperties though.

Created attachment 4242 [details] [review]
Current DDC -> pScrn->monitor code.

This can be split out quite easily into three different patches:
- Load/DumpDDC
- moving out of xf86CVTMode from cvt.c to xf86cvt.c
- DDC info converted to pScrn->monitor ranges and Modes.

- cvt code: the only way i'm apparently (thanks ajax :)) able to have
xf86CVTMode linked into the server and into the cvt generator at the same time
seems to be by linking ../../common/xf86cvt.o (with only xf86CVTMode and no
further dependencies) into utils/cvt/cvt directly. I'm not sure wether that is
an acceptable solution.
- ddc code:
 - Some never tested bugs in the old ddc code were found. We never really cared
much about ddc before it seems.
 - all direct DDC handling in xf86Mode.c is dropped (welcome drop in case of   
   validateModes).
 - ranges are used when found and when unset previously.
 - a modelist is generated.
   - through established timings from a table of vesa modes.
   - through detailed timings directly.
   - generated with xf86CVTMode from standard timings and detailed standard timings.
 - modelist is used to set ranges when unset previously.
 - modelist is added to pScrn->monitor->modes


TODO:
 - DisplayModeRec->type should gain M_T_DDC_*** types, and a whole priority
   system in xf86Mode.c needs to be devised so that:
   - the preferred EDID timing might eventually make the Modes directive of the
     Screen section might become unneccessary.
   - user modes have priority over DDC modes, which have priority over built-in  
     modes.
   - duplicate modes (of which there are a lot now) can be scrapped keeping only
     higher priority ones.
 - fix up drivers.
 - rework edid parsing and add edid extensions.

Extra TODO:
- gather maximum resolution from generated modes list.
  - this means that i can remove the native resolution from edid retrieval from
the unichrome drivers panel code.
  - also should become the upper limit of resolution when not using a Modes
directive (Modes might become optional in future as preferred edid timing will
be chosen first).

The last one was dug up by barry scott on xfree86 4.5.0: an older unichrome
version (with only a limited number of good dotclocks - newer version generates
good modes instead of relying on a table) refused the native resolution of a
panel (preferred timing: 1280x768). Xfree86 4.5.0 then tried to set 1280x960
instead, a mode that the panel was unable to handle. And so the driver developer
was blamed :)

Maximum resolution from EDID isn't a good idea... it will break on a lot of
CRTs. CRTs should, I think, default to "default" modes that are standard modes
that fit in the EDID nicely with some margin, the user can always manually try
to push them up.

I'm not even sure finding the max resolution is a good idea for flat panels... I
think you should really mark the first detailed timing entry from the EDID and
use that as your native resolution.

(In reply to comment #17)
> Maximum resolution from EDID isn't a good idea... it will break on a lot of
> CRTs. CRTs should, I think, default to "default" modes that are standard modes
> that fit in the EDID nicely with some margin, the user can always manually try
> to push them up.
> 
> I'm not even sure finding the max resolution is a good idea for flat panels... I
> think you should really mark the first detailed timing entry from the EDID and
> use that as your native resolution.
> 
But then we run into the very same issue again: there's nothing in edid that
says wether the display device is a panel or a CRT. And relying on the digital
bit here isn't an option as a very large number of panels get sold analog only,
and even those with DVI unset the digital bit when using the analog input.

Basically, as with all the edid stuff, the max resolution isn't a hard limit. We
simply can't put full and complete trust in edid information. What i want to see
happen with the max resolution is that the mode code never chooses, on its own,
to use a resolution higher than what edid provides.

My crt here reports a max resolution of 1600x1200, the preferred timing is
1280x1024, this while it can handle 1920x1440 (yuck). If I want 1920x1440, then
I'll have to provide it in the Modes directive, in the Display subsection of the
Screen section, like i'd do now. If this is not present, then any resolution
higher than 1600x1200 should get scrapped.

And even then, only built-in modes get scrapped, higher priority modes, such as
those coming from a Modes Section or the driver do not get scrapped. It all
boils down to the same type priority again.

Preferred timing should indeed have the highest priority, save user and driver
defined modes. But the problem here was that xfree86 4.5.0 for some reason chose
a higher resolution when the prefered timing was flagged as bad by the driver.
So this is a user-irritation which we can avoid now.

*** Bug 5715 has been marked as a duplicate of this bug. ***

(In reply to comment #18)
> But then we run into the very same issue again: there's nothing in edid that
> says wether the display device is a panel or a CRT. And relying on the digital
> bit here isn't an option as a very large number of panels get sold analog only,
> and even those with DVI unset the digital bit when using the analog input.
> 
> Basically, as with all the edid stuff, the max resolution isn't a hard limit. We
> simply can't put full and complete trust in edid information.

This is an important point you make here.  I'm not sure how many displays
have been sold which have invalid or incorrect EDID data, but over the
years I've seen a number of them creep up in bug reports.  One issue that
comes to mind in particular is a number of Sony displays which were all
made with the same serial number, even though the displays were very
different.  SNY0000 or SNY0001 seems to ring a bell.

I think there's no question that we should try to use EDID data whenever
it is available, and try to use it in the smartest way possible, but there
are always likely to be corner cases due to broken hardware.  It's probably
best to simply handle those cases with blacklisting or some other kludges,
which while ugly, seem to be required in general in hardware drivers et al.
in order to work around hardware/firmware design flaws in consumer goods,
without penalizing the user for having bad hardware.  ;o)


(In reply to comment #7)
> Also: Some quick yet waterproof way to know that a gathered EDID block is new,
> so that a monitor change can be detected. Some sort of hash perhaps, or are
> bytes 08-17 (aka complete serial number) deemed sufficient?

I wouldn't rely on all manufacturers properly setting the hardware ID or
serial number on anything, including displays.  As mentioned above, some
displays have the same ID for example due to manufacturer stupidity.  It
seems likely that someone could easily goof up any data.  There are reports
of hard drives for example which all have the same serial number, even though
they're supposed to be unique.

Hardware manufacturers suck ass it seems.  Film at 11.  ;)

Well, the manufacturer + a signature (md5 ?) of the EDID block (minus the serial
number) might do the trick to identify them in near to all cases.

We'll need a database of monitors. We don't have do have it in the x server
project (could be some separate module in CVS or whatever) but I think we do
need it, with things like EDID override or even replacement blocks, whatever we
come up with and additional infos not covered by EDID in most case (well, they
are covered by extended EDID which is rarely present) like subpixel ordering.

What about some XML-like storate ? sounds quite suited for that, so we can add
arbitrary new "attributes" to monitors in some structured way... I'd still vote
for one monitor=one file though.

To make things perfect, we could look into importing whatever files monitor
manufacturers provide on their windows install CDs ... If a given monitor isn't
in the database and the user knows where to get that file (or has an accessible
windows install on the machine), we could have a tool to import that and submit
the data back to some robot email address that adds it to the database... would
there be any copyright issue on such things ? I doubt it ...

Ben.

(In reply to comment #21)
> Well, the manufacturer + a signature (md5 ?) of the EDID block (minus the serial
> number) might do the trick to identify them in near to all cases.
> 
Yes, you're right, that should do the trick. I wouldn't care about serial number
though, the check should be quick so that it can be done "often". If the monitor
has changed, even if it's the same make and model, we might as well go through
the motions too. We just don't want to fire up the whole (future) modesetting
stack at random.

In fact, it might be better if we just get make, model and serial, and stop
reading there if they're the same. Anyway, all of this is pretty much in the
future, it requires better handling of output devices first imho, as it is part
of reprobing all outputs and seeing wether anything changed.

> We'll need a database of monitors. We don't have do have it in the x server
> project (could be some separate module in CVS or whatever) but I think we do
> need it, with things like EDID override or even replacement blocks, whatever we
> come up with and additional infos not covered by EDID in most case (well, they
> are covered by extended EDID which is rarely present) like subpixel ordering.
> 
> What about some XML-like storate ? sounds quite suited for that, so we can add
> arbitrary new "attributes" to monitors in some structured way... I'd still vote
> for one monitor=one file though.
> 
> To make things perfect, we could look into importing whatever files monitor
> manufacturers provide on their windows install CDs ... If a given monitor isn't
> in the database and the user knows where to get that file (or has an accessible
> windows install on the machine), we could have a tool to import that and submit
> the data back to some robot email address that adds it to the database... would
> there be any copyright issue on such things ? I doubt it ...

I looked up my DVI-Ded CRT a few weeks ago. Those infs are very human readable.
The .inf for this monitor only held: manufacturer and  device IDS, maximum
resolution and "DPMS = TRUE", very little info altogether, and without digging
things out properly, the info for the other monitors was just as sparse.

The max resolution was set at 1920x1440, with the edid having a max resolution
of 1600x1200. In this case, a 19" CRT, i'd very much prefer not ever
encountering 1920x1440 at all, I even steer clear of 1600x1200. So in the case
of this specific monitor, i wouldn't include it in a database at all, except to
flag it being "incapable" of reduced blanking (which is just a one-liner in the
DDC code actually - this does seem to be a one off).

I'd be using a real and proper database project side, and dump that to xml or
whatever (as long as it's human editable) for distribution. You will also
require a web front end for viewing entries, and you will probably require a
form for users to fill in. When all that's done, you'll be amazed at how
disappointingly little reports you get, because unless something goes wrong,
users don't care :)

Imho, you might as well mail xorg@... and start setting up an fd.o project for
it already, with a mailing list, and get discussions started. Ajax, i believe
that rh internal document of yours is about this too, when can you release this
and get things rolling?

I am going to repeat myself once more though: doing a complete monitor database
is; 1) huge, especially if you don't limit yourself to broken monitors or very
sparse missing information. 2) a very boring and very timeconsuming job, that
you'd have to be willing to do for years and years. I have some experience
waiting for user reports on subsystem ids and digging out .tw sites for
northbridge info; a monitor database is just about the last thing i'd ever
volunteer for, and i'm rewriting the tseng driver.

But maybe it is time that all those professing this (this is not towards you or
ajax, because i know that you actually mean business) start getting their hands
dirty.

Comment on attachment 4233 [details] [review]
DumpDDC/LoadDDC: Probably a horrible bit of code, but loads of fun :)

Keep this. Splitting up #4242.

Created attachment 4526 [details] [review]
Move xf86CVTmode from utils/cvt/cvt.c to common/xf86cvt.c.

This moves the CVT generator routine to a file of it's own, so that it can be
linked into the server and into the cvt util at the same time. This provides
xf86CVTMode() to the whole of the server and to all the drivers that wish to
make use of this.

Created attachment 4527 [details] [review]
xf86DDCMonitorSet: Fill MonRec from DDC information.

This patch is the second leg of the EDID injection.

This code hooks xf86DDCMonitorSet off xf86SetDDCproperties, and fills out the
MonRec with information retrieved during the parsing of the EDID block.

Depends on xf86CVTMode (attachment 4526 [details] [review]). Adds a lot of duplicate modes with
normal use. Requires DisplayModeRec.type expansion and extended type handling
before this is acceptable.

Also a note about max res & digital vs. analog. You said that monitors tends to
be unreliable in their EDID infos telling us wether they are analog or digital.
That is true, however, most drivers know fairly well if a given monitor is
hooked using an analog or a digital connection, which is a good info to have. I
would expect pretty much any monitor hooked to a digital connection to be
happiest with the first detailed timing in the EDID block as a default mode. It
would be nice if drivers could at least do that...

That is a very good point that i hadn't considered.

My thoughts about it are the following:
- There will be instances where the pI2CBus might be the same for the DVI device
as for the CRT, while both might be different and not worked into the same
DVI-I. Sure, bad vendor, but most of this bug is about bad vendors, and we will
run into the same problem :)
- Requires more clued drivers and good handling of output devices (which is
neccessary anyway, but will be a bit long in the tooth).
- This shows the strength of the xf86SetDDCProperties usage over Dawes'
xf86validateModes code: The driver has _full_ control over wether or not it uses
the EDID block it retrieved, wether or not it uses the standard handling, and,
even if it uses the standard handling, wether or not to change something the
standard handling set for it.

About the last bullet, a normal implementation would then become: (PreInit)
- Set up pI2CBusses.
- xf86DoEDID_DDC1/2
- (check for output devices)
- xf86SetDDCproperties (translation; run xf86DDCMonitorSet on pScrn->monitor)
- alter pScrn->monitor if so desired. In case of the reduced blanking: the
driver can choose to also enable reduced blanking for a TTL or LVDS panel (if
it's not shared with a CRT that is).
- go into mode validation (this is of course the really short term view - one
step at a time)

If a driver handles EDID itself, then it can just not call xf86SetDDCproperties
and handle the setting of pScrn->monitor itself. Setting pScrn->monitor->DDC is
no longer needed with the xf86ValidateModes parsing of DDC gone, but a driver
will probably want to keep it around for reference anyway.

As an extension to your point: In the general TTL panel case (most laptops), the
maximum resolution (however retrieved, from an EDID block or from a scratch
area, or from config) should just be fed to xf86CVTMode with Reduced=TRUE (the
driver should do this and attach this mode to the list). Maybe
M_T_DRIVER_NATIVE, or perhaps M_T_DRIVER_PREF (with an M_T_EDID_PREF sibling) is
a good name for this type?

In any case, you're absolutely right, and yes, there should be nothing server
side that prohibits setting MonRec fields directly from the driver.

See also bug #5892 "Improvement of auto-config by using more EDID data in mode
validation process" where Henry has posted a suggested patch.

Right, i'm going to take some time to outline the EDID priority solution i see:

We already talked about it being based on DisplayModePtr->type, and we do want
to expand the current types. So 0x40 (xf86str.h) seems like the type to use.

I don't like M_T_EDID at all, i'd prefer M_T_DRIVER instead.
* For edid, with attachment 4527 [details] [review], edid is under full driver control.
* If the driver needs to attach modes, and there many times where this is
necessary. CH7011 tv encoder only knows a set of specific modes, so we just add
those modes to the modelist. These modes need a higher priority too.
* Then there is the case of the panel of a laptop. For cheap panels, which don't
come with DDC/EDID, we usually fall back to a scratch area. So we can cvt -r a
mode for that one and attach that with M_T_DRIVER as well.

I don't see why EDID or DRIVER need seperate handling. It's up to the driver to
decice which modeslist goes into validation, and which are kept out. This based
on output devices used.

We do need seperate handling for at least 2 cases:
* EDID preferred.
* panel (edid less) native resolution.

I suggest using M_T_DRIVER_PREFERRED 0x48.

The low quad isn't used except for M_T_BUILTIN, to flag generation of the
synthClock and the Crtc entries of the DisplayModeRec. So we can use that to
flag the preferred mode. We don't want to use 0x41 as this won't allow us any
future granularity here.

So, handling. First of all, this is after _actual_ mode validation.

We need to keep the use of the Modes directive (of the Display subsection of the
Screen section) "as is". We keep the highest priority type of each Modename. I
do think that M_T_USERDEF (provided in the conf in a Modes section) should have
higher priority here.

When the Modes directive isn't there, then we need proper autoconfiguration.

If there's M_T_DRIVER, then we use the full list of M_T_DRIVER. The first mode
showing up should be M_T_DRIVER_PREFERRED.

If there's no M_T_DRIVER, we should try to keep current behaviour again.

So the "only" time we use autoconfiguration, is without a Modes directive and
with M_T_DRIVER. Which is, in my view, exactly what we were looking for when we
started this.

*** Bug 6195 has been marked as a duplicate of this bug. ***

*** Bug 6038 has been marked as a duplicate of this bug. ***

*** Bug 6306 has been marked as a duplicate of this bug. ***

*** Bug 6503 has been marked as a duplicate of this bug. ***

Kai-Uwe, i'm CCing you here as we are apparently running in some gamma akwardness.

The code here uses the edid provided gamma (usually 2.2 or thereabouts) is used
to set the MonPtr->gamma.red/green/blue gamma values directly. Most CRTs I've
seen use 2.2 while the usual gamma value is defaulted to 1.0, quite an alarming
difference if you do not know what's going on.

Is it correct to use this value directly? If not, how should it be used, or
should we be using the per colour values in some way?

Comment on attachment 4526 [details] [review]
Move xf86CVTmode from utils/cvt/cvt.c to common/xf86cvt.c.

Committed.

The gamma used by xgamma and in the Xvidmode extension 
to set grafic cards one dimansional LUTs are a correction 
to the standard 2.2, like a multipier.
Dividing the EDID gamma by 2.2 would give a
correction factor for PC style xgamma.
Anyway, I would not think it is correct to use EDID to 
change any colors by the (X) driver. Normally 
simply sRGB colorimetrics are written in the EDID block.
Did someone find other values than 2.2?

On the other hand the identification strings in EDID are
highest welcome.

(In reply to comment #36)
> The gamma used by xgamma and in the Xvidmode extension 
> to set grafic cards one dimansional LUTs are a correction 
> to the standard 2.2, like a multipier.
> Dividing the EDID gamma by 2.2 would give a
> correction factor for PC style xgamma.
> Anyway, I would not think it is correct to use EDID to 
> change any colors by the (X) driver. Normally 
> simply sRGB colorimetrics are written in the EDID block.
> Did someone find other values than 2.2?

Dell 2000FP reports a gamma of 2.5.  I'm sure there are others.

Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.

(In reply to comment #37)
> (In reply to comment #36)
> > The gamma used by xgamma and in the Xvidmode extension 
> > to set grafic cards one dimansional LUTs are a correction 
> > to the standard 2.2, like a multipier.
> > Dividing the EDID gamma by 2.2 would give a
> > correction factor for PC style xgamma.
> > Anyway, I would not think it is correct to use EDID to 
> > change any colors by the (X) driver. Normally 
> > simply sRGB colorimetrics are written in the EDID block.
> > Did someone find other values than 2.2?
> 
> Dell 2000FP reports a gamma of 2.5.  I'm sure there are others.

Kai-uwe, you're right, current strategy of ignoring EDID gamma is probably the best option.

Comment on attachment 4527 [details] [review]
xf86DDCMonitorSet: Fill MonRec from DDC information.

Was committed half a year or so ago.

(In reply to comment #13)
> drivers needing work:
> apm: doesn't call xf86SetDDCproperties
> cyrix: PROBE_DETECT only.
> glint: doesn't call xf86SetDDCproperties, DDC after validation.
> i128: sets pScrn->monitor->DDC directly, doesn't call xf86SetDDCproperties
> i740: Should ditch the #defines surrounding DDC code and DDC handling should be
> moved to before validation
> i810: sets pScrn->monitor->DDC directly, doesn't call xf86SetDDCproperties
> i830: calls xf86SetDDCproperties properly, but pokes at MonRec from the edid
> block itself. Is probably ok, as it probably just overwrites values (check after
> implementation has taken on some shape).
> nsc: PROBE_DETECT only.
> rendition: ok, but should get a once-over (have rv2100 somewhere myself so np).
> sis: pokes at MonRec from the edid block itself.
> sisusb: probably does have the DDC bus implemented.
> sunffb: i2c bus implemented, but no real DDC code.
> tdfx: DDC handling after validation. (have voodoo 3k)
> voodoo: includes xf86DDC.h but not even an i2c bus.
> 
> All in all, very doable and not much work. Verification is going to be most of
> the work. i830 and sis might want to ignore xf86SetDDCproperties though.

Most of this is still a TODO here, and the reason to keep this bug open.

(In reply to comment #41)
> (In reply to comment #13)
> > drivers needing work:
> > apm: doesn't call xf86SetDDCproperties

Done.

> > cyrix: PROBE_DETECT only.

Deprecated driver, port to geode if you care.

> > glint: doesn't call xf86SetDDCproperties, DDC after validation.

Yes it does.  Moved DDC before validation though.

> > i128: sets pScrn->monitor->DDC directly, doesn't call xf86SetDDCproperties

Did both.  Only does the one now.

> > i740: Should ditch the #defines surrounding DDC code and DDC handling should be
> > moved to before validation

Blindly done, will test once I apply power to a sufficiently old AGP system.  Still does a vbeDoEDID in PreInit.  Not the best thing ever.

> > i810: sets pScrn->monitor->DDC directly, doesn't call xf86SetDDCproperties

Did both, only does the one now.

> > i830: calls xf86SetDDCproperties properly, but pokes at MonRec from the edid
> > block itself. Is probably ok, as it probably just overwrites values (check after
> > implementation has taken on some shape).

Obsolete observation, intel is randrful now.

> > nsc: PROBE_DETECT only.

See cyrix above.

> > sis: pokes at MonRec from the edid block itself.
> > sisusb: probably does have the DDC bus implemented.
> > sunffb: i2c bus implemented, but no real DDC code.

These need astonishing amounts of love anyway.  Should file new bugs for these I suppose.

> > tdfx: DDC handling after validation. (have voodoo 3k)

Fixed ages ago.

> > voodoo: includes xf86DDC.h but not even an i2c bus.

Doesn't anymore.

Done enough to call fixed.