Bug 65524

Of course, that commit is just the icing on the case. After Egbert's patch, we successfully read the EDID for the SDVO panel and then things go wrong...

Before:
[    0.610486] [drm:drm_helper_probe_single_connector_modes], [CONNECTOR:15:LVDS-2] probed modes :
[    0.610490] [drm:drm_mode_debug_printmodeline], Modeline 17:"1024x600" 60 50500 1024 1034 1066 1348 600 612 616 624 0x48 0xa

After:
[    0.653730] [drm:drm_helper_probe_single_connector_modes], [CONNECTOR:15:LVDS-2] probed modes :
[    0.653733] [drm:drm_mode_debug_printmodeline], Modeline 18:"1024x600" 60 44700 1024 1034 1066 1194 600 612 616 624 0x48 0xa
[    0.653739] [drm:drm_mode_debug_printmodeline], Modeline 19:"1024x600" 40 29800 1024 1034 1066 1194 600 612 616 624 0x40 0xa

And then there is an even greater discrepancy in the adjusted modes for the first modeset:

Before:
[    1.110223] [drm:intel_dump_pipe_config], adjusted mode:
[    1.110228] [drm:drm_mode_debug_printmodeline], Modeline 0:"1024x600" 60 101000 1024 1034 1066 1348 600 612 616 624 0x48 0xa

After:
[    1.186459] [drm:intel_dump_pipe_config], adjusted mode:
[    1.186463] [drm:drm_mode_debug_printmodeline], Modeline 0:"1024x600" 60 178800 1024 1034 1066 1194 600 612 616 624 0x48 0xa


Before we have a pixel multiplier of about 2, and afterwards closer to 4.

Created attachment 80497 [details]
working dmesg

Created attachment 80498 [details]
Broken dmesg

Note in broken "[    1.425654] [drm:intel_enable_sdvo], First SDVOB output reported failure to sync"

Indeed tweaking the pixel multiplier so that it comes out at 2 makes the SDVO LVDS panel work again:

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 45aa32a..20a6c79 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -577,9 +577,9 @@ log_fail:
 
 static int intel_sdvo_get_pixel_multiplier(struct drm_display_mode *mode)
 {
-	if (mode->clock >= 100000)
+	if (mode->clock >= 80000)
 		return 1;
-	else if (mode->clock >= 50000)
+	else if (mode->clock >= 40000)
 		return 2;
 	else
 		return 4;

Since sdvo lvds apparently worked since forever ... should we just do the edid read _after_ the vbt reading?

Iirc there's been quite a few machines with sdvo lvds out there and the bug Egbert found is really old, so I'm fairly positive that it should work.

Created attachment 80525 [details] [review]
prefer vbt over edid for sdvo lvds

The modelines in the EDID look sane, so I think we do have an issue in the way we compute the pixel multiplier raising it above the bandwidth limit.

Spec says clearly that the sdvo signal must be at least 100MHz, and at least the two sdvo encoders I have here (one DVI, other HDMI) have no issues with the low-res modes.

My pet theory here is simply that this board/sdvo encoder combination has a problem with 4x pixel multiplier mode and the firmware engineer "fixed" this by frobbing the mode in the vbt a bit to push it over 50MHz dotclock.

We could test this if you force the 4x mutliplier on the vbt mode, sdvo dotclock should be in the range 100-225MHz iirc, so it should still work.

Using the EDID:

requested: Modeline 0:"1024x600" 60 44700 1024 1034 1066 1194 600 612 616 624 0x48 0xa
preferred input: Modeline 0:"1024x600" 60 44700 1024 1034 1066 1194 600 612 616 624 0x48 0xa
adjusted: Modeline 0:"1024x600" 60 178800 1024 1034 1066 1194 600 612 616 624 0x48 0xa

-> refclk=120000, clock=(.m1=14, .m2=7, n=4, .p1=1, .p2=10), computed dotclock = 178000 [broken]

with pixel_multiplier=2:

refclk=120000, clock=(.m1=14, .m2=7, n=2, .p1=3, .p2=10), computed dotclock = 89000 [works]

Using VBT:

request: Modeline 0:"1024x600" 60 50500 1024 1034 1066 1348 600 612 616 624 0x48 0xa
preferred input: Modeline 0:"1024x600" 60 50500 1024 1034 1066 1348 600 612 616 624 0x48 0xa
adjusted:  Modeline 0:"1024x600" 60 101000 1024 1034 1066 1348 600 612 616 624 0x48 0xa

->  refclk=120000, clock=(.m1=16, .m2=9, n=2, .p1=3, .p2=10), computed dotclock = 101000 [works]

with pixel_multiplier=4:
-> refclk=120000, clock=(.m1=13, .m2=7, n=3, .p1=1, .p2=10), computed dotclock = 201600 [broken]


The bspec is very clear in that the allowable dotclock range for sDVO is 100-225MHz on Ironlake. So I have no idea why either the forced pixel_multiplier=2 works for the EDID mode, nor why it fails with pixel_multiplier=4. I also distrust the VBT modes, on other machines they are garbage. It is a less than satisfactory situation.

Personally your experimental results reinforces my opinion that someone ordered the shittiest sdvo encoder and the tiniest panel they could find and then noticed that stuff doesn't work with a pixel mutliplier of 4.

To fix this they've frobbed the panel's mode so that it's just barely above 50MHz and this way tricked the Windows driver into picking a pixel mutliplier of 2 without overtly offending the panel.

I share your fear for the reliability of VBT modes, but in this specific case of SDVO-LVDS it looks like the vbt is actually more reliable. Especially if you combine this with the fact that we've ignored the edid modes for a few years completely. Maybe we need to double-check with egbert, but iirc the panel which fails for him didn't have any vbt mode at all.

Egbert, can you please check whether my patch doesn't break your system?

I'm positive that your patch will not harm the TGCS (foermerly IBM) POS system on which I discovered that reading EDID was broken.
For entertainment let me share the story that led to this discovery:
The vendor's QA complained that the driver we ship in the next SP (SP3) of our enterprise product only allows to set an 800x600 mode on the built-in 1024x768 panel correctly when the VBIOS option "Windows 7" is selected but not when "All Others" is set. The additional claim that this used to work with SP1 let me to investigate the issue further: it turned out that the 'Windows 7' VBIOS contained a VBT table, while the other one didn't. A test with the SP1 kernel revealed that on this kernel the panel provided an EDID thus the fixed panel mode was set even in the absence of a VBT.
The rest was then easy to spot.

Patch merged to -fixes as:

commit c3456fb3e4712d0448592af3c5d644c9472cd3c1
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Jun 10 09:47:58 2013 +0200

    drm/i915: prefer VBT modes for SVDO-LVDS over EDID

Amusing.. I was going to commit a fix to the upstream kernel when I came across this bug.   

The complaint was that they were not able read the /sys/class/drm/card0-LVDS-1/edid information from their lvds display.  Sound familiar?

Long story short it was the same &intel_sdvo->ddc change.  However I didn't know about the pixel multiplier discrepancy.  Good job fixing that!