2009-01-22 18:23:00

by Risto Suominen

[permalink] [raw]
Subject: [PATCH] atyfb: fix distorted image on PowerMacs

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 <[email protected]>
---
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);


Attachments:
aty-mach64-pmac.diff (1.28 kB)

2009-01-22 20:31:43

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [PATCH] atyfb: fix distorted image on PowerMacs

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 <[email protected]>
> ---
> 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?
[email protected]
http://www.sci.fi/~syrjala/

2009-01-23 14:22:17

by Mikael Pettersson

[permalink] [raw]
Subject: Re: [PATCH] atyfb: fix distorted image on PowerMacs

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 <[email protected]>

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 <[email protected]>


> ---
> 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 <[email protected]>
> ---
> 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);

2009-01-30 03:43:41

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] atyfb: fix distorted image on PowerMacs

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 <[email protected]>
> ---
> 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);

2009-01-30 04:38:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] atyfb: fix distorted image on PowerMacs

On Fri, 30 Jan 2009 14:42:50 +1100 Benjamin Herrenschmidt <[email protected]> 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 <[email protected]>
> > ---
> > 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: <[email protected]>' to the changelog
if you consider a backport to be appropriate.