Re: [PATCH 2/4] drm/radeon: convert fence to uint64_t

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

On 03.05.2012 18:34, Jerome Glisse wrote:
> On Thu, May 3, 2012 at 12:29 PM, Alex Deucher<alexdeucher@gmail.com>  wrote:
>> On Thu, May 3, 2012 at 11:56 AM, Jerome Glisse<j.glisse@gmail.com>  wrote:
>>> On Thu, May 3, 2012 at 7:39 AM, Christian König<deathsimple@vodafone.de>  wrote:
>>>> On 03.05.2012 09:21, Michel Dänzer wrote:
>>>>> On Mit, 2012-05-02 at 16:20 -0400, j.glisse@gmail.com wrote:
>>>>>> From: Jerome Glisse<jglisse@redhat.com>
>>>>>>
>>>>>> 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<jglisse@redhat.com>
>>>>> [...]
>>>>>
>>>>>> 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 =rdev->wb.wb[index/4];
>>>>>> +       rdev->fence_drv[ring].cpu_addr =u64*)&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 fences, 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.