diff mbox

[1/2] drm/radeon: do not move bo to different placement at each cs

Message ID 1354203342-3961-2-git-send-email-j.glisse@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jerome Glisse Nov. 29, 2012, 3:35 p.m. UTC
From: Jerome Glisse <jglisse@redhat.com>

The bo creation placement is where the bo will be. Instead of trying
to move bo at each command stream let this work to another worker
thread that will use more advance heuristic.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h        |  1 +
 drivers/gpu/drm/radeon/radeon_object.c | 17 ++++++++---------
 2 files changed, 9 insertions(+), 9 deletions(-)

Comments

Jerome Glisse Dec. 10, 2012, 8:16 p.m. UTC | #1
On Thu, Nov 29, 2012 at 10:35 AM,  <j.glisse@gmail.com> wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> The bo creation placement is where the bo will be. Instead of trying
> to move bo at each command stream let this work to another worker
> thread that will use more advance heuristic.
>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>

What about including this for 3.8 it will mostly fix all regression
performance and is a first valid step for proper bo placement.

Cheers,
Jerome

> ---
>  drivers/gpu/drm/radeon/radeon.h        |  1 +
>  drivers/gpu/drm/radeon/radeon_object.c | 17 ++++++++---------
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 8c42d54..0a2664c 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -313,6 +313,7 @@ struct radeon_bo {
>         struct list_head                list;
>         /* Protected by tbo.reserved */
>         u32                             placements[3];
> +       u32                             busy_placements[3];
>         struct ttm_placement            placement;
>         struct ttm_buffer_object        tbo;
>         struct ttm_bo_kmap_obj          kmap;
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 3f9f3bb..e25ae20 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -84,7 +84,6 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>         rbo->placement.fpfn = 0;
>         rbo->placement.lpfn = 0;
>         rbo->placement.placement = rbo->placements;
> -       rbo->placement.busy_placement = rbo->placements;
>         if (domain & RADEON_GEM_DOMAIN_VRAM)
>                 rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>                                         TTM_PL_FLAG_VRAM;
> @@ -105,6 +104,14 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>         if (!c)
>                 rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
>         rbo->placement.num_placement = c;
> +
> +       c = 0;
> +       rbo->placement.busy_placement = rbo->busy_placements;
> +       if (rbo->rdev->flags & RADEON_IS_AGP) {
> +               rbo->busy_placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_TT;
> +       } else {
> +               rbo->busy_placements[c++] = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_TT;
> +       }
>         rbo->placement.num_busy_placement = c;
>  }
>
> @@ -360,17 +367,9 @@ int radeon_bo_list_validate(struct list_head *head)
>         list_for_each_entry(lobj, head, tv.head) {
>                 bo = lobj->bo;
>                 if (!bo->pin_count) {
> -                       domain = lobj->wdomain ? lobj->wdomain : lobj->rdomain;
> -
> -               retry:
> -                       radeon_ttm_placement_from_domain(bo, domain);
>                         r = ttm_bo_validate(&bo->tbo, &bo->placement,
>                                                 true, false, false);
>                         if (unlikely(r)) {
> -                               if (r != -ERESTARTSYS && domain == RADEON_GEM_DOMAIN_VRAM) {
> -                                       domain |= RADEON_GEM_DOMAIN_GTT;
> -                                       goto retry;
> -                               }
>                                 return r;
>                         }
>                 }
> --
> 1.7.11.7
>
Alex Deucher Dec. 11, 2012, 5:18 p.m. UTC | #2
On Mon, Dec 10, 2012 at 3:16 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Thu, Nov 29, 2012 at 10:35 AM,  <j.glisse@gmail.com> wrote:
>> From: Jerome Glisse <jglisse@redhat.com>
>>
>> The bo creation placement is where the bo will be. Instead of trying
>> to move bo at each command stream let this work to another worker
>> thread that will use more advance heuristic.
>>
>> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>
> What about including this for 3.8 it will mostly fix all regression
> performance and is a first valid step for proper bo placement.

Looks good to me.  I'll add it to my 3.8 tree unless there are any objections.

Alex

>
> Cheers,
> Jerome
>
>> ---
>>  drivers/gpu/drm/radeon/radeon.h        |  1 +
>>  drivers/gpu/drm/radeon/radeon_object.c | 17 ++++++++---------
>>  2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
>> index 8c42d54..0a2664c 100644
>> --- a/drivers/gpu/drm/radeon/radeon.h
>> +++ b/drivers/gpu/drm/radeon/radeon.h
>> @@ -313,6 +313,7 @@ struct radeon_bo {
>>         struct list_head                list;
>>         /* Protected by tbo.reserved */
>>         u32                             placements[3];
>> +       u32                             busy_placements[3];
>>         struct ttm_placement            placement;
>>         struct ttm_buffer_object        tbo;
>>         struct ttm_bo_kmap_obj          kmap;
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
>> index 3f9f3bb..e25ae20 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.c
>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
>> @@ -84,7 +84,6 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>>         rbo->placement.fpfn = 0;
>>         rbo->placement.lpfn = 0;
>>         rbo->placement.placement = rbo->placements;
>> -       rbo->placement.busy_placement = rbo->placements;
>>         if (domain & RADEON_GEM_DOMAIN_VRAM)
>>                 rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>>                                         TTM_PL_FLAG_VRAM;
>> @@ -105,6 +104,14 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>>         if (!c)
>>                 rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
>>         rbo->placement.num_placement = c;
>> +
>> +       c = 0;
>> +       rbo->placement.busy_placement = rbo->busy_placements;
>> +       if (rbo->rdev->flags & RADEON_IS_AGP) {
>> +               rbo->busy_placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_TT;
>> +       } else {
>> +               rbo->busy_placements[c++] = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_TT;
>> +       }
>>         rbo->placement.num_busy_placement = c;
>>  }
>>
>> @@ -360,17 +367,9 @@ int radeon_bo_list_validate(struct list_head *head)
>>         list_for_each_entry(lobj, head, tv.head) {
>>                 bo = lobj->bo;
>>                 if (!bo->pin_count) {
>> -                       domain = lobj->wdomain ? lobj->wdomain : lobj->rdomain;
>> -
>> -               retry:
>> -                       radeon_ttm_placement_from_domain(bo, domain);
>>                         r = ttm_bo_validate(&bo->tbo, &bo->placement,
>>                                                 true, false, false);
>>                         if (unlikely(r)) {
>> -                               if (r != -ERESTARTSYS && domain == RADEON_GEM_DOMAIN_VRAM) {
>> -                                       domain |= RADEON_GEM_DOMAIN_GTT;
>> -                                       goto retry;
>> -                               }
>>                                 return r;
>>                         }
>>                 }
>> --
>> 1.7.11.7
>>
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 8c42d54..0a2664c 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -313,6 +313,7 @@  struct radeon_bo {
 	struct list_head		list;
 	/* Protected by tbo.reserved */
 	u32				placements[3];
+	u32				busy_placements[3];
 	struct ttm_placement		placement;
 	struct ttm_buffer_object	tbo;
 	struct ttm_bo_kmap_obj		kmap;
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 3f9f3bb..e25ae20 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -84,7 +84,6 @@  void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
 	rbo->placement.fpfn = 0;
 	rbo->placement.lpfn = 0;
 	rbo->placement.placement = rbo->placements;
-	rbo->placement.busy_placement = rbo->placements;
 	if (domain & RADEON_GEM_DOMAIN_VRAM)
 		rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
 					TTM_PL_FLAG_VRAM;
@@ -105,6 +104,14 @@  void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
 	if (!c)
 		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
 	rbo->placement.num_placement = c;
+
+	c = 0;
+	rbo->placement.busy_placement = rbo->busy_placements;
+	if (rbo->rdev->flags & RADEON_IS_AGP) {
+		rbo->busy_placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_TT;
+	} else {
+		rbo->busy_placements[c++] = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_TT;
+	}
 	rbo->placement.num_busy_placement = c;
 }
 
@@ -360,17 +367,9 @@  int radeon_bo_list_validate(struct list_head *head)
 	list_for_each_entry(lobj, head, tv.head) {
 		bo = lobj->bo;
 		if (!bo->pin_count) {
-			domain = lobj->wdomain ? lobj->wdomain : lobj->rdomain;
-			
-		retry:
-			radeon_ttm_placement_from_domain(bo, domain);
 			r = ttm_bo_validate(&bo->tbo, &bo->placement,
 						true, false, false);
 			if (unlikely(r)) {
-				if (r != -ERESTARTSYS && domain == RADEON_GEM_DOMAIN_VRAM) {
-					domain |= RADEON_GEM_DOMAIN_GTT;
-					goto retry;
-				}
 				return r;
 			}
 		}