Skip to contentSkip to conversation

atyfb: Replace busy loop with mdelay()

PATCH In review Ville Syrjälä 4 participants 5 messages Feb 22, 2007 – Mar 7, 2007 Framebuffer layer

lore
Share & tools
lore

Ville Syrjälä

author
#0 lore raw

PATCH 1/1 atyfb: Replace busy loop with mdelay()

Replace a busy loop with mdelay() in atyfb clock setup code. This fixes boot hangs several people have experienced.

Signed-off-by Ville Syrjala <[email protected]>

---

This patch was already forwareded by James to some of you, but here it is with the proper changelog, signed-off-by line, and a language fix in the comment part. Also Cc:d [email protected] as this seems like -stable material to me.

 drivers/video/aty/mach64_ct.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/video/aty/mach64_ct.c +3 -6
Index: linux-2.6.20.1/drivers/video/aty/mach64_ct.c
===================================================================
@@ -598,7 +598,6 @@ static void aty_resume_pll_ct(const stru
 	struct atyfb_par *par = info->par;
 
 	if (par->mclk_per != par->xclk_per) {
-		int i;
 		/*
 		* This disables the sclk, crashes the computer as reported:
 		* aty_st_pll_ct(SPLL_CNTL2, 3, info);

@@ -609,12 +608,10 @@ static void aty_resume_pll_ct(const stru
 		aty_st_pll_ct(SCLK_FB_DIV, pll->ct.sclk_fb_div, par);
 		aty_st_pll_ct(SPLL_CNTL2, pll->ct.spll_cntl2, par);
 		/*
-		 * The sclk has been started. However, I believe the first clock
-		 * ticks it generates are not very stable. Hope this primitive loop
-		 * helps for Rage Mobilities that sometimes crash when
-		 * we switch to sclk. (Daniel Mantione, 13-05-2003)
+		 * SCLK has been started. Wait for the PLL to lock. 5 ms
+		 * should be enough according to mach64 programmer's guide.
 		 */
-		for (i=0;i<=0x1ffff;i++);
+		mdelay(5);
 	}
 
 	aty_st_pll_ct(PLL_REF_DIV, pll->ct.pll_ref_div, par);
Signature · 9 lines
Ville Syrjälä
[email protected]
http://www.sci.fi/~syrjala/

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
#1 lore raw
Quoted 2 lines · Ville Syrjälä

Replace a busy loop with mdelay() in atyfb clock setup code. This fixes boot hangs several people have experienced.

Agree that is a candidate for the stable tree. If not, I'll take care of this patch.

Tony

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Linux-fbdev-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

#2 lore raw
In reply to Ville Syrjälä
Quoted 33 lines · On Thu, 22 Feb 2007 02:20:33 +0200 Ville Syrj__l__ <[email protected]> wrote:

Replace a busy loop with mdelay() in atyfb clock setup code. This fixes boot hangs several people have experienced.

Signed-off-by Ville Syrjala <[email protected]>

---

This patch was already forwareded by James to some of you, but here it is with the proper changelog, signed-off-by line, and a language fix in the comment part. Also Cc:d [email protected] as this seems like -stable material to me.

 drivers/video/aty/mach64_ct.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Index: linux-2.6.20.1/drivers/video/aty/mach64_ct.c
===================================================================

drivers/video/aty/mach64_ct.c +0 -3
@@ -598,7 +598,6 @@ static void aty_resume_pll_ct(const stru
 	struct atyfb_par *par = info->par;
 
 	if (par->mclk_per != par->xclk_per) {
-		int i;
 		/*
 		* This disables the sclk, crashes the computer as reported:
 		* aty_st_pll_ct(SPLL_CNTL2, 3, info);

@@ -609,12 +608,10 @@ static void aty_resume_pll_ct(const stru
 		aty_st_pll_ct(SCLK_FB_DIV, pll->ct.sclk_fb_div, par);
 		aty_st_pll_ct(SPLL_CNTL2, pll->ct.spll_cntl2, par);
 		/*
-		 * The sclk has been started. However, I believe the first clock
-		 * ticks it generates are not very stable. Hope this primitive loop

drivers/video/aty/mach64_ct.c static void aty_resume_pll_ct(const stru · line 617

Quoted 10 lines
drivers/video/aty/mach64_ct.c +3 -3
-		 * helps for Rage Mobilities that sometimes crash when
-		 * we switch to sclk. (Daniel Mantione, 13-05-2003)
+		 * SCLK has been started. Wait for the PLL to lock. 5 ms
+		 * should be enough according to mach64 programmer's guide.
 		 */
-		for (i=0;i<=0x1ffff;i++);
+		mdelay(5);
 	}
 
 	aty_st_pll_ct(PLL_REF_DIV, pll->ct.pll_ref_div, par);

A similar but different patch just got merged into mainline:

commit 8690ba446defe2e2b81803756c099d2943dfd5fd

Author: David Miller <[email protected]>
Date:   Mon Mar 5 01:35:36 2007 -0800

[PATCH] video/aty/mach64_ct.c: fix bogus delay loop

CT based mach64 cards were reported to hang on sparc64 boxes when compiled with gcc-4.1.x and later.

Looking at this piece of code, it's no surprise. A critical delay was implemented as an empty for() loop, and gcc 4.0.x and previous did not optimize it away, so we did get a delay.

But gcc-4.1.x and later can optimize it away, and we get crashes.

Use a real udelay() to fix this. Fix verified on SunBlade100.

Signed-off-by David S. Miller <[email protected]>
Signed-off-by Linus Torvalds <[email protected]>
drivers/video/aty/mach64_ct.c +1 -2
index f3b487b..1fdcfdb 100644
@@ -598,7 +598,6 @@ static void aty_resume_pll_ct(const stru
        struct atyfb_par *par = info->par;
 
        if (par->mclk_per != par->xclk_per) {
-               int i;
                /*
                * This disables the sclk, crashes the computer as reported:
                * aty_st_pll_ct(SPLL_CNTL2, 3, info);

@@ -614,7 +613,7 @@ static void aty_resume_pll_ct(const stru
                 * helps for Rage Mobilities that sometimes crash when
                 * we switch to sclk. (Daniel Mantione, 13-05-2003)
                 */
-               for (i=0;i<=0x1ffff;i++);
+               udelay(500);
        }
 
        aty_st_pll_ct(PLL_REF_DIV, pll->ct.pll_ref_div, par);

hopefully that works OK for you.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#3 lore raw

drivers/video/aty/mach64_ct.c static void aty_resume_pll_ct(const stru · line 616

Quoted 6 lines · Andrew Morton
drivers/video/aty/mach64_ct.c +1 -1
@@ -614,7 +613,7 @@ static void aty_resume_pll_ct(const stru
                 * helps for Rage Mobilities that sometimes crash when
                 * we switch to sclk. (Daniel Mantione, 13-05-2003)
                 */
-               for (i=0;i<=0x1ffff;i++);
+               udelay(500);

I'm not sure if 500 us is enough. The programmer's guide says you should allow 5 ms for the PLL to lock.

Signature · 9 lines
Ville Syrjälä
[email protected]
http://www.sci.fi/~syrjala/

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
#4 lore raw
Quoted 10 lines · Ville Syrj?l?

drivers/video/aty/mach64_ct.c static void aty_resume_pll_ct(const stru · line 616

Quoted 6 lines · Andrew Morton
drivers/video/aty/mach64_ct.c +1 -1
@@ -614,7 +613,7 @@ static void aty_resume_pll_ct(const stru
                 * helps for Rage Mobilities that sometimes crash when
                 * we switch to sclk. (Daniel Mantione, 13-05-2003)
                 */
-               for (i=0;i<=0x1ffff;i++);
+               udelay(500);

I'm not sure if 500 us is enough. The programmer's guide says you should allow 5 ms for the PLL to lock.

Then can you correct the patch in mainline, and forward what gets accepted there to the stable team if you wish the change to also be made there?

thanks,

greg k-h

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV