Bug 17892

Summary: [GM965] Xorg crashes when started with VGA projector
Product: xorg Reporter: Ben Gamari <bgamari>
Component: Driver/intelAssignee: Jesse Barnes <jbarnes>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium CC: orion, quanxian.wang, zhenyu.z.wang
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 16926    
Attachments:
Description Flags
Xorg log
none
Xorg.conf
none
Don't destroy GPIO DDC bus if there's a DDC failure
none
Init & teardown GPIOA as needed
none
crt probe test patch
none
Xorg log from xserver crashing at startup with patched driver
none
crt probe take2
none
right take2 patch none

Description Ben Gamari 2008-10-03 11:43:10 UTC
Created attachment 19354 [details]
Xorg log

Xorg segfaults when start on i965 with the VGA output active.
Comment 1 Gordon Jin 2008-10-05 05:05:24 UTC
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.
Comment 2 Ben Gamari 2008-10-05 09:37:27 UTC
Created attachment 19389 [details]
Xorg.conf

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
Comment 3 Jesse Barnes 2008-10-06 13:59:08 UTC
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?
Comment 4 Jesse Barnes 2008-10-06 14:01:37 UTC
Zhenyu, what do you think?  Looks like you made the last changes here...
Comment 5 Ben Gamari 2008-10-06 14:20:27 UTC
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.
Comment 6 Wang Zhenyu 2008-10-06 18:01:56 UTC
Jesse, yeah, I think the patch looks reasonable to me, thanks for catch this!
Comment 7 Jesse Barnes 2008-10-08 08:55:55 UTC
*** Bug 17948 has been marked as a duplicate of this bug. ***
Comment 8 Jesse Barnes 2008-10-08 08:57:10 UTC
Ben, have you had a chance to test yet?  I wanted to get confirmation before pushing this.
Comment 9 Orion Poplawski 2008-10-08 10:47:21 UTC
Fixes my crash.
Comment 10 Jesse Barnes 2008-10-08 11:12:24 UTC
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.

Zhenyu?
Comment 11 Wang Zhenyu 2008-10-08 18:55:09 UTC
Jesse, yes, looks right to me. I'm testing with this change.
Comment 12 Wang Zhenyu 2008-10-08 19:11:29 UTC
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.
Comment 13 Jesse Barnes 2008-10-08 20:41:48 UTC
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?
Comment 14 Wang Zhenyu 2008-10-08 20:57:31 UTC
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.
Comment 15 Ben Gamari 2008-10-08 20:59:03 UTC
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].
Comment 16 Wang Zhenyu 2008-10-08 22:42:06 UTC
Created attachment 19510 [details] [review]
crt probe take2

Please try this one. Thanks.
Comment 17 Wang Zhenyu 2008-10-08 22:48:45 UTC
Created attachment 19511 [details] [review]
right take2 patch

oops, attach the origin patch again.
Comment 18 Gordon Jin 2008-10-09 19:30:16 UTC
*** Bug 17989 has been marked as a duplicate of this bug. ***
Comment 19 Gordon Jin 2008-10-09 19:32:21 UTC
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.
Comment 20 Ben Gamari 2008-10-10 06:25:49 UTC
Yep, seems to work here. Thanks,

- Ben
Comment 21 Orion Poplawski 2008-10-10 08:23:03 UTC
take 2 patch works for me too.
Comment 22 Gordon Jin 2008-10-11 02:44:23 UTC
Jesse/Zhenyu, I think you could commit the patch now? I suppose 2.5.0 will be released on next Monday?
Comment 23 Wang Zhenyu 2008-10-11 09:00:17 UTC
ok, pushed to master commit 6cb4150160bb1e1365773561fb53294ad9248a0e.

Thanks for reporting and testing this. Close.

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.