[PATCH] drm/radeon: fix possible lack of synchronization btw ttm and other ring

[PATCH] drm/radeon: fix possible lack of synchronization btw ttm and other ring

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

We need to sync with the GFX ring as ttm might have schedule bo move
on it and new command scheduled for other ring need to wait for bo
data to be in place.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_cs.c |   12 ++++++------
 include/drm/radeon_drm.h           |    1 -
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index 82f2e7b0..e5f8e17 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -122,15 +122,15 @@ static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
 	int i, r;
 
 	for (i = 0; i < p->nrelocs; i++) {
+		struct radeon_fence *fence;
+
 		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 (fence->ring != p->ring && !radeon_fence_signaled(fence)) {
-				sync_to_ring[fence->ring] = true;
-				need_sync = true;
-			}
+		fence = p->relocs[i].robj->tbo.sync_obj;
+		if (fence->ring != p->ring && !radeon_fence_signaled(fence)) {
+			sync_to_ring[fence->ring] = true;
+			need_sync = true;
 		}
 	}
 
diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
index 7c491b4..5805686 100644
--- a/include/drm/radeon_drm.h
+++ b/include/drm/radeon_drm.h
@@ -926,7 +926,6 @@ struct drm_radeon_cs_chunk {
 };
 
 /* drm_radeon_cs_reloc.flags */
-#define RADEON_RELOC_DONT_SYNC		0x01
 
 struct drm_radeon_cs_reloc {
 	uint32_t		handle;
-- 
1.7.7.6

Re: [PATCH] drm/radeon: fix possible lack of synchronization btw ttm and other ring

[Alex Deucher]

On Thu, May 3, 2012 at 1:44 PM,  <j.glisse@gmail.com> wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> We need to sync with the GFX ring as ttm might have schedule bo move
> on it and new command scheduled for other ring need to wait for bo
> data to be in place.
>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>

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

> ---
>  drivers/gpu/drm/radeon/radeon_cs.c |   12 ++++++------
>  include/drm/radeon_drm.h           |    1 -
>  2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
> index 82f2e7b0..e5f8e17 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -122,15 +122,15 @@ static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
>        int i, r;
>
>        for (i = 0; i < p->nrelocs; i++) {
> +               struct radeon_fence *fence;
> +
>                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 (fence->ring != p->ring && !radeon_fence_signaled(fence)) {
> -                               sync_to_ring[fence->ring] = true;
> -                               need_sync = true;
> -                       }
> +               fence = p->relocs[i].robj->tbo.sync_obj;
> +               if (fence->ring != p->ring && !radeon_fence_signaled(fence)) {
> +                       sync_to_ring[fence->ring] = true;
> +                       need_sync = true;
>                }
>        }
>
> diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
> index 7c491b4..5805686 100644
> --- a/include/drm/radeon_drm.h
> +++ b/include/drm/radeon_drm.h
> @@ -926,7 +926,6 @@ struct drm_radeon_cs_chunk {
>  };
>
>  /* drm_radeon_cs_reloc.flags */
> -#define RADEON_RELOC_DONT_SYNC         0x01
>
>  struct drm_radeon_cs_reloc {
>        uint32_t                handle;
> --
> 1.7.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: fix possible lack of synchronization btw ttm and other ring

[Christian König]

On 03.05.2012 19:44, j.glisse@gmail.com wrote:
> From: Jerome Glisse<jglisse@redhat.com>
>
> We need to sync with the GFX ring as ttm might have schedule bo move
> on it and new command scheduled for other ring need to wait for bo
> data to be in place.
>
> Signed-off-by: Jerome Glisse<jglisse@redhat.com>
Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_cs.c |   12 ++++++------
>   include/drm/radeon_drm.h           |    1 -
>   2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
> index 82f2e7b0..e5f8e17 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -122,15 +122,15 @@ static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
>   	int i, r;
>
>   	for (i = 0; i<  p->nrelocs; i++) {
> +		struct radeon_fence *fence;
> +
>   		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 (fence->ring != p->ring&&  !radeon_fence_signaled(fence)) {
> -				sync_to_ring[fence->ring] = true;
> -				need_sync = true;
> -			}
> +		fence = p->relocs[i].robj->tbo.sync_obj;
> +		if (fence->ring != p->ring&&  !radeon_fence_signaled(fence)) {
> +			sync_to_ring[fence->ring] = true;
> +			need_sync = true;
>   		}
>   	}
>
> diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
> index 7c491b4..5805686 100644
> --- a/include/drm/radeon_drm.h
> +++ b/include/drm/radeon_drm.h
> @@ -926,7 +926,6 @@ struct drm_radeon_cs_chunk {
>   };
>
>   /* drm_radeon_cs_reloc.flags */
> -#define RADEON_RELOC_DONT_SYNC		0x01
>
>   struct drm_radeon_cs_reloc {
>   	uint32_t		handle;

Re: [PATCH] drm/radeon: fix possible lack of synchronization btw ttm and other ring

[Dave Airlie]

>>
>> From: Jerome Glisse<jglisse@redhat.com>
>>
>> We need to sync with the GFX ring as ttm might have schedule bo move
>> on it and new command scheduled for other ring need to wait for bo
>> data to be in place.
>>
Is this required for -fixes? if so it doesn't apply clean.

Dave.

Re: [PATCH] drm/radeon: fix possible lack of synchronization btw ttm and other ring

[Christian König]

On 04.05.2012 10:42, Dave Airlie wrote:
>>> From: Jerome Glisse<jglisse@redhat.com>
>>>
>>> We need to sync with the GFX ring as ttm might have schedule bo move
>>> on it and new command scheduled for other ring need to wait for bo
>>> data to be in place.
>>>
> Is this required for -fixes? if so it doesn't apply clean.
Nope, only drm-next, since there isn't any public available client that 
makes use of it.

Christian.