From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH 2/2] drm/radeon: fix bank tiling parameters on SI Date: Tue, 31 Jul 2012 17:21:49 +0200 Message-ID: <5017F80D.6060500@vodafone.de> References: <1343735331-4922-1-git-send-email-deathsimple@vodafone.de> <1343735331-4922-2-git-send-email-deathsimple@vodafone.de> <5017E9EC.70704@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from outgoing.email.vodafone.de (outgoing.email.vodafone.de [139.7.28.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 9E9B49E7E9 for ; Tue, 31 Jul 2012 08:21:53 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Alex Deucher Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 31.07.2012 16:42, Alex Deucher wrote: > On Tue, Jul 31, 2012 at 10:21 AM, Christian K=F6nig > wrote: >> On 31.07.2012 16:02, Alex Deucher wrote: >>> On Tue, Jul 31, 2012 at 7:48 AM, Christian K=F6nig >>> 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 registe= rs. >> So I tried changing it from 8 to 16 and hurray it started working (ok, m= ore >> 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 > >> 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=F6nig >>>> 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 |=3D (3 << 0); >>>> break; >>>> - } >>>> - if ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) >>>> - rdev->config.si.tile_config |=3D 1 << 4; >>>> - else >>>> + } >>>> + switch ((mc_arb_ramcfg & NOOFBANK_MASK) >> NOOFBANK_SHIFT) { >>>> + case 0: /* four banks */ >>>> rdev->config.si.tile_config |=3D 0 << 4; >>>> + break; >>>> + case 1: /* eight banks */ >>>> + rdev->config.si.tile_config |=3D 1 << 4; >>>> + break; >>>> + case 2: /* sixteen banks */ >>>> + default: >>>> + rdev->config.si.tile_config |=3D 2 << 4; >>>> + break; >>>> + } >>>> rdev->config.si.tile_config |=3D >>>> ((gb_addr_config & PIPE_INTERLEAVE_SIZE_MASK) >> >>>> PIPE_INTERLEAVE_SIZE_SHIFT) << 8; >>>> rdev->config.si.tile_config |=3D >>>> -- >>>> 1.7.9.5 >>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel@lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >>