diff mbox series

[2/5] drm/ttm: add busy and idle placement flags

Message ID 20210831112110.113196-2-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/5] drm/ttm: cleanup ttm_resource_compat | expand

Commit Message

Christian König Aug. 31, 2021, 11:21 a.m. UTC
More flexible than the busy placements.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    | 8 +++++++-
 include/drm/ttm/ttm_placement.h | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Aug. 31, 2021, 1:18 p.m. UTC | #1
On Tue, Aug 31, 2021 at 01:21:07PM +0200, Christian König wrote:
> More flexible than the busy placements.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c    | 8 +++++++-
>  include/drm/ttm/ttm_placement.h | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 0a3127436f61..c7034040c67f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -834,6 +834,9 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>  		const struct ttm_place *place = &placement->placement[i];
>  		struct ttm_resource_manager *man;
>  
> +		if (place->flags & TTM_PL_FLAG_BUSY)
> +			continue;
> +
>  		man = ttm_manager_type(bdev, place->mem_type);
>  		if (!man || !ttm_resource_manager_used(man))
>  			continue;
> @@ -860,6 +863,9 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>  		const struct ttm_place *place = &placement->busy_placement[i];
>  		struct ttm_resource_manager *man;
>  
> +		if (place->flags & TTM_PL_FLAG_IDLE)
> +			continue;
> +
>  		man = ttm_manager_type(bdev, place->mem_type);
>  		if (!man || !ttm_resource_manager_used(man))
>  			continue;
> @@ -869,7 +875,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>  		if (likely(!ret))
>  			return 0;
>  
> -		if (ret && ret != -EBUSY)
> +		if (ret != -EBUSY)
>  			goto error;
>  	}
>  
> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
> index 8995c9e4ec1b..63f7217354c0 100644
> --- a/include/drm/ttm/ttm_placement.h
> +++ b/include/drm/ttm/ttm_placement.h
> @@ -53,6 +53,12 @@
>  /* For multihop handling */
>  #define TTM_PL_FLAG_TEMPORARY   (1 << 2)
>  
> +/* Placement is only used when we are evicting */
> +#define TTM_PL_FLAG_BUSY	(1 << 3)
> +
> +/* Placement is only used when we are not evicting */
> +#define TTM_PL_FLAG_IDLE	(1 << 4)

Using an enum for this (with BIT() macro or so) and then slapping
kerneldoc on top would be nice. That way you can also use the same enum in
parameters and structures and it's all a bit easier to find and connect.

Otherwise I think this series makes sense, but probably better for nouveau
folks to do review/testing.
-Daniel

> +
>  /**
>   * struct ttm_place
>   *
> -- 
> 2.25.1
>
Christian König Sept. 1, 2021, 7:35 a.m. UTC | #2
Am 31.08.21 um 15:18 schrieb Daniel Vetter:
> On Tue, Aug 31, 2021 at 01:21:07PM +0200, Christian König wrote:
>> More flexible than the busy placements.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c    | 8 +++++++-
>>   include/drm/ttm/ttm_placement.h | 6 ++++++
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 0a3127436f61..c7034040c67f 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -834,6 +834,9 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>>   		const struct ttm_place *place = &placement->placement[i];
>>   		struct ttm_resource_manager *man;
>>   
>> +		if (place->flags & TTM_PL_FLAG_BUSY)
>> +			continue;
>> +
>>   		man = ttm_manager_type(bdev, place->mem_type);
>>   		if (!man || !ttm_resource_manager_used(man))
>>   			continue;
>> @@ -860,6 +863,9 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>>   		const struct ttm_place *place = &placement->busy_placement[i];
>>   		struct ttm_resource_manager *man;
>>   
>> +		if (place->flags & TTM_PL_FLAG_IDLE)
>> +			continue;
>> +
>>   		man = ttm_manager_type(bdev, place->mem_type);
>>   		if (!man || !ttm_resource_manager_used(man))
>>   			continue;
>> @@ -869,7 +875,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>>   		if (likely(!ret))
>>   			return 0;
>>   
>> -		if (ret && ret != -EBUSY)
>> +		if (ret != -EBUSY)
>>   			goto error;
>>   	}
>>   
>> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
>> index 8995c9e4ec1b..63f7217354c0 100644
>> --- a/include/drm/ttm/ttm_placement.h
>> +++ b/include/drm/ttm/ttm_placement.h
>> @@ -53,6 +53,12 @@
>>   /* For multihop handling */
>>   #define TTM_PL_FLAG_TEMPORARY   (1 << 2)
>>   
>> +/* Placement is only used when we are evicting */
>> +#define TTM_PL_FLAG_BUSY	(1 << 3)
>> +
>> +/* Placement is only used when we are not evicting */
>> +#define TTM_PL_FLAG_IDLE	(1 << 4)
> Using an enum for this (with BIT() macro or so) and then slapping
> kerneldoc on top would be nice. That way you can also use the same enum in
> parameters and structures and it's all a bit easier to find and connect.

I don't really like to define flags as enums since they are not really 
an enumeration.

But some more kerneldoc sounds like a good idea to me.

> Otherwise I think this series makes sense, but probably better for nouveau
> folks to do review/testing.

Yeah, agree completely. I can do some smoke testing with nouveau, but 
that's about it.

Christian.

> -Daniel
>
>> +
>>   /**
>>    * struct ttm_place
>>    *
>> -- 
>> 2.25.1
>>
Huang Rui Sept. 2, 2021, 9:19 a.m. UTC | #3
On Tue, Aug 31, 2021 at 07:21:07PM +0800, Christian König wrote:
> More flexible than the busy placements.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Patch 2 -> 5 are Acked-by: Huang Rui <ray.huang@amd.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c    | 8 +++++++-
>  include/drm/ttm/ttm_placement.h | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 0a3127436f61..c7034040c67f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -834,6 +834,9 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>  		const struct ttm_place *place = &placement->placement[i];
>  		struct ttm_resource_manager *man;
>  
> +		if (place->flags & TTM_PL_FLAG_BUSY)
> +			continue;
> +
>  		man = ttm_manager_type(bdev, place->mem_type);
>  		if (!man || !ttm_resource_manager_used(man))
>  			continue;
> @@ -860,6 +863,9 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>  		const struct ttm_place *place = &placement->busy_placement[i];
>  		struct ttm_resource_manager *man;
>  
> +		if (place->flags & TTM_PL_FLAG_IDLE)
> +			continue;
> +
>  		man = ttm_manager_type(bdev, place->mem_type);
>  		if (!man || !ttm_resource_manager_used(man))
>  			continue;
> @@ -869,7 +875,7 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
>  		if (likely(!ret))
>  			return 0;
>  
> -		if (ret && ret != -EBUSY)
> +		if (ret != -EBUSY)
>  			goto error;
>  	}
>  
> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
> index 8995c9e4ec1b..63f7217354c0 100644
> --- a/include/drm/ttm/ttm_placement.h
> +++ b/include/drm/ttm/ttm_placement.h
> @@ -53,6 +53,12 @@
>  /* For multihop handling */
>  #define TTM_PL_FLAG_TEMPORARY   (1 << 2)
>  
> +/* Placement is only used when we are evicting */
> +#define TTM_PL_FLAG_BUSY	(1 << 3)
> +
> +/* Placement is only used when we are not evicting */
> +#define TTM_PL_FLAG_IDLE	(1 << 4)
> +
>  /**
>   * struct ttm_place
>   *
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 0a3127436f61..c7034040c67f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -834,6 +834,9 @@  int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 		const struct ttm_place *place = &placement->placement[i];
 		struct ttm_resource_manager *man;
 
+		if (place->flags & TTM_PL_FLAG_BUSY)
+			continue;
+
 		man = ttm_manager_type(bdev, place->mem_type);
 		if (!man || !ttm_resource_manager_used(man))
 			continue;
@@ -860,6 +863,9 @@  int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 		const struct ttm_place *place = &placement->busy_placement[i];
 		struct ttm_resource_manager *man;
 
+		if (place->flags & TTM_PL_FLAG_IDLE)
+			continue;
+
 		man = ttm_manager_type(bdev, place->mem_type);
 		if (!man || !ttm_resource_manager_used(man))
 			continue;
@@ -869,7 +875,7 @@  int ttm_bo_mem_space(struct ttm_buffer_object *bo,
 		if (likely(!ret))
 			return 0;
 
-		if (ret && ret != -EBUSY)
+		if (ret != -EBUSY)
 			goto error;
 	}
 
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index 8995c9e4ec1b..63f7217354c0 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -53,6 +53,12 @@ 
 /* For multihop handling */
 #define TTM_PL_FLAG_TEMPORARY   (1 << 2)
 
+/* Placement is only used when we are evicting */
+#define TTM_PL_FLAG_BUSY	(1 << 3)
+
+/* Placement is only used when we are not evicting */
+#define TTM_PL_FLAG_IDLE	(1 << 4)
+
 /**
  * struct ttm_place
  *