[PATCH] atyfb: fix distorted image on PowerMacs

[PATCH] atyfb: fix distorted image on PowerMacs

[Risto Suominen]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
columns in their framebuffer image. This seems to depend on a value (4) read
from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
be written to the chip. This patch uses a value calculated by aty_init_pll_ct
instead, as a starting point.

Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
---
The testing is done on kernel version 2.6.18.

--- a/drivers/video/aty/mach64_ct.c.org	2006-09-20 06:42:06.000000000 +0300
+++ b/drivers/video/aty/mach64_ct.c	2009-01-22 14:28:13.000000000 +0200
@@ -7,6 +7,7 @@
 #include <linux/delay.h>
 #include <asm/io.h>
 #include <video/mach64.h>
+#include <asm/machdep.h>
 #include "atyfb.h"

 #undef DEBUG
@@ -536,6 +537,14 @@ static int __init aty_init_pll_ct(const
 	pll->ct.xclk_post_div_real = postdividers[xpost_div];
 	pll->ct.mclk_fb_div = q * pll->ct.xclk_post_div_real / 8;

+#ifdef CONFIG_PPC
+	if (machine_is(powermac)) {
+		/* Override PLL_EXT_CNTL & 0x07. */
+		pll->ct.xclk_post_div = xpost_div;
+		pll->ct.xclk_ref_div = 1;
+	}
+#endif
+
 #ifdef DEBUG
 	pllmclk = (1000000 * pll->ct.mclk_fb_mult * pll->ct.mclk_fb_div) /
 			(par->ref_clk_per * pll->ct.pll_ref_div);

[-- Attachment #2: aty-mach64-pmac.diff --]
[-- Type: text/x-diff, Size: 1306 bytes --]

Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
columns in their framebuffer image. This seems to depend on a value (4) read
from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
be written to the chip. This patch uses a value calculated by aty_init_pll_ct
instead, as a starting point.

Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
---
The testing is done on kernel version 2.6.18.

--- a/drivers/video/aty/mach64_ct.c.org	2006-09-20 06:42:06.000000000 +0300
+++ b/drivers/video/aty/mach64_ct.c	2009-01-22 14:28:13.000000000 +0200
@@ -7,6 +7,7 @@
 #include <linux/delay.h>
 #include <asm/io.h>
 #include <video/mach64.h>
+#include <asm/machdep.h>
 #include "atyfb.h"
 
 #undef DEBUG
@@ -536,6 +537,14 @@ static int __init aty_init_pll_ct(const 
 	pll->ct.xclk_post_div_real = postdividers[xpost_div];
 	pll->ct.mclk_fb_div = q * pll->ct.xclk_post_div_real / 8;
 
+#ifdef CONFIG_PPC
+	if (machine_is(powermac)) {
+		/* Override PLL_EXT_CNTL & 0x07. */
+		pll->ct.xclk_post_div = xpost_div;
+		pll->ct.xclk_ref_div = 1;
+	}
+#endif
+
 #ifdef DEBUG
 	pllmclk = (1000000 * pll->ct.mclk_fb_mult * pll->ct.mclk_fb_div) /
 			(par->ref_clk_per * pll->ct.pll_ref_div);

Re: [PATCH] atyfb: fix distorted image on PowerMacs

[Ville Syrjälä]

On Thu, Jan 22, 2009 at 08:22:48PM +0200, Risto Suominen wrote:
> Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
> and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
> columns in their framebuffer image. This seems to depend on a value (4) read
> from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
> be written to the chip. This patch uses a value calculated by aty_init_pll_ct
> instead, as a starting point.

AFAICS it should be the right thing to do on other systems too. I have
a good collection of mach64 pci cards and laptops so I'll try to find
some time next week to test that theory on x86.

Also cc:ing fbdev-devel...

> Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
> ---
> The testing is done on kernel version 2.6.18.
> 
> --- a/drivers/video/aty/mach64_ct.c.org	2006-09-20 06:42:06.000000000 +0300
> +++ b/drivers/video/aty/mach64_ct.c	2009-01-22 14:28:13.000000000 +0200
> @@ -7,6 +7,7 @@
>  #include <linux/delay.h>
>  #include <asm/io.h>
>  #include <video/mach64.h>
> +#include <asm/machdep.h>
>  #include "atyfb.h"
> 
>  #undef DEBUG
> @@ -536,6 +537,14 @@ static int __init aty_init_pll_ct(const
>  	pll->ct.xclk_post_div_real = postdividers[xpost_div];
>  	pll->ct.mclk_fb_div = q * pll->ct.xclk_post_div_real / 8;
> 
> +#ifdef CONFIG_PPC
> +	if (machine_is(powermac)) {
> +		/* Override PLL_EXT_CNTL & 0x07. */
> +		pll->ct.xclk_post_div = xpost_div;
> +		pll->ct.xclk_ref_div = 1;
> +	}
> +#endif
> +
>  #ifdef DEBUG
>  	pllmclk = (1000000 * pll->ct.mclk_fb_mult * pll->ct.mclk_fb_div) /
>  			(par->ref_clk_per * pll->ct.pll_ref_div);

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [Linux-fbdev-devel] [PATCH] atyfb: fix distorted image on PowerMacs

[Ville Syrjälä]

On Thu, Jan 22, 2009 at 10:12:04PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 22, 2009 at 08:22:48PM +0200, Risto Suominen wrote:
> > Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
> > and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
> > columns in their framebuffer image. This seems to depend on a value (4) read
> > from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
> > be written to the chip. This patch uses a value calculated by aty_init_pll_ct
> > instead, as a starting point.
> 
> AFAICS it should be the right thing to do on other systems too. I have
> a good collection of mach64 pci cards and laptops so I'll try to find
> some time next week to test that theory on x86.

Well, the experiment failed on the first Mobility I tried. The FIFO
calculations aren't obvious from the code and there's no mention how
to actually calculate them in the programming guide. So I guess special
casing powermacs is the best way forward.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [Linux-fbdev-devel] [PATCH] atyfb: fix distorted image on PowerMacs

[Benjamin Herrenschmidt]

On Tue, 2009-02-03 at 20:56 +0200, Ville Syrjälä wrote:
> On Thu, Jan 22, 2009 at 10:12:04PM +0200, Ville Syrjälä wrote:
> > On Thu, Jan 22, 2009 at 08:22:48PM +0200, Risto Suominen wrote:
> > > Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
> > > and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
> > > columns in their framebuffer image. This seems to depend on a value (4) read
> > > from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
> > > be written to the chip. This patch uses a value calculated by aty_init_pll_ct
> > > instead, as a starting point.
> > 
> > AFAICS it should be the right thing to do on other systems too. I have
> > a good collection of mach64 pci cards and laptops so I'll try to find
> > some time next week to test that theory on x86.
> 
> Well, the experiment failed on the first Mobility I tried. The FIFO
> calculations aren't obvious from the code and there's no mention how
> to actually calculate them in the programming guide. So I guess special
> casing powermacs is the best way forward.

There's one powermac with a M1 (the first clamshell ibook). I don't have
such a machine however, it would be useful if somebody who did could
test if the patch caused a regression there...

It's possible that we should instead base the code on the chip variant.

Ben.

Re: [PATCH] atyfb: fix distorted image on PowerMacs

[Mikael Pettersson]

Risto Suominen writes:
 > Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
 > and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
 > columns in their framebuffer image. This seems to depend on a value (4) read
 > from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
 > be written to the chip. This patch uses a value calculated by aty_init_pll_ct
 > instead, as a starting point.
 > 
 > Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>

Thank you. I reported this issue way back in March 2005
<http://marc.info/?l=linux-kernel&m=111175371705665&w=2>
but nothing ever came out of that. Your patch applies
cleanly to 2.6.29-rc2 and fixes the issue on my Beige G3.

Tested-by: Mikael Pettersson <mikpe@it.uu.se>


 > ---
 > The testing is done on kernel version 2.6.18.
 > 
 > --- a/drivers/video/aty/mach64_ct.c.org	2006-09-20 06:42:06.000000000 +0300
 > +++ b/drivers/video/aty/mach64_ct.c	2009-01-22 14:28:13.000000000 +0200
 > @@ -7,6 +7,7 @@
 >  #include <linux/delay.h>
 >  #include <asm/io.h>
 >  #include <video/mach64.h>
 > +#include <asm/machdep.h>
 >  #include "atyfb.h"
 > 
 >  #undef DEBUG
 > @@ -536,6 +537,14 @@ static int __init aty_init_pll_ct(const
 >  	pll->ct.xclk_post_div_real = postdividers[xpost_div];
 >  	pll->ct.mclk_fb_div = q * pll->ct.xclk_post_div_real / 8;
 > 
 > +#ifdef CONFIG_PPC
 > +	if (machine_is(powermac)) {
 > +		/* Override PLL_EXT_CNTL & 0x07. */
 > +		pll->ct.xclk_post_div = xpost_div;
 > +		pll->ct.xclk_ref_div = 1;
 > +	}
 > +#endif
 > +
 >  #ifdef DEBUG
 >  	pllmclk = (1000000 * pll->ct.mclk_fb_mult * pll->ct.mclk_fb_div) /
 >  			(par->ref_clk_per * pll->ct.pll_ref_div);
 > Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
 > and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
 > columns in their framebuffer image. This seems to depend on a value (4) read
 > from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
 > be written to the chip. This patch uses a value calculated by aty_init_pll_ct
 > instead, as a starting point.
 > 
 > Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
 > ---
 > The testing is done on kernel version 2.6.18.
 > 
 > --- a/drivers/video/aty/mach64_ct.c.org	2006-09-20 06:42:06.000000000 +0300
 > +++ b/drivers/video/aty/mach64_ct.c	2009-01-22 14:28:13.000000000 +0200
 > @@ -7,6 +7,7 @@
 >  #include <linux/delay.h>
 >  #include <asm/io.h>
 >  #include <video/mach64.h>
 > +#include <asm/machdep.h>
 >  #include "atyfb.h"
 >  
 >  #undef DEBUG
 > @@ -536,6 +537,14 @@ static int __init aty_init_pll_ct(const 
 >  	pll->ct.xclk_post_div_real = postdividers[xpost_div];
 >  	pll->ct.mclk_fb_div = q * pll->ct.xclk_post_div_real / 8;
 >  
 > +#ifdef CONFIG_PPC
 > +	if (machine_is(powermac)) {
 > +		/* Override PLL_EXT_CNTL & 0x07. */
 > +		pll->ct.xclk_post_div = xpost_div;
 > +		pll->ct.xclk_ref_div = 1;
 > +	}
 > +#endif
 > +
 >  #ifdef DEBUG
 >  	pllmclk = (1000000 * pll->ct.mclk_fb_mult * pll->ct.mclk_fb_div) /
 >  			(par->ref_clk_per * pll->ct.pll_ref_div);

Re: [PATCH] atyfb: fix distorted image on PowerMacs

[Benjamin Herrenschmidt]

On Thu, 2009-01-22 at 20:22 +0200, Risto Suominen wrote:
> Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
> and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
> columns in their framebuffer image. This seems to depend on a value (4) read
> from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
> be written to the chip. This patch uses a value calculated by aty_init_pll_ct
> instead, as a starting point.
> 
> Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
> ---
> The testing is done on kernel version 2.6.18.

This also fixes my oooold wallstreet powerbook.

Andrew, do you mind if I include that in the powerpc tree ?

Cheers,
Ben.

> --- a/drivers/video/aty/mach64_ct.c.org	2006-09-20 06:42:06.000000000 +0300
> +++ b/drivers/video/aty/mach64_ct.c	2009-01-22 14:28:13.000000000 +0200
> @@ -7,6 +7,7 @@
>  #include <linux/delay.h>
>  #include <asm/io.h>
>  #include <video/mach64.h>
> +#include <asm/machdep.h>
>  #include "atyfb.h"
> 
>  #undef DEBUG
> @@ -536,6 +537,14 @@ static int __init aty_init_pll_ct(const
>  	pll->ct.xclk_post_div_real = postdividers[xpost_div];
>  	pll->ct.mclk_fb_div = q * pll->ct.xclk_post_div_real / 8;
> 
> +#ifdef CONFIG_PPC
> +	if (machine_is(powermac)) {
> +		/* Override PLL_EXT_CNTL & 0x07. */
> +		pll->ct.xclk_post_div = xpost_div;
> +		pll->ct.xclk_ref_div = 1;
> +	}
> +#endif
> +
>  #ifdef DEBUG
>  	pllmclk = (1000000 * pll->ct.mclk_fb_mult * pll->ct.mclk_fb_div) /
>  			(par->ref_clk_per * pll->ct.pll_ref_div);

Re: [PATCH] atyfb: fix distorted image on PowerMacs

[Andrew Morton]

On Fri, 30 Jan 2009 14:42:50 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Thu, 2009-01-22 at 20:22 +0200, Risto Suominen wrote:
> > Since the complete re-write in 2.6.10, some PowerMacs (At least PowerMac 5500
> > and PowerMac G3 Beige rev A) with ATI Mach64 chip have suffered from unstable
> > columns in their framebuffer image. This seems to depend on a value (4) read
> > from PLL_EXT_CNTL register, which leads to incorrect DSP config parameters to
> > be written to the chip. This patch uses a value calculated by aty_init_pll_ct
> > instead, as a starting point.
> > 
> > Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
> > ---
> > The testing is done on kernel version 2.6.18.
> 
> This also fixes my oooold wallstreet powerbook.
> 
> Andrew, do you mind if I include that in the powerpc tree ?

Go wild.

Please don't to forget to add 'Cc: <stable@kernel.org>' to the changelog
if you consider a backport to be appropriate.