Bug 12596 - resume from S3 broken in the latest release
Summary: resume from S3 broken in the latest release
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/Radeon (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: medium major
Assignee: xf86-video-ati maintainers
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
: 14070 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-09-27 11:20 UTC by Raffaele Sandrini
Modified: 2008-02-01 00:08 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Attempt to fix the resume problem for current HEAD. (390 bytes, patch)
2007-11-28 08:50 UTC, Raffaele Sandrini
no flags Details | Splinter Review

Description Raffaele Sandrini 2007-09-27 11:20:27 UTC
Somewhere in the diff (800 lines) from xf86-video-ati-6.7.193 to xf86-video-ati-6.7.194 resume from S3 was broken on my "ATI Technologies Inc M24GL [Mobility FireGL V3200]".

The system seems to crash on resume (everytime), however with 6.7.193 it works every time.
Comment 1 Alex Deucher 2007-09-27 11:48:13 UTC
Can you use git bisect to track down what commit caused the problem?
Comment 2 Raffaele Sandrini 2007-09-27 12:42:20 UTC
Ok i found the faulty patch through git bisect:

RADEON: fix up LVDS handling for r3xx and newer
b27135bce8b41d69290613b440a338b0a7fe0200

reverting that patch fixes the problem.
Comment 3 Alex Deucher 2007-09-27 13:44:56 UTC
does reverting just this part fix it?

-    save->lvds_pll_cntl = info->SavedReg.lvds_pll_cntl;
+    save->lvds_pll_cntl = (info->SavedReg.lvds_pll_cntl |
+                           RADEON_LVDS_PLL_EN);
+
+    save->lvds_pll_cntl &= ~RADEON_LVDS_PLL_RESET;

How about just removing the RADEON_LVDS_PLL_EN or the RADEON_LVDS_PLL_RESET line?
Comment 4 Raffaele Sandrini 2007-09-27 14:38:08 UTC
No that seemd not to help.

At least this patch (against xf86-video-ati-6.7.194):
--- a/src/radeon_output.c
+++ b/src/radeon_output.c
@@ -812,10 +812,7 @@ static void RADEONInitLVDSRegisters(xf86OutputPtr output, RADEONSavePtr save,
     ScrnInfoPtr pScrn = output->scrn;
     RADEONInfoPtr  info       = RADEONPTR(pScrn);
 
-    save->lvds_pll_cntl = (info->SavedReg.lvds_pll_cntl |
-                          RADEON_LVDS_PLL_EN);
-
-    save->lvds_pll_cntl &= ~RADEON_LVDS_PLL_RESET;
+    save->lvds_pll_cntl = info->SavedReg.lvds_pll_cntl;
 
     save->lvds_gen_cntl = info->SavedReg.lvds_gen_cntl;
     save->lvds_gen_cntl |= RADEON_LVDS_DISPLAY_DIS;

did not solve the problem.
Comment 5 Alex Deucher 2007-10-04 12:40:19 UTC
how about just reverting this part?

-        /*OUTREG(RADEON_LVDS_PLL_CNTL,  restore->lvds_pll_cntl);  
-        OUTREG(RADEON_BIOS_4_SCRATCH, restore->bios_4_scratch);
+        OUTREG(RADEON_LVDS_PLL_CNTL,  restore->lvds_pll_cntl);  
+        /*OUTREG(RADEON_BIOS_4_SCRATCH, restore->bios_4_scratch);

the LVDS PLL must need some special reset sequence.
Comment 6 Raffaele Sandrini 2007-10-16 00:19:19 UTC
This did indeed help. However i was not able to test that with todays HEAD. The revert-patch could not be merged with current HEAD.

Sorry for the late answer.
Comment 7 Raffaele Sandrini 2007-11-28 08:50:04 UTC
Created attachment 12784 [details] [review]
Attempt to fix the resume problem for current HEAD.

It still does not work with the current master (6f080d00e6f4f84d5e0d6b4eff302bf42c230e81).

I traked the problem down to the same codeline introduced by the initial ofending patch. I have no clue what it does ;) I simply know that applying the attached patch to the current HEAD solves the problem.
Comment 8 Alex Deucher 2008-01-14 10:28:29 UTC
*** Bug 14070 has been marked as a duplicate of this bug. ***
Comment 9 Krzysztof Kotlenga 2008-01-26 12:47:13 UTC
I can confirm that the patch from comment #7 fixes the problem for me as well (with the current master).
Comment 10 Alex Deucher 2008-02-01 00:08:35 UTC
I've committed a workaround for now.


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.