| Summary: | intel: Implement option to ignore external fixed mode settings | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | xorg | Reporter: | Mike Isely <isely> | ||||||
| Component: | Driver/intel | Assignee: | Wang Zhenyu <zhenyu.z.wang> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> | ||||||
| Severity: | enhancement | ||||||||
| Priority: | medium | CC: | isely, michael.fu | ||||||
| Version: | 7.3 (2007.09) | ||||||||
| Hardware: | x86 (IA32) | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| i915 platform: | i915 features: | ||||||||
| Attachments: |
|
||||||||
|
Description
Mike Isely
2008-03-25 07:57:57 UTC
Please attach the patch. 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 I think this might be better become a quirk option. we will accept this option. zhenyu to review and check in the patch. 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? 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 ok, we'd better describe the new option in man page, could you update your patches with #15201 to reflect these options? 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 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 Thanks Mike, sorry for delay this patch till now. I've pushed it to master. 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.