Bug 64178 - [Intel GM45] xrandr --output <output> --mode <prev_mode> is not able to light up an output that has been disabled with xrandr --off.
Summary: [Intel GM45] xrandr --output <output> --mode <prev_mode> is not able to ligh...
Status: CLOSED DUPLICATE of bug 61642
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Imre Deak
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-03 09:58 UTC by Egbert Eich
Modified: 2017-07-24 22:58 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Attachments
Proposed Fix. (2.25 KB, text/plain)
2013-05-03 09:58 UTC, Egbert Eich 		no flags Details
Imre's patch modified. (1.58 KB, patch)
2013-05-03 15:14 UTC, Egbert Eich 		no flags Details | Splinter Review
Imre's patch modified V_2. (1.61 KB, patch)
2013-05-03 15:59 UTC, Egbert Eich 		no flags Details | Splinter Review
Show Obsolete (1) View All

Description Egbert Eich 2013-05-03 09:58:10 UTC 
Created attachment 78802 [details]
Proposed Fix.

This issue has been found on a Intel GM45 (0x8086 0x2a42) (among others):

1. Start an Xserver on a system with multiple outputs attached.
2. Ensure all outputs are lit.
3. Disable one output with 'xrandr --output <output_ID> --off
4. Observe this output become blank and enter power saving state.
5. Issue xrandr --output <output_ID> --mode <previous_mode> 

=> One will observe that this output will remain blank.

Investigation results: 

The problem is an asymetric behavior in intel_crtc_set_config(): If a full mode
set is required any encoders in power save state are activated as a side
effect. Otherwise they are simply left in their provious state.
This asymetry in behavior can cause a surprising and unexpected behavior.

The attached patch will fix this problem. There are possibly other ways to fix the issue - to me this seemed to be the simplest.

Ticket opened to put the patch up for discussion.
Comment 1 Daniel Vetter 2013-05-03 10:20:04 UTC 
Imre is already working on an almost identical fix for similar issues we have with igt testscases randomly failing.
Comment 2 Chris Wilson 2013-05-03 10:55:38 UTC 
This isn't exactly a kernel bug - but incorrect state tracking in UXA.

*** This bug has been marked as a duplicate of bug 61642 ***
Comment 3 Imre Deak 2013-05-03 11:24:46 UTC 
(In reply to comment #1)
> Imre is already working on an almost identical fix for similar issues we
> have with igt testscases randomly failing.

Yes, it's bug #59834, the patch is attached to bug #60002. It may fix the first bug, but it hasn't been confirmed yet. But I think the patch fixes an issue anyway. I've sent it to the mailing list now.
Comment 4 Egbert Eich 2013-05-03 15:14:32 UTC 
Created attachment 78812 [details] [review]
Imre's patch modified.

(In reply to comment #3)
> (In reply to comment #1)
> > Imre is already working on an almost identical fix for similar issues we
> > have with igt testscases randomly failing.
> 
> Yes, it's bug #59834, the patch is attached to bug #60002. It may fix the
> first bug, but it hasn't been confirmed yet. But I think the patch fixes an
> issue anyway. I've sent it to the mailing list now.

Imre, your patch looks nicer, unfortunately it doesn't work here.

The reason is that the test is inside the "if (set->crtc->fb != set->fb)" test.
In this case the fb has not changed however thus the extra test doesn't even run.

With the modified patch that's attached it works.
Comment 5 Imre Deak 2013-05-03 15:46:41 UTC 
(In reply to comment #4)
> Created attachment 78812 [details] [review] [review]
> Imre's patch modified.
> 
> (In reply to comment #3)
> > (In reply to comment #1)
> > > Imre is already working on an almost identical fix for similar issues we
> > > have with igt testscases randomly failing.
> > 
> > Yes, it's bug #59834, the patch is attached to bug #60002. It may fix the
> > first bug, but it hasn't been confirmed yet. But I think the patch fixes an
> > issue anyway. I've sent it to the mailing list now.
> 
> Imre, your patch looks nicer, unfortunately it doesn't work here.
> 
> The reason is that the test is inside the "if (set->crtc->fb != set->fb)"
> test.
> In this case the fb has not changed however thus the extra test doesn't even
> run.
> 
> With the modified patch that's attached it works.

Yes, you are right, I wasn't thinking of FB remaining the same.
Comment 6 Egbert Eich 2013-05-03 15:59:45 UTC 
Created attachment 78813 [details] [review]
Imre's patch modified V_2.

(In reply to comment #5)
> (In reply to comment #4)
> 
> Yes, you are right, I wasn't thinking of FB remaining the same.

Another thing: 
it turns out that doing a '*set->connectors' without testing for set->connectors != NULL is not very healthy.

This new patch saves one from unpleasant surprises.
Comment 7 Imre Deak 2013-05-03 16:30:10 UTC 
(In reply to comment #6)
> Created attachment 78813 [details] [review] [review]
> Imre's patch modified V_2.
> 
> (In reply to comment #5)
> > (In reply to comment #4)
> > 
> > Yes, you are right, I wasn't thinking of FB remaining the same.
> 
> Another thing: 
> it turns out that doing a '*set->connectors' without testing for
> set->connectors != NULL is not very healthy.
> 
> This new patch saves one from unpleasant surprises.

Right, though this wasn't a problem in the original, where connectors couldn't be NULL. I think it'd be better to post these to the ML btw.

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.