From patchwork Fri Jul 5 14:11:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Deucher X-Patchwork-Id: 2824231 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 11153BF4A1 for ; Fri, 5 Jul 2013 14:12:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BA26220165 for ; Fri, 5 Jul 2013 14:12:21 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2060D20164 for ; Fri, 5 Jul 2013 14:12:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DEB3EE5CB0 for ; Fri, 5 Jul 2013 07:12:19 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-vc0-f178.google.com (mail-vc0-f178.google.com [209.85.220.178]) by gabe.freedesktop.org (Postfix) with ESMTP id 146CBE5D1C for ; Fri, 5 Jul 2013 07:11:18 -0700 (PDT) Received: by mail-vc0-f178.google.com with SMTP id m17so1662587vca.23 for ; Fri, 05 Jul 2013 07:11:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=NgP8RZ3NfpiSV4maPi0dju0VJbIpUwRVUlHdZxf7fDA=; b=bUTh7hfOih5KkiZ5y1ErU4oV/RfRv2tWFDLI5cVphonutlkb4DRAHj1H6qxaTfAG2n D/umPXVbE/AqevccwocodpQo+P9ce+tmPkyCdGPz9SyDhw8zh1DNxyeb/11axgQJbAtq qk00oSX6xtmrfnoGPXsgEMzn5WFQ2F6yN89bqkKPISkaSQs8snhdNn/JvJc4ULg7vNnV Sh8GYqjecXYbEPWGm+1F1+hPEyRGaOd/ROmm5EQAAisrGbJA1vVNXFkt9/UD+GbSi4qm oZYAWjTVym5am1TufLD222+BctrCr7VE7DjzaFJB7weKOXYwxnRlRQhFCI1vzI1pAim+ b6ww== MIME-Version: 1.0 X-Received: by 10.220.83.69 with SMTP id e5mr6886740vcl.53.1373033478412; Fri, 05 Jul 2013 07:11:18 -0700 (PDT) Received: by 10.58.29.3 with HTTP; Fri, 5 Jul 2013 07:11:18 -0700 (PDT) In-Reply-To: <201307032240.r63MeW8L002902@glazunov.sibelius.xs4all.nl> References: <201307032240.r63MeW8L002902@glazunov.sibelius.xs4all.nl> Date: Fri, 5 Jul 2013 10:11:18 -0400 Message-ID: Subject: Re: [PATCH] radeon: Fix surface register on r100 From: Alex Deucher To: Mark Kettenis Cc: dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Jul 3, 2013 at 6:40 PM, Mark Kettenis wrote: > Working on KMS support on OpenBSD/sparc64, I ended up with the initial > framebuffer on a Sun XVR-100 card (Radeon 7000/VE, RV100 with > OpenFirmware) being tiled when none of the tiling flags were set. > Tracked it down to an issue with r100_set_surface_reg(). The tiling > bits on these older chips are a bit different than the later ones. > There is no real flag for macro tiled buffer; > RADEON_SURF_TILE_COLOR_MACRO is 0. So if we aren't actually tiling, > we can't actually indicate that by not setting that bit. Instead we > should make sure that we set the size of tiles to 0. The diff below > reorganizes the code handling these variants a bit to do that. It > also seems that you can't have a buffer that's only micro tiled. The > diff turns that into a BUG(), but perhaps that isn't such a good idea > since I believe that userland can actually request such a tiling and > the ioctl code doesn't check this. So perhaps it should just fall > through into the no-tiling case. It's a little bit funny on r1xx compared to newer asics: SURFACE0_INFO - RW - 32 bits - [MMReg:0xB0C] Field Name Bits Default Description SURF0_PITCHSEL 9:0 0x0 Pitch in octawords (16 bytes) of Surface 0. A value of 0 disables tiling in Surface 0. SURF0_TILE_MODE 17:16 0x0 Mode of tiling for Surface 0. Set SURF0_PITCHSEL to 0 to disable tiling surface 0. 0=Disable MicroTiling 1=Enable MicroTiling 2=32 bit Z tiling 3=16 bit Z tiling Does the attached patch fix it? Alex > > With this fixed, I'm pretty sure the special RN50 handling can go. > The extra /= 16 was probably enough to make the pitch small enough to > keep the hardware from tiling. > > Signed-off-by: Mark Kettenis > > diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c > index d0314ec..84d832a 100644 > --- a/drivers/gpu/drm/radeon/r100.c > +++ b/drivers/gpu/drm/radeon/r100.c > @@ -3072,11 +3072,20 @@ int r100_set_surface_reg(struct radeon_device *rdev, int reg, > int flags = 0; > > if (rdev->family <= CHIP_RS200) { > - if ((tiling_flags & (RADEON_TILING_MACRO|RADEON_TILING_MICRO)) > - == (RADEON_TILING_MACRO|RADEON_TILING_MICRO)) > + switch (tiling_flags & (RADEON_TILING_MACRO|RADEON_TILING_MICRO)) { > + case RADEON_TILING_MACRO|RADEON_TILING_MICRO: > flags |= RADEON_SURF_TILE_COLOR_BOTH; > - if (tiling_flags & RADEON_TILING_MACRO) > + break; > + case RADEON_TILING_MACRO: > flags |= RADEON_SURF_TILE_COLOR_MACRO; > + break; > + case RADEON_TILING_MICRO: > + BUG(); > + break; > + case 0: > + pitch = 0; > + break; > + } > } else if (rdev->family <= CHIP_RV280) { > if (tiling_flags & (RADEON_TILING_MACRO)) > flags |= R200_SURF_TILE_COLOR_MACRO; > @@ -3094,13 +3103,6 @@ int r100_set_surface_reg(struct radeon_device *rdev, int reg, > if (tiling_flags & RADEON_TILING_SWAP_32BIT) > flags |= RADEON_SURF_AP0_SWP_32BPP | RADEON_SURF_AP1_SWP_32BPP; > > - /* when we aren't tiling the pitch seems to needs to be furtherdivided down. - tested on power5 + rn50 server */ > - if (tiling_flags & (RADEON_TILING_SWAP_16BIT | RADEON_TILING_SWAP_32BIT)) { > - if (!(tiling_flags & (RADEON_TILING_MACRO | RADEON_TILING_MICRO))) > - if (ASIC_IS_RN50(rdev)) > - pitch /= 16; > - } > - > /* r100/r200 divide by 16 */ > if (rdev->family < CHIP_R300) > flags |= pitch / 16; > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel From ebaef472e37eb3f844f16ff9d4b9fa4a09e3d805 Mon Sep 17 00:00:00 2001 From: Alex Deucher Date: Fri, 5 Jul 2013 10:05:49 -0400 Subject: [PATCH] drm/radeon: fix surface setup on r1xx r1xx asics have a slightly different surface register setup compared to newer asics. There is no specific enable bit for macro tiling, rather, to disable macro tiling, you need to set the surface pitch to 0. With this fixed, the special rn50 handling can go. Noticed-by: Mark Kettenis Signed-off-by: Alex Deucher --- drivers/gpu/drm/radeon/r100.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index d0314ec..c9affef 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -3077,6 +3077,10 @@ int r100_set_surface_reg(struct radeon_device *rdev, int reg, flags |= RADEON_SURF_TILE_COLOR_BOTH; if (tiling_flags & RADEON_TILING_MACRO) flags |= RADEON_SURF_TILE_COLOR_MACRO; + /* setting pitch to 0 disables tiling */ + if ((tiling_flags & (RADEON_TILING_MACRO|RADEON_TILING_MICRO)) + == 0) + pitch = 0; } else if (rdev->family <= CHIP_RV280) { if (tiling_flags & (RADEON_TILING_MACRO)) flags |= R200_SURF_TILE_COLOR_MACRO; @@ -3094,13 +3098,6 @@ int r100_set_surface_reg(struct radeon_device *rdev, int reg, if (tiling_flags & RADEON_TILING_SWAP_32BIT) flags |= RADEON_SURF_AP0_SWP_32BPP | RADEON_SURF_AP1_SWP_32BPP; - /* when we aren't tiling the pitch seems to needs to be furtherdivided down. - tested on power5 + rn50 server */ - if (tiling_flags & (RADEON_TILING_SWAP_16BIT | RADEON_TILING_SWAP_32BIT)) { - if (!(tiling_flags & (RADEON_TILING_MACRO | RADEON_TILING_MICRO))) - if (ASIC_IS_RN50(rdev)) - pitch /= 16; - } - /* r100/r200 divide by 16 */ if (rdev->family < CHIP_R300) flags |= pitch / 16; -- 1.7.7.5