diff mbox

drm/radeon: make 64bit fences more robust v3

Message ID CADnq5_NCwN-Y6S_evxLrJieJ7ARTsYs-n-QGrhKuzbjVo0_yPA@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Deucher Sept. 13, 2012, 1:54 p.m. UTC
On Thu, Sep 13, 2012 at 4:33 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Only increase the higher 32bits if we really detect a wrap around.
>
> v2: instead of increasing the higher 32bits just use the higher
>     32bits from the last emitted fence.
> v3: also use last emitted fence value as upper limit.
>
> The intention of this patch is to make fences as robust as
> they where before introducing 64bit fences. This is
> necessary because on older systems it looks like the fence
> value gets corrupted on initialization.
>
> Should fix:
> https://bugs.freedesktop.org/show_bug.cgi?id=54129
> https://bugs.freedesktop.org/show_bug.cgi?id=51344
> https://bugs.freedesktop.org/show_bug.cgi?id=54662
> https://bugzilla.redhat.com/show_bug.cgi?id=846505
> https://bugzilla.redhat.com/show_bug.cgi?id=845639
>
> Needs to be adopted for 3.5, but that shouldn't be to
> much of a problem.
>
> Signed-off-by: Christian König <deathsimple@vodafone.de>
> Cc: stable@vger.kernel.org

This looks good to me.  I made a few minor changes to the commit
message to note that 3.5.x requires a different patch which will be
sent out separately due to changes in the fence code.  If there are no
objections I'll send this out in my next -fixes pull request later
today.

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_fence.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> index 7b737b9..2a59375 100644
> --- a/drivers/gpu/drm/radeon/radeon_fence.c
> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> @@ -131,7 +131,7 @@ int radeon_fence_emit(struct radeon_device *rdev,
>   */
>  void radeon_fence_process(struct radeon_device *rdev, int ring)
>  {
> -       uint64_t seq, last_seq;
> +       uint64_t seq, last_seq, last_emitted;
>         unsigned count_loop = 0;
>         bool wake = false;
>
> @@ -158,13 +158,15 @@ void radeon_fence_process(struct radeon_device *rdev, int ring)
>          */
>         last_seq = atomic64_read(&rdev->fence_drv[ring].last_seq);
>         do {
> +               last_emitted = rdev->fence_drv[ring].sync_seq[ring];
>                 seq = radeon_fence_read(rdev, ring);
>                 seq |= last_seq & 0xffffffff00000000LL;
>                 if (seq < last_seq) {
> -                       seq += 0x100000000LL;
> +                       seq &= 0xffffffff;
> +                       seq |= last_emitted & 0xffffffff00000000LL;
>                 }
>
> -               if (seq == last_seq) {
> +               if (seq <= last_seq || seq > last_emitted) {
>                         break;
>                 }
>                 /* If we loop over we don't want to return without
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Jerome Glisse Sept. 13, 2012, 1:55 p.m. UTC | #1
On Thu, Sep 13, 2012 at 9:54 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, Sep 13, 2012 at 4:33 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Only increase the higher 32bits if we really detect a wrap around.
>>
>> v2: instead of increasing the higher 32bits just use the higher
>>     32bits from the last emitted fence.
>> v3: also use last emitted fence value as upper limit.
>>
>> The intention of this patch is to make fences as robust as
>> they where before introducing 64bit fences. This is
>> necessary because on older systems it looks like the fence
>> value gets corrupted on initialization.
>>
>> Should fix:
>> https://bugs.freedesktop.org/show_bug.cgi?id=54129
>> https://bugs.freedesktop.org/show_bug.cgi?id=51344
>> https://bugs.freedesktop.org/show_bug.cgi?id=54662
>> https://bugzilla.redhat.com/show_bug.cgi?id=846505
>> https://bugzilla.redhat.com/show_bug.cgi?id=845639
>>
>> Needs to be adopted for 3.5, but that shouldn't be to
>> much of a problem.
>>
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
>> Cc: stable@vger.kernel.org
>
> This looks good to me.  I made a few minor changes to the commit
> message to note that 3.5.x requires a different patch which will be
> sent out separately due to changes in the fence code.  If there are no
> objections I'll send this out in my next -fixes pull request later
> today.

Yeah looks good here too

Cheers,
Jerome

>
> Alex
>
>> ---
>>  drivers/gpu/drm/radeon/radeon_fence.c |    8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
>> index 7b737b9..2a59375 100644
>> --- a/drivers/gpu/drm/radeon/radeon_fence.c
>> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
>> @@ -131,7 +131,7 @@ int radeon_fence_emit(struct radeon_device *rdev,
>>   */
>>  void radeon_fence_process(struct radeon_device *rdev, int ring)
>>  {
>> -       uint64_t seq, last_seq;
>> +       uint64_t seq, last_seq, last_emitted;
>>         unsigned count_loop = 0;
>>         bool wake = false;
>>
>> @@ -158,13 +158,15 @@ void radeon_fence_process(struct radeon_device *rdev, int ring)
>>          */
>>         last_seq = atomic64_read(&rdev->fence_drv[ring].last_seq);
>>         do {
>> +               last_emitted = rdev->fence_drv[ring].sync_seq[ring];
>>                 seq = radeon_fence_read(rdev, ring);
>>                 seq |= last_seq & 0xffffffff00000000LL;
>>                 if (seq < last_seq) {
>> -                       seq += 0x100000000LL;
>> +                       seq &= 0xffffffff;
>> +                       seq |= last_emitted & 0xffffffff00000000LL;
>>                 }
>>
>> -               if (seq == last_seq) {
>> +               if (seq <= last_seq || seq > last_emitted) {
>>                         break;
>>                 }
>>                 /* If we loop over we don't want to return without
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox

Patch

From b6c778e5742da8eb337ef1e0074a07cb0b89d361 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20K=C3=B6nig?= <deathsimple@vodafone.de>
Date: Thu, 13 Sep 2012 10:33:47 +0200
Subject: [PATCH] drm/radeon: make 64bit fences more robust v3
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Only increase the higher 32bits if we really detect a wrap around.

v2: instead of increasing the higher 32bits just use the higher
    32bits from the last emitted fence.
v3: also use last emitted fence value as upper limit.

The intention of this patch is to make fences as robust as
they where before introducing 64bit fences. This is
necessary because on older systems it looks like the fence
value gets corrupted on initialization.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=51344

Should also fix:
https://bugs.freedesktop.org/show_bug.cgi?id=54129
https://bugs.freedesktop.org/show_bug.cgi?id=54662
https://bugzilla.redhat.com/show_bug.cgi?id=846505
https://bugzilla.redhat.com/show_bug.cgi?id=845639

3.5 needs a separate patch due to changes in the
fence code.  Will send that out separately.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_fence.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 7b737b9..2a59375 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -131,7 +131,7 @@  int radeon_fence_emit(struct radeon_device *rdev,
  */
 void radeon_fence_process(struct radeon_device *rdev, int ring)
 {
-	uint64_t seq, last_seq;
+	uint64_t seq, last_seq, last_emitted;
 	unsigned count_loop = 0;
 	bool wake = false;
 
@@ -158,13 +158,15 @@  void radeon_fence_process(struct radeon_device *rdev, int ring)
 	 */
 	last_seq = atomic64_read(&rdev->fence_drv[ring].last_seq);
 	do {
+		last_emitted = rdev->fence_drv[ring].sync_seq[ring];
 		seq = radeon_fence_read(rdev, ring);
 		seq |= last_seq & 0xffffffff00000000LL;
 		if (seq < last_seq) {
-			seq += 0x100000000LL;
+			seq &= 0xffffffff;
+			seq |= last_emitted & 0xffffffff00000000LL;
 		}
 
-		if (seq == last_seq) {
+		if (seq <= last_seq || seq > last_emitted) {
 			break;
 		}
 		/* If we loop over we don't want to return without
-- 
1.7.7.5