From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH 2/4] drm/radeon: convert fence to uint64_t Date: Thu, 03 May 2012 18:45:50 +0200 Message-ID: <4FA2B63E.2070401@vodafone.de> References: <1335990013-5877-1-git-send-email-j.glisse@gmail.com> <1335990013-5877-3-git-send-email-j.glisse@gmail.com> <1336029696.17679.43.camel@thor.local> <4FA26E85.8020303@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 BD1229E7A4 for ; Thu, 3 May 2012 09:45: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: Jerome Glisse Cc: =?ISO-8859-1?Q?r?= , dri-devel@lists.freedesktop.org, =?ISO-8859-1?Q?Michel_D=E4nze?=@freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 03.05.2012 18:34, Jerome Glisse wrote: > On Thu, May 3, 2012 at 12:29 PM, Alex Deucher wro= te: >> On Thu, May 3, 2012 at 11:56 AM, Jerome Glisse wrot= e: >>> On Thu, May 3, 2012 at 7:39 AM, Christian K=F6nig wrote: >>>> On 03.05.2012 09:21, Michel D=E4nzer wrote: >>>>> On Mit, 2012-05-02 at 16:20 -0400, j.glisse@gmail.com wrote: >>>>>> From: Jerome Glisse >>>>>> >>>>>> This convert fence to use uint64_t sequence number intention is >>>>>> to use the fact that uin64_t is big enough that we don't need to >>>>>> care about wrap around. >>>>>> >>>>>> Tested with and without writeback using 0xFFFFF000 as initial >>>>>> fence sequence and thus allowing to test the wrap around from >>>>>> 32bits to 64bits. >>>>>> >>>>>> Signed-off-by: Jerome Glisse >>>>> [...] >>>>> >>>>>> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c >>>>>> b/drivers/gpu/drm/radeon/radeon_fence.c >>>>>> index 7733429..6da1535 100644 >>>>>> --- a/drivers/gpu/drm/radeon/radeon_fence.c >>>>>> +++ b/drivers/gpu/drm/radeon/radeon_fence.c >>>>>> @@ -386,9 +388,9 @@ int radeon_fence_driver_start_ring(struct >>>>>> radeon_device *rdev, int ring) >>>>>> rdev->fence_drv[ring].scratch_reg - >>>>>> rdev->scratch.reg_base; >>>>>> } >>>>>> - rdev->fence_drv[ring].cpu_addr =3Drdev->wb.wb[index/4]; >>>>>> + rdev->fence_drv[ring].cpu_addr =3Du64*)&rdev->wb.wb[index/4]; >>>>> Might want to ensure cpu_addr is 64 bit aligned, or there might be >>>>> trouble on some architectures. >>>>> >>>>> >>>>> With this change, Cayman cards will already use six scratch registers >>>>> for the rings. It won't be possible to extend this scheme for even one >>>>> additional ring, will it? >>>> >>>> That won't work anyway, since not all rings can deal with 64 bit fence= s, so >>>> we need to still use 32 bit signaling and extend them to 64 bit while >>>> processing the fence value. >>>> >>>> Already working on that. >>>> >>>> Christian. >>> This patch is fine with ring that can't emit directly 64bits, all you >>> have to do is fix the emit_fence callback to properly handle it and >>> then you have to fix the radeon_fence_read which can be move to a ring >>> specific callback. Anyway point is that patchset works and is fine on >>> current set of ring we have and it can work as easily for ring without >>> easy 64bits value emitting. So please explain further why those patch >>> can't work because as i just explained i don't see why. >>> >>> I have updated some v2 version of those patchset to handle the cayman >>> and newer possibly running out of scratch reg and i also fix the >>> alignment issue to be 64bits >> FWIW, we don't actually use scratch regs any more on r6xx+ (non-AGP at >> least), it's just memory writes so we could make the scratch pool >> bigger. >> >> Alex >> > That's what my v2 does, just drop scratch reg for cayman and newer. > > Cheers, > Jerome > I actually always wanted to change that in a way that scratch regs are = only allocated if wb is really disabled, just never had time to do so = (sigh). Cheers, Christian.