Created attachment 19354 [details]
Xorg segfaults when start on i965 with the VGA output active.
Does this ever work for you with older driver version?
And you indicate X can startup ok if not using VGA, right?
Please attach xorg.conf too.
Created attachment 19389 [details]
I'll go and check with a few older driver versions. Things work quite fine if X is started without VGA. I've attached my xorg.conf
Created attachment 19419 [details] [review]
Don't destroy GPIO DDC bus if there's a DDC failure
Hm, works for me with VGA attached... Are you running with any extra patches? Does the same thing happen with the latest git bits (though it looks like you were running pretty recent bits).
Based on the backtrace, I'd say the driver's CRT ->detect and ->get_modes functions are getting called (though we could be crashing in one of the other outputs it seems likely that the VGA ->get_modes is to blame given your configuration). It looks like there's a case where we might destroy the GPIO DDC bus and then re-use it. If that's happening in your case, this patch might help. Can you give it a try?
Zhenyu, what do you think? Looks like you made the last changes here...
I'll check that patch as soon as I get a chance (probably in a few hours). However, it probably is important to note that I was going through a really funky-looking VGA switch-box when the crash occurred. Given this, it's entirely possible that EDID failed or something to that effect.
Jesse, yeah, I think the patch looks reasonable to me, thanks for catch this!
*** Bug 17948 has been marked as a duplicate of this bug. ***
Ben, have you had a chance to test yet? I wanted to get confirmation before pushing this.
Fixes my crash.
Created attachment 19502 [details] [review]
Init & teardown GPIOA as needed
After looking at it a little more I don't think my patch would work in all cases.
i830_crt_get_modes() can get called multiple times by the core X server, and with my patch we might leave a destroyed I2C bus in intel_output->pDDCBus if we ended up getting modes from say GPIOD. It looks like i830_crt_detect_ddc() was the only thing really expecting for a DDCBus to be set up already, so I think we can just set up the GPIOA bus there, and remove the GPIOA special case from i830_get_edid().
The one open question I have is whether we should be doing the same thing in i830_crt_detect_ddc() as we do in i830_crt_get_modes(), i.e. checking all of the GPIO pins to see if something is attached. If so some further consolidation might be possible.
Jesse, yes, looks right to me. I'm testing with this change.
Created attachment 19507 [details] [review]
crt probe test patch
This might be cleaner, setup and destroy DDC bus in any probe action, which should work with different CRT usage and reprobe event.
Zhenyu, yeah your patch looks good, though I'm still worried about the DDC detection path... Shouldn't we be probing all the GPIO ports there too?
We always depend on hotplug detection on newer chip, and my testing on 915G here works fine. Yes, we can have another function for DDC probe for DDC detect.
I think older chipsets don't ship any DVI-I port, or we haven't seen any one need alternative DDC scanning.
Created attachment 19508 [details]
Xorg log from xserver crashing at startup with patched driver
(In reply to comment #12)
> Created an attachment (id=19507) [details]
> crt probe test patch
> This might be cleaner, setup and destroy DDC bus in any probe action, which
> should work with different CRT usage and reprobe event.
Unfortunately, this doesn't seem to help. Xorg still crashes when one attempts to enable the output on a running server with "xrandr --output VGA --auto" and also appears to crash when the server is started with the output connected. Log attached of server crashing at start up with driver with attachment 19507 [details] [review].
Created attachment 19510 [details] [review]
crt probe take2
Please try this one. Thanks.
Created attachment 19511 [details] [review]
right take2 patch
oops, attach the origin patch again.
*** Bug 17989 has been marked as a duplicate of this bug. ***
Ben/Quanxian, could you take high priority on trying the patch? I hope the patch could be committed before end of this week to catch 2.5.0 release.
Yep, seems to work here. Thanks,
take 2 patch works for me too.
Jesse/Zhenyu, I think you could commit the patch now? I suppose 2.5.0 will be released on next Monday?
ok, pushed to master commit 6cb4150160bb1e1365773561fb53294ad9248a0e.
Thanks for reporting and testing this. Close.