Re: [PATCH 2/2] drm/radeon: fix bank tiling parameters on SI

2012-07-31 Thread Alex Deucher
On Tue, Jul 31, 2012 at 7:48 AM, Christian König
deathsim...@vodafone.de wrote:
 The sixteen bank case wasn't handled here, leading to GPU
 crashes because of userspace miscalculation.

You mean a GPU hang or a segfault in userspace?  IIRC, from the hw
perspective numbers of banks over 8 are considered 8 for tiling.

Alex


 Signed-off-by: Christian König deathsim...@vodafone.de
 Cc: sta...@vger.kernel.org
 ---
  drivers/gpu/drm/radeon/si.c |   16 
  1 file changed, 12 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
 index 0b02792..0182fc8 100644
 --- a/drivers/gpu/drm/radeon/si.c
 +++ b/drivers/gpu/drm/radeon/si.c
 @@ -1639,11 +1639,19 @@ static void si_gpu_init(struct radeon_device *rdev)
 /* XXX what about 12? */
 rdev-config.si.tile_config |= (3  0);
 break;
 -   }
 -   if ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT)
 -   rdev-config.si.tile_config |= 1  4;
 -   else
 +   }
 +   switch ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT) {
 +   case 0: /* four banks */
 rdev-config.si.tile_config |= 0  4;
 +   break;
 +   case 1: /* eight banks */
 +   rdev-config.si.tile_config |= 1  4;
 +   break;
 +   case 2: /* sixteen banks */
 +   default:
 +   rdev-config.si.tile_config |= 2  4;
 +   break;
 +   }
 rdev-config.si.tile_config |=
 ((gb_addr_config  PIPE_INTERLEAVE_SIZE_MASK)  
 PIPE_INTERLEAVE_SIZE_SHIFT)  8;
 rdev-config.si.tile_config |=
 --
 1.7.9.5

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/2] drm/radeon: fix bank tiling parameters on SI

2012-07-31 Thread Christian König

On 31.07.2012 16:02, Alex Deucher wrote:

On Tue, Jul 31, 2012 at 7:48 AM, Christian König
deathsim...@vodafone.de wrote:

The sixteen bank case wasn't handled here, leading to GPU
crashes because of userspace miscalculation.

You mean a GPU hang or a segfault in userspace?  IIRC, from the hw
perspective numbers of banks over 8 are considered 8 for tiling.
Well it was a GPU segfault :) The GPU was accessing memory regions that 
weren't VM mapped.


As far as i could figure it out it wasn't happy with the alignment of 
the color buffer. The number of banks we used for that calculation 
seemed to be different from what the kernel programmed into the tiling 
config registers. So I tried changing it from 8 to 16 and hurray it 
started working (ok, more or less currently digging into the next problem).


It is possible that this is just masquerading another bug, but as far as 
I can see it is the most logical explanation.


Why are 16 banks equal to 8 banks on the hw side?

Christian.



Alex


Signed-off-by: Christian König deathsim...@vodafone.de
Cc: sta...@vger.kernel.org
---
  drivers/gpu/drm/radeon/si.c |   16 
  1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 0b02792..0182fc8 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -1639,11 +1639,19 @@ static void si_gpu_init(struct radeon_device *rdev)
 /* XXX what about 12? */
 rdev-config.si.tile_config |= (3  0);
 break;
-   }
-   if ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT)
-   rdev-config.si.tile_config |= 1  4;
-   else
+   }
+   switch ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT) {
+   case 0: /* four banks */
 rdev-config.si.tile_config |= 0  4;
+   break;
+   case 1: /* eight banks */
+   rdev-config.si.tile_config |= 1  4;
+   break;
+   case 2: /* sixteen banks */
+   default:
+   rdev-config.si.tile_config |= 2  4;
+   break;
+   }
 rdev-config.si.tile_config |=
 ((gb_addr_config  PIPE_INTERLEAVE_SIZE_MASK)  
PIPE_INTERLEAVE_SIZE_SHIFT)  8;
 rdev-config.si.tile_config |=
--
1.7.9.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/2] drm/radeon: fix bank tiling parameters on SI

2012-07-31 Thread Alex Deucher
On Tue, Jul 31, 2012 at 10:21 AM, Christian König
deathsim...@vodafone.de wrote:
 On 31.07.2012 16:02, Alex Deucher wrote:

 On Tue, Jul 31, 2012 at 7:48 AM, Christian König
 deathsim...@vodafone.de wrote:

 The sixteen bank case wasn't handled here, leading to GPU
 crashes because of userspace miscalculation.

 You mean a GPU hang or a segfault in userspace?  IIRC, from the hw
 perspective numbers of banks over 8 are considered 8 for tiling.

 Well it was a GPU segfault :) The GPU was accessing memory regions that
 weren't VM mapped.

 As far as i could figure it out it wasn't happy with the alignment of the
 color buffer. The number of banks we used for that calculation seemed to be
 different from what the kernel programmed into the tiling config registers.
 So I tried changing it from 8 to 16 and hurray it started working (ok, more
 or less currently digging into the next problem).

 It is possible that this is just masquerading another bug, but as far as I
 can see it is the most logical explanation.

Seems reasonable.

Reviewed-by: Alex Deucher alexander.deuc...@amd.com


 Why are 16 banks equal to 8 banks on the hw side?

I was talking to one of the hw guys about some issues with tiling.
Certain 6xx or 7xx chips report more than 8 banks in the MC config,
but the tiling configuration on them only supports 4 and 8 banks.
IIRC, he said number of banks above 8 should be treated as 8 at least
for 6xx-9xx.  Although, looking at it again it looks like maybe we
want 16 bank support for evergreen/cayman as well after all.  It
shouldn't hurt other than possibly over allocating a bit.

Alex


 Christian.



 Alex

 Signed-off-by: Christian König deathsim...@vodafone.de
 Cc: sta...@vger.kernel.org
 ---
   drivers/gpu/drm/radeon/si.c |   16 
   1 file changed, 12 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
 index 0b02792..0182fc8 100644
 --- a/drivers/gpu/drm/radeon/si.c
 +++ b/drivers/gpu/drm/radeon/si.c
 @@ -1639,11 +1639,19 @@ static void si_gpu_init(struct radeon_device
 *rdev)
  /* XXX what about 12? */
  rdev-config.si.tile_config |= (3  0);
  break;
 -   }
 -   if ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT)
 -   rdev-config.si.tile_config |= 1  4;
 -   else
 +   }
 +   switch ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT) {
 +   case 0: /* four banks */
  rdev-config.si.tile_config |= 0  4;
 +   break;
 +   case 1: /* eight banks */
 +   rdev-config.si.tile_config |= 1  4;
 +   break;
 +   case 2: /* sixteen banks */
 +   default:
 +   rdev-config.si.tile_config |= 2  4;
 +   break;
 +   }
  rdev-config.si.tile_config |=
  ((gb_addr_config  PIPE_INTERLEAVE_SIZE_MASK) 
 PIPE_INTERLEAVE_SIZE_SHIFT)  8;
  rdev-config.si.tile_config |=
 --
 1.7.9.5

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/2] drm/radeon: fix bank tiling parameters on SI

2012-07-31 Thread Christian König

On 31.07.2012 16:42, Alex Deucher wrote:

On Tue, Jul 31, 2012 at 10:21 AM, Christian König
deathsim...@vodafone.de wrote:

On 31.07.2012 16:02, Alex Deucher wrote:

On Tue, Jul 31, 2012 at 7:48 AM, Christian König
deathsim...@vodafone.de wrote:

The sixteen bank case wasn't handled here, leading to GPU
crashes because of userspace miscalculation.

You mean a GPU hang or a segfault in userspace?  IIRC, from the hw
perspective numbers of banks over 8 are considered 8 for tiling.

Well it was a GPU segfault :) The GPU was accessing memory regions that
weren't VM mapped.

As far as i could figure it out it wasn't happy with the alignment of the
color buffer. The number of banks we used for that calculation seemed to be
different from what the kernel programmed into the tiling config registers.
So I tried changing it from 8 to 16 and hurray it started working (ok, more
or less currently digging into the next problem).

It is possible that this is just masquerading another bug, but as far as I
can see it is the most logical explanation.

Seems reasonable.

Reviewed-by: Alex Deucher alexander.deuc...@amd.com


Why are 16 banks equal to 8 banks on the hw side?

I was talking to one of the hw guys about some issues with tiling.
Certain 6xx or 7xx chips report more than 8 banks in the MC config,
but the tiling configuration on them only supports 4 and 8 banks.
IIRC, he said number of banks above 8 should be treated as 8 at least
for 6xx-9xx.  Although, looking at it again it looks like maybe we
want 16 bank support for evergreen/cayman as well after all.  It
shouldn't hurt other than possibly over allocating a bit.

The alternative is to reduce the number of banks in the gb_tile_mode* regs.

I should also note that I have encountered a couple of more problems 
with SI and 16 banks, they go away if I reduce the banks to 8. Not sure 
what the reason is but it is possible that our surface_manager can't 
handle 16 banks yet.


So be careful if you enable that for cayman and evergreen.

Cheers,
Christian.



Alex


Christian.



Alex


Signed-off-by: Christian König deathsim...@vodafone.de
Cc: sta...@vger.kernel.org
---
   drivers/gpu/drm/radeon/si.c |   16 
   1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 0b02792..0182fc8 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -1639,11 +1639,19 @@ static void si_gpu_init(struct radeon_device
*rdev)
  /* XXX what about 12? */
  rdev-config.si.tile_config |= (3  0);
  break;
-   }
-   if ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT)
-   rdev-config.si.tile_config |= 1  4;
-   else
+   }
+   switch ((mc_arb_ramcfg  NOOFBANK_MASK)  NOOFBANK_SHIFT) {
+   case 0: /* four banks */
  rdev-config.si.tile_config |= 0  4;
+   break;
+   case 1: /* eight banks */
+   rdev-config.si.tile_config |= 1  4;
+   break;
+   case 2: /* sixteen banks */
+   default:
+   rdev-config.si.tile_config |= 2  4;
+   break;
+   }
  rdev-config.si.tile_config |=
  ((gb_addr_config  PIPE_INTERLEAVE_SIZE_MASK) 
PIPE_INTERLEAVE_SIZE_SHIFT)  8;
  rdev-config.si.tile_config |=
--
1.7.9.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel




___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel