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

[Alex Deucher]

On Thu, May 3, 2012 at 4:46 PM, Jerome Glisse <j.glisse at gmail.com> wrote:
> On Thu, May 3, 2012 at 12:34 PM, Jerome Glisse <j.glisse at gmail.com> wrote:
>> On Thu, May 3, 2012 at 12:29 PM, Alex Deucher <alexdeucher at gmail.com> wrote:
>>> On Thu, May 3, 2012 at 11:56 AM, Jerome Glisse <j.glisse at gmail.com> wrote:
>>>> On Thu, May 3, 2012 at 7:39 AM, Christian König <deathsimple at vodafone.de> wrote:
>>>>> On 03.05.2012 09:21, Michel Dänzer wrote:
>>>>>>
>>>>>> On Mit, 2012-05-02 at 16:20 -0400, j.glisse at gmail.com wrote:
>>>>>>>
>>>>>>> From: Jerome Glisse<jglisse at 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 at 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
>
> Btw uploaded a v3 with some more thing like warnononce for extra
> safety, better comment and trying to mitigate race btw cpu reading and
> gpu writing fence.
> http://people.freedesktop.org/~glisse/reset3/

In patch:
http://people.freedesktop.org/~glisse/reset3/0003-drm-radeon-rework-fence-handling-drop-fence-list-v3.patch

You can drop this hunk:
+	if (rdev->family >= CHIP_CAYMAN) {
+		/* because there is so many ring on newer GPU we can't use
+		 * scratch reg thus we need to use event, on those GPU there
+		 * is no AGP version and writting to system ram have always
+		 * been reliable so far
+		 */
+		rdev->wb.enabled = true;
+		rdev->wb.use_event = true;
+	}

As the code right below it does the exact same thing.  It could
probably be extended to APUs as well since they will never have AGP
support.  I'll send out a patch.

Alex

>
> Cheers,
> Jerome