diff mbox

[1/2] drm/radeon: add error handling to fence_wait_empty_locked

Message ID 1341390322-5984-1-git-send-email-deathsimple@vodafone.de (mailing list archive)
State New, archived
Headers show

Commit Message

Christian König July 4, 2012, 8:25 a.m. UTC
Instead of returning the error handle it directly
and while at it fix the comments about the ring lock.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon.h       |    2 +-
 drivers/gpu/drm/radeon/radeon_fence.c |   33 +++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 13 deletions(-)

Comments

Alex Deucher July 4, 2012, 1:25 p.m. UTC | #1
On Wed, Jul 4, 2012 at 4:25 AM, Christian König <deathsimple@vodafone.de> wrote:
> Instead of returning the error handle it directly
> and while at it fix the comments about the ring lock.
>
> Signed-off-by: Christian König <deathsimple@vodafone.de>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>

For the series:

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/radeon.h       |    2 +-
>  drivers/gpu/drm/radeon/radeon_fence.c |   33 +++++++++++++++++++++------------
>  2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 77b4519b..5861ec8 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -239,7 +239,7 @@ void radeon_fence_process(struct radeon_device *rdev, int ring);
>  bool radeon_fence_signaled(struct radeon_fence *fence);
>  int radeon_fence_wait(struct radeon_fence *fence, bool interruptible);
>  int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring);
> -int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
> +void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
>  int radeon_fence_wait_any(struct radeon_device *rdev,
>                           struct radeon_fence **fences,
>                           bool intr);
> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> index 7b55625..be4e4f3 100644
> --- a/drivers/gpu/drm/radeon/radeon_fence.c
> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> @@ -440,14 +440,11 @@ int radeon_fence_wait_any(struct radeon_device *rdev,
>         return 0;
>  }
>
> +/* caller must hold ring lock */
>  int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
>  {
>         uint64_t seq;
>
> -       /* We are not protected by ring lock when reading current seq but
> -        * it's ok as worst case is we return to early while we could have
> -        * wait.
> -        */
>         seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL;
>         if (seq >= rdev->fence_drv[ring].sync_seq[ring]) {
>                 /* nothing to wait for, last_seq is
> @@ -457,15 +454,27 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
>         return radeon_fence_wait_seq(rdev, seq, ring, false, false);
>  }
>
> -int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
> +/* caller must hold ring lock */
> +void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
>  {
> -       /* We are not protected by ring lock when reading current seq
> -        * but it's ok as wait empty is call from place where no more
> -        * activity can be scheduled so there won't be concurrent access
> -        * to seq value.
> -        */
> -       return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring],
> -                                    ring, false, false);
> +       uint64_t seq = rdev->fence_drv[ring].sync_seq[ring];
> +
> +       while(1) {
> +               int r;
> +               r = radeon_fence_wait_seq(rdev, seq, ring, false, false);
> +               if (r == -EDEADLK) {
> +                       mutex_unlock(&rdev->ring_lock);
> +                       r = radeon_gpu_reset(rdev);
> +                       mutex_lock(&rdev->ring_lock);
> +                       if (!r)
> +                               continue;
> +               }
> +               if (r) {
> +                       dev_err(rdev->dev, "error waiting for ring to become"
> +                               " idle (%d)\n", r);
> +               }
> +               return;
> +       }
>  }
>
>  struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence)
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Jerome Glisse July 4, 2012, 3:55 p.m. UTC | #2
On Wed, Jul 4, 2012 at 9:25 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, Jul 4, 2012 at 4:25 AM, Christian König <deathsimple@vodafone.de> wrote:
>> Instead of returning the error handle it directly
>> and while at it fix the comments about the ring lock.
>>
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>
> For the series:
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Jerome Glisse <jglisse@redhat.com>

>> ---
>>  drivers/gpu/drm/radeon/radeon.h       |    2 +-
>>  drivers/gpu/drm/radeon/radeon_fence.c |   33 +++++++++++++++++++++------------
>>  2 files changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
>> index 77b4519b..5861ec8 100644
>> --- a/drivers/gpu/drm/radeon/radeon.h
>> +++ b/drivers/gpu/drm/radeon/radeon.h
>> @@ -239,7 +239,7 @@ void radeon_fence_process(struct radeon_device *rdev, int ring);
>>  bool radeon_fence_signaled(struct radeon_fence *fence);
>>  int radeon_fence_wait(struct radeon_fence *fence, bool interruptible);
>>  int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring);
>> -int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
>> +void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
>>  int radeon_fence_wait_any(struct radeon_device *rdev,
>>                           struct radeon_fence **fences,
>>                           bool intr);
>> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
>> index 7b55625..be4e4f3 100644
>> --- a/drivers/gpu/drm/radeon/radeon_fence.c
>> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
>> @@ -440,14 +440,11 @@ int radeon_fence_wait_any(struct radeon_device *rdev,
>>         return 0;
>>  }
>>
>> +/* caller must hold ring lock */
>>  int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
>>  {
>>         uint64_t seq;
>>
>> -       /* We are not protected by ring lock when reading current seq but
>> -        * it's ok as worst case is we return to early while we could have
>> -        * wait.
>> -        */
>>         seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL;
>>         if (seq >= rdev->fence_drv[ring].sync_seq[ring]) {
>>                 /* nothing to wait for, last_seq is
>> @@ -457,15 +454,27 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
>>         return radeon_fence_wait_seq(rdev, seq, ring, false, false);
>>  }
>>
>> -int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
>> +/* caller must hold ring lock */
>> +void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
>>  {
>> -       /* We are not protected by ring lock when reading current seq
>> -        * but it's ok as wait empty is call from place where no more
>> -        * activity can be scheduled so there won't be concurrent access
>> -        * to seq value.
>> -        */
>> -       return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring],
>> -                                    ring, false, false);
>> +       uint64_t seq = rdev->fence_drv[ring].sync_seq[ring];
>> +
>> +       while(1) {
>> +               int r;
>> +               r = radeon_fence_wait_seq(rdev, seq, ring, false, false);
>> +               if (r == -EDEADLK) {
>> +                       mutex_unlock(&rdev->ring_lock);
>> +                       r = radeon_gpu_reset(rdev);
>> +                       mutex_lock(&rdev->ring_lock);
>> +                       if (!r)
>> +                               continue;
>> +               }
>> +               if (r) {
>> +                       dev_err(rdev->dev, "error waiting for ring to become"
>> +                               " idle (%d)\n", r);
>> +               }
>> +               return;
>> +       }
>>  }
>>
>>  struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence)
>> --
>> 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

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..5861ec8 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -239,7 +239,7 @@  void radeon_fence_process(struct radeon_device *rdev, int ring);
 bool radeon_fence_signaled(struct radeon_fence *fence);
 int radeon_fence_wait(struct radeon_fence *fence, bool interruptible);
 int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring);
-int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
+void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
 int radeon_fence_wait_any(struct radeon_device *rdev,
 			  struct radeon_fence **fences,
 			  bool intr);
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 7b55625..be4e4f3 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -440,14 +440,11 @@  int radeon_fence_wait_any(struct radeon_device *rdev,
 	return 0;
 }
 
+/* caller must hold ring lock */
 int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
 {
 	uint64_t seq;
 
-	/* We are not protected by ring lock when reading current seq but
-	 * it's ok as worst case is we return to early while we could have
-	 * wait.
-	 */
 	seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL;
 	if (seq >= rdev->fence_drv[ring].sync_seq[ring]) {
 		/* nothing to wait for, last_seq is
@@ -457,15 +454,27 @@  int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
 	return radeon_fence_wait_seq(rdev, seq, ring, false, false);
 }
 
-int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
+/* caller must hold ring lock */
+void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
 {
-	/* We are not protected by ring lock when reading current seq
-	 * but it's ok as wait empty is call from place where no more
-	 * activity can be scheduled so there won't be concurrent access
-	 * to seq value.
-	 */
-	return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring],
-				     ring, false, false);
+	uint64_t seq = rdev->fence_drv[ring].sync_seq[ring];
+
+	while(1) {
+		int r;
+		r = radeon_fence_wait_seq(rdev, seq, ring, false, false);
+		if (r == -EDEADLK) {
+			mutex_unlock(&rdev->ring_lock);
+			r = radeon_gpu_reset(rdev);
+			mutex_lock(&rdev->ring_lock);
+			if (!r)
+				continue;
+		}
+		if (r) {
+			dev_err(rdev->dev, "error waiting for ring to become"
+				" idle (%d)\n", r);
+		}
+		return;
+	}
 }
 
 struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence)