[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Jerome Glisse]

On Thu, 2012-02-23 at 10:17 -0500, Alex Deucher wrote:
> 2012/2/23 Christian K?nig :
> > The function radeon_bo_list_validate can cause a
> > bo to move, resulting in a different sync_obj
> > and a dependency to wait for this move to finish.
> >
> > Signed-off-by: Christian K?nig 
> > Reviewed-by: Alex Deucher 
> 
> For the series:
> 
> Reviewed-by: Alex Deucher 

Same
Reviewed-by: Jerome Glisse 

> 
> > ---
> >  drivers/gpu/drm/radeon/radeon.h|1 -
> >  drivers/gpu/drm/radeon/radeon_cs.c |   21 ++---
> >  2 files changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon.h 
> > b/drivers/gpu/drm/radeon/radeon.h
> > index 884e0d4..4c1b981 100644
> > --- a/drivers/gpu/drm/radeon/radeon.h
> > +++ b/drivers/gpu/drm/radeon/radeon.h
> > @@ -836,7 +836,6 @@ struct radeon_cs_parser {
> >struct radeon_cs_reloc  *relocs;
> >struct radeon_cs_reloc  **relocs_ptr;
> >struct list_headvalidated;
> > -   boolsync_to_ring[RADEON_NUM_RINGS];
> >/* indices of various chunks */
> >int chunk_ib_idx;
> >int chunk_relocs_idx;
> > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
> > b/drivers/gpu/drm/radeon/radeon_cs.c
> > index 435a3d9..7fd0987 100644
> > --- a/drivers/gpu/drm/radeon/radeon_cs.c
> > +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> > @@ -85,12 +85,6 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
> >radeon_bo_list_add_object(>relocs[i].lobj,
> >  >validated);
> >
> > -   if (p->relocs[i].robj->tbo.sync_obj && !(r->flags & 
> > RADEON_RELOC_DONT_SYNC)) {
> > -   struct radeon_fence *fence = 
> > p->relocs[i].robj->tbo.sync_obj;
> > -   if (!radeon_fence_signaled(fence)) {
> > -   p->sync_to_ring[fence->ring] = true;
> > -   }
> > -   }
> >} else
> >p->relocs[i].handle = 0;
> >}
> > @@ -118,11 +112,24 @@ static int radeon_cs_get_ring(struct radeon_cs_parser 
> > *p, u32 ring, s32 priority
> >
> >  static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
> >  {
> > +   bool sync_to_ring[RADEON_NUM_RINGS] = { };
> >int i, r;
> >
> > +   for (i = 0; i < p->nrelocs; i++) {
> > +   if (!p->relocs[i].robj || !p->relocs[i].robj->tbo.sync_obj)
> > +   continue;
> > +
> > +   if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {
> > +   struct radeon_fence *fence = 
> > p->relocs[i].robj->tbo.sync_obj;
> > +   if (!radeon_fence_signaled(fence)) {
> > +   sync_to_ring[fence->ring] = true;
> > +   }
> > +   }
> > +   }
> > +
> >for (i = 0; i < RADEON_NUM_RINGS; ++i) {
> >/* no need to sync to our own or unused rings */
> > -   if (i == p->ring || !p->sync_to_ring[i] || 
> > !p->rdev->ring[i].ready)
> > +   if (i == p->ring || !sync_to_ring[i] || 
> > !p->rdev->ring[i].ready)
> >continue;
> >
> >if (!p->ib->fence->semaphore) {
> > --
> > 1.7.5.4
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Jerome Glisse]

On Thu, 2012-02-23 at 10:17 -0500, Alex Deucher wrote:
 2012/2/23 Christian König deathsim...@vodafone.de:
  The function radeon_bo_list_validate can cause a
  bo to move, resulting in a different sync_obj
  and a dependency to wait for this move to finish.
 
  Signed-off-by: Christian König deathsim...@vodafone.de
  Reviewed-by: Alex Deucher alexander.deuc...@amd.com
 
 For the series:
 
 Reviewed-by: Alex Deucher alexander.deuc...@amd.com

Same
Reviewed-by: Jerome Glisse jgli...@redhat.com

 
  ---
   drivers/gpu/drm/radeon/radeon.h|1 -
   drivers/gpu/drm/radeon/radeon_cs.c |   21 ++---
   2 files changed, 14 insertions(+), 8 deletions(-)
 
  diff --git a/drivers/gpu/drm/radeon/radeon.h 
  b/drivers/gpu/drm/radeon/radeon.h
  index 884e0d4..4c1b981 100644
  --- a/drivers/gpu/drm/radeon/radeon.h
  +++ b/drivers/gpu/drm/radeon/radeon.h
  @@ -836,7 +836,6 @@ struct radeon_cs_parser {
 struct radeon_cs_reloc  *relocs;
 struct radeon_cs_reloc  **relocs_ptr;
 struct list_headvalidated;
  -   boolsync_to_ring[RADEON_NUM_RINGS];
 /* indices of various chunks */
 int chunk_ib_idx;
 int chunk_relocs_idx;
  diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
  b/drivers/gpu/drm/radeon/radeon_cs.c
  index 435a3d9..7fd0987 100644
  --- a/drivers/gpu/drm/radeon/radeon_cs.c
  +++ b/drivers/gpu/drm/radeon/radeon_cs.c
  @@ -85,12 +85,6 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
 radeon_bo_list_add_object(p-relocs[i].lobj,
   p-validated);
 
  -   if (p-relocs[i].robj-tbo.sync_obj  !(r-flags  
  RADEON_RELOC_DONT_SYNC)) {
  -   struct radeon_fence *fence = 
  p-relocs[i].robj-tbo.sync_obj;
  -   if (!radeon_fence_signaled(fence)) {
  -   p-sync_to_ring[fence-ring] = true;
  -   }
  -   }
 } else
 p-relocs[i].handle = 0;
 }
  @@ -118,11 +112,24 @@ static int radeon_cs_get_ring(struct radeon_cs_parser 
  *p, u32 ring, s32 priority
 
   static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
   {
  +   bool sync_to_ring[RADEON_NUM_RINGS] = { };
 int i, r;
 
  +   for (i = 0; i  p-nrelocs; i++) {
  +   if (!p-relocs[i].robj || !p-relocs[i].robj-tbo.sync_obj)
  +   continue;
  +
  +   if (!(p-relocs[i].flags  RADEON_RELOC_DONT_SYNC)) {
  +   struct radeon_fence *fence = 
  p-relocs[i].robj-tbo.sync_obj;
  +   if (!radeon_fence_signaled(fence)) {
  +   sync_to_ring[fence-ring] = true;
  +   }
  +   }
  +   }
  +
 for (i = 0; i  RADEON_NUM_RINGS; ++i) {
 /* no need to sync to our own or unused rings */
  -   if (i == p-ring || !p-sync_to_ring[i] || 
  !p-rdev-ring[i].ready)
  +   if (i == p-ring || !sync_to_ring[i] || 
  !p-rdev-ring[i].ready)
 continue;
 
 if (!p-ib-fence-semaphore) {
  --
  1.7.5.4
 
  ___
  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


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Christian König]

On 23.02.2012 18:32, Jerome Glisse wrote:
> 2012/2/23 Mathias Fr?hlich  >
>
>
> Christian,
>
> On Thursday, February 23, 2012 15:18:42 Christian K?nig wrote:
> > The function radeon_bo_list_validate can cause a
> > bo to move, resulting in a different sync_obj
> > and a dependency to wait for this move to finish.
> >
> > Signed-off-by: Christian K?nig  >
> > Reviewed-by: Alex Deucher  >
>
> I am not sure, but to me this looks like this could fix these kind
> of gpu
> lockups that I experience since some time every now and then.
> The usual symptom is that I get the
>
> radeon :01:00.0: GPU lockup CP stall for more than 1msec
> GPU lockup (waiting for 0x00682AC3 last fence id 0x00682AC2)
> [...]
>
> kernel message. Each time with the fence being off by one like in
> the example
> above.
>
> If this change has the potential to fix this issue I think this
> particular
> patch should be considered for the current upstream kernel release.
>
> Mathias
>
>
> No this patch doesn't. This patch is all about getting proper sync btw 
> different rings.
That's unfortunately true, since we wasn't able to release any code that 
makes direct use of the different rings (yet) it shouldn't really matter 
in practice. I Just wanted to have that fix upstream since it is an 
obvious bug.

Christian.
>
> Sorry for the deception.
>
> Cheers,
> Jerome

-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Mathias Fröhlich]

Christian,

On Thursday, February 23, 2012 15:18:42 Christian K?nig wrote:
> The function radeon_bo_list_validate can cause a
> bo to move, resulting in a different sync_obj
> and a dependency to wait for this move to finish.
> 
> Signed-off-by: Christian K?nig 
> Reviewed-by: Alex Deucher 

I am not sure, but to me this looks like this could fix these kind of gpu 
lockups that I experience since some time every now and then.
The usual symptom is that I get the 

radeon :01:00.0: GPU lockup CP stall for more than 1msec
GPU lockup (waiting for 0x00682AC3 last fence id 0x00682AC2)
[...]

kernel message. Each time with the fence being off by one like in the example 
above.

If this change has the potential to fix this issue I think this particular 
patch should be considered for the current upstream kernel release.

Mathias

[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Christian König]

The function radeon_bo_list_validate can cause a
bo to move, resulting in a different sync_obj
and a dependency to wait for this move to finish.

Signed-off-by: Christian K?nig 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/radeon.h|1 -
 drivers/gpu/drm/radeon/radeon_cs.c |   21 ++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 884e0d4..4c1b981 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -836,7 +836,6 @@ struct radeon_cs_parser {
struct radeon_cs_reloc  *relocs;
struct radeon_cs_reloc  **relocs_ptr;
struct list_headvalidated;
-   boolsync_to_ring[RADEON_NUM_RINGS];
/* indices of various chunks */
int chunk_ib_idx;
int chunk_relocs_idx;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index 435a3d9..7fd0987 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -85,12 +85,6 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
radeon_bo_list_add_object(>relocs[i].lobj,
  >validated);

-   if (p->relocs[i].robj->tbo.sync_obj && !(r->flags & 
RADEON_RELOC_DONT_SYNC)) {
-   struct radeon_fence *fence = 
p->relocs[i].robj->tbo.sync_obj;
-   if (!radeon_fence_signaled(fence)) {
-   p->sync_to_ring[fence->ring] = true;
-   }
-   }
} else
p->relocs[i].handle = 0;
}
@@ -118,11 +112,24 @@ static int radeon_cs_get_ring(struct radeon_cs_parser *p, 
u32 ring, s32 priority

 static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
 {
+   bool sync_to_ring[RADEON_NUM_RINGS] = { };
int i, r;

+   for (i = 0; i < p->nrelocs; i++) {
+   if (!p->relocs[i].robj || !p->relocs[i].robj->tbo.sync_obj)
+   continue;
+
+   if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {
+   struct radeon_fence *fence = 
p->relocs[i].robj->tbo.sync_obj;
+   if (!radeon_fence_signaled(fence)) {
+   sync_to_ring[fence->ring] = true;
+   }
+   }
+   }
+
for (i = 0; i < RADEON_NUM_RINGS; ++i) {
/* no need to sync to our own or unused rings */
-   if (i == p->ring || !p->sync_to_ring[i] || 
!p->rdev->ring[i].ready)
+   if (i == p->ring || !sync_to_ring[i] || !p->rdev->ring[i].ready)
continue;

if (!p->ib->fence->semaphore) {
-- 
1.7.5.4

[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Jerome Glisse]

2012/2/23 Mathias Fr?hlich 

>
> Christian,
>
> On Thursday, February 23, 2012 15:18:42 Christian K?nig wrote:
> > The function radeon_bo_list_validate can cause a
> > bo to move, resulting in a different sync_obj
> > and a dependency to wait for this move to finish.
> >
> > Signed-off-by: Christian K?nig 
> > Reviewed-by: Alex Deucher 
>
> I am not sure, but to me this looks like this could fix these kind of gpu
> lockups that I experience since some time every now and then.
> The usual symptom is that I get the
>
> radeon :01:00.0: GPU lockup CP stall for more than 1msec
> GPU lockup (waiting for 0x00682AC3 last fence id 0x00682AC2)
> [...]
>
> kernel message. Each time with the fence being off by one like in the
> example
> above.
>
> If this change has the potential to fix this issue I think this particular
> patch should be considered for the current upstream kernel release.
>
> Mathias
>

No this patch doesn't. This patch is all about getting proper sync btw
different rings.

Sorry for the deception.

Cheers,
Jerome
-- next part --
An HTML attachment was scrubbed...
URL: 

[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Alex Deucher]

2012/2/23 Christian K?nig :
> The function radeon_bo_list_validate can cause a
> bo to move, resulting in a different sync_obj
> and a dependency to wait for this move to finish.
>
> Signed-off-by: Christian K?nig 
> Reviewed-by: Alex Deucher 

For the series:

Reviewed-by: Alex Deucher 

> ---
> ?drivers/gpu/drm/radeon/radeon.h ? ?| ? ?1 -
> ?drivers/gpu/drm/radeon/radeon_cs.c | ? 21 ++---
> ?2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 884e0d4..4c1b981 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -836,7 +836,6 @@ struct radeon_cs_parser {
> ? ? ? ?struct radeon_cs_reloc ?*relocs;
> ? ? ? ?struct radeon_cs_reloc ?**relocs_ptr;
> ? ? ? ?struct list_head ? ? ? ?validated;
> - ? ? ? bool ? ? ? ? ? ? ? ? ? ?sync_to_ring[RADEON_NUM_RINGS];
> ? ? ? ?/* indices of various chunks */
> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? chunk_ib_idx;
> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? chunk_relocs_idx;
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index 435a3d9..7fd0987 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -85,12 +85,6 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
> ? ? ? ? ? ? ? ? ? ? ? ?radeon_bo_list_add_object(>relocs[i].lobj,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?>validated);
>
> - ? ? ? ? ? ? ? ? ? ? ? if (p->relocs[i].robj->tbo.sync_obj && !(r->flags & 
> RADEON_RELOC_DONT_SYNC)) {
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct radeon_fence *fence = 
> p->relocs[i].robj->tbo.sync_obj;
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (!radeon_fence_signaled(fence)) {
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? p->sync_to_ring[fence->ring] = true;
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? }
> - ? ? ? ? ? ? ? ? ? ? ? }
> ? ? ? ? ? ? ? ?} else
> ? ? ? ? ? ? ? ? ? ? ? ?p->relocs[i].handle = 0;
> ? ? ? ?}
> @@ -118,11 +112,24 @@ static int radeon_cs_get_ring(struct radeon_cs_parser 
> *p, u32 ring, s32 priority
>
> ?static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
> ?{
> + ? ? ? bool sync_to_ring[RADEON_NUM_RINGS] = { };
> ? ? ? ?int i, r;
>
> + ? ? ? for (i = 0; i < p->nrelocs; i++) {
> + ? ? ? ? ? ? ? if (!p->relocs[i].robj || !p->relocs[i].robj->tbo.sync_obj)
> + ? ? ? ? ? ? ? ? ? ? ? continue;
> +
> + ? ? ? ? ? ? ? if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {
> + ? ? ? ? ? ? ? ? ? ? ? struct radeon_fence *fence = 
> p->relocs[i].robj->tbo.sync_obj;
> + ? ? ? ? ? ? ? ? ? ? ? if (!radeon_fence_signaled(fence)) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sync_to_ring[fence->ring] = true;
> + ? ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? }
> + ? ? ? }
> +
> ? ? ? ?for (i = 0; i < RADEON_NUM_RINGS; ++i) {
> ? ? ? ? ? ? ? ?/* no need to sync to our own or unused rings */
> - ? ? ? ? ? ? ? if (i == p->ring || !p->sync_to_ring[i] || 
> !p->rdev->ring[i].ready)
> + ? ? ? ? ? ? ? if (i == p->ring || !sync_to_ring[i] || 
> !p->rdev->ring[i].ready)
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
>
> ? ? ? ? ? ? ? ?if (!p->ib->fence->semaphore) {
> --
> 1.7.5.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Christian König]

The function radeon_bo_list_validate can cause a
bo to move, resulting in a different sync_obj
and a dependency to wait for this move to finish.

Signed-off-by: Christian König deathsim...@vodafone.de
Reviewed-by: Alex Deucher alexander.deuc...@amd.com
---
 drivers/gpu/drm/radeon/radeon.h|1 -
 drivers/gpu/drm/radeon/radeon_cs.c |   21 ++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 884e0d4..4c1b981 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -836,7 +836,6 @@ struct radeon_cs_parser {
struct radeon_cs_reloc  *relocs;
struct radeon_cs_reloc  **relocs_ptr;
struct list_headvalidated;
-   boolsync_to_ring[RADEON_NUM_RINGS];
/* indices of various chunks */
int chunk_ib_idx;
int chunk_relocs_idx;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index 435a3d9..7fd0987 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -85,12 +85,6 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
radeon_bo_list_add_object(p-relocs[i].lobj,
  p-validated);
 
-   if (p-relocs[i].robj-tbo.sync_obj  !(r-flags  
RADEON_RELOC_DONT_SYNC)) {
-   struct radeon_fence *fence = 
p-relocs[i].robj-tbo.sync_obj;
-   if (!radeon_fence_signaled(fence)) {
-   p-sync_to_ring[fence-ring] = true;
-   }
-   }
} else
p-relocs[i].handle = 0;
}
@@ -118,11 +112,24 @@ static int radeon_cs_get_ring(struct radeon_cs_parser *p, 
u32 ring, s32 priority
 
 static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
 {
+   bool sync_to_ring[RADEON_NUM_RINGS] = { };
int i, r;
 
+   for (i = 0; i  p-nrelocs; i++) {
+   if (!p-relocs[i].robj || !p-relocs[i].robj-tbo.sync_obj)
+   continue;
+
+   if (!(p-relocs[i].flags  RADEON_RELOC_DONT_SYNC)) {
+   struct radeon_fence *fence = 
p-relocs[i].robj-tbo.sync_obj;
+   if (!radeon_fence_signaled(fence)) {
+   sync_to_ring[fence-ring] = true;
+   }
+   }
+   }
+
for (i = 0; i  RADEON_NUM_RINGS; ++i) {
/* no need to sync to our own or unused rings */
-   if (i == p-ring || !p-sync_to_ring[i] || 
!p-rdev-ring[i].ready)
+   if (i == p-ring || !sync_to_ring[i] || !p-rdev-ring[i].ready)
continue;
 
if (!p-ib-fence-semaphore) {
-- 
1.7.5.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Alex Deucher]

2012/2/23 Christian König deathsim...@vodafone.de:
 The function radeon_bo_list_validate can cause a
 bo to move, resulting in a different sync_obj
 and a dependency to wait for this move to finish.

 Signed-off-by: Christian König deathsim...@vodafone.de
 Reviewed-by: Alex Deucher alexander.deuc...@amd.com

For the series:

Reviewed-by: Alex Deucher alexander.deuc...@amd.com

 ---
  drivers/gpu/drm/radeon/radeon.h    |    1 -
  drivers/gpu/drm/radeon/radeon_cs.c |   21 ++---
  2 files changed, 14 insertions(+), 8 deletions(-)

 diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
 index 884e0d4..4c1b981 100644
 --- a/drivers/gpu/drm/radeon/radeon.h
 +++ b/drivers/gpu/drm/radeon/radeon.h
 @@ -836,7 +836,6 @@ struct radeon_cs_parser {
        struct radeon_cs_reloc  *relocs;
        struct radeon_cs_reloc  **relocs_ptr;
        struct list_head        validated;
 -       bool                    sync_to_ring[RADEON_NUM_RINGS];
        /* indices of various chunks */
        int                     chunk_ib_idx;
        int                     chunk_relocs_idx;
 diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
 b/drivers/gpu/drm/radeon/radeon_cs.c
 index 435a3d9..7fd0987 100644
 --- a/drivers/gpu/drm/radeon/radeon_cs.c
 +++ b/drivers/gpu/drm/radeon/radeon_cs.c
 @@ -85,12 +85,6 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
                        radeon_bo_list_add_object(p-relocs[i].lobj,
                                                  p-validated);

 -                       if (p-relocs[i].robj-tbo.sync_obj  !(r-flags  
 RADEON_RELOC_DONT_SYNC)) {
 -                               struct radeon_fence *fence = 
 p-relocs[i].robj-tbo.sync_obj;
 -                               if (!radeon_fence_signaled(fence)) {
 -                                       p-sync_to_ring[fence-ring] = true;
 -                               }
 -                       }
                } else
                        p-relocs[i].handle = 0;
        }
 @@ -118,11 +112,24 @@ static int radeon_cs_get_ring(struct radeon_cs_parser 
 *p, u32 ring, s32 priority

  static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
  {
 +       bool sync_to_ring[RADEON_NUM_RINGS] = { };
        int i, r;

 +       for (i = 0; i  p-nrelocs; i++) {
 +               if (!p-relocs[i].robj || !p-relocs[i].robj-tbo.sync_obj)
 +                       continue;
 +
 +               if (!(p-relocs[i].flags  RADEON_RELOC_DONT_SYNC)) {
 +                       struct radeon_fence *fence = 
 p-relocs[i].robj-tbo.sync_obj;
 +                       if (!radeon_fence_signaled(fence)) {
 +                               sync_to_ring[fence-ring] = true;
 +                       }
 +               }
 +       }
 +
        for (i = 0; i  RADEON_NUM_RINGS; ++i) {
                /* no need to sync to our own or unused rings */
 -               if (i == p-ring || !p-sync_to_ring[i] || 
 !p-rdev-ring[i].ready)
 +               if (i == p-ring || !sync_to_ring[i] || 
 !p-rdev-ring[i].ready)
                        continue;

                if (!p-ib-fence-semaphore) {
 --
 1.7.5.4

 ___
 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

Re: [PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Mathias Fröhlich]

Christian,

On Thursday, February 23, 2012 15:18:42 Christian König wrote:
 The function radeon_bo_list_validate can cause a
 bo to move, resulting in a different sync_obj
 and a dependency to wait for this move to finish.
 
 Signed-off-by: Christian König deathsim...@vodafone.de
 Reviewed-by: Alex Deucher alexander.deuc...@amd.com

I am not sure, but to me this looks like this could fix these kind of gpu 
lockups that I experience since some time every now and then.
The usual symptom is that I get the 

radeon :01:00.0: GPU lockup CP stall for more than 1msec
GPU lockup (waiting for 0x00682AC3 last fence id 0x00682AC2)
[...]

kernel message. Each time with the fence being off by one like in the example 
above.

If this change has the potential to fix this issue I think this particular 
patch should be considered for the current upstream kernel release.

Mathias
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Jerome Glisse]

2012/2/23 Mathias Fröhlich mathias.froehl...@gmx.net


 Christian,

 On Thursday, February 23, 2012 15:18:42 Christian König wrote:
  The function radeon_bo_list_validate can cause a
  bo to move, resulting in a different sync_obj
  and a dependency to wait for this move to finish.
 
  Signed-off-by: Christian König deathsim...@vodafone.de
  Reviewed-by: Alex Deucher alexander.deuc...@amd.com

 I am not sure, but to me this looks like this could fix these kind of gpu
 lockups that I experience since some time every now and then.
 The usual symptom is that I get the

 radeon :01:00.0: GPU lockup CP stall for more than 1msec
 GPU lockup (waiting for 0x00682AC3 last fence id 0x00682AC2)
 [...]

 kernel message. Each time with the fence being off by one like in the
 example
 above.

 If this change has the potential to fix this issue I think this particular
 patch should be considered for the current upstream kernel release.

 Mathias


No this patch doesn't. This patch is all about getting proper sync btw
different rings.

Sorry for the deception.

Cheers,
Jerome
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/radeon: move ring syncing after bo validation

[Christian König]

On 23.02.2012 18:32, Jerome Glisse wrote:
2012/2/23 Mathias Fröhlich mathias.froehl...@gmx.net 
mailto:mathias.froehl...@gmx.net



Christian,

On Thursday, February 23, 2012 15:18:42 Christian König wrote:
 The function radeon_bo_list_validate can cause a
 bo to move, resulting in a different sync_obj
 and a dependency to wait for this move to finish.

 Signed-off-by: Christian König deathsim...@vodafone.de
mailto:deathsim...@vodafone.de
 Reviewed-by: Alex Deucher alexander.deuc...@amd.com
mailto:alexander.deuc...@amd.com

I am not sure, but to me this looks like this could fix these kind
of gpu
lockups that I experience since some time every now and then.
The usual symptom is that I get the

radeon :01:00.0: GPU lockup CP stall for more than 1msec
GPU lockup (waiting for 0x00682AC3 last fence id 0x00682AC2)
[...]

kernel message. Each time with the fence being off by one like in
the example
above.

If this change has the potential to fix this issue I think this
particular
patch should be considered for the current upstream kernel release.

Mathias


No this patch doesn't. This patch is all about getting proper sync btw 
different rings.
That's unfortunately true, since we wasn't able to release any code that 
makes direct use of the different rings (yet) it shouldn't really matter 
in practice. I Just wanted to have that fix upstream since it is an 
obvious bug.


Christian.


Sorry for the deception.

Cheers,
Jerome


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel