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

["Christian König" <deathsimple@vodafone.de>]

On 31.07.2012 16:42, Alex Deucher wrote:
> On Tue, Jul 31, 2012 at 10:21 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 31.07.2012 16:02, Alex Deucher wrote:
>>> On Tue, Jul 31, 2012 at 7:48 AM, Christian König
>>> <deathsimple@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.deucher@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 <deathsimple@vodafone.de>
>>>> Cc: stable@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
>>