2005-12-15 04:32:20

by Mark M. Hoffman

[permalink] [raw]
Subject: [BUG] Xserver startup locks system... git bisect results

Hello:

git bisect said:
> 47807ce381acc34a7ffee2b42e35e96c0f322e52 is first bad commit
> diff-tree 47807ce381acc34a7ffee2b42e35e96c0f322e52 (from 0e670506668a43e1355b8f10c33d081a676bd521)
> Author: Dave Airlie <[email protected]>
> Date: Tue Dec 13 04:18:41 2005 +0000
>
> [drm] fix radeon aperture issue

With this one applied, my machine locks up tight just after starting the
Xserver. Some info (dmesg, lspci, config) is here:

http://members.dca.net/mhoffman/lkml-20051214/

I can put a serial console on it if necessary, but not until about this
time tomorrow.

Regards,

--
Mark M. Hoffman
[email protected]


2005-12-15 04:56:57

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

On Wed, 2005-12-14 at 23:32 -0500, Mark M. Hoffman wrote:
> Hello:
>
> git bisect said:
> > 47807ce381acc34a7ffee2b42e35e96c0f322e52 is first bad commit
> > diff-tree 47807ce381acc34a7ffee2b42e35e96c0f322e52 (from 0e670506668a43e1355b8f10c33d081a676bd521)
> > Author: Dave Airlie <[email protected]>
> > Date: Tue Dec 13 04:18:41 2005 +0000
> >
> > [drm] fix radeon aperture issue
>
> With this one applied, my machine locks up tight just after starting the
> Xserver. Some info (dmesg, lspci, config) is here:
>
> http://members.dca.net/mhoffman/lkml-20051214/
>
> I can put a serial console on it if necessary, but not until about this
> time tomorrow.

You have to love this X radeon driver ... you can't fix one bug without
breaking something else, it's one of the worst piece of crap I've ever
seen...

What would be useful now is the X version and maybe trying a little hack
in the X driver. Do you have ways to rebuild the X driver at all ?

The problem is, that patch actually fixes some users... Ah, also, could
you maybe add some printk's around the code that is modified by that
patch and try to catch the value it tries to use before the lockup ?

That is, print the values of:

dev_priv->fb_location

and

RADEON_READ(RADEON_CONFIG_APER_SIZE)

Thanks,
Ben.


2005-12-15 05:11:13

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

Ah, also, something else you can try, is replace

dev_priv->gart_vm_start = dev_priv->fb_location
+ RADEON_READ(RADEON_CONFIG_APER_SIZE);
with

#define RADEON_CONFIG_MEMSIZE 0x00F8
dev_priv->gart_vm_start = dev_priv->fb_location
+ RADEON_READ(RADEON_CONFIG_MEMSIZE);

And tell us if it helps ?

Ben.


2005-12-15 05:19:38

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

On Thu, 2005-12-15 at 16:07 +1100, Benjamin Herrenschmidt wrote:
> Ah, also, something else you can try, is replace
>
> dev_priv->gart_vm_start = dev_priv->fb_location
> + RADEON_READ(RADEON_CONFIG_APER_SIZE);

Actually, the above should read

+ RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;

With the patch that is causing your problem...

> with
>
> #define RADEON_CONFIG_MEMSIZE 0x00F8
> dev_priv->gart_vm_start = dev_priv->fb_location
> + RADEON_READ(RADEON_CONFIG_MEMSIZE);
>
> And tell us if it helps ?
>
> Ben.
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2005-12-15 07:55:38

by Dave Airlie

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

> > [drm] fix radeon aperture issue
>
> With this one applied, my machine locks up tight just after starting the
> Xserver. Some info (dmesg, lspci, config) is here:

Can you give me an Xorg.0.log and /etc/X11/xorg.conf

I've got the same card here and it seems to work for me .. so maybe
its a configuration issue..

Dave.

2005-12-15 09:04:03

by Paul Mackerras

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

Benjamin Herrenschmidt writes:

> On Thu, 2005-12-15 at 16:07 +1100, Benjamin Herrenschmidt wrote:
> > Ah, also, something else you can try, is replace
> >
> > dev_priv->gart_vm_start = dev_priv->fb_location
> > + RADEON_READ(RADEON_CONFIG_APER_SIZE);
>
> Actually, the above should read
>
> + RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;

With the patch below, my powerbook will sleep and wake up
successfully.

Paul.

diff --git a/drivers/char/drm/radeon_cp.c b/drivers/char/drm/radeon_cp.c
index 9f2b4ef..465fdc2 100644
--- a/drivers/char/drm/radeon_cp.c
+++ b/drivers/char/drm/radeon_cp.c
@@ -1522,7 +1522,7 @@ static int radeon_do_init_cp(drm_device_

dev_priv->gart_size = init->gart_size;
dev_priv->gart_vm_start = dev_priv->fb_location
- + RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;
+ + RADEON_READ(RADEON_CONFIG_MEMSIZE);

#if __OS_HAS_AGP
if (!dev_priv->is_pci)
diff --git a/drivers/char/drm/radeon_drv.h b/drivers/char/drm/radeon_drv.h
index 7bda7e3..d92ccee 100644
--- a/drivers/char/drm/radeon_drv.h
+++ b/drivers/char/drm/radeon_drv.h
@@ -379,6 +379,7 @@ extern int r300_do_cp_cmdbuf(drm_device_
# define RADEON_PLL_WR_EN (1 << 7)
#define RADEON_CLOCK_CNTL_INDEX 0x0008
#define RADEON_CONFIG_APER_SIZE 0x0108
+#define RADEON_CONFIG_MEMSIZE 0x00f8
#define RADEON_CRTC_OFFSET 0x0224
#define RADEON_CRTC_OFFSET_CNTL 0x0228
# define RADEON_CRTC_TILE_EN (1 << 15)

2005-12-15 13:40:54

by Mark M. Hoffman

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

Hi Dave:

* Dave Airlie <[email protected]> [2005-12-15 18:55:37 +1100]:
> > > [drm] fix radeon aperture issue
> >
> > With this one applied, my machine locks up tight just after starting the
> > Xserver. Some info (dmesg, lspci, config) is here:
>
> Can you give me an Xorg.0.log and /etc/X11/xorg.conf
>
> I've got the same card here and it seems to work for me .. so maybe
> its a configuration issue..

I've added both files here:

http://members.dca.net/mhoffman/lkml-20051214/

Regards,

--
Mark M. Hoffman
[email protected]

2005-12-15 13:45:47

by Mark M. Hoffman

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

Hi Benjamin:

* Benjamin Herrenschmidt <[email protected]> [2005-12-15 15:53:03 +1100]:
> On Wed, 2005-12-14 at 23:32 -0500, Mark M. Hoffman wrote:
> > Hello:
> >
> > git bisect said:
> > > 47807ce381acc34a7ffee2b42e35e96c0f322e52 is first bad commit
> > > diff-tree 47807ce381acc34a7ffee2b42e35e96c0f322e52 (from 0e670506668a43e1355b8f10c33d081a676bd521)
> > > Author: Dave Airlie <[email protected]>
> > > Date: Tue Dec 13 04:18:41 2005 +0000
> > >
> > > [drm] fix radeon aperture issue
> >
> > With this one applied, my machine locks up tight just after starting the
> > Xserver. Some info (dmesg, lspci, config) is here:
> >
> > http://members.dca.net/mhoffman/lkml-20051214/
> >
> > I can put a serial console on it if necessary, but not until about this
> > time tomorrow.
>
> You have to love this X radeon driver ... you can't fix one bug without
> breaking something else, it's one of the worst piece of crap I've ever
> seen...
>
> What would be useful now is the X version and maybe trying a little hack
> in the X driver. Do you have ways to rebuild the X driver at all ?

Check the link again for log & config. I have not built any part of X
from source before. But if it's necessary, I will try it.

> The problem is, that patch actually fixes some users... Ah, also, could
> you maybe add some printk's around the code that is modified by that
> patch and try to catch the value it tries to use before the lockup ?
>
> That is, print the values of:
>
> dev_priv->fb_location
>
> and
>
> RADEON_READ(RADEON_CONFIG_APER_SIZE)

These suggestions, along with later ones that involve recompiles... I will try
them late this evening (EST).

Regards,

--
Mark M. Hoffman
[email protected]

2005-12-16 03:50:41

by Mark M. Hoffman

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

Hi Paul, Benjamin:

* Paul Mackerras <[email protected]> [2005-12-15 20:03:59 +1100]:
> Benjamin Herrenschmidt writes:
>
> > On Thu, 2005-12-15 at 16:07 +1100, Benjamin Herrenschmidt wrote:
> > > Ah, also, something else you can try, is replace
> > >
> > > dev_priv->gart_vm_start = dev_priv->fb_location
> > > + RADEON_READ(RADEON_CONFIG_APER_SIZE);
> >
> > Actually, the above should read
> >
> > + RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;
>
> With the patch below, my powerbook will sleep and wake up
> successfully.

I added the printk's you (BH) asked for to Paul's patch, resulting in the
patch below. It works fine so far. Here's the relevant kernel log:

Dec 15 22:39:47 jupiter kernel: dev_priv->fb_location is 0xe8000000
Dec 15 22:39:47 jupiter kernel: RADEON_READ(RADEON_CONFIG_APER_SIZE) is 0x08000000
Dec 15 22:39:47 jupiter kernel: [drm] Loading R200 Microcode

Let me know if you need any more info. Thanks.

--- linux-2.6.15-rc5-radeon-test.orig/drivers/char/drm/radeon_cp.c
+++ linux-2.6.15-rc5-radeon-test/drivers/char/drm/radeon_cp.c
@@ -1522,7 +1522,12 @@ static int radeon_do_init_cp(drm_device_

dev_priv->gart_size = init->gart_size;
dev_priv->gart_vm_start = dev_priv->fb_location
- + RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;
+ + RADEON_READ(RADEON_CONFIG_MEMSIZE);
+
+printk(KERN_INFO "dev_priv->fb_location is 0x%08x\n",
+ dev_priv->fb_location);
+printk(KERN_INFO "RADEON_READ(RADEON_CONFIG_APER_SIZE) is 0x%08x\n",
+ RADEON_READ(RADEON_CONFIG_APER_SIZE));

#if __OS_HAS_AGP
if (!dev_priv->is_pci)
--- linux-2.6.15-rc5-radeon-test.orig/drivers/char/drm/radeon_drv.h
+++ linux-2.6.15-rc5-radeon-test/drivers/char/drm/radeon_drv.h
@@ -379,6 +379,7 @@ extern int r300_do_cp_cmdbuf(drm_device_
# define RADEON_PLL_WR_EN (1 << 7)
#define RADEON_CLOCK_CNTL_INDEX 0x0008
#define RADEON_CONFIG_APER_SIZE 0x0108
+#define RADEON_CONFIG_MEMSIZE 0x00f8
#define RADEON_CRTC_OFFSET 0x0224
#define RADEON_CRTC_OFFSET_CNTL 0x0228
# define RADEON_CRTC_TILE_EN (1 << 15)

--
Mark M. Hoffman
[email protected]

2005-12-16 04:47:52

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

On Thu, 2005-12-15 at 22:50 -0500, Mark M. Hoffman wrote:

> I added the printk's you (BH) asked for to Paul's patch, resulting in the
> patch below. It works fine so far. Here's the relevant kernel log:

Ok, sounds good, it's also the right way to go. However, Linus, don't
merge that patch "as-is" as some chips have a bug with CONFIG_MEMSIZE
being 0 instead of 8k. I'll send you a proper patch and I'll fix the
remaining problems on X side (it's still bogus, though the DRM "fixes it
up" now).

Ben.


2005-12-16 05:52:45

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

Finally fixes the radeon memory mapping bug that was incorrectly
fixed by the previous patch. This time, we use the actual vram
size as the size to calculate how far to move the AGP aperture
from the framebuffer in card's memory space. If there are still
issues with this patch, they are due to bugs in the X driver that
I'm working on fixing too.

Signed-off-by: Benjamin Herrenschmidt <[email protected]>

Index: linux-work/drivers/char/drm/radeon_cp.c
===================================================================
--- linux-work.orig/drivers/char/drm/radeon_cp.c 2005-12-13 20:23:00.000000000 +1100
+++ linux-work/drivers/char/drm/radeon_cp.c 2005-12-16 16:17:30.000000000 +1100
@@ -1312,6 +1312,8 @@
static int radeon_do_init_cp(drm_device_t * dev, drm_radeon_init_t * init)
{
drm_radeon_private_t *dev_priv = dev->dev_private;;
+ unsigned int mem_size;
+
DRM_DEBUG("\n");

dev_priv->is_pci = init->is_pci;
@@ -1521,8 +1523,11 @@
+ dev_priv->fb_location) >> 10));

dev_priv->gart_size = init->gart_size;
- dev_priv->gart_vm_start = dev_priv->fb_location
- + RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;
+
+ mem_size = RADEON_READ(RADEON_CONFIG_MEMSIZE);
+ if (mem_size == 0)
+ mem_size = 0x800000;
+ dev_priv->gart_vm_start = dev_priv->fb_location + mem_size;

#if __OS_HAS_AGP
if (!dev_priv->is_pci)
Index: linux-work/drivers/char/drm/radeon_drv.h
===================================================================
--- linux-work.orig/drivers/char/drm/radeon_drv.h 2005-11-25 15:03:35.000000000 +1100
+++ linux-work/drivers/char/drm/radeon_drv.h 2005-12-16 16:14:43.000000000 +1100
@@ -379,6 +379,7 @@
# define RADEON_PLL_WR_EN (1 << 7)
#define RADEON_CLOCK_CNTL_INDEX 0x0008
#define RADEON_CONFIG_APER_SIZE 0x0108
+#define RADEON_CONFIG_MEMSIZE 0x00f8
#define RADEON_CRTC_OFFSET 0x0224
#define RADEON_CRTC_OFFSET_CNTL 0x0228
# define RADEON_CRTC_TILE_EN (1 << 15)


2005-12-16 06:59:15

by Dave Airlie

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results


> Finally fixes the radeon memory mapping bug that was incorrectly
> fixed by the previous patch. This time, we use the actual vram
> size as the size to calculate how far to move the AGP aperture> from the
> framebuffer in card's memory space. If there are still issues with this
> patch, they are due to bugs in the X driver that I'm working on fixing
> too.
>
> Signed-off-by: Benjamin Herrenschmidt <[email protected]>

Acked-by: Dave Airlie <[email protected]>

>
> Index: linux-work/drivers/char/drm/radeon_cp.c
> ===================================================================
> --- linux-work.orig/drivers/char/drm/radeon_cp.c 2005-12-13 20:23:00.000000000 +1100
> +++ linux-work/drivers/char/drm/radeon_cp.c 2005-12-16 16:17:30.000000000 +1100
> @@ -1312,6 +1312,8 @@
> static int radeon_do_init_cp(drm_device_t * dev, drm_radeon_init_t * init)
> {
> drm_radeon_private_t *dev_priv = dev->dev_private;;
> + unsigned int mem_size;
> +
> DRM_DEBUG("\n");
>
> dev_priv->is_pci = init->is_pci;
> @@ -1521,8 +1523,11 @@
> + dev_priv->fb_location) >> 10));
>
> dev_priv->gart_size = init->gart_size;
> - dev_priv->gart_vm_start = dev_priv->fb_location
> - + RADEON_READ(RADEON_CONFIG_APER_SIZE) * 2;
> +
> + mem_size = RADEON_READ(RADEON_CONFIG_MEMSIZE);
> + if (mem_size == 0)
> + mem_size = 0x800000;
> + dev_priv->gart_vm_start = dev_priv->fb_location + mem_size;
>
> #if __OS_HAS_AGP
> if (!dev_priv->is_pci)
> Index: linux-work/drivers/char/drm/radeon_drv.h
> ===================================================================
> --- linux-work.orig/drivers/char/drm/radeon_drv.h 2005-11-25 15:03:35.000000000 +1100
> +++ linux-work/drivers/char/drm/radeon_drv.h 2005-12-16 16:14:43.000000000 +1100
> @@ -379,6 +379,7 @@
> # define RADEON_PLL_WR_EN (1 << 7)
> #define RADEON_CLOCK_CNTL_INDEX 0x0008
> #define RADEON_CONFIG_APER_SIZE 0x0108
> +#define RADEON_CONFIG_MEMSIZE 0x00f8
> #define RADEON_CRTC_OFFSET 0x0224
> #define RADEON_CRTC_OFFSET_CNTL 0x0228
> # define RADEON_CRTC_TILE_EN (1 << 15)
>
>

--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG

2005-12-27 07:48:43

by Will Dyson

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

On 12/16/05, Benjamin Herrenschmidt <[email protected]> wrote:
> Finally fixes the radeon memory mapping bug that was incorrectly
> fixed by the previous patch. This time, we use the actual vram
> size as the size to calculate how far to move the AGP aperture
> from the framebuffer in card's memory space. If there are still
> issues with this patch, they are due to bugs in the X driver that
> I'm working on fixing too.

My amd64 machine with an agp radeon9200SE locks up tight on xserver
start (with an MCE) unless I revert both patches. I'm using x.org 6.9
compiled from the debian svn tree. The output of 'lspci -vv' is
available here:

http://www.lucidts.com/~will/lspcivv.txt

Please let me know of any testing I can do on x.org radeon DDX
patches, or any additional information I can provide.

--
Will Dyson

2005-12-27 09:09:44

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG] Xserver startup locks system... git bisect results

On Tue, 2005-12-27 at 02:48 -0500, Will Dyson wrote:
> On 12/16/05, Benjamin Herrenschmidt <[email protected]> wrote:
> > Finally fixes the radeon memory mapping bug that was incorrectly
> > fixed by the previous patch. This time, we use the actual vram
> > size as the size to calculate how far to move the AGP aperture
> > from the framebuffer in card's memory space. If there are still
> > issues with this patch, they are due to bugs in the X driver that
> > I'm working on fixing too.
>
> My amd64 machine with an agp radeon9200SE locks up tight on xserver
> start (with an MCE) unless I revert both patches. I'm using x.org 6.9
> compiled from the debian svn tree. The output of 'lspci -vv' is
> available here:
>
> http://www.lucidts.com/~will/lspcivv.txt
>
> Please let me know of any testing I can do on x.org radeon DDX
> patches, or any additional information I can provide.

Can you try the additional patch I sent ? If it doesn't help, I'll try
to send later today or tomorow a patch adding more debug informations so
I can figure out what's going on...

Ben.