Bug 15200 - intel: Implement option to ignore external fixed mode settings
Summary: intel: Implement option to ignore external fixed mode settings
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: 7.3 (2007.09)
Hardware: x86 (IA32) All
: medium enhancement
Assignee: Wang Zhenyu
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-25 07:57 UTC by Mike Isely
Modified: 2008-05-25 17:13 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch for the described feature (5.04 KB, patch)
2008-03-25 19:52 UTC, Mike Isely
no flags Details | Splinter Review
Man page change to document the earlier-implemented LVDSFixedMode option (6.25 KB, patch)
2008-04-14 10:09 UTC, Mike Isely
no flags Details | Splinter Review

Description Mike Isely 2008-03-25 07:57:57 UTC
Hi,

The following is a patch that adds a feature to the xorg intel driver.  This compatibly tweaks the algorithm for determination of the "fixed panel mode" associated with the LVDS output, and was needed in order to allow correct operation of a 100Hz digital display direct connected to the LVDS output of an Ampro CoreModule 800 embedded SBC (item described at: http://www.ampro.com/Products/CoreModules/CoreModule_800/, GPU is Intel 82855GME).  The application is embedded; there's no EDID for the display and since a COTS part is being used for the SBC, the video BIOS in the processor lacks any specific knowledge about the connected display.

This feature is conditioned by a new driver option; if the option is not specified then the behavior is unchanged.

Attached to this report is a signed-off patch against the driver sources.  I debugged this against a Debian-compiled variant of the driver (version 2:2.2.0-1), but I believe it should be close enough to be able to cleanly apply against the upstream sources.  Copied below are the patch comments:

<cut>
xorg-video-intel: Implement option to ignore external fixed mode settings

From: Mike Isely <isely@pobox.com>

The Intel xorg driver tries mightily to determine the native fixed
panel mode settings for the LVDS output.  It does this through various
means, including scanning video BIOS tables, and noticing if the pipe
in question has already been set up by somebody else (and adopting
those timings).  This strategy works well for say a laptop where the
LCD panel is an integral part of the machine.  But for other
applications where the display is unrelated to the system's BIOS or
other software, then the BIOS will likely have no clue how to
configure the LVDS output.  Worse still, the BIOS can simply "get it
wrong", leaving the pipe misconfigured.  Unfortunately the Intel
driver can potentially notice this, adopt the same settings, leaving a
messed up display.

All of this complexity normally happens independently, behind the
scenes, from the mode timings that might otherwise be specified by the
user.  This driver has a concept of fixed, i.e. "native" mode, and
then user-specified mode.  If the corresponding resolutions between
those concepts don't match, then the driver in theory will arrange for
scaling to take place while adhering to the actual native mode of the
panel.  Said another way, if the user says 800x600 but the driver
mistakenly (see above) thinks the native mode is 640x480, then 640x480
is the mode set with scaling to an 800x600 frame buffer.  If the
driver gets the wrong native mode, then the result is a miserable mess
with no way for the user to override what the driver thinks is right.

This patch provides a means to override the driver.  This implements a
new driver option, "LVDSFixedMode" which defaults to true (the normal,
probe-what-I-need behavior).  However when set to false, then all the
guessing is skipped and the driver will assume no fixed, i.e. "native"
mode for the display device.  Instead with this option set to false,
the driver will directly set the timings specified by the user,
providing an escape hatch for situations where the driver can't
correctly figure out the right mode.

Under most scenarios of course, this option should not be needed.  But
in situations where the Intel video BIOS is hopelessly fouled up
related to the LVDS output, this option provides the escape hatch for
the user to get a working display in spite of the BIOS situation.

Signed-off-by: Mike Isely <isely@pobox.com>
</cut>
Comment 1 Gordon Jin 2008-03-25 19:21:40 UTC
Please attach the patch.
Comment 2 Mike Isely 2008-03-25 19:52:42 UTC
Created attachment 15462 [details] [review]
Patch for the described feature

Well I don't understand what happened here because I *swear* I had attached it before.  And yet it's not there.  So here it is...  (And the same thing happened to 15201 as well.)

  -Mike
Comment 3 Wang Zhenyu 2008-03-30 20:06:42 UTC
I think this might be better become a quirk option.
Comment 4 Michael Fu 2008-04-01 18:19:15 UTC
we will accept this option. zhenyu to review and check in the patch.
Comment 5 Eric Anholt 2008-04-01 18:21:58 UTC
Doesn't seem like a quirk option to me.

You really want to be able to specify that one specific mode is the fixed timing for the panel, so that you can also use the panel scaling support for other resolutions.  It looks like with this patch, no fixed mode will be selected, so any different-resolution mode will be sent directly to the panel and only one of them will actually work.  Is this right?
Comment 6 Mike Isely 2008-04-01 19:33:39 UTC
I'm not sure how one can code this as a quirk.  I hadn't replied yet to comment #3 because I wanted to go back into the code first and figure out if that even is possible.  But one sense I have about quirk options is that these are unusual things typically detected by the driver, to work around strange hardware behavior regarding a specific GPU variant or board environment.  However in this case the issue arises with the specific combination of board video BIOS and attached display device.  This combination can't be known by the board vendor (they make embedded SBC products and are not in the business of trying to support every conceivable panel timing for whatever their customers might attach).  The whole problem here is that the driver can't correctly _detect_ this situation so how is the driver going to know to set a quirk flag for it?

I agree with the point in comment #5, but without this patch and if that one specific mode cannot otherwise be figured out then the driver aborts initialization of the pipe, leaving one without any way at all to configure / use the display.  With this patch, the behavior is basically to fall back to all the usual mode / timing selection logic that one would otherwise use for an arbitrary display device.  Said another way, by turning on this option, the user must then ensure that the video timings are correct through the usual means in xorg.conf, same as one might need to do in order to configure an old style CRT lacking EDID data.  Yes, that isn't exactly automatic or easy, but at least it is possible (and with the option off / unspecified behavior remains as before).  Without the patch, it's not even possible.

True, "only one of them will work", but that's really the same as one might expect for any display device with picky timing requirements.  If there were a way via xorg.conf to actually specify the desired fixed mode without inteference from the video BIOS then sure panel scaling can be brought to bear again.  But IMHO not having the luxury of scaling is a better choice than not having a viable display at all.

I arrived at this solution after determining that this driver was completely relying on determination of the fixed panel mode through one of (1) video BIOS already has the registers set (i.e. adopt the active BIOS config), (2) the video BIOS has a viable mode defined, or (3) there is EDID data present for the display device.  For our application, the video BIOS misconfigures the part for our display, the driver also fails to find a viable table entry in the BIOS, and we don't have any EDID data available (this is an embedded application, using a 3rd party SBC for the processor and GPU).  Failing all three of those cases, the driver gives up and shuts down the pipe.  There's simply no way for the driver to be externally configured via xorg.conf to override this behavior and allow configuration to continue without a fixed mode.  I traced every code path with an eye towards analyzing what happens without a fixed mode, and in every case the driver does the right thing (no scaling, use mode from xorg config).  This patch implements the option to allow the driver to continue without requiring any of those 3 cases to be true / correct.

Looked at another way, turning this option on further divorces the intel driver from the video BIOS, with the price that more responsibility falls upon the user to correctly configure the display timings.  I think that's a fair trade especially since it is controlled by an option which is normally off, and in some situations (like the one I'm dealing with), this is a showstopper issue since the BIOS is borked and would otherwise fatally mislead the driver.

  -Mike
Comment 7 Wang Zhenyu 2008-04-01 20:14:22 UTC
ok, we'd better describe the new option in man page, could you update your patches with #15201 to reflect these options? 
Comment 8 Mike Isely 2008-04-02 09:16:47 UTC
I was afraid I'd be asked that question :-)

Yes, I will generate / update appropriate patches to update the driver man page (also for bug 15201).  It will however be a few days yet.

Also, I'm open to renaming those options if better names can be suggested.

  -Mike
Comment 9 Mike Isely 2008-04-14 10:09:28 UTC
Created attachment 15911 [details] [review]
Man page change to document the earlier-implemented LVDSFixedMode option

I'm not entirely happy with this man page change.  I wrote it last week, but I've been sitting on it, waiting for some inspiration for how to compact it.  I made two changes; one briefly describes the option.  The second change is a new section that discusses fixed mode, scaling, and tries to make it clear for when messing with the LVDSFixedMode option makes sense.  I'm not sure I succeeded, and the change adds more than a few lines to the man page.  So I'm open to ideas for how to improve it.  In the mean time, here's what I've got.  If you're happy with it, go on ahead and commit it.  But I won't be offended if you think there's a better way to do this.

  -Mike
Comment 10 Wang Zhenyu 2008-05-07 19:17:43 UTC
Thanks Mike, sorry for delay this patch till now. I've pushed it to master.
Comment 11 Mike Isely 2008-05-25 17:13:09 UTC
No worries.  Glad I could contribute.

  -Mike


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.