* [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 10:34 ` Matthew Auld
2022-02-14 9:34 ` [PATCH 03/11] drm/ttm: move the LRU into resource handling v3 Christian König
` (10 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
It makes sense to have this in the common manager for debugging and
accounting of how much resources are used.
v2: cleanup kerneldoc a bit
v3: drop the atomic, update counter under lock instead
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com> (v1)
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/ttm/ttm_resource.c | 30 ++++++++++++++++++++++++++++++
include/drm/ttm/ttm_resource.h | 11 +++++++++--
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index ae40e144e728..bbb8a0f7aa14 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res)
{
+ struct ttm_resource_manager *man;
+
res->start = 0;
res->num_pages = PFN_UP(bo->base.size);
res->mem_type = place->mem_type;
@@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
res->bo = bo;
+
+ man = ttm_manager_type(bo->bdev, place->mem_type);
+ spin_lock(&bo->bdev->lru_lock);
+ man->usage += bo->base.size;
+ spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init);
void ttm_resource_fini(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
+ spin_lock(&man->bdev->lru_lock);
+ man->usage -= res->bo->base.size;
+ spin_unlock(&man->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_fini);
@@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
spin_lock_init(&man->move_lock);
man->bdev = bdev;
man->size = size;
+ man->usage = 0;
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
INIT_LIST_HEAD(&man->lru[i]);
@@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
}
EXPORT_SYMBOL(ttm_resource_manager_evict_all);
+/**
+ * ttm_resource_manager_usage
+ *
+ * @man: A memory manager object.
+ *
+ * Return how many resources are currently used.
+ */
+uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man)
+{
+ uint64_t usage;
+
+ spin_lock(&man->bdev->lru_lock);
+ usage = man->usage;
+ spin_unlock(&man->bdev->lru_lock);
+ return usage;
+}
+EXPORT_SYMBOL(ttm_resource_manager_usage);
+
/**
* ttm_resource_manager_debug
*
@@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
drm_printf(p, " use_type: %d\n", man->use_type);
drm_printf(p, " use_tt: %d\n", man->use_tt);
drm_printf(p, " size: %llu\n", man->size);
+ drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man));
if (man->func->debug)
man->func->debug(man, p);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 555a11fb8a7f..323c14a30c6b 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -27,6 +27,7 @@
#include <linux/types.h>
#include <linux/mutex.h>
+#include <linux/atomic.h>
#include <linux/dma-buf-map.h>
#include <linux/dma-fence.h>
#include <drm/drm_print.h>
@@ -130,10 +131,15 @@ struct ttm_resource_manager {
struct dma_fence *move;
/*
- * Protected by the global->lru_lock.
+ * Protected by the bdev->lru_lock.
*/
-
struct list_head lru[TTM_MAX_BO_PRIORITY];
+
+ /**
+ * @usage: How much of the resources are used, protected by the
+ * bdev->lru_lock.
+ */
+ uint64_t usage;
};
/**
@@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
int ttm_resource_manager_evict_all(struct ttm_device *bdev,
struct ttm_resource_manager *man);
+uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
void ttm_resource_manager_debug(struct ttm_resource_manager *man,
struct drm_printer *p);
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
2022-02-14 9:34 ` [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3 Christian König
@ 2022-02-14 10:34 ` Matthew Auld
2022-02-14 13:23 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Matthew Auld @ 2022-02-14 10:34 UTC (permalink / raw)
To: Christian König; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
On Mon, 14 Feb 2022 at 09:34, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> It makes sense to have this in the common manager for debugging and
> accounting of how much resources are used.
>
> v2: cleanup kerneldoc a bit
> v3: drop the atomic, update counter under lock instead
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com> (v1)
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/ttm/ttm_resource.c | 30 ++++++++++++++++++++++++++++++
> include/drm/ttm/ttm_resource.h | 11 +++++++++--
> 2 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index ae40e144e728..bbb8a0f7aa14 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> const struct ttm_place *place,
> struct ttm_resource *res)
> {
> + struct ttm_resource_manager *man;
> +
> res->start = 0;
> res->num_pages = PFN_UP(bo->base.size);
> res->mem_type = place->mem_type;
> @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> res->bus.is_iomem = false;
> res->bus.caching = ttm_cached;
> res->bo = bo;
> +
> + man = ttm_manager_type(bo->bdev, place->mem_type);
> + spin_lock(&bo->bdev->lru_lock);
> + man->usage += bo->base.size;
> + spin_unlock(&bo->bdev->lru_lock);
> }
> EXPORT_SYMBOL(ttm_resource_init);
>
> @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init);
> void ttm_resource_fini(struct ttm_resource_manager *man,
> struct ttm_resource *res)
> {
> + spin_lock(&man->bdev->lru_lock);
> + man->usage -= res->bo->base.size;
> + spin_unlock(&man->bdev->lru_lock);
> }
> EXPORT_SYMBOL(ttm_resource_fini);
>
> @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
> spin_lock_init(&man->move_lock);
> man->bdev = bdev;
> man->size = size;
> + man->usage = 0;
>
> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> INIT_LIST_HEAD(&man->lru[i]);
> @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
> }
> EXPORT_SYMBOL(ttm_resource_manager_evict_all);
>
> +/**
> + * ttm_resource_manager_usage
> + *
> + * @man: A memory manager object.
> + *
> + * Return how many resources are currently used.
Maybe mention the units here?
"Return how many resources are currently used, in bytes."
Anyway,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> + */
> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man)
> +{
> + uint64_t usage;
> +
> + spin_lock(&man->bdev->lru_lock);
> + usage = man->usage;
> + spin_unlock(&man->bdev->lru_lock);
> + return usage;
> +}
> +EXPORT_SYMBOL(ttm_resource_manager_usage);
> +
> /**
> * ttm_resource_manager_debug
> *
> @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> drm_printf(p, " use_type: %d\n", man->use_type);
> drm_printf(p, " use_tt: %d\n", man->use_tt);
> drm_printf(p, " size: %llu\n", man->size);
> + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man));
> if (man->func->debug)
> man->func->debug(man, p);
> }
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 555a11fb8a7f..323c14a30c6b 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -27,6 +27,7 @@
>
> #include <linux/types.h>
> #include <linux/mutex.h>
> +#include <linux/atomic.h>
> #include <linux/dma-buf-map.h>
> #include <linux/dma-fence.h>
> #include <drm/drm_print.h>
> @@ -130,10 +131,15 @@ struct ttm_resource_manager {
> struct dma_fence *move;
>
> /*
> - * Protected by the global->lru_lock.
> + * Protected by the bdev->lru_lock.
> */
> -
> struct list_head lru[TTM_MAX_BO_PRIORITY];
> +
> + /**
> + * @usage: How much of the resources are used, protected by the
> + * bdev->lru_lock.
> + */
> + uint64_t usage;
> };
>
> /**
> @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
> int ttm_resource_manager_evict_all(struct ttm_device *bdev,
> struct ttm_resource_manager *man);
>
> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
> void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> struct drm_printer *p);
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
2022-02-14 10:34 ` Matthew Auld
@ 2022-02-14 13:23 ` Christian König
2022-02-14 14:29 ` Matthew Auld
0 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2022-02-14 13:23 UTC (permalink / raw)
To: Matthew Auld; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
Am 14.02.22 um 11:34 schrieb Matthew Auld:
> On Mon, 14 Feb 2022 at 09:34, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> It makes sense to have this in the common manager for debugging and
>> accounting of how much resources are used.
>>
>> v2: cleanup kerneldoc a bit
>> v3: drop the atomic, update counter under lock instead
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Reviewed-by: Huang Rui <ray.huang@amd.com> (v1)
>> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> ---
>> drivers/gpu/drm/ttm/ttm_resource.c | 30 ++++++++++++++++++++++++++++++
>> include/drm/ttm/ttm_resource.h | 11 +++++++++--
>> 2 files changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
>> index ae40e144e728..bbb8a0f7aa14 100644
>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>> @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>> const struct ttm_place *place,
>> struct ttm_resource *res)
>> {
>> + struct ttm_resource_manager *man;
>> +
>> res->start = 0;
>> res->num_pages = PFN_UP(bo->base.size);
>> res->mem_type = place->mem_type;
>> @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>> res->bus.is_iomem = false;
>> res->bus.caching = ttm_cached;
>> res->bo = bo;
>> +
>> + man = ttm_manager_type(bo->bdev, place->mem_type);
>> + spin_lock(&bo->bdev->lru_lock);
>> + man->usage += bo->base.size;
>> + spin_unlock(&bo->bdev->lru_lock);
>> }
>> EXPORT_SYMBOL(ttm_resource_init);
>>
>> @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init);
>> void ttm_resource_fini(struct ttm_resource_manager *man,
>> struct ttm_resource *res)
>> {
>> + spin_lock(&man->bdev->lru_lock);
>> + man->usage -= res->bo->base.size;
>> + spin_unlock(&man->bdev->lru_lock);
>> }
>> EXPORT_SYMBOL(ttm_resource_fini);
>>
>> @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
>> spin_lock_init(&man->move_lock);
>> man->bdev = bdev;
>> man->size = size;
>> + man->usage = 0;
>>
>> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
>> INIT_LIST_HEAD(&man->lru[i]);
>> @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
>> }
>> EXPORT_SYMBOL(ttm_resource_manager_evict_all);
>>
>> +/**
>> + * ttm_resource_manager_usage
>> + *
>> + * @man: A memory manager object.
>> + *
>> + * Return how many resources are currently used.
> Maybe mention the units here?
>
> "Return how many resources are currently used, in bytes."
Well exactly that's not correct. The whole idea here is that these are
driver defined units.
E.g. for the AMDGPU OA and GWS resources it's essentially a hardware block.
Regards,
Christian.
>
> Anyway,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
>> + */
>> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man)
>> +{
>> + uint64_t usage;
>> +
>> + spin_lock(&man->bdev->lru_lock);
>> + usage = man->usage;
>> + spin_unlock(&man->bdev->lru_lock);
>> + return usage;
>> +}
>> +EXPORT_SYMBOL(ttm_resource_manager_usage);
>> +
>> /**
>> * ttm_resource_manager_debug
>> *
>> @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
>> drm_printf(p, " use_type: %d\n", man->use_type);
>> drm_printf(p, " use_tt: %d\n", man->use_tt);
>> drm_printf(p, " size: %llu\n", man->size);
>> + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man));
>> if (man->func->debug)
>> man->func->debug(man, p);
>> }
>> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
>> index 555a11fb8a7f..323c14a30c6b 100644
>> --- a/include/drm/ttm/ttm_resource.h
>> +++ b/include/drm/ttm/ttm_resource.h
>> @@ -27,6 +27,7 @@
>>
>> #include <linux/types.h>
>> #include <linux/mutex.h>
>> +#include <linux/atomic.h>
>> #include <linux/dma-buf-map.h>
>> #include <linux/dma-fence.h>
>> #include <drm/drm_print.h>
>> @@ -130,10 +131,15 @@ struct ttm_resource_manager {
>> struct dma_fence *move;
>>
>> /*
>> - * Protected by the global->lru_lock.
>> + * Protected by the bdev->lru_lock.
>> */
>> -
>> struct list_head lru[TTM_MAX_BO_PRIORITY];
>> +
>> + /**
>> + * @usage: How much of the resources are used, protected by the
>> + * bdev->lru_lock.
>> + */
>> + uint64_t usage;
>> };
>>
>> /**
>> @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
>> int ttm_resource_manager_evict_all(struct ttm_device *bdev,
>> struct ttm_resource_manager *man);
>>
>> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
>> void ttm_resource_manager_debug(struct ttm_resource_manager *man,
>> struct drm_printer *p);
>>
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
2022-02-14 13:23 ` Christian König
@ 2022-02-14 14:29 ` Matthew Auld
2022-02-14 15:36 ` Christian König
0 siblings, 1 reply; 25+ messages in thread
From: Matthew Auld @ 2022-02-14 14:29 UTC (permalink / raw)
To: Christian König; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
On Mon, 14 Feb 2022 at 13:23, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 14.02.22 um 11:34 schrieb Matthew Auld:
> > On Mon, 14 Feb 2022 at 09:34, Christian König
> > <ckoenig.leichtzumerken@gmail.com> wrote:
> >> It makes sense to have this in the common manager for debugging and
> >> accounting of how much resources are used.
> >>
> >> v2: cleanup kerneldoc a bit
> >> v3: drop the atomic, update counter under lock instead
> >>
> >> Signed-off-by: Christian König <christian.koenig@amd.com>
> >> Reviewed-by: Huang Rui <ray.huang@amd.com> (v1)
> >> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> >> ---
> >> drivers/gpu/drm/ttm/ttm_resource.c | 30 ++++++++++++++++++++++++++++++
> >> include/drm/ttm/ttm_resource.h | 11 +++++++++--
> >> 2 files changed, 39 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> >> index ae40e144e728..bbb8a0f7aa14 100644
> >> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> >> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> >> @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> >> const struct ttm_place *place,
> >> struct ttm_resource *res)
> >> {
> >> + struct ttm_resource_manager *man;
> >> +
> >> res->start = 0;
> >> res->num_pages = PFN_UP(bo->base.size);
> >> res->mem_type = place->mem_type;
> >> @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
> >> res->bus.is_iomem = false;
> >> res->bus.caching = ttm_cached;
> >> res->bo = bo;
> >> +
> >> + man = ttm_manager_type(bo->bdev, place->mem_type);
> >> + spin_lock(&bo->bdev->lru_lock);
> >> + man->usage += bo->base.size;
> >> + spin_unlock(&bo->bdev->lru_lock);
> >> }
> >> EXPORT_SYMBOL(ttm_resource_init);
> >>
> >> @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init);
> >> void ttm_resource_fini(struct ttm_resource_manager *man,
> >> struct ttm_resource *res)
> >> {
> >> + spin_lock(&man->bdev->lru_lock);
> >> + man->usage -= res->bo->base.size;
> >> + spin_unlock(&man->bdev->lru_lock);
> >> }
> >> EXPORT_SYMBOL(ttm_resource_fini);
> >>
> >> @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
> >> spin_lock_init(&man->move_lock);
> >> man->bdev = bdev;
> >> man->size = size;
> >> + man->usage = 0;
> >>
> >> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> >> INIT_LIST_HEAD(&man->lru[i]);
> >> @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
> >> }
> >> EXPORT_SYMBOL(ttm_resource_manager_evict_all);
> >>
> >> +/**
> >> + * ttm_resource_manager_usage
> >> + *
> >> + * @man: A memory manager object.
> >> + *
> >> + * Return how many resources are currently used.
> > Maybe mention the units here?
> >
> > "Return how many resources are currently used, in bytes."
>
> Well exactly that's not correct. The whole idea here is that these are
> driver defined units.
Ok, I was assuming bo->base.size to operate in bytes(the kernel-doc
seems to indicate that) and ttm_resource_{init, fini} are using this
to track the man->usage.
>
> E.g. for the AMDGPU OA and GWS resources it's essentially a hardware block.
>
> Regards,
> Christian.
>
> >
> > Anyway,
> > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> >
> >> + */
> >> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man)
> >> +{
> >> + uint64_t usage;
> >> +
> >> + spin_lock(&man->bdev->lru_lock);
> >> + usage = man->usage;
> >> + spin_unlock(&man->bdev->lru_lock);
> >> + return usage;
> >> +}
> >> +EXPORT_SYMBOL(ttm_resource_manager_usage);
> >> +
> >> /**
> >> * ttm_resource_manager_debug
> >> *
> >> @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> >> drm_printf(p, " use_type: %d\n", man->use_type);
> >> drm_printf(p, " use_tt: %d\n", man->use_tt);
> >> drm_printf(p, " size: %llu\n", man->size);
> >> + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man));
> >> if (man->func->debug)
> >> man->func->debug(man, p);
> >> }
> >> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> >> index 555a11fb8a7f..323c14a30c6b 100644
> >> --- a/include/drm/ttm/ttm_resource.h
> >> +++ b/include/drm/ttm/ttm_resource.h
> >> @@ -27,6 +27,7 @@
> >>
> >> #include <linux/types.h>
> >> #include <linux/mutex.h>
> >> +#include <linux/atomic.h>
> >> #include <linux/dma-buf-map.h>
> >> #include <linux/dma-fence.h>
> >> #include <drm/drm_print.h>
> >> @@ -130,10 +131,15 @@ struct ttm_resource_manager {
> >> struct dma_fence *move;
> >>
> >> /*
> >> - * Protected by the global->lru_lock.
> >> + * Protected by the bdev->lru_lock.
> >> */
> >> -
> >> struct list_head lru[TTM_MAX_BO_PRIORITY];
> >> +
> >> + /**
> >> + * @usage: How much of the resources are used, protected by the
> >> + * bdev->lru_lock.
> >> + */
> >> + uint64_t usage;
> >> };
> >>
> >> /**
> >> @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
> >> int ttm_resource_manager_evict_all(struct ttm_device *bdev,
> >> struct ttm_resource_manager *man);
> >>
> >> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
> >> void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> >> struct drm_printer *p);
> >>
> >> --
> >> 2.25.1
> >>
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3
2022-02-14 14:29 ` Matthew Auld
@ 2022-02-14 15:36 ` Christian König
0 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 15:36 UTC (permalink / raw)
To: Matthew Auld; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
Am 14.02.22 um 15:29 schrieb Matthew Auld:
> On Mon, 14 Feb 2022 at 13:23, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 14.02.22 um 11:34 schrieb Matthew Auld:
>>> On Mon, 14 Feb 2022 at 09:34, Christian König
>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>> It makes sense to have this in the common manager for debugging and
>>>> accounting of how much resources are used.
>>>>
>>>> v2: cleanup kerneldoc a bit
>>>> v3: drop the atomic, update counter under lock instead
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> Reviewed-by: Huang Rui <ray.huang@amd.com> (v1)
>>>> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>>>> ---
>>>> drivers/gpu/drm/ttm/ttm_resource.c | 30 ++++++++++++++++++++++++++++++
>>>> include/drm/ttm/ttm_resource.h | 11 +++++++++--
>>>> 2 files changed, 39 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> index ae40e144e728..bbb8a0f7aa14 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> @@ -41,6 +41,8 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>>>> const struct ttm_place *place,
>>>> struct ttm_resource *res)
>>>> {
>>>> + struct ttm_resource_manager *man;
>>>> +
>>>> res->start = 0;
>>>> res->num_pages = PFN_UP(bo->base.size);
>>>> res->mem_type = place->mem_type;
>>>> @@ -50,6 +52,11 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>>>> res->bus.is_iomem = false;
>>>> res->bus.caching = ttm_cached;
>>>> res->bo = bo;
>>>> +
>>>> + man = ttm_manager_type(bo->bdev, place->mem_type);
>>>> + spin_lock(&bo->bdev->lru_lock);
>>>> + man->usage += bo->base.size;
>>>> + spin_unlock(&bo->bdev->lru_lock);
>>>> }
>>>> EXPORT_SYMBOL(ttm_resource_init);
>>>>
>>>> @@ -65,6 +72,9 @@ EXPORT_SYMBOL(ttm_resource_init);
>>>> void ttm_resource_fini(struct ttm_resource_manager *man,
>>>> struct ttm_resource *res)
>>>> {
>>>> + spin_lock(&man->bdev->lru_lock);
>>>> + man->usage -= res->bo->base.size;
>>>> + spin_unlock(&man->bdev->lru_lock);
>>>> }
>>>> EXPORT_SYMBOL(ttm_resource_fini);
>>>>
>>>> @@ -166,6 +176,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
>>>> spin_lock_init(&man->move_lock);
>>>> man->bdev = bdev;
>>>> man->size = size;
>>>> + man->usage = 0;
>>>>
>>>> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
>>>> INIT_LIST_HEAD(&man->lru[i]);
>>>> @@ -226,6 +237,24 @@ int ttm_resource_manager_evict_all(struct ttm_device *bdev,
>>>> }
>>>> EXPORT_SYMBOL(ttm_resource_manager_evict_all);
>>>>
>>>> +/**
>>>> + * ttm_resource_manager_usage
>>>> + *
>>>> + * @man: A memory manager object.
>>>> + *
>>>> + * Return how many resources are currently used.
>>> Maybe mention the units here?
>>>
>>> "Return how many resources are currently used, in bytes."
>> Well exactly that's not correct. The whole idea here is that these are
>> driver defined units.
> Ok, I was assuming bo->base.size to operate in bytes(the kernel-doc
> seems to indicate that) and ttm_resource_{init, fini} are using this
> to track the man->usage.
Yeah, good point. That one is the next on my TODO list to fix.
Christian.
>
>> E.g. for the AMDGPU OA and GWS resources it's essentially a hardware block.
>>
>> Regards,
>> Christian.
>>
>>> Anyway,
>>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>>>
>>>> + */
>>>> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man)
>>>> +{
>>>> + uint64_t usage;
>>>> +
>>>> + spin_lock(&man->bdev->lru_lock);
>>>> + usage = man->usage;
>>>> + spin_unlock(&man->bdev->lru_lock);
>>>> + return usage;
>>>> +}
>>>> +EXPORT_SYMBOL(ttm_resource_manager_usage);
>>>> +
>>>> /**
>>>> * ttm_resource_manager_debug
>>>> *
>>>> @@ -238,6 +267,7 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
>>>> drm_printf(p, " use_type: %d\n", man->use_type);
>>>> drm_printf(p, " use_tt: %d\n", man->use_tt);
>>>> drm_printf(p, " size: %llu\n", man->size);
>>>> + drm_printf(p, " usage: %llu\n", ttm_resource_manager_usage(man));
>>>> if (man->func->debug)
>>>> man->func->debug(man, p);
>>>> }
>>>> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
>>>> index 555a11fb8a7f..323c14a30c6b 100644
>>>> --- a/include/drm/ttm/ttm_resource.h
>>>> +++ b/include/drm/ttm/ttm_resource.h
>>>> @@ -27,6 +27,7 @@
>>>>
>>>> #include <linux/types.h>
>>>> #include <linux/mutex.h>
>>>> +#include <linux/atomic.h>
>>>> #include <linux/dma-buf-map.h>
>>>> #include <linux/dma-fence.h>
>>>> #include <drm/drm_print.h>
>>>> @@ -130,10 +131,15 @@ struct ttm_resource_manager {
>>>> struct dma_fence *move;
>>>>
>>>> /*
>>>> - * Protected by the global->lru_lock.
>>>> + * Protected by the bdev->lru_lock.
>>>> */
>>>> -
>>>> struct list_head lru[TTM_MAX_BO_PRIORITY];
>>>> +
>>>> + /**
>>>> + * @usage: How much of the resources are used, protected by the
>>>> + * bdev->lru_lock.
>>>> + */
>>>> + uint64_t usage;
>>>> };
>>>>
>>>> /**
>>>> @@ -283,6 +289,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
>>>> int ttm_resource_manager_evict_all(struct ttm_device *bdev,
>>>> struct ttm_resource_manager *man);
>>>>
>>>> +uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
>>>> void ttm_resource_manager_debug(struct ttm_resource_manager *man,
>>>> struct drm_printer *p);
>>>>
>>>> --
>>>> 2.25.1
>>>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 03/11] drm/ttm: move the LRU into resource handling v3
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
2022-02-14 9:34 ` [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3 Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 9:34 ` [PATCH 04/11] drm/ttm: add resource iterator v2 Christian König
` (9 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
This way we finally fix the problem that new resource are
not immediately evict-able after allocation.
That has caused numerous problems including OOM on GDS handling
and not being able to use TTM as general resource manager.
v2: stop assuming in ttm_resource_fini that res->bo is still valid.
v3: cleanup kerneldoc, add more lockdep annotation
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +-
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 115 ++--------------------
drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
drivers/gpu/drm/ttm/ttm_device.c | 64 ++++++-------
drivers/gpu/drm/ttm/ttm_resource.c | 122 +++++++++++++++++++++++-
include/drm/ttm/ttm_bo_api.h | 16 ----
include/drm/ttm/ttm_bo_driver.h | 29 +-----
include/drm/ttm/ttm_resource.h | 35 +++++++
9 files changed, 197 insertions(+), 195 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b37fc7d7d2c7..f2ce5a0defd9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -683,12 +683,12 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
if (vm->bulk_moveable) {
spin_lock(&adev->mman.bdev.lru_lock);
- ttm_bo_bulk_move_lru_tail(&vm->lru_bulk_move);
+ ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
spin_unlock(&adev->mman.bdev.lru_lock);
return;
}
- memset(&vm->lru_bulk_move, 0, sizeof(vm->lru_bulk_move));
+ ttm_lru_bulk_move_init(&vm->lru_bulk_move);
spin_lock(&adev->mman.bdev.lru_lock);
list_for_each_entry(bo_base, &vm->idle, vm_status) {
@@ -698,11 +698,9 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
if (!bo->parent)
continue;
- ttm_bo_move_to_lru_tail(&bo->tbo, bo->tbo.resource,
- &vm->lru_bulk_move);
+ ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
if (shadow)
ttm_bo_move_to_lru_tail(&shadow->tbo,
- shadow->tbo.resource,
&vm->lru_bulk_move);
}
spin_unlock(&adev->mman.bdev.lru_lock);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index de3fe79b665a..582e8dc9bc8c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -849,7 +849,7 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
bo->priority = I915_TTM_PRIO_NO_PAGES;
}
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_bo_move_to_lru_tail(bo, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index db3dc7ef5382..cb0fa932d495 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,108 +69,16 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
}
}
-static inline void ttm_bo_move_to_pinned(struct ttm_buffer_object *bo)
-{
- struct ttm_device *bdev = bo->bdev;
-
- list_move_tail(&bo->lru, &bdev->pinned);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-}
-
-static inline void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
-{
- struct ttm_device *bdev = bo->bdev;
-
- list_del_init(&bo->lru);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-}
-
-static void ttm_bo_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
- struct ttm_buffer_object *bo)
-{
- if (!pos->first)
- pos->first = bo;
- pos->last = bo;
-}
-
void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_resource *mem,
struct ttm_lru_bulk_move *bulk)
{
- struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
-
- if (!bo->deleted)
- dma_resv_assert_held(bo->base.resv);
-
- if (bo->pin_count) {
- ttm_bo_move_to_pinned(bo);
- return;
- }
-
- if (!mem)
- return;
-
- man = ttm_manager_type(bdev, mem->mem_type);
- list_move_tail(&bo->lru, &man->lru[bo->priority]);
-
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-
- if (bulk && !bo->pin_count) {
- switch (bo->resource->mem_type) {
- case TTM_PL_TT:
- ttm_bo_bulk_move_set_pos(&bulk->tt[bo->priority], bo);
- break;
+ dma_resv_assert_held(bo->base.resv);
- case TTM_PL_VRAM:
- ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
- break;
- }
- }
+ if (bo->resource)
+ ttm_resource_move_to_lru_tail(bo->resource, bulk);
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
-void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
-{
- unsigned i;
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
- struct ttm_resource_manager *man;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- man = ttm_manager_type(pos->first->bdev, TTM_PL_TT);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
- struct ttm_resource_manager *man;
-
- if (!pos->first)
- continue;
-
- dma_resv_assert_held(pos->first->base.resv);
- dma_resv_assert_held(pos->last->base.resv);
-
- man = ttm_manager_type(pos->first->bdev, TTM_PL_VRAM);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-}
-EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
-
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
struct ttm_resource *mem, bool evict,
struct ttm_operation_ctx *ctx,
@@ -344,7 +252,6 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo,
return ret;
}
- ttm_bo_move_to_pinned(bo);
list_del_init(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
ttm_bo_cleanup_memtype_use(bo);
@@ -445,7 +352,7 @@ static void ttm_bo_release(struct kref *kref)
*/
if (bo->pin_count) {
bo->pin_count = 0;
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_resource_move_to_lru_tail(bo->resource, NULL);
}
kref_init(&bo->kref);
@@ -458,7 +365,6 @@ static void ttm_bo_release(struct kref *kref)
}
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_del_from_lru(bo);
list_del(&bo->ddestroy);
spin_unlock(&bo->bdev->lru_lock);
@@ -673,15 +579,17 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource *res;
bool locked = false;
unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(bo, &man->lru[i], lru) {
+ list_for_each_entry(res, &man->lru[i], lru) {
bool busy;
+ bo = res->bo;
if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
&locked, &busy)) {
if (busy && !busy_bo && ticket !=
@@ -699,7 +607,7 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
}
/* If the inner loop terminated early, we have our candidate */
- if (&bo->lru != &man->lru[i])
+ if (&res->lru != &man->lru[i])
break;
bo = NULL;
@@ -875,9 +783,6 @@ int ttm_bo_mem_space(struct ttm_buffer_object *bo,
}
error:
- if (bo->resource->mem_type == TTM_PL_SYSTEM && !bo->pin_count)
- ttm_bo_move_to_lru_tail_unlocked(bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_mem_space);
@@ -971,7 +876,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
kref_init(&bo->kref);
- INIT_LIST_HEAD(&bo->lru);
INIT_LIST_HEAD(&bo->ddestroy);
bo->bdev = bdev;
bo->type = type;
@@ -1021,8 +925,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
return ret;
}
- ttm_bo_move_to_lru_tail_unlocked(bo);
-
return ret;
}
EXPORT_SYMBOL(ttm_bo_init_reserved);
@@ -1123,7 +1025,6 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
return ret == -EBUSY ? -ENOSPC : ret;
}
- ttm_bo_move_to_pinned(bo);
/* TODO: Cleanup the locking */
spin_unlock(&bo->bdev->lru_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 544a84fa6589..0163e92b57af 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -231,7 +231,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
atomic_inc(&ttm_glob.bo_count);
INIT_LIST_HEAD(&fbo->base.ddestroy);
- INIT_LIST_HEAD(&fbo->base.lru);
fbo->base.moving = NULL;
drm_vma_node_reset(&fbo->base.base.vma_node);
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index be24bb6cefd0..ba35887147ba 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -144,6 +144,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
{
struct ttm_resource_manager *man;
struct ttm_buffer_object *bo;
+ struct ttm_resource *res;
unsigned i, j;
int ret;
@@ -154,8 +155,11 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
continue;
for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(bo, &man->lru[j], lru) {
- uint32_t num_pages = PFN_UP(bo->base.size);
+ list_for_each_entry(res, &man->lru[j], lru) {
+ uint32_t num_pages;
+
+ bo = res->bo;
+ num_pages = PFN_UP(bo->base.size);
ret = ttm_bo_swapout(bo, ctx, gfp_flags);
/* ttm_bo_swapout has dropped the lru_lock */
@@ -259,49 +263,45 @@ void ttm_device_fini(struct ttm_device *bdev)
}
EXPORT_SYMBOL(ttm_device_fini);
-void ttm_device_clear_dma_mappings(struct ttm_device *bdev)
+static void ttm_device_clear_lru_dma_mappings(struct ttm_device *bdev,
+ struct list_head *list)
{
- struct ttm_resource_manager *man;
- struct ttm_buffer_object *bo;
- unsigned int i, j;
+ struct ttm_resource *res;
spin_lock(&bdev->lru_lock);
- while (!list_empty(&bdev->pinned)) {
- bo = list_first_entry(&bdev->pinned, struct ttm_buffer_object, lru);
+ while ((res = list_first_entry_or_null(list, typeof(*res), lru))) {
+ struct ttm_buffer_object *bo = res->bo;
+
/* Take ref against racing releases once lru_lock is unlocked */
- if (ttm_bo_get_unless_zero(bo)) {
- list_del_init(&bo->lru);
- spin_unlock(&bdev->lru_lock);
+ if (!ttm_bo_get_unless_zero(bo))
+ continue;
- if (bo->ttm)
- ttm_tt_unpopulate(bo->bdev, bo->ttm);
+ list_del_init(&res->lru);
+ spin_unlock(&bdev->lru_lock);
- ttm_bo_put(bo);
- spin_lock(&bdev->lru_lock);
- }
+ if (bo->ttm)
+ ttm_tt_unpopulate(bo->bdev, bo->ttm);
+
+ ttm_bo_put(bo);
+ spin_lock(&bdev->lru_lock);
}
+ spin_unlock(&bdev->lru_lock);
+}
+
+void ttm_device_clear_dma_mappings(struct ttm_device *bdev)
+{
+ struct ttm_resource_manager *man;
+ unsigned int i, j;
+
+ ttm_device_clear_lru_dma_mappings(bdev, &bdev->pinned);
for (i = TTM_PL_SYSTEM; i < TTM_NUM_MEM_TYPES; ++i) {
man = ttm_manager_type(bdev, i);
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- while (!list_empty(&man->lru[j])) {
- bo = list_first_entry(&man->lru[j], struct ttm_buffer_object, lru);
- if (ttm_bo_get_unless_zero(bo)) {
- list_del_init(&bo->lru);
- spin_unlock(&bdev->lru_lock);
-
- if (bo->ttm)
- ttm_tt_unpopulate(bo->bdev, bo->ttm);
-
- ttm_bo_put(bo);
- spin_lock(&bdev->lru_lock);
- }
- }
- }
+ for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j)
+ ttm_device_clear_lru_dma_mappings(bdev, &man->lru[j]);
}
- spin_unlock(&bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_device_clear_dma_mappings);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index bbb8a0f7aa14..411b3f001eeb 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -29,6 +29,110 @@
#include <drm/ttm/ttm_resource.h>
#include <drm/ttm/ttm_bo_driver.h>
+/**
+ * ttm_lru_bulk_move_init - initialize a bulk move structure
+ * @bulk: the structure to init
+ *
+ * For now just memset the structure to zero.
+ */
+void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk)
+{
+ memset(bulk, 0, sizeof(*bulk));
+}
+EXPORT_SYMBOL(ttm_lru_bulk_move_init);
+
+/**
+ * ttm_lru_bulk_move_tail - bulk move range of resources to the LRU tail.
+ *
+ * @bulk: bulk move structure
+ *
+ * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
+ * resource order never changes. Should be called with &ttm_device.lru_lock held.
+ */
+void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
+{
+ unsigned i;
+
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
+ struct ttm_resource_manager *man;
+
+ if (!pos->first)
+ continue;
+
+ lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
+
+ man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
+ list_bulk_move_tail(&man->lru[i], &pos->first->lru,
+ &pos->last->lru);
+ }
+
+ for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
+ struct ttm_resource_manager *man;
+
+ if (!pos->first)
+ continue;
+
+ lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
+
+ man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
+ list_bulk_move_tail(&man->lru[i], &pos->first->lru,
+ &pos->last->lru);
+ }
+}
+EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
+
+/* Record a resource position in a bulk move structure */
+static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
+ struct ttm_resource *res)
+{
+ if (!pos->first)
+ pos->first = res;
+ pos->last = res;
+}
+
+/* Move a resource to the LRU tail and track the bulk position */
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
+ struct ttm_lru_bulk_move *bulk)
+{
+ struct ttm_buffer_object *bo = res->bo;
+ struct ttm_device *bdev = bo->bdev;
+ struct ttm_resource_manager *man;
+
+ lockdep_assert_held(&bo->bdev->lru_lock);
+
+ if (bo->pin_count) {
+ list_move_tail(&res->lru, &bdev->pinned);
+ if (bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(res->bo);
+ return;
+ }
+
+ man = ttm_manager_type(bdev, res->mem_type);
+ list_move_tail(&res->lru, &man->lru[bo->priority]);
+
+ if (bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(bo);
+
+ if (!bulk)
+ return;
+
+ switch (res->mem_type) {
+ case TTM_PL_TT:
+ ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
+ break;
+
+ case TTM_PL_VRAM:
+ ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
+ break;
+ }
+}
+
/**
* ttm_resource_init - resource object constructure
* @bo: buffer object this resources is allocated for
@@ -52,10 +156,12 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
res->bus.is_iomem = false;
res->bus.caching = ttm_cached;
res->bo = bo;
+ INIT_LIST_HEAD(&res->lru);
man = ttm_manager_type(bo->bdev, place->mem_type);
spin_lock(&bo->bdev->lru_lock);
man->usage += bo->base.size;
+ ttm_resource_move_to_lru_tail(res, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -66,15 +172,21 @@ EXPORT_SYMBOL(ttm_resource_init);
* @res: the resource to clean up
*
* Should be used by resource manager backends to clean up the TTM resource
- * objects before freeing the underlying structure. Counterpart of
- * &ttm_resource_init
+ * objects before freeing the underlying structure. Makes sure the resource is
+ * removed from the LRU before destruction.
+ * Counterpart of &ttm_resource_init.
*/
void ttm_resource_fini(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
- spin_lock(&man->bdev->lru_lock);
- man->usage -= res->bo->base.size;
- spin_unlock(&man->bdev->lru_lock);
+ struct ttm_device *bdev = man->bdev;
+
+ spin_lock(&bdev->lru_lock);
+ list_del_init(&res->lru);
+ if (res->bo && bdev->funcs->del_from_lru_notify)
+ bdev->funcs->del_from_lru_notify(res->bo);
+ man->usage -= res->num_pages << PAGE_SHIFT;
+ spin_unlock(&bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_fini);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c17b2df9178b..3da77fc54552 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -55,8 +55,6 @@ struct ttm_placement;
struct ttm_place;
-struct ttm_lru_bulk_move;
-
/**
* enum ttm_bo_type
*
@@ -94,7 +92,6 @@ struct ttm_tt;
* @ttm: TTM structure holding system pages.
* @evicted: Whether the object was evicted without user-space knowing.
* @deleted: True if the object is only a zombie and already deleted.
- * @lru: List head for the lru list.
* @ddestroy: List head for the delayed destroy list.
* @swap: List head for swap LRU list.
* @moving: Fence set when BO is moving
@@ -143,7 +140,6 @@ struct ttm_buffer_object {
* Members protected by the bdev::lru_lock.
*/
- struct list_head lru;
struct list_head ddestroy;
/**
@@ -295,7 +291,6 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
* ttm_bo_move_to_lru_tail
*
* @bo: The buffer object.
- * @mem: Resource object.
* @bulk: optional bulk move structure to remember BO positions
*
* Move this BO to the tail of all lru lists used to lookup and reserve an
@@ -303,19 +298,8 @@ void ttm_bo_put(struct ttm_buffer_object *bo);
* held, and is used to make a BO less likely to be considered for eviction.
*/
void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_resource *mem,
struct ttm_lru_bulk_move *bulk);
-/**
- * ttm_bo_bulk_move_lru_tail
- *
- * @bulk: bulk move structure
- *
- * Bulk move BOs to the LRU tail, only valid to use when driver makes sure that
- * BO order never changes. Should be called with ttm_global::lru_lock held.
- */
-void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk);
-
/**
* ttm_bo_lock_delayed_workqueue
*
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 5f087575194b..6c7352e13708 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -45,33 +45,6 @@
#include "ttm_tt.h"
#include "ttm_pool.h"
-/**
- * struct ttm_lru_bulk_move_pos
- *
- * @first: first BO in the bulk move range
- * @last: last BO in the bulk move range
- *
- * Positions for a lru bulk move.
- */
-struct ttm_lru_bulk_move_pos {
- struct ttm_buffer_object *first;
- struct ttm_buffer_object *last;
-};
-
-/**
- * struct ttm_lru_bulk_move
- *
- * @tt: first/last lru entry for BOs in the TT domain
- * @vram: first/last lru entry for BOs in the VRAM domain
- * @swap: first/last lru entry for BOs on the swap list
- *
- * Helper structure for bulk moves on the LRU list.
- */
-struct ttm_lru_bulk_move {
- struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
- struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
-};
-
/*
* ttm_bo.c
*/
@@ -182,7 +155,7 @@ static inline void
ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
{
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_move_to_lru_tail(bo, bo->resource, NULL);
+ ttm_bo_move_to_lru_tail(bo, NULL);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 323c14a30c6b..181e82e3d806 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -26,10 +26,12 @@
#define _TTM_RESOURCE_H_
#include <linux/types.h>
+#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/atomic.h>
#include <linux/dma-buf-map.h>
#include <linux/dma-fence.h>
+
#include <drm/drm_print.h>
#include <drm/ttm/ttm_caching.h>
#include <drm/ttm/ttm_kmap_iter.h>
@@ -179,6 +181,33 @@ struct ttm_resource {
uint32_t placement;
struct ttm_bus_placement bus;
struct ttm_buffer_object *bo;
+ struct list_head lru;
+};
+
+/**
+ * struct ttm_lru_bulk_move_pos
+ *
+ * @first: first res in the bulk move range
+ * @last: last res in the bulk move range
+ *
+ * Positions for a lru bulk move.
+ */
+struct ttm_lru_bulk_move_pos {
+ struct ttm_resource *first;
+ struct ttm_resource *last;
+};
+
+/**
+ * struct ttm_lru_bulk_move
+ *
+ * @tt: first/last lru entry for resources in the TT domain
+ * @vram: first/last lru entry for resources in the VRAM domain
+ *
+ * Helper structure for bulk moves on the LRU list.
+ */
+struct ttm_lru_bulk_move {
+ struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
+ struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
};
/**
@@ -267,6 +296,12 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
man->move = NULL;
}
+void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
+void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
+
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
+ struct ttm_lru_bulk_move *bulk);
+
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
struct ttm_resource *res);
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 04/11] drm/ttm: add resource iterator v2
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
2022-02-14 9:34 ` [PATCH 02/11] drm/ttm: add common accounting to the resource mgr v3 Christian König
2022-02-14 9:34 ` [PATCH 03/11] drm/ttm: move the LRU into resource handling v3 Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 16:09 ` Felix Kuehling
2022-02-14 9:34 ` [PATCH 05/11] drm/radeon: remove resource accounting Christian König
` (8 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
Instead of duplicating that at different places add an iterator over all
the resources in a resource manager.
v2: add lockdep annotation and kerneldoc
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/ttm/ttm_bo.c | 41 ++++++++++---------------
drivers/gpu/drm/ttm/ttm_device.c | 26 +++++++---------
drivers/gpu/drm/ttm/ttm_resource.c | 49 ++++++++++++++++++++++++++++++
include/drm/ttm/ttm_resource.h | 23 ++++++++++++++
4 files changed, 99 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index cb0fa932d495..599be3dda8a9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -579,38 +579,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
struct ww_acquire_ctx *ticket)
{
struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
+ struct ttm_resource_cursor cursor;
struct ttm_resource *res;
bool locked = false;
- unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- list_for_each_entry(res, &man->lru[i], lru) {
- bool busy;
-
- bo = res->bo;
- if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
- &locked, &busy)) {
- if (busy && !busy_bo && ticket !=
- dma_resv_locking_ctx(bo->base.resv))
- busy_bo = bo;
- continue;
- }
-
- if (!ttm_bo_get_unless_zero(bo)) {
- if (locked)
- dma_resv_unlock(bo->base.resv);
- continue;
- }
- break;
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ bool busy;
+
+ if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
+ &locked, &busy)) {
+ if (busy && !busy_bo && ticket !=
+ dma_resv_locking_ctx(bo->base.resv))
+ busy_bo = res->bo;
+ continue;
}
- /* If the inner loop terminated early, we have our candidate */
- if (&res->lru != &man->lru[i])
- break;
-
- bo = NULL;
+ if (!ttm_bo_get_unless_zero(res->bo)) {
+ if (locked)
+ dma_resv_unlock(res->bo->base.resv);
+ continue;
+ }
+ bo = res->bo;
}
if (!bo) {
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index ba35887147ba..a0562ab386f5 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout);
int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
gfp_t gfp_flags)
{
+ struct ttm_resource_cursor cursor;
struct ttm_resource_manager *man;
- struct ttm_buffer_object *bo;
struct ttm_resource *res;
- unsigned i, j;
+ unsigned i;
int ret;
spin_lock(&bdev->lru_lock);
@@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
if (!man || !man->use_tt)
continue;
- for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
- list_for_each_entry(res, &man->lru[j], lru) {
- uint32_t num_pages;
-
- bo = res->bo;
- num_pages = PFN_UP(bo->base.size);
+ ttm_resource_manager_for_each_res(man, &cursor, res) {
+ struct ttm_buffer_object *bo = res->bo;
+ uint32_t num_pages = PFN_UP(bo->base.size);
- ret = ttm_bo_swapout(bo, ctx, gfp_flags);
- /* ttm_bo_swapout has dropped the lru_lock */
- if (!ret)
- return num_pages;
- if (ret != -EBUSY)
- return ret;
- }
+ ret = ttm_bo_swapout(bo, ctx, gfp_flags);
+ /* ttm_bo_swapout has dropped the lru_lock */
+ if (!ret)
+ return num_pages;
+ if (ret != -EBUSY)
+ return ret;
}
}
spin_unlock(&bdev->lru_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 411b3f001eeb..1f9b8205b3a5 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -385,6 +385,55 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
}
EXPORT_SYMBOL(ttm_resource_manager_debug);
+/**
+ * ttm_resource_manager_first
+ *
+ * @man: resource manager to iterate over
+ * @cursor: cursor to record the position
+ *
+ * Returns the first resource from the resource manager.
+ */
+struct ttm_resource *
+ttm_resource_manager_first(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor)
+{
+ struct ttm_resource *res;
+
+ lockdep_assert_held(&man->bdev->lru_lock);
+
+ for (cursor->priority = 0; cursor->priority < TTM_MAX_BO_PRIORITY;
+ ++cursor->priority)
+ list_for_each_entry(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ return NULL;
+}
+
+/**
+ * ttm_resource_manager_next
+ *
+ * @man: resource manager to iterate over
+ * @cursor: cursor to record the position
+ *
+ * Returns the next resource from the resource manager.
+ */
+struct ttm_resource *
+ttm_resource_manager_next(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor,
+ struct ttm_resource *res)
+{
+ lockdep_assert_held(&man->bdev->lru_lock);
+
+ list_for_each_entry_continue(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ for (; cursor->priority < TTM_MAX_BO_PRIORITY; ++cursor->priority)
+ list_for_each_entry(res, &man->lru[cursor->priority], lru)
+ return res;
+
+ return NULL;
+}
+
static void ttm_kmap_iter_iomap_map_local(struct ttm_kmap_iter *iter,
struct dma_buf_map *dmap,
pgoff_t i)
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 181e82e3d806..ef0ec700e896 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -184,6 +184,17 @@ struct ttm_resource {
struct list_head lru;
};
+/**
+ * struct ttm_resource_cursor
+ *
+ * @priority: the current priority
+ *
+ * Cursor to iterate over the resources in a manager.
+ */
+struct ttm_resource_cursor {
+ unsigned int priority;
+};
+
/**
* struct ttm_lru_bulk_move_pos
*
@@ -328,6 +339,18 @@ uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
void ttm_resource_manager_debug(struct ttm_resource_manager *man,
struct drm_printer *p);
+struct ttm_resource *
+ttm_resource_manager_first(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor);
+struct ttm_resource *
+ttm_resource_manager_next(struct ttm_resource_manager *man,
+ struct ttm_resource_cursor *cursor,
+ struct ttm_resource *res);
+
+#define ttm_resource_manager_for_each_res(man, cursor, res) \
+ for (res = ttm_resource_manager_first(man, cursor); res; \
+ res = ttm_resource_manager_next(man, cursor, res))
+
struct ttm_kmap_iter *
ttm_kmap_iter_iomap_init(struct ttm_kmap_iter_iomap *iter_io,
struct io_mapping *iomap,
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 04/11] drm/ttm: add resource iterator v2
2022-02-14 9:34 ` [PATCH 04/11] drm/ttm: add resource iterator v2 Christian König
@ 2022-02-14 16:09 ` Felix Kuehling
0 siblings, 0 replies; 25+ messages in thread
From: Felix Kuehling @ 2022-02-14 16:09 UTC (permalink / raw)
To: Christian König, matthew.william.auld, daniel,
thomas.hellstrom, dri-devel
Am 2022-02-14 um 04:34 schrieb Christian König:
> Instead of duplicating that at different places add an iterator over all
> the resources in a resource manager.
>
> v2: add lockdep annotation and kerneldoc
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 41 ++++++++++---------------
> drivers/gpu/drm/ttm/ttm_device.c | 26 +++++++---------
> drivers/gpu/drm/ttm/ttm_resource.c | 49 ++++++++++++++++++++++++++++++
> include/drm/ttm/ttm_resource.h | 23 ++++++++++++++
> 4 files changed, 99 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index cb0fa932d495..599be3dda8a9 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -579,38 +579,29 @@ int ttm_mem_evict_first(struct ttm_device *bdev,
> struct ww_acquire_ctx *ticket)
> {
> struct ttm_buffer_object *bo = NULL, *busy_bo = NULL;
> + struct ttm_resource_cursor cursor;
> struct ttm_resource *res;
> bool locked = false;
> - unsigned i;
> int ret;
>
> spin_lock(&bdev->lru_lock);
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - list_for_each_entry(res, &man->lru[i], lru) {
> - bool busy;
> -
> - bo = res->bo;
> - if (!ttm_bo_evict_swapout_allowable(bo, ctx, place,
> - &locked, &busy)) {
> - if (busy && !busy_bo && ticket !=
> - dma_resv_locking_ctx(bo->base.resv))
> - busy_bo = bo;
> - continue;
> - }
> -
> - if (!ttm_bo_get_unless_zero(bo)) {
> - if (locked)
> - dma_resv_unlock(bo->base.resv);
> - continue;
> - }
> - break;
> + ttm_resource_manager_for_each_res(man, &cursor, res) {
> + bool busy;
> +
> + if (!ttm_bo_evict_swapout_allowable(res->bo, ctx, place,
> + &locked, &busy)) {
> + if (busy && !busy_bo && ticket !=
> + dma_resv_locking_ctx(bo->base.resv))
> + busy_bo = res->bo;
> + continue;
> }
>
> - /* If the inner loop terminated early, we have our candidate */
> - if (&res->lru != &man->lru[i])
> - break;
> -
> - bo = NULL;
> + if (!ttm_bo_get_unless_zero(res->bo)) {
> + if (locked)
> + dma_resv_unlock(res->bo->base.resv);
> + continue;
> + }
> + bo = res->bo;
> }
>
> if (!bo) {
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index ba35887147ba..a0562ab386f5 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -142,10 +142,10 @@ EXPORT_SYMBOL(ttm_global_swapout);
> int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
> gfp_t gfp_flags)
> {
> + struct ttm_resource_cursor cursor;
> struct ttm_resource_manager *man;
> - struct ttm_buffer_object *bo;
> struct ttm_resource *res;
> - unsigned i, j;
> + unsigned i;
> int ret;
>
> spin_lock(&bdev->lru_lock);
> @@ -154,20 +154,16 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
> if (!man || !man->use_tt)
> continue;
>
> - for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
> - list_for_each_entry(res, &man->lru[j], lru) {
> - uint32_t num_pages;
> -
> - bo = res->bo;
> - num_pages = PFN_UP(bo->base.size);
> + ttm_resource_manager_for_each_res(man, &cursor, res) {
> + struct ttm_buffer_object *bo = res->bo;
> + uint32_t num_pages = PFN_UP(bo->base.size);
>
> - ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> - /* ttm_bo_swapout has dropped the lru_lock */
> - if (!ret)
> - return num_pages;
> - if (ret != -EBUSY)
> - return ret;
> - }
> + ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> + /* ttm_bo_swapout has dropped the lru_lock */
> + if (!ret)
> + return num_pages;
> + if (ret != -EBUSY)
> + return ret;
> }
> }
> spin_unlock(&bdev->lru_lock);
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 411b3f001eeb..1f9b8205b3a5 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -385,6 +385,55 @@ void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> }
> EXPORT_SYMBOL(ttm_resource_manager_debug);
>
> +/**
> + * ttm_resource_manager_first
> + *
> + * @man: resource manager to iterate over
> + * @cursor: cursor to record the position
> + *
> + * Returns the first resource from the resource manager.
> + */
> +struct ttm_resource *
> +ttm_resource_manager_first(struct ttm_resource_manager *man,
> + struct ttm_resource_cursor *cursor)
> +{
> + struct ttm_resource *res;
> +
> + lockdep_assert_held(&man->bdev->lru_lock);
> +
> + for (cursor->priority = 0; cursor->priority < TTM_MAX_BO_PRIORITY;
> + ++cursor->priority)
> + list_for_each_entry(res, &man->lru[cursor->priority], lru)
> + return res;
> +
> + return NULL;
> +}
> +
> +/**
> + * ttm_resource_manager_next
> + *
> + * @man: resource manager to iterate over
> + * @cursor: cursor to record the position
Documentation of the "res" parameter is missing.
> + *
> + * Returns the next resource from the resource manager.
> + */
> +struct ttm_resource *
> +ttm_resource_manager_next(struct ttm_resource_manager *man,
> + struct ttm_resource_cursor *cursor,
> + struct ttm_resource *res)
> +{
> + lockdep_assert_held(&man->bdev->lru_lock);
> +
> + list_for_each_entry_continue(res, &man->lru[cursor->priority], lru)
> + return res;
> +
> + for (; cursor->priority < TTM_MAX_BO_PRIORITY; ++cursor->priority)
If you get here, the previous loop has finished iterating over the
current cursor->priority level. Therefore I think you need to increment
cursor->priority before the first loop iteration here. Otherwise you'll
just keep iterating over the same priority level over and over.
Regards,
Felix
> + list_for_each_entry(res, &man->lru[cursor->priority], lru)
> + return res;
> +
> + return NULL;
> +}
> +
> static void ttm_kmap_iter_iomap_map_local(struct ttm_kmap_iter *iter,
> struct dma_buf_map *dmap,
> pgoff_t i)
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 181e82e3d806..ef0ec700e896 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -184,6 +184,17 @@ struct ttm_resource {
> struct list_head lru;
> };
>
> +/**
> + * struct ttm_resource_cursor
> + *
> + * @priority: the current priority
> + *
> + * Cursor to iterate over the resources in a manager.
> + */
> +struct ttm_resource_cursor {
> + unsigned int priority;
> +};
> +
> /**
> * struct ttm_lru_bulk_move_pos
> *
> @@ -328,6 +339,18 @@ uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
> void ttm_resource_manager_debug(struct ttm_resource_manager *man,
> struct drm_printer *p);
>
> +struct ttm_resource *
> +ttm_resource_manager_first(struct ttm_resource_manager *man,
> + struct ttm_resource_cursor *cursor);
> +struct ttm_resource *
> +ttm_resource_manager_next(struct ttm_resource_manager *man,
> + struct ttm_resource_cursor *cursor,
> + struct ttm_resource *res);
> +
> +#define ttm_resource_manager_for_each_res(man, cursor, res) \
> + for (res = ttm_resource_manager_first(man, cursor); res; \
> + res = ttm_resource_manager_next(man, cursor, res))
> +
> struct ttm_kmap_iter *
> ttm_kmap_iter_iomap_init(struct ttm_kmap_iter_iomap *iter_io,
> struct io_mapping *iomap,
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 05/11] drm/radeon: remove resource accounting
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (2 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 04/11] drm/ttm: add resource iterator v2 Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 10:59 ` Matthew Auld
2022-02-14 9:34 ` [PATCH 06/11] drm/amdgpu: remove GTT accounting v2 Christian König
` (7 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
Use the one provided by TTM instead.
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/radeon/radeon.h | 2 --
drivers/gpu/drm/radeon/radeon_kms.c | 7 ++++--
drivers/gpu/drm/radeon/radeon_object.c | 30 +++-----------------------
drivers/gpu/drm/radeon/radeon_object.h | 1 -
drivers/gpu/drm/radeon/radeon_ttm.c | 18 ++--------------
5 files changed, 10 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 895776c421d4..08f83bf2c330 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2462,8 +2462,6 @@ struct radeon_device {
struct radeon_vm_manager vm_manager;
struct mutex gpu_clock_mutex;
/* memory stats */
- atomic64_t vram_usage;
- atomic64_t gtt_usage;
atomic64_t num_bytes_moved;
atomic_t gpu_reset_counter;
/* ACPI interface */
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 11ad210919c8..965161b8565b 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
struct drm_radeon_info *info = data;
struct radeon_mode_info *minfo = &rdev->mode_info;
uint32_t *value, value_tmp, *value_ptr, value_size;
+ struct ttm_resource_manager *man;
uint64_t value64;
struct drm_crtc *crtc;
int i, found;
@@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
case RADEON_INFO_VRAM_USAGE:
value = (uint32_t*)&value64;
value_size = sizeof(uint64_t);
- value64 = atomic64_read(&rdev->vram_usage);
+ man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
+ value64 = ttm_resource_manager_usage(man);
break;
case RADEON_INFO_GTT_USAGE:
value = (uint32_t*)&value64;
value_size = sizeof(uint64_t);
- value64 = atomic64_read(&rdev->gtt_usage);
+ man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT);
+ value64 = ttm_resource_manager_usage(man);
break;
case RADEON_INFO_ACTIVE_CU_COUNT:
if (rdev->family >= CHIP_BONAIRE)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 56ede9d63b12..c9bbed2a25ad 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo *bo);
* function are calling it.
*/
-static void radeon_update_memory_usage(struct ttm_buffer_object *bo,
- unsigned int mem_type, int sign)
-{
- struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
- switch (mem_type) {
- case TTM_PL_TT:
- if (sign > 0)
- atomic64_add(bo->base.size, &rdev->gtt_usage);
- else
- atomic64_sub(bo->base.size, &rdev->gtt_usage);
- break;
- case TTM_PL_VRAM:
- if (sign > 0)
- atomic64_add(bo->base.size, &rdev->vram_usage);
- else
- atomic64_sub(bo->base.size, &rdev->vram_usage);
- break;
- }
-}
-
static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
{
struct radeon_bo *bo;
@@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev)
static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev)
{
u64 real_vram_size = rdev->mc.real_vram_size;
- u64 vram_usage = atomic64_read(&rdev->vram_usage);
+ struct ttm_resource_manager *man =
+ ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
+ u64 vram_usage = ttm_resource_manager_usage(man);
/* This function is based on the current VRAM usage.
*
@@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
}
void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- unsigned int old_type,
struct ttm_resource *new_mem)
{
struct radeon_bo *rbo;
- radeon_update_memory_usage(bo, old_type, -1);
- if (new_mem)
- radeon_update_memory_usage(bo, new_mem->mem_type, 1);
-
if (!radeon_ttm_bo_is_radeon_bo(bo))
return;
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 1afc7992ef91..0b64e202577b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -161,7 +161,6 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
extern int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
bool force_drop);
extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
- unsigned int old_type,
struct ttm_resource *new_mem);
extern vm_fault_t radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
extern int radeon_bo_get_surface_reg(struct radeon_bo *bo);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 0d1283cdc8fb..ae09a91a486a 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -199,7 +199,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
struct ttm_resource *old_mem = bo->resource;
struct radeon_device *rdev;
struct radeon_bo *rbo;
- int r, old_type;
+ int r;
if (new_mem->mem_type == TTM_PL_TT) {
r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem);
@@ -216,9 +216,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
if (WARN_ON_ONCE(rbo->tbo.pin_count > 0))
return -EINVAL;
- /* Save old type for statistics update */
- old_type = old_mem->mem_type;
-
rdev = radeon_get_rdev(bo->bdev);
if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
ttm_bo_move_null(bo, new_mem);
@@ -264,7 +261,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
out:
/* update statistics */
atomic64_add(bo->base.size, &rdev->num_bytes_moved);
- radeon_bo_move_notify(bo, old_type, new_mem);
+ radeon_bo_move_notify(bo, new_mem);
return 0;
}
@@ -679,16 +676,6 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
}
-static void
-radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
-{
- unsigned int old_type = TTM_PL_SYSTEM;
-
- if (bo->resource)
- old_type = bo->resource->mem_type;
- radeon_bo_move_notify(bo, old_type, NULL);
-}
-
static struct ttm_device_funcs radeon_bo_driver = {
.ttm_tt_create = &radeon_ttm_tt_create,
.ttm_tt_populate = &radeon_ttm_tt_populate,
@@ -697,7 +684,6 @@ static struct ttm_device_funcs radeon_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &radeon_evict_flags,
.move = &radeon_bo_move,
- .delete_mem_notify = &radeon_bo_delete_mem_notify,
.io_mem_reserve = &radeon_ttm_io_mem_reserve,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 05/11] drm/radeon: remove resource accounting
2022-02-14 9:34 ` [PATCH 05/11] drm/radeon: remove resource accounting Christian König
@ 2022-02-14 10:59 ` Matthew Auld
0 siblings, 0 replies; 25+ messages in thread
From: Matthew Auld @ 2022-02-14 10:59 UTC (permalink / raw)
To: Christian König; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
On Mon, 14 Feb 2022 at 09:34, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Use the one provided by TTM instead.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/radeon/radeon.h | 2 --
> drivers/gpu/drm/radeon/radeon_kms.c | 7 ++++--
> drivers/gpu/drm/radeon/radeon_object.c | 30 +++-----------------------
> drivers/gpu/drm/radeon/radeon_object.h | 1 -
> drivers/gpu/drm/radeon/radeon_ttm.c | 18 ++--------------
> 5 files changed, 10 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 895776c421d4..08f83bf2c330 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2462,8 +2462,6 @@ struct radeon_device {
> struct radeon_vm_manager vm_manager;
> struct mutex gpu_clock_mutex;
> /* memory stats */
> - atomic64_t vram_usage;
> - atomic64_t gtt_usage;
> atomic64_t num_bytes_moved;
> atomic_t gpu_reset_counter;
> /* ACPI interface */
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index 11ad210919c8..965161b8565b 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -241,6 +241,7 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> struct drm_radeon_info *info = data;
> struct radeon_mode_info *minfo = &rdev->mode_info;
> uint32_t *value, value_tmp, *value_ptr, value_size;
> + struct ttm_resource_manager *man;
> uint64_t value64;
> struct drm_crtc *crtc;
> int i, found;
> @@ -550,12 +551,14 @@ int radeon_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> case RADEON_INFO_VRAM_USAGE:
> value = (uint32_t*)&value64;
> value_size = sizeof(uint64_t);
> - value64 = atomic64_read(&rdev->vram_usage);
> + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
> + value64 = ttm_resource_manager_usage(man);
> break;
> case RADEON_INFO_GTT_USAGE:
> value = (uint32_t*)&value64;
> value_size = sizeof(uint64_t);
> - value64 = atomic64_read(&rdev->gtt_usage);
> + man = ttm_manager_type(&rdev->mman.bdev, TTM_PL_TT);
> + value64 = ttm_resource_manager_usage(man);
> break;
> case RADEON_INFO_ACTIVE_CU_COUNT:
> if (rdev->family >= CHIP_BONAIRE)
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 56ede9d63b12..c9bbed2a25ad 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -49,27 +49,6 @@ static void radeon_bo_clear_surface_reg(struct radeon_bo *bo);
> * function are calling it.
> */
>
> -static void radeon_update_memory_usage(struct ttm_buffer_object *bo,
> - unsigned int mem_type, int sign)
> -{
> - struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
> -
> - switch (mem_type) {
> - case TTM_PL_TT:
> - if (sign > 0)
> - atomic64_add(bo->base.size, &rdev->gtt_usage);
> - else
> - atomic64_sub(bo->base.size, &rdev->gtt_usage);
> - break;
> - case TTM_PL_VRAM:
> - if (sign > 0)
> - atomic64_add(bo->base.size, &rdev->vram_usage);
> - else
> - atomic64_sub(bo->base.size, &rdev->vram_usage);
> - break;
> - }
> -}
> -
> static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
> {
> struct radeon_bo *bo;
> @@ -434,7 +413,9 @@ void radeon_bo_fini(struct radeon_device *rdev)
> static u64 radeon_bo_get_threshold_for_moves(struct radeon_device *rdev)
> {
> u64 real_vram_size = rdev->mc.real_vram_size;
> - u64 vram_usage = atomic64_read(&rdev->vram_usage);
> + struct ttm_resource_manager *man =
> + ttm_manager_type(&rdev->mman.bdev, TTM_PL_VRAM);
> + u64 vram_usage = ttm_resource_manager_usage(man);
>
> /* This function is based on the current VRAM usage.
> *
> @@ -725,15 +706,10 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
> }
>
> void radeon_bo_move_notify(struct ttm_buffer_object *bo,
> - unsigned int old_type,
> struct ttm_resource *new_mem)
new_mem looks to also be unused now?
> {
> struct radeon_bo *rbo;
>
> - radeon_update_memory_usage(bo, old_type, -1);
> - if (new_mem)
> - radeon_update_memory_usage(bo, new_mem->mem_type, 1);
> -
> if (!radeon_ttm_bo_is_radeon_bo(bo))
> return;
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
> index 1afc7992ef91..0b64e202577b 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -161,7 +161,6 @@ extern void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
> extern int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
> bool force_drop);
> extern void radeon_bo_move_notify(struct ttm_buffer_object *bo,
> - unsigned int old_type,
> struct ttm_resource *new_mem);
> extern vm_fault_t radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo);
> extern int radeon_bo_get_surface_reg(struct radeon_bo *bo);
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 0d1283cdc8fb..ae09a91a486a 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -199,7 +199,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
> struct ttm_resource *old_mem = bo->resource;
> struct radeon_device *rdev;
> struct radeon_bo *rbo;
> - int r, old_type;
> + int r;
>
> if (new_mem->mem_type == TTM_PL_TT) {
> r = radeon_ttm_tt_bind(bo->bdev, bo->ttm, new_mem);
> @@ -216,9 +216,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
> if (WARN_ON_ONCE(rbo->tbo.pin_count > 0))
> return -EINVAL;
>
> - /* Save old type for statistics update */
> - old_type = old_mem->mem_type;
> -
> rdev = radeon_get_rdev(bo->bdev);
> if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) {
> ttm_bo_move_null(bo, new_mem);
> @@ -264,7 +261,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
> out:
> /* update statistics */
> atomic64_add(bo->base.size, &rdev->num_bytes_moved);
> - radeon_bo_move_notify(bo, old_type, new_mem);
> + radeon_bo_move_notify(bo, new_mem);
> return 0;
> }
>
> @@ -679,16 +676,6 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
> return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
> }
>
> -static void
> -radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> -{
> - unsigned int old_type = TTM_PL_SYSTEM;
> -
> - if (bo->resource)
> - old_type = bo->resource->mem_type;
> - radeon_bo_move_notify(bo, old_type, NULL);
Can we really drop the entire move_notify() here? It looks like it
does more than just accounting?
Otherwise changes look mechanical,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> -}
> -
> static struct ttm_device_funcs radeon_bo_driver = {
> .ttm_tt_create = &radeon_ttm_tt_create,
> .ttm_tt_populate = &radeon_ttm_tt_populate,
> @@ -697,7 +684,6 @@ static struct ttm_device_funcs radeon_bo_driver = {
> .eviction_valuable = ttm_bo_eviction_valuable,
> .evict_flags = &radeon_evict_flags,
> .move = &radeon_bo_move,
> - .delete_mem_notify = &radeon_bo_delete_mem_notify,
> .io_mem_reserve = &radeon_ttm_io_mem_reserve,
> };
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (3 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 05/11] drm/radeon: remove resource accounting Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 11:10 ` Matthew Auld
2022-03-09 14:10 ` Mike Lothian
2022-02-14 9:34 ` [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting Christian König
` (6 subsequent siblings)
11 siblings, 2 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
This is provided by TTM now.
Also switch man->size to bytes instead of pages and fix the double
printing of size and usage in debugfs.
v2: fix size checking as well
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
4 files changed, 16 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index e0c7fbe01d93..3bcd27ae379d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
struct ttm_resource_manager *man;
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
- return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
+ return sysfs_emit(buf, "%llu\n", man->size);
}
/**
@@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
+ struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
- return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
@@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
struct amdgpu_gtt_node *node;
int r;
- if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
- atomic64_add_return(num_pages, &mgr->used) > man->size) {
- atomic64_sub(num_pages, &mgr->used);
- return -ENOSPC;
- }
-
node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
- if (!node) {
- r = -ENOMEM;
- goto err_out;
- }
+ if (!node)
+ return -ENOMEM;
node->tbo = tbo;
ttm_resource_init(tbo, place, &node->base.base);
+ if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
+ ttm_resource_manager_usage(man) > man->size) {
+ r = -ENOSPC;
+ goto err_free;
+ }
if (place->lpfn) {
spin_lock(&mgr->lock);
@@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
err_free:
ttm_resource_fini(man, &node->base.base);
kfree(node);
-
-err_out:
- if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
- atomic64_sub(num_pages, &mgr->used);
-
return r;
}
@@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
drm_mm_remove_node(&node->base.mm_nodes[0]);
spin_unlock(&mgr->lock);
- if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
- atomic64_sub(res->num_pages, &mgr->used);
-
ttm_resource_fini(man, res);
kfree(node);
}
-/**
- * amdgpu_gtt_mgr_usage - return usage of GTT domain
- *
- * @mgr: amdgpu_gtt_mgr pointer
- *
- * Return how many bytes are used in the GTT domain
- */
-uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
-{
- return atomic64_read(&mgr->used) * PAGE_SIZE;
-}
-
/**
* amdgpu_gtt_mgr_recover - re-init gart
*
@@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
spin_lock(&mgr->lock);
drm_mm_print(&mgr->mm, printer);
spin_unlock(&mgr->lock);
-
- drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
- man->size, atomic64_read(&mgr->used));
}
static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
@@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
man->use_tt = true;
man->func = &amdgpu_gtt_mgr_func;
- ttm_resource_manager_init(man, &adev->mman.bdev,
- gtt_size >> PAGE_SHIFT);
+ ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
drm_mm_init(&mgr->mm, start, size);
spin_lock_init(&mgr->lock);
- atomic64_set(&mgr->used, 0);
ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
ttm_resource_manager_set_used(man, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 1ebb91db2274..9ff4aced5da7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GTT_USAGE:
- ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
+ ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GDS_CONFIG: {
struct drm_amdgpu_info_gds gds_info;
@@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
case AMDGPU_INFO_MEMORY: {
struct drm_amdgpu_memory_info mem;
struct ttm_resource_manager *gtt_man =
- ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
+ &adev->mman.gtt_mgr.manager;
+
memset(&mem, 0, sizeof(mem));
mem.vram.total_heap_size = adev->gmc.real_vram_size;
mem.vram.usable_heap_size = adev->gmc.real_vram_size -
@@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
mem.gtt.total_heap_size *= PAGE_SIZE;
mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
atomic64_read(&adev->gart_pin_size);
- mem.gtt.heap_usage =
- amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
+ mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
return copy_to_user(out, &mem,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 5661b82d84d4..514754142f69 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -451,7 +451,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
if (domain & AMDGPU_GEM_DOMAIN_GTT) {
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
- if (size < (man->size << PAGE_SHIFT))
+ if (size < man->size)
return true;
else
goto fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index f8f48be16d80..120b69ec9885 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
struct ttm_resource_manager manager;
struct drm_mm mm;
spinlock_t lock;
- atomic64_t used;
};
struct amdgpu_preempt_mgr {
@@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
-uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr);
int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
2022-02-14 9:34 ` [PATCH 06/11] drm/amdgpu: remove GTT accounting v2 Christian König
@ 2022-02-14 11:10 ` Matthew Auld
2022-02-14 13:31 ` Christian König
2022-03-09 14:10 ` Mike Lothian
1 sibling, 1 reply; 25+ messages in thread
From: Matthew Auld @ 2022-02-14 11:10 UTC (permalink / raw)
To: Christian König; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
On Mon, 14 Feb 2022 at 09:34, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> This is provided by TTM now.
>
> Also switch man->size to bytes instead of pages and fix the double
> printing of size and usage in debugfs.
>
> v2: fix size checking as well
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
> 4 files changed, 16 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index e0c7fbe01d93..3bcd27ae379d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
> struct ttm_resource_manager *man;
>
> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
> + return sysfs_emit(buf, "%llu\n", man->size);
> }
>
> /**
> @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
>
> - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
> }
>
> static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
> @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> struct amdgpu_gtt_node *node;
> int r;
>
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> - atomic64_add_return(num_pages, &mgr->used) > man->size) {
> - atomic64_sub(num_pages, &mgr->used);
I guess this behaviour is now slightly different, since TEMPORARY will
now get accounted like everything else. Hopefully that is not a
concern.
Otherwise seems mechanical,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> - return -ENOSPC;
> - }
> -
> node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
> - if (!node) {
> - r = -ENOMEM;
> - goto err_out;
> - }
> + if (!node)
> + return -ENOMEM;
>
> node->tbo = tbo;
> ttm_resource_init(tbo, place, &node->base.base);
> + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> + ttm_resource_manager_usage(man) > man->size) {
> + r = -ENOSPC;
> + goto err_free;
> + }
>
> if (place->lpfn) {
> spin_lock(&mgr->lock);
> @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> err_free:
> ttm_resource_fini(man, &node->base.base);
> kfree(node);
> -
> -err_out:
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(num_pages, &mgr->used);
> -
> return r;
> }
>
> @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
> drm_mm_remove_node(&node->base.mm_nodes[0]);
> spin_unlock(&mgr->lock);
>
> - if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(res->num_pages, &mgr->used);
> -
> ttm_resource_fini(man, res);
> kfree(node);
> }
>
> -/**
> - * amdgpu_gtt_mgr_usage - return usage of GTT domain
> - *
> - * @mgr: amdgpu_gtt_mgr pointer
> - *
> - * Return how many bytes are used in the GTT domain
> - */
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
> -{
> - return atomic64_read(&mgr->used) * PAGE_SIZE;
> -}
> -
> /**
> * amdgpu_gtt_mgr_recover - re-init gart
> *
> @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
> spin_lock(&mgr->lock);
> drm_mm_print(&mgr->mm, printer);
> spin_unlock(&mgr->lock);
> -
> - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
> - man->size, atomic64_read(&mgr->used));
> }
>
> static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
> @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
> man->use_tt = true;
> man->func = &amdgpu_gtt_mgr_func;
>
> - ttm_resource_manager_init(man, &adev->mman.bdev,
> - gtt_size >> PAGE_SHIFT);
> + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
>
> start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
> drm_mm_init(&mgr->mm, start, size);
> spin_lock_init(&mgr->lock);
> - atomic64_set(&mgr->used, 0);
>
> ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
> ttm_resource_manager_set_used(man, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 1ebb91db2274..9ff4aced5da7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_GTT_USAGE:
> - ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
> + ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_GDS_CONFIG: {
> struct drm_amdgpu_info_gds gds_info;
> @@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> case AMDGPU_INFO_MEMORY: {
> struct drm_amdgpu_memory_info mem;
> struct ttm_resource_manager *gtt_man =
> - ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> + &adev->mman.gtt_mgr.manager;
> +
> memset(&mem, 0, sizeof(mem));
> mem.vram.total_heap_size = adev->gmc.real_vram_size;
> mem.vram.usable_heap_size = adev->gmc.real_vram_size -
> @@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> mem.gtt.total_heap_size *= PAGE_SIZE;
> mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
> atomic64_read(&adev->gart_pin_size);
> - mem.gtt.heap_usage =
> - amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
> + mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
> mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
>
> return copy_to_user(out, &mem,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 5661b82d84d4..514754142f69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -451,7 +451,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>
> - if (size < (man->size << PAGE_SHIFT))
> + if (size < man->size)
> return true;
> else
> goto fail;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index f8f48be16d80..120b69ec9885 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
> struct ttm_resource_manager manager;
> struct drm_mm mm;
> spinlock_t lock;
> - atomic64_t used;
> };
>
> struct amdgpu_preempt_mgr {
> @@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
> void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
>
> bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr);
> int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
>
> uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
2022-02-14 11:10 ` Matthew Auld
@ 2022-02-14 13:31 ` Christian König
0 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 13:31 UTC (permalink / raw)
To: Matthew Auld; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
Am 14.02.22 um 12:10 schrieb Matthew Auld:
> On Mon, 14 Feb 2022 at 09:34, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> This is provided by TTM now.
>>
>> Also switch man->size to bytes instead of pages and fix the double
>> printing of size and usage in debugfs.
>>
>> v2: fix size checking as well
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
>> 4 files changed, 16 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> index e0c7fbe01d93..3bcd27ae379d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
>> struct ttm_resource_manager *man;
>>
>> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>> - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
>> + return sysfs_emit(buf, "%llu\n", man->size);
>> }
>>
>> /**
>> @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
>> {
>> struct drm_device *ddev = dev_get_drvdata(dev);
>> struct amdgpu_device *adev = drm_to_adev(ddev);
>> + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
>>
>> - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
>> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
>> }
>>
>> static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
>> @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
>> struct amdgpu_gtt_node *node;
>> int r;
>>
>> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
>> - atomic64_add_return(num_pages, &mgr->used) > man->size) {
>> - atomic64_sub(num_pages, &mgr->used);
> I guess this behaviour is now slightly different, since TEMPORARY will
> now get accounted like everything else. Hopefully that is not a
> concern.
Yeah, that's correct but also unproblematic. See a few lines below.
>
> Otherwise seems mechanical,
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>
>
>> - return -ENOSPC;
>> - }
>> -
>> node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
>> - if (!node) {
>> - r = -ENOMEM;
>> - goto err_out;
>> - }
>> + if (!node)
>> + return -ENOMEM;
>>
>> node->tbo = tbo;
>> ttm_resource_init(tbo, place, &node->base.base);
>> + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
>> + ttm_resource_manager_usage(man) > man->size) {
>> + r = -ENOSPC;
>> + goto err_free;
>> + }
This here now takes care of TTM_PL_FLAG_TEMPORARY.
Regards,
Christian.
>>
>> if (place->lpfn) {
>> spin_lock(&mgr->lock);
>> @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
>> err_free:
>> ttm_resource_fini(man, &node->base.base);
>> kfree(node);
>> -
>> -err_out:
>> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
>> - atomic64_sub(num_pages, &mgr->used);
>> -
>> return r;
>> }
>>
>> @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
>> drm_mm_remove_node(&node->base.mm_nodes[0]);
>> spin_unlock(&mgr->lock);
>>
>> - if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
>> - atomic64_sub(res->num_pages, &mgr->used);
>> -
>> ttm_resource_fini(man, res);
>> kfree(node);
>> }
>>
>> -/**
>> - * amdgpu_gtt_mgr_usage - return usage of GTT domain
>> - *
>> - * @mgr: amdgpu_gtt_mgr pointer
>> - *
>> - * Return how many bytes are used in the GTT domain
>> - */
>> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
>> -{
>> - return atomic64_read(&mgr->used) * PAGE_SIZE;
>> -}
>> -
>> /**
>> * amdgpu_gtt_mgr_recover - re-init gart
>> *
>> @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
>> spin_lock(&mgr->lock);
>> drm_mm_print(&mgr->mm, printer);
>> spin_unlock(&mgr->lock);
>> -
>> - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
>> - man->size, atomic64_read(&mgr->used));
>> }
>>
>> static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
>> @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
>> man->use_tt = true;
>> man->func = &amdgpu_gtt_mgr_func;
>>
>> - ttm_resource_manager_init(man, &adev->mman.bdev,
>> - gtt_size >> PAGE_SHIFT);
>> + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
>>
>> start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
>> size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
>> drm_mm_init(&mgr->mm, start, size);
>> spin_lock_init(&mgr->lock);
>> - atomic64_set(&mgr->used, 0);
>>
>> ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
>> ttm_resource_manager_set_used(man, true);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index 1ebb91db2274..9ff4aced5da7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
>> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
>> case AMDGPU_INFO_GTT_USAGE:
>> - ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
>> + ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
>> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
>> case AMDGPU_INFO_GDS_CONFIG: {
>> struct drm_amdgpu_info_gds gds_info;
>> @@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> case AMDGPU_INFO_MEMORY: {
>> struct drm_amdgpu_memory_info mem;
>> struct ttm_resource_manager *gtt_man =
>> - ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>> + &adev->mman.gtt_mgr.manager;
>> +
>> memset(&mem, 0, sizeof(mem));
>> mem.vram.total_heap_size = adev->gmc.real_vram_size;
>> mem.vram.usable_heap_size = adev->gmc.real_vram_size -
>> @@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> mem.gtt.total_heap_size *= PAGE_SIZE;
>> mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
>> atomic64_read(&adev->gart_pin_size);
>> - mem.gtt.heap_usage =
>> - amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
>> + mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
>> mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
>>
>> return copy_to_user(out, &mem,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 5661b82d84d4..514754142f69 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -451,7 +451,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>>
>> - if (size < (man->size << PAGE_SHIFT))
>> + if (size < man->size)
>> return true;
>> else
>> goto fail;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index f8f48be16d80..120b69ec9885 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
>> struct ttm_resource_manager manager;
>> struct drm_mm mm;
>> spinlock_t lock;
>> - atomic64_t used;
>> };
>>
>> struct amdgpu_preempt_mgr {
>> @@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
>> void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
>>
>> bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
>> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr);
>> int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
>>
>> uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/11] drm/amdgpu: remove GTT accounting v2
2022-02-14 9:34 ` [PATCH 06/11] drm/amdgpu: remove GTT accounting v2 Christian König
2022-02-14 11:10 ` Matthew Auld
@ 2022-03-09 14:10 ` Mike Lothian
1 sibling, 0 replies; 25+ messages in thread
From: Mike Lothian @ 2022-03-09 14:10 UTC (permalink / raw)
To: Christian König
Cc: thomas.hellstrom, Felix.Kuehling, matthew.william.auld, dri-devel
Hi
This patch seems to be causing me problems
https://gitlab.freedesktop.org/drm/amd/-/issues/1927
There are 3 issues I'm experiencing, two kernel bugs and a mesa bug
Cheers
Mike
On Mon, 14 Feb 2022 at 09:34, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> This is provided by TTM now.
>
> Also switch man->size to bytes instead of pages and fix the double
> printing of size and usage in debugfs.
>
> v2: fix size checking as well
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 +++++----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
> 4 files changed, 16 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index e0c7fbe01d93..3bcd27ae379d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -60,7 +60,7 @@ static ssize_t amdgpu_mem_info_gtt_total_show(struct device *dev,
> struct ttm_resource_manager *man;
>
> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> - return sysfs_emit(buf, "%llu\n", man->size * PAGE_SIZE);
> + return sysfs_emit(buf, "%llu\n", man->size);
> }
>
> /**
> @@ -77,8 +77,9 @@ static ssize_t amdgpu_mem_info_gtt_used_show(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct ttm_resource_manager *man = &adev->mman.gtt_mgr.manager;
>
> - return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr));
> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
> }
>
> static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
> @@ -130,20 +131,17 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> struct amdgpu_gtt_node *node;
> int r;
>
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> - atomic64_add_return(num_pages, &mgr->used) > man->size) {
> - atomic64_sub(num_pages, &mgr->used);
> - return -ENOSPC;
> - }
> -
> node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
> - if (!node) {
> - r = -ENOMEM;
> - goto err_out;
> - }
> + if (!node)
> + return -ENOMEM;
>
> node->tbo = tbo;
> ttm_resource_init(tbo, place, &node->base.base);
> + if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
> + ttm_resource_manager_usage(man) > man->size) {
> + r = -ENOSPC;
> + goto err_free;
> + }
>
> if (place->lpfn) {
> spin_lock(&mgr->lock);
> @@ -169,11 +167,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> err_free:
> ttm_resource_fini(man, &node->base.base);
> kfree(node);
> -
> -err_out:
> - if (!(place->flags & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(num_pages, &mgr->used);
> -
> return r;
> }
>
> @@ -196,25 +189,10 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
> drm_mm_remove_node(&node->base.mm_nodes[0]);
> spin_unlock(&mgr->lock);
>
> - if (!(res->placement & TTM_PL_FLAG_TEMPORARY))
> - atomic64_sub(res->num_pages, &mgr->used);
> -
> ttm_resource_fini(man, res);
> kfree(node);
> }
>
> -/**
> - * amdgpu_gtt_mgr_usage - return usage of GTT domain
> - *
> - * @mgr: amdgpu_gtt_mgr pointer
> - *
> - * Return how many bytes are used in the GTT domain
> - */
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr)
> -{
> - return atomic64_read(&mgr->used) * PAGE_SIZE;
> -}
> -
> /**
> * amdgpu_gtt_mgr_recover - re-init gart
> *
> @@ -260,9 +238,6 @@ static void amdgpu_gtt_mgr_debug(struct ttm_resource_manager *man,
> spin_lock(&mgr->lock);
> drm_mm_print(&mgr->mm, printer);
> spin_unlock(&mgr->lock);
> -
> - drm_printf(printer, "man size:%llu pages, gtt used:%llu pages\n",
> - man->size, atomic64_read(&mgr->used));
> }
>
> static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func = {
> @@ -288,14 +263,12 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
> man->use_tt = true;
> man->func = &amdgpu_gtt_mgr_func;
>
> - ttm_resource_manager_init(man, &adev->mman.bdev,
> - gtt_size >> PAGE_SHIFT);
> + ttm_resource_manager_init(man, &adev->mman.bdev, gtt_size);
>
> start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
> size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
> drm_mm_init(&mgr->mm, start, size);
> spin_lock_init(&mgr->lock);
> - atomic64_set(&mgr->used, 0);
>
> ttm_set_driver_manager(&adev->mman.bdev, TTM_PL_TT, &mgr->manager);
> ttm_resource_manager_set_used(man, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 1ebb91db2274..9ff4aced5da7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -684,7 +684,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_GTT_USAGE:
> - ui64 = amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
> + ui64 = ttm_resource_manager_usage(&adev->mman.gtt_mgr.manager);
> return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
> case AMDGPU_INFO_GDS_CONFIG: {
> struct drm_amdgpu_info_gds gds_info;
> @@ -716,7 +716,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> case AMDGPU_INFO_MEMORY: {
> struct drm_amdgpu_memory_info mem;
> struct ttm_resource_manager *gtt_man =
> - ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> + &adev->mman.gtt_mgr.manager;
> +
> memset(&mem, 0, sizeof(mem));
> mem.vram.total_heap_size = adev->gmc.real_vram_size;
> mem.vram.usable_heap_size = adev->gmc.real_vram_size -
> @@ -741,8 +742,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> mem.gtt.total_heap_size *= PAGE_SIZE;
> mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
> atomic64_read(&adev->gart_pin_size);
> - mem.gtt.heap_usage =
> - amdgpu_gtt_mgr_usage(&adev->mman.gtt_mgr);
> + mem.gtt.heap_usage = ttm_resource_manager_usage(gtt_man);
> mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
>
> return copy_to_user(out, &mem,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 5661b82d84d4..514754142f69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -451,7 +451,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
>
> - if (size < (man->size << PAGE_SHIFT))
> + if (size < man->size)
> return true;
> else
> goto fail;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index f8f48be16d80..120b69ec9885 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -52,7 +52,6 @@ struct amdgpu_gtt_mgr {
> struct ttm_resource_manager manager;
> struct drm_mm mm;
> spinlock_t lock;
> - atomic64_t used;
> };
>
> struct amdgpu_preempt_mgr {
> @@ -114,7 +113,6 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev);
> void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
>
> bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
> -uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_gtt_mgr *mgr);
> int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
>
> uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (4 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 06/11] drm/amdgpu: remove GTT accounting v2 Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 11:14 ` Matthew Auld
2022-02-14 16:14 ` Felix Kuehling
2022-02-14 9:34 ` [PATCH 08/11] drm/amdgpu: remove VRAM accounting v2 Christian König
` (5 subsequent siblings)
11 siblings, 2 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
This is provided by TTM now.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 62 ++-----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +--
2 files changed, 6 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
index 0d85c2096ab5..e8adfd0a570a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
@@ -25,12 +25,6 @@
#include "amdgpu.h"
-static inline struct amdgpu_preempt_mgr *
-to_preempt_mgr(struct ttm_resource_manager *man)
-{
- return container_of(man, struct amdgpu_preempt_mgr, manager);
-}
-
/**
* DOC: mem_info_preempt_used
*
@@ -45,10 +39,9 @@ static ssize_t mem_info_preempt_used_show(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
- struct ttm_resource_manager *man;
+ struct ttm_resource_manager *man = &adev->mman.preempt_mgr;
- man = ttm_manager_type(&adev->mman.bdev, AMDGPU_PL_PREEMPT);
- return sysfs_emit(buf, "%llu\n", amdgpu_preempt_mgr_usage(man));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
static DEVICE_ATTR_RO(mem_info_preempt_used);
@@ -68,16 +61,12 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man,
const struct ttm_place *place,
struct ttm_resource **res)
{
- struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
-
*res = kzalloc(sizeof(**res), GFP_KERNEL);
if (!*res)
return -ENOMEM;
ttm_resource_init(tbo, place, *res);
(*res)->start = AMDGPU_BO_INVALID_OFFSET;
-
- atomic64_add((*res)->num_pages, &mgr->used);
return 0;
}
@@ -92,49 +81,13 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man,
static void amdgpu_preempt_mgr_del(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
- struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
-
- atomic64_sub(res->num_pages, &mgr->used);
ttm_resource_fini(man, res);
kfree(res);
}
-/**
- * amdgpu_preempt_mgr_usage - return usage of PREEMPT domain
- *
- * @man: TTM memory type manager
- *
- * Return how many bytes are used in the GTT domain
- */
-uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man)
-{
- struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
- s64 result = atomic64_read(&mgr->used);
-
- return (result > 0 ? result : 0) * PAGE_SIZE;
-}
-
-/**
- * amdgpu_preempt_mgr_debug - dump VRAM table
- *
- * @man: TTM memory type manager
- * @printer: DRM printer to use
- *
- * Dump the table content using printk.
- */
-static void amdgpu_preempt_mgr_debug(struct ttm_resource_manager *man,
- struct drm_printer *printer)
-{
- struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
-
- drm_printf(printer, "man size:%llu pages, preempt used:%lld pages\n",
- man->size, (u64)atomic64_read(&mgr->used));
-}
-
static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = {
.alloc = amdgpu_preempt_mgr_new,
.free = amdgpu_preempt_mgr_del,
- .debug = amdgpu_preempt_mgr_debug
};
/**
@@ -146,8 +99,7 @@ static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = {
*/
int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
{
- struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr;
- struct ttm_resource_manager *man = &mgr->manager;
+ struct ttm_resource_manager *man = &adev->mman.preempt_mgr;
int ret;
man->use_tt = true;
@@ -155,16 +107,13 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
ttm_resource_manager_init(man, &adev->mman.bdev, (1 << 30));
- atomic64_set(&mgr->used, 0);
-
ret = device_create_file(adev->dev, &dev_attr_mem_info_preempt_used);
if (ret) {
DRM_ERROR("Failed to create device file mem_info_preempt_used\n");
return ret;
}
- ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT,
- &mgr->manager);
+ ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT, man);
ttm_resource_manager_set_used(man, true);
return 0;
}
@@ -179,8 +128,7 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
*/
void amdgpu_preempt_mgr_fini(struct amdgpu_device *adev)
{
- struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr;
- struct ttm_resource_manager *man = &mgr->manager;
+ struct ttm_resource_manager *man = &adev->mman.preempt_mgr;
int ret;
ttm_resource_manager_set_used(man, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 120b69ec9885..4e8577dad16a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -54,11 +54,6 @@ struct amdgpu_gtt_mgr {
spinlock_t lock;
};
-struct amdgpu_preempt_mgr {
- struct ttm_resource_manager manager;
- atomic64_t used;
-};
-
struct amdgpu_mman {
struct ttm_device bdev;
bool initialized;
@@ -75,7 +70,7 @@ struct amdgpu_mman {
struct amdgpu_vram_mgr vram_mgr;
struct amdgpu_gtt_mgr gtt_mgr;
- struct amdgpu_preempt_mgr preempt_mgr;
+ struct ttm_resource_manager preempt_mgr;
uint64_t stolen_vga_size;
struct amdgpu_bo *stolen_vga_memory;
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting
2022-02-14 9:34 ` [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting Christian König
@ 2022-02-14 11:14 ` Matthew Auld
2022-02-14 16:14 ` Felix Kuehling
1 sibling, 0 replies; 25+ messages in thread
From: Matthew Auld @ 2022-02-14 11:14 UTC (permalink / raw)
To: Christian König; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
On Mon, 14 Feb 2022 at 09:34, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> This is provided by TTM now.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting
2022-02-14 9:34 ` [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting Christian König
2022-02-14 11:14 ` Matthew Auld
@ 2022-02-14 16:14 ` Felix Kuehling
1 sibling, 0 replies; 25+ messages in thread
From: Felix Kuehling @ 2022-02-14 16:14 UTC (permalink / raw)
To: Christian König, matthew.william.auld, daniel,
thomas.hellstrom, dri-devel
Am 2022-02-14 um 04:34 schrieb Christian König:
> This is provided by TTM now.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
This patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c | 62 ++-----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 +--
> 2 files changed, 6 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
> index 0d85c2096ab5..e8adfd0a570a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_preempt_mgr.c
> @@ -25,12 +25,6 @@
>
> #include "amdgpu.h"
>
> -static inline struct amdgpu_preempt_mgr *
> -to_preempt_mgr(struct ttm_resource_manager *man)
> -{
> - return container_of(man, struct amdgpu_preempt_mgr, manager);
> -}
> -
> /**
> * DOC: mem_info_preempt_used
> *
> @@ -45,10 +39,9 @@ static ssize_t mem_info_preempt_used_show(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - struct ttm_resource_manager *man;
> + struct ttm_resource_manager *man = &adev->mman.preempt_mgr;
>
> - man = ttm_manager_type(&adev->mman.bdev, AMDGPU_PL_PREEMPT);
> - return sysfs_emit(buf, "%llu\n", amdgpu_preempt_mgr_usage(man));
> + return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
> }
>
> static DEVICE_ATTR_RO(mem_info_preempt_used);
> @@ -68,16 +61,12 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man,
> const struct ttm_place *place,
> struct ttm_resource **res)
> {
> - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
> -
> *res = kzalloc(sizeof(**res), GFP_KERNEL);
> if (!*res)
> return -ENOMEM;
>
> ttm_resource_init(tbo, place, *res);
> (*res)->start = AMDGPU_BO_INVALID_OFFSET;
> -
> - atomic64_add((*res)->num_pages, &mgr->used);
> return 0;
> }
>
> @@ -92,49 +81,13 @@ static int amdgpu_preempt_mgr_new(struct ttm_resource_manager *man,
> static void amdgpu_preempt_mgr_del(struct ttm_resource_manager *man,
> struct ttm_resource *res)
> {
> - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
> -
> - atomic64_sub(res->num_pages, &mgr->used);
> ttm_resource_fini(man, res);
> kfree(res);
> }
>
> -/**
> - * amdgpu_preempt_mgr_usage - return usage of PREEMPT domain
> - *
> - * @man: TTM memory type manager
> - *
> - * Return how many bytes are used in the GTT domain
> - */
> -uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager *man)
> -{
> - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
> - s64 result = atomic64_read(&mgr->used);
> -
> - return (result > 0 ? result : 0) * PAGE_SIZE;
> -}
> -
> -/**
> - * amdgpu_preempt_mgr_debug - dump VRAM table
> - *
> - * @man: TTM memory type manager
> - * @printer: DRM printer to use
> - *
> - * Dump the table content using printk.
> - */
> -static void amdgpu_preempt_mgr_debug(struct ttm_resource_manager *man,
> - struct drm_printer *printer)
> -{
> - struct amdgpu_preempt_mgr *mgr = to_preempt_mgr(man);
> -
> - drm_printf(printer, "man size:%llu pages, preempt used:%lld pages\n",
> - man->size, (u64)atomic64_read(&mgr->used));
> -}
> -
> static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = {
> .alloc = amdgpu_preempt_mgr_new,
> .free = amdgpu_preempt_mgr_del,
> - .debug = amdgpu_preempt_mgr_debug
> };
>
> /**
> @@ -146,8 +99,7 @@ static const struct ttm_resource_manager_func amdgpu_preempt_mgr_func = {
> */
> int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
> {
> - struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr;
> - struct ttm_resource_manager *man = &mgr->manager;
> + struct ttm_resource_manager *man = &adev->mman.preempt_mgr;
> int ret;
>
> man->use_tt = true;
> @@ -155,16 +107,13 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
>
> ttm_resource_manager_init(man, &adev->mman.bdev, (1 << 30));
>
> - atomic64_set(&mgr->used, 0);
> -
> ret = device_create_file(adev->dev, &dev_attr_mem_info_preempt_used);
> if (ret) {
> DRM_ERROR("Failed to create device file mem_info_preempt_used\n");
> return ret;
> }
>
> - ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT,
> - &mgr->manager);
> + ttm_set_driver_manager(&adev->mman.bdev, AMDGPU_PL_PREEMPT, man);
> ttm_resource_manager_set_used(man, true);
> return 0;
> }
> @@ -179,8 +128,7 @@ int amdgpu_preempt_mgr_init(struct amdgpu_device *adev)
> */
> void amdgpu_preempt_mgr_fini(struct amdgpu_device *adev)
> {
> - struct amdgpu_preempt_mgr *mgr = &adev->mman.preempt_mgr;
> - struct ttm_resource_manager *man = &mgr->manager;
> + struct ttm_resource_manager *man = &adev->mman.preempt_mgr;
> int ret;
>
> ttm_resource_manager_set_used(man, false);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 120b69ec9885..4e8577dad16a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -54,11 +54,6 @@ struct amdgpu_gtt_mgr {
> spinlock_t lock;
> };
>
> -struct amdgpu_preempt_mgr {
> - struct ttm_resource_manager manager;
> - atomic64_t used;
> -};
> -
> struct amdgpu_mman {
> struct ttm_device bdev;
> bool initialized;
> @@ -75,7 +70,7 @@ struct amdgpu_mman {
>
> struct amdgpu_vram_mgr vram_mgr;
> struct amdgpu_gtt_mgr gtt_mgr;
> - struct amdgpu_preempt_mgr preempt_mgr;
> + struct ttm_resource_manager preempt_mgr;
>
> uint64_t stolen_vga_size;
> struct amdgpu_bo *stolen_vga_memory;
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 08/11] drm/amdgpu: remove VRAM accounting v2
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (5 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 07/11] drm/amdgpu: remove PL_PREEMPT accounting Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 11:20 ` Matthew Auld
2022-02-14 9:34 ` [PATCH 09/11] drm/amdgpu: drop amdgpu_gtt_node Christian König
` (4 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
This is provided by TTM now.
Also switch man->size to bytes instead of pages and fix the double
printing of size and usage in debugfs.
v2: fix size checking as well
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 2 -
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 58 +++++++-------------
7 files changed, 32 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index e8440d306496..025748e9c772 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -314,7 +314,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
}
total_vram = adev->gmc.real_vram_size - atomic64_read(&adev->vram_pin_size);
- used_vram = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
+ used_vram = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram;
spin_lock(&adev->mm_stats.lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 9ff4aced5da7..0beab961b18b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
ui64 = atomic64_read(&adev->num_vram_cpu_page_faults);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VRAM_USAGE:
- ui64 = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
+ ui64 = ttm_resource_manager_usage(&adev->mman.vram_mgr.manager);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VIS_VRAM_USAGE:
ui64 = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr);
@@ -717,6 +717,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
struct drm_amdgpu_memory_info mem;
struct ttm_resource_manager *gtt_man =
&adev->mman.gtt_mgr.manager;
+ struct ttm_resource_manager *vram_man =
+ &adev->mman.vram_mgr.manager;
memset(&mem, 0, sizeof(mem));
mem.vram.total_heap_size = adev->gmc.real_vram_size;
@@ -724,7 +726,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
atomic64_read(&adev->vram_pin_size) -
AMDGPU_VM_RESERVED_VRAM;
mem.vram.heap_usage =
- amdgpu_vram_mgr_usage(&adev->mman.vram_mgr);
+ ttm_resource_manager_usage(vram_man);
mem.vram.max_allocation = mem.vram.usable_heap_size * 3 / 4;
mem.cpu_accessible_vram.total_heap_size =
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 514754142f69..ea0cde4904f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -460,7 +460,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
if (domain & AMDGPU_GEM_DOMAIN_VRAM) {
man = ttm_manager_type(&adev->mman.bdev, TTM_PL_VRAM);
- if (size < (man->size << PAGE_SHIFT))
+ if (size < man->size)
return true;
else
goto fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d178fbec7048..5859ed0552a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1884,7 +1884,7 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
size = adev->gmc.real_vram_size;
else
size = adev->gmc.visible_vram_size;
- man->size = size >> PAGE_SHIFT;
+ man->size = size;
adev->mman.buffer_funcs_enabled = enable;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 4e8577dad16a..58c64871c94a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -44,7 +44,6 @@ struct amdgpu_vram_mgr {
spinlock_t lock;
struct list_head reservations_pending;
struct list_head reserved_pages;
- atomic64_t usage;
atomic64_t vis_usage;
};
@@ -122,7 +121,6 @@ int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
void amdgpu_vram_mgr_free_sgt(struct device *dev,
enum dma_data_direction dir,
struct sg_table *sgt);
-uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr);
uint64_t amdgpu_vram_mgr_vis_usage(struct amdgpu_vram_mgr *mgr);
int amdgpu_vram_mgr_reserve_range(struct amdgpu_vram_mgr *mgr,
uint64_t start, uint64_t size);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 07bc0f504713..3a25dd220786 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -575,8 +575,10 @@ static int amdgpu_virt_write_vf2pf_data(struct amdgpu_device *adev)
vf2pf_info->driver_cert = 0;
vf2pf_info->os_info.all = 0;
- vf2pf_info->fb_usage = amdgpu_vram_mgr_usage(&adev->mman.vram_mgr) >> 20;
- vf2pf_info->fb_vis_usage = amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
+ vf2pf_info->fb_usage =
+ ttm_resource_manager_usage(&adev->mman.vram_mgr.manager) >> 20;
+ vf2pf_info->fb_vis_usage =
+ amdgpu_vram_mgr_vis_usage(&adev->mman.vram_mgr) >> 20;
vf2pf_info->fb_size = adev->gmc.real_vram_size >> 20;
vf2pf_info->fb_vis_size = adev->gmc.visible_vram_size >> 20;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 7442095f089c..e50fe25fbcb8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -96,9 +96,9 @@ static ssize_t amdgpu_mem_info_vram_used_show(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
+ struct ttm_resource_manager *man = &adev->mman.vram_mgr.manager;
- return sysfs_emit(buf, "%llu\n",
- amdgpu_vram_mgr_usage(&adev->mman.vram_mgr));
+ return sysfs_emit(buf, "%llu\n", ttm_resource_manager_usage(man));
}
/**
@@ -253,7 +253,9 @@ static void amdgpu_vram_mgr_do_reserve(struct ttm_resource_manager *man)
vis_usage = amdgpu_vram_mgr_vis_size(adev, &rsv->mm_node);
atomic64_add(vis_usage, &mgr->vis_usage);
- atomic64_add(rsv->mm_node.size << PAGE_SHIFT, &mgr->usage);
+ spin_lock(&man->bdev->lru_lock);
+ man->usage += rsv->mm_node.size << PAGE_SHIFT;
+ spin_unlock(&man->bdev->lru_lock);
list_move(&rsv->node, &mgr->reserved_pages);
}
}
@@ -378,19 +380,13 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
lpfn = place->lpfn;
if (!lpfn)
- lpfn = man->size;
+ lpfn = man->size >> PAGE_SHIFT;
max_bytes = adev->gmc.mc_vram_size;
if (tbo->type != ttm_bo_type_kernel)
max_bytes -= AMDGPU_VM_RESERVED_VRAM;
- /* bail out quickly if there's likely not enough VRAM for this BO */
mem_bytes = tbo->base.size;
- if (atomic64_add_return(mem_bytes, &mgr->usage) > max_bytes) {
- r = -ENOSPC;
- goto error_sub;
- }
-
if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
pages_per_node = ~0ul;
num_nodes = 1;
@@ -408,13 +404,17 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
node = kvmalloc(struct_size(node, mm_nodes, num_nodes),
GFP_KERNEL | __GFP_ZERO);
- if (!node) {
- r = -ENOMEM;
- goto error_sub;
- }
+ if (!node)
+ return -ENOMEM;
ttm_resource_init(tbo, place, &node->base);
+ /* bail out quickly if there's likely not enough VRAM for this BO */
+ if (ttm_resource_manager_usage(man) > max_bytes) {
+ r = -ENOSPC;
+ goto error_fini;
+ }
+
mode = DRM_MM_INSERT_BEST;
if (place->flags & TTM_PL_FLAG_TOPDOWN)
mode = DRM_MM_INSERT_HIGH;
@@ -472,11 +472,10 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
while (i--)
drm_mm_remove_node(&node->mm_nodes[i]);
spin_unlock(&mgr->lock);
+error_fini:
ttm_resource_fini(man, &node->base);
kvfree(node);
-error_sub:
- atomic64_sub(mem_bytes, &mgr->usage);
return r;
}
@@ -494,7 +493,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
struct amdgpu_device *adev = to_amdgpu_device(mgr);
- uint64_t usage = 0, vis_usage = 0;
+ uint64_t vis_usage = 0;
unsigned i, pages;
spin_lock(&mgr->lock);
@@ -503,13 +502,11 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager *man,
struct drm_mm_node *mm = &node->mm_nodes[i];
drm_mm_remove_node(mm);
- usage += mm->size << PAGE_SHIFT;
vis_usage += amdgpu_vram_mgr_vis_size(adev, mm);
}
amdgpu_vram_mgr_do_reserve(man);
spin_unlock(&mgr->lock);
- atomic64_sub(usage, &mgr->usage);
atomic64_sub(vis_usage, &mgr->vis_usage);
ttm_resource_fini(man, res);
@@ -627,18 +624,6 @@ void amdgpu_vram_mgr_free_sgt(struct device *dev,
kfree(sgt);
}
-/**
- * amdgpu_vram_mgr_usage - how many bytes are used in this domain
- *
- * @mgr: amdgpu_vram_mgr pointer
- *
- * Returns how many bytes are used in this domain.
- */
-uint64_t amdgpu_vram_mgr_usage(struct amdgpu_vram_mgr *mgr)
-{
- return atomic64_read(&mgr->usage);
-}
-
/**
* amdgpu_vram_mgr_vis_usage - how many bytes are used in the visible part
*
@@ -664,13 +649,12 @@ static void amdgpu_vram_mgr_debug(struct ttm_resource_manager *man,
{
struct amdgpu_vram_mgr *mgr = to_vram_mgr(man);
+ drm_printf(printer, " vis usage:%llu\n",
+ amdgpu_vram_mgr_vis_usage(mgr));
+
spin_lock(&mgr->lock);
drm_mm_print(&mgr->mm, printer);
spin_unlock(&mgr->lock);
-
- drm_printf(printer, "man size:%llu pages, ram usage:%lluMB, vis usage:%lluMB\n",
- man->size, amdgpu_vram_mgr_usage(mgr) >> 20,
- amdgpu_vram_mgr_vis_usage(mgr) >> 20);
}
static const struct ttm_resource_manager_func amdgpu_vram_mgr_func = {
@@ -692,11 +676,11 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
struct ttm_resource_manager *man = &mgr->manager;
ttm_resource_manager_init(man, &adev->mman.bdev,
- adev->gmc.real_vram_size >> PAGE_SHIFT);
+ adev->gmc.real_vram_size);
man->func = &amdgpu_vram_mgr_func;
- drm_mm_init(&mgr->mm, 0, man->size);
+ drm_mm_init(&mgr->mm, 0, man->size >> PAGE_SHIFT);
spin_lock_init(&mgr->lock);
INIT_LIST_HEAD(&mgr->reservations_pending);
INIT_LIST_HEAD(&mgr->reserved_pages);
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/11] drm/amdgpu: drop amdgpu_gtt_node
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (6 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 08/11] drm/amdgpu: remove VRAM accounting v2 Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 9:34 ` [PATCH 10/11] drm/ttm: allow bulk moves for all domains Christian König
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
We have the BO pointer in the base structure now as well.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 49 ++++++++-------------
include/drm/ttm/ttm_resource.h | 8 ++++
2 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 3bcd27ae379d..68494b959116 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -26,23 +26,12 @@
#include "amdgpu.h"
-struct amdgpu_gtt_node {
- struct ttm_buffer_object *tbo;
- struct ttm_range_mgr_node base;
-};
-
static inline struct amdgpu_gtt_mgr *
to_gtt_mgr(struct ttm_resource_manager *man)
{
return container_of(man, struct amdgpu_gtt_mgr, manager);
}
-static inline struct amdgpu_gtt_node *
-to_amdgpu_gtt_node(struct ttm_resource *res)
-{
- return container_of(res, struct amdgpu_gtt_node, base.base);
-}
-
/**
* DOC: mem_info_gtt_total
*
@@ -106,9 +95,9 @@ const struct attribute_group amdgpu_gtt_mgr_attr_group = {
*/
bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *res)
{
- struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
+ struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
- return drm_mm_node_allocated(&node->base.mm_nodes[0]);
+ return drm_mm_node_allocated(&node->mm_nodes[0]);
}
/**
@@ -128,15 +117,14 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
{
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
uint32_t num_pages = PFN_UP(tbo->base.size);
- struct amdgpu_gtt_node *node;
+ struct ttm_range_mgr_node *node;
int r;
- node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
+ node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL);
if (!node)
return -ENOMEM;
- node->tbo = tbo;
- ttm_resource_init(tbo, place, &node->base.base);
+ ttm_resource_init(tbo, place, &node->base);
if (!(place->flags & TTM_PL_FLAG_TEMPORARY) &&
ttm_resource_manager_usage(man) > man->size) {
r = -ENOSPC;
@@ -145,8 +133,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (place->lpfn) {
spin_lock(&mgr->lock);
- r = drm_mm_insert_node_in_range(&mgr->mm,
- &node->base.mm_nodes[0],
+ r = drm_mm_insert_node_in_range(&mgr->mm, &node->mm_nodes[0],
num_pages, tbo->page_alignment,
0, place->fpfn, place->lpfn,
DRM_MM_INSERT_BEST);
@@ -154,18 +141,18 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (unlikely(r))
goto err_free;
- node->base.base.start = node->base.mm_nodes[0].start;
+ node->base.start = node->mm_nodes[0].start;
} else {
- node->base.mm_nodes[0].start = 0;
- node->base.mm_nodes[0].size = node->base.base.num_pages;
- node->base.base.start = AMDGPU_BO_INVALID_OFFSET;
+ node->mm_nodes[0].start = 0;
+ node->mm_nodes[0].size = node->base.num_pages;
+ node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
- *res = &node->base.base;
+ *res = &node->base;
return 0;
err_free:
- ttm_resource_fini(man, &node->base.base);
+ ttm_resource_fini(man, &node->base);
kfree(node);
return r;
}
@@ -181,12 +168,12 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
struct ttm_resource *res)
{
- struct amdgpu_gtt_node *node = to_amdgpu_gtt_node(res);
+ struct ttm_range_mgr_node *node = to_ttm_range_mgr_node(res);
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
spin_lock(&mgr->lock);
- if (drm_mm_node_allocated(&node->base.mm_nodes[0]))
- drm_mm_remove_node(&node->base.mm_nodes[0]);
+ if (drm_mm_node_allocated(&node->mm_nodes[0]))
+ drm_mm_remove_node(&node->mm_nodes[0]);
spin_unlock(&mgr->lock);
ttm_resource_fini(man, res);
@@ -202,7 +189,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
*/
int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr)
{
- struct amdgpu_gtt_node *node;
+ struct ttm_range_mgr_node *node;
struct drm_mm_node *mm_node;
struct amdgpu_device *adev;
int r = 0;
@@ -210,8 +197,8 @@ int amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr)
adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
spin_lock(&mgr->lock);
drm_mm_for_each_node(mm_node, &mgr->mm) {
- node = container_of(mm_node, typeof(*node), base.mm_nodes[0]);
- r = amdgpu_ttm_recover_gart(node->tbo);
+ node = container_of(mm_node, typeof(*node), mm_nodes[0]);
+ r = amdgpu_ttm_recover_gart(node->base.bo);
if (r)
break;
}
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index ef0ec700e896..8a780a348025 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -347,6 +347,14 @@ ttm_resource_manager_next(struct ttm_resource_manager *man,
struct ttm_resource_cursor *cursor,
struct ttm_resource *res);
+/**
+ * ttm_resource_manager_for_each_res - iterate over all resources
+ * @man: the resource manager
+ * @cursor: struct ttm_resource_cursor for the current position
+ * @res: the current resource
+ *
+ * Iterate over all the evictable resources in a resource manager.
+ */
#define ttm_resource_manager_for_each_res(man, cursor, res) \
for (res = ttm_resource_manager_first(man, cursor); res; \
res = ttm_resource_manager_next(man, cursor, res))
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/11] drm/ttm: allow bulk moves for all domains
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (7 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 09/11] drm/amdgpu: drop amdgpu_gtt_node Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 9:34 ` [PATCH 11/11] drm/ttm: rework bulk move handling v2 Christian König
` (2 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
Not just TT and VRAM.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/ttm/ttm_resource.c | 52 +++++++++---------------------
include/drm/ttm/ttm_device.h | 2 --
include/drm/ttm/ttm_resource.h | 4 +--
3 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 1f9b8205b3a5..9df8fcc7e479 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -51,38 +51,24 @@ EXPORT_SYMBOL(ttm_lru_bulk_move_init);
*/
void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
{
- unsigned i;
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->tt[i];
- struct ttm_resource_manager *man;
+ unsigned i, j;
- if (!pos->first)
- continue;
+ for (i = 0; i < TTM_NUM_MEM_TYPES; ++i) {
+ for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
+ struct ttm_lru_bulk_move_pos *pos = &bulk->pos[i][j];
+ struct ttm_resource_manager *man;
- lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
- dma_resv_assert_held(pos->first->bo->base.resv);
- dma_resv_assert_held(pos->last->bo->base.resv);
+ if (!pos->first)
+ continue;
- man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_TT);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
- }
-
- for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
- struct ttm_lru_bulk_move_pos *pos = &bulk->vram[i];
- struct ttm_resource_manager *man;
+ lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
+ dma_resv_assert_held(pos->first->bo->base.resv);
+ dma_resv_assert_held(pos->last->bo->base.resv);
- if (!pos->first)
- continue;
-
- lockdep_assert_held(&pos->first->bo->bdev->lru_lock);
- dma_resv_assert_held(pos->first->bo->base.resv);
- dma_resv_assert_held(pos->last->bo->base.resv);
-
- man = ttm_manager_type(pos->first->bo->bdev, TTM_PL_VRAM);
- list_bulk_move_tail(&man->lru[i], &pos->first->lru,
- &pos->last->lru);
+ man = ttm_manager_type(pos->first->bo->bdev, i);
+ list_bulk_move_tail(&man->lru[j], &pos->first->lru,
+ &pos->last->lru);
+ }
}
}
EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
@@ -122,15 +108,7 @@ void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
if (!bulk)
return;
- switch (res->mem_type) {
- case TTM_PL_TT:
- ttm_lru_bulk_move_set_pos(&bulk->tt[bo->priority], res);
- break;
-
- case TTM_PL_VRAM:
- ttm_lru_bulk_move_set_pos(&bulk->vram[bo->priority], res);
- break;
- }
+ ttm_lru_bulk_move_set_pos(&bulk->pos[res->mem_type][bo->priority], res);
}
/**
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 0a4ddec78d8f..425150f35fbe 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -30,8 +30,6 @@
#include <drm/ttm/ttm_resource.h>
#include <drm/ttm/ttm_pool.h>
-#define TTM_NUM_MEM_TYPES 8
-
struct ttm_device;
struct ttm_placement;
struct ttm_buffer_object;
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 8a780a348025..990ed0b289a2 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -37,6 +37,7 @@
#include <drm/ttm/ttm_kmap_iter.h>
#define TTM_MAX_BO_PRIORITY 4U
+#define TTM_NUM_MEM_TYPES 8
struct ttm_device;
struct ttm_resource_manager;
@@ -217,8 +218,7 @@ struct ttm_lru_bulk_move_pos {
* Helper structure for bulk moves on the LRU list.
*/
struct ttm_lru_bulk_move {
- struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
- struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
+ struct ttm_lru_bulk_move_pos pos[TTM_NUM_MEM_TYPES][TTM_MAX_BO_PRIORITY];
};
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 11/11] drm/ttm: rework bulk move handling v2
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (8 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 10/11] drm/ttm: allow bulk moves for all domains Christian König
@ 2022-02-14 9:34 ` Christian König
2022-02-14 9:36 ` [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
2022-02-14 10:27 ` Matthew Auld
11 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:34 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
Instead of providing the bulk move structure for each LRU update set
this as property of the BO. This should avoid costly bulk move rebuilds
with some games under RADV.
v2: some name polishing, add a few more kerneldoc words.
v3: add some lockdep
Signed-off-by: Christian König <christian.koenig@amd.com>
Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 72 +++-----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 47 +++++++++++--
drivers/gpu/drm/ttm/ttm_resource.c | 87 ++++++++++++++++++-------
include/drm/ttm/ttm_bo_api.h | 16 ++---
include/drm/ttm/ttm_bo_driver.h | 2 +-
include/drm/ttm/ttm_device.h | 9 ---
include/drm/ttm/ttm_resource.h | 9 ++-
10 files changed, 128 insertions(+), 120 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 5859ed0552a4..57ac118fc266 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1498,7 +1498,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = {
.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
.access_memory = &amdgpu_ttm_access_memory,
- .del_from_lru_notify = &amdgpu_vm_del_from_lru_notify
};
/*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f2ce5a0defd9..28f5e8b21a99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -375,7 +375,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
return;
- vm->bulk_moveable = false;
+ ttm_bo_set_bulk_move(&bo->tbo, &vm->lru_bulk_move);
if (bo->tbo.type == ttm_bo_type_kernel && bo->parent)
amdgpu_vm_bo_relocated(base);
else
@@ -637,36 +637,6 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
list_add(&entry->tv.head, validated);
}
-/**
- * amdgpu_vm_del_from_lru_notify - update bulk_moveable flag
- *
- * @bo: BO which was removed from the LRU
- *
- * Make sure the bulk_moveable flag is updated when a BO is removed from the
- * LRU.
- */
-void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo)
-{
- struct amdgpu_bo *abo;
- struct amdgpu_vm_bo_base *bo_base;
-
- if (!amdgpu_bo_is_amdgpu_bo(bo))
- return;
-
- if (bo->pin_count)
- return;
-
- abo = ttm_to_amdgpu_bo(bo);
- if (!abo->parent)
- return;
- for (bo_base = abo->vm_bo; bo_base; bo_base = bo_base->next) {
- struct amdgpu_vm *vm = bo_base->vm;
-
- if (abo->tbo.base.resv == vm->root.bo->tbo.base.resv)
- vm->bulk_moveable = false;
- }
-
-}
/**
* amdgpu_vm_move_to_lru_tail - move all BOs to the end of LRU
*
@@ -679,33 +649,9 @@ void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo)
void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
{
- struct amdgpu_vm_bo_base *bo_base;
-
- if (vm->bulk_moveable) {
- spin_lock(&adev->mman.bdev.lru_lock);
- ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
- spin_unlock(&adev->mman.bdev.lru_lock);
- return;
- }
-
- ttm_lru_bulk_move_init(&vm->lru_bulk_move);
-
spin_lock(&adev->mman.bdev.lru_lock);
- list_for_each_entry(bo_base, &vm->idle, vm_status) {
- struct amdgpu_bo *bo = bo_base->bo;
- struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
-
- if (!bo->parent)
- continue;
-
- ttm_bo_move_to_lru_tail(&bo->tbo, &vm->lru_bulk_move);
- if (shadow)
- ttm_bo_move_to_lru_tail(&shadow->tbo,
- &vm->lru_bulk_move);
- }
+ ttm_lru_bulk_move_tail(&vm->lru_bulk_move);
spin_unlock(&adev->mman.bdev.lru_lock);
-
- vm->bulk_moveable = true;
}
/**
@@ -728,8 +674,6 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_vm_bo_base *bo_base, *tmp;
int r;
- vm->bulk_moveable &= list_empty(&vm->evicted);
-
list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
struct amdgpu_bo *bo = bo_base->bo;
struct amdgpu_bo *shadow = amdgpu_bo_shadowed(bo);
@@ -1047,10 +991,16 @@ static void amdgpu_vm_free_table(struct amdgpu_vm_bo_base *entry)
if (!entry->bo)
return;
+
shadow = amdgpu_bo_shadowed(entry->bo);
+ if (shadow) {
+ ttm_bo_set_bulk_move(&shadow->tbo, NULL);
+ amdgpu_bo_unref(&shadow);
+ }
+
+ ttm_bo_set_bulk_move(&entry->bo->tbo, NULL);
entry->bo->vm_bo = NULL;
list_del(&entry->vm_status);
- amdgpu_bo_unref(&shadow);
amdgpu_bo_unref(&entry->bo);
}
@@ -1070,8 +1020,6 @@ static void amdgpu_vm_free_pts(struct amdgpu_device *adev,
struct amdgpu_vm_pt_cursor cursor;
struct amdgpu_vm_bo_base *entry;
- vm->bulk_moveable = false;
-
for_each_amdgpu_vm_pt_dfs_safe(adev, vm, start, cursor, entry)
amdgpu_vm_free_table(entry);
@@ -2651,7 +2599,7 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
if (bo) {
if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
- vm->bulk_moveable = false;
+ ttm_bo_set_bulk_move(&bo->tbo, NULL);
for (base = &bo_va->base.bo->vm_bo; *base;
base = &(*base)->next) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 85fcfb8c5efd..4d236682a118 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -317,8 +317,6 @@ struct amdgpu_vm {
/* Store positions of group of BOs */
struct ttm_lru_bulk_move lru_bulk_move;
- /* mark whether can do the bulk move */
- bool bulk_moveable;
/* Flag to indicate if VM is used for compute */
bool is_compute_context;
};
@@ -454,7 +452,6 @@ void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);
void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
struct amdgpu_vm *vm);
-void amdgpu_vm_del_from_lru_notify(struct ttm_buffer_object *bo);
void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem,
uint64_t *gtt_mem, uint64_t *cpu_mem);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 582e8dc9bc8c..6fc192082d8c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -849,7 +849,7 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
bo->priority = I915_TTM_PRIO_NO_PAGES;
}
- ttm_bo_move_to_lru_tail(bo, NULL);
+ ttm_bo_move_to_lru_tail(bo);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 599be3dda8a9..d445a6c534d7 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -69,16 +69,53 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
}
}
-void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_lru_bulk_move *bulk)
+/**
+ * ttm_bo_move_to_lru_tail
+ *
+ * @bo: The buffer object.
+ *
+ * Move this BO to the tail of all lru lists used to lookup and reserve an
+ * object. This function must be called with struct ttm_global::lru_lock
+ * held, and is used to make a BO less likely to be considered for eviction.
+ */
+void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo)
{
dma_resv_assert_held(bo->base.resv);
if (bo->resource)
- ttm_resource_move_to_lru_tail(bo->resource, bulk);
+ ttm_resource_move_to_lru_tail(bo->resource);
}
EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
+/**
+ * ttm_bo_set_bulk_move - update BOs bulk move object
+ *
+ * @bo: The buffer object.
+ *
+ * Update the BOs bulk move object, making sure that resources are added/removed
+ * as well. A bulk move allows to move many resource on the LRU at once,
+ * resulting in much less overhead of maintaining the LRU.
+ * The only requirement is that the resources stay together on the LRU and are
+ * never separated. This is enforces by setting the bulk_move structure on a BO.
+ */
+void ttm_bo_set_bulk_move(struct ttm_buffer_object *bo,
+ struct ttm_lru_bulk_move *bulk)
+{
+ dma_resv_assert_held(bo->base.resv);
+
+ if (bo->bulk_move == bulk)
+ return;
+
+ spin_lock(&bo->bdev->lru_lock);
+ if (bo->bulk_move && bo->resource)
+ ttm_lru_bulk_move_del(bo->bulk_move, bo->resource);
+ bo->bulk_move = bulk;
+ if (bo->bulk_move && bo->resource)
+ ttm_lru_bulk_move_add(bo->bulk_move, bo->resource);
+ spin_unlock(&bo->bdev->lru_lock);
+}
+EXPORT_SYMBOL(ttm_bo_set_bulk_move);
+
static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
struct ttm_resource *mem, bool evict,
struct ttm_operation_ctx *ctx,
@@ -316,6 +353,7 @@ static void ttm_bo_release(struct kref *kref)
int ret;
WARN_ON_ONCE(bo->pin_count);
+ WARN_ON_ONCE(bo->bulk_move);
if (!bo->deleted) {
ret = ttm_bo_individualize_resv(bo);
@@ -352,7 +390,7 @@ static void ttm_bo_release(struct kref *kref)
*/
if (bo->pin_count) {
bo->pin_count = 0;
- ttm_resource_move_to_lru_tail(bo->resource, NULL);
+ ttm_resource_move_to_lru_tail(bo->resource);
}
kref_init(&bo->kref);
@@ -874,6 +912,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
bo->moving = NULL;
bo->pin_count = 0;
bo->sg = sg;
+ bo->bulk_move = NULL;
if (resv) {
bo->base.resv = resv;
dma_resv_assert_held(bo->base.resv);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 9df8fcc7e479..8c827eedc4df 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -73,42 +73,77 @@ void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk)
}
EXPORT_SYMBOL(ttm_lru_bulk_move_tail);
-/* Record a resource position in a bulk move structure */
-static void ttm_lru_bulk_move_set_pos(struct ttm_lru_bulk_move_pos *pos,
- struct ttm_resource *res)
+/* Return the bulk move pos object for this resource */
+static struct ttm_lru_bulk_move_pos *
+ttm_lru_bulk_move_pos(struct ttm_lru_bulk_move *bulk, struct ttm_resource *res)
{
- if (!pos->first)
+ return &bulk->pos[res->mem_type][res->bo->priority];
+}
+
+/* Move the resource to the tail of the bulk move range */
+static void ttm_lru_bulk_move_pos_tail(struct ttm_lru_bulk_move_pos *pos,
+ struct ttm_resource *res)
+{
+ if (pos->last != res) {
+ list_move(&res->lru, &pos->last->lru);
+ pos->last = res;
+ }
+}
+
+/* Add the resource to a bulk_move cursor */
+void ttm_lru_bulk_move_add(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res)
+{
+ struct ttm_lru_bulk_move_pos *pos = ttm_lru_bulk_move_pos(bulk, res);
+
+ if (!pos->first) {
pos->first = res;
- pos->last = res;
+ pos->last = res;
+ } else {
+ ttm_lru_bulk_move_pos_tail(pos, res);
+ }
+}
+
+/* Remove the resource from a bulk_move range */
+void ttm_lru_bulk_move_del(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res)
+{
+ struct ttm_lru_bulk_move_pos *pos = ttm_lru_bulk_move_pos(bulk, res);
+
+ if (unlikely(pos->first == res && pos->last == res)) {
+ pos->first = NULL;
+ pos->last = NULL;
+ } else if (pos->first == res) {
+ pos->first = list_next_entry(res, lru);
+ } else if (pos->last == res) {
+ pos->last = list_prev_entry(res, lru);
+ } else {
+ list_move(&res->lru, &pos->last->lru);
+ }
}
-/* Move a resource to the LRU tail and track the bulk position */
-void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
- struct ttm_lru_bulk_move *bulk)
+/* Move a resource to the LRU or bulk tail */
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res)
{
struct ttm_buffer_object *bo = res->bo;
struct ttm_device *bdev = bo->bdev;
- struct ttm_resource_manager *man;
lockdep_assert_held(&bo->bdev->lru_lock);
if (bo->pin_count) {
list_move_tail(&res->lru, &bdev->pinned);
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(res->bo);
- return;
- }
- man = ttm_manager_type(bdev, res->mem_type);
- list_move_tail(&res->lru, &man->lru[bo->priority]);
+ } else if (bo->bulk_move) {
+ struct ttm_lru_bulk_move_pos *pos =
+ ttm_lru_bulk_move_pos(bo->bulk_move, res);
- if (bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(bo);
-
- if (!bulk)
- return;
+ ttm_lru_bulk_move_pos_tail(pos, res);
+ } else {
+ struct ttm_resource_manager *man;
- ttm_lru_bulk_move_set_pos(&bulk->pos[res->mem_type][bo->priority], res);
+ man = ttm_manager_type(bdev, res->mem_type);
+ list_move_tail(&res->lru, &man->lru[bo->priority]);
+ }
}
/**
@@ -139,7 +174,7 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
man = ttm_manager_type(bo->bdev, place->mem_type);
spin_lock(&bo->bdev->lru_lock);
man->usage += bo->base.size;
- ttm_resource_move_to_lru_tail(res, NULL);
+ ttm_resource_move_to_lru_tail(res);
spin_unlock(&bo->bdev->lru_lock);
}
EXPORT_SYMBOL(ttm_resource_init);
@@ -161,8 +196,6 @@ void ttm_resource_fini(struct ttm_resource_manager *man,
spin_lock(&bdev->lru_lock);
list_del_init(&res->lru);
- if (res->bo && bdev->funcs->del_from_lru_notify)
- bdev->funcs->del_from_lru_notify(res->bo);
man->usage -= res->num_pages << PAGE_SHIFT;
spin_unlock(&bdev->lru_lock);
}
@@ -185,6 +218,12 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
if (!*res)
return;
+ if (bo->bulk_move) {
+ spin_lock(&bo->bdev->lru_lock);
+ ttm_lru_bulk_move_del(bo->bulk_move, *res);
+ spin_unlock(&bo->bdev->lru_lock);
+ }
+
man = ttm_manager_type(bo->bdev, (*res)->mem_type);
man->func->free(man, *res);
*res = NULL;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 3da77fc54552..a6d335895685 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -135,6 +135,7 @@ struct ttm_buffer_object {
struct ttm_resource *resource;
struct ttm_tt *ttm;
bool deleted;
+ struct ttm_lru_bulk_move *bulk_move;
/**
* Members protected by the bdev::lru_lock.
@@ -287,18 +288,9 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
*/
void ttm_bo_put(struct ttm_buffer_object *bo);
-/**
- * ttm_bo_move_to_lru_tail
- *
- * @bo: The buffer object.
- * @bulk: optional bulk move structure to remember BO positions
- *
- * Move this BO to the tail of all lru lists used to lookup and reserve an
- * object. This function must be called with struct ttm_global::lru_lock
- * held, and is used to make a BO less likely to be considered for eviction.
- */
-void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
- struct ttm_lru_bulk_move *bulk);
+void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo);
+void ttm_bo_set_bulk_move(struct ttm_buffer_object *bo,
+ struct ttm_lru_bulk_move *bulk);
/**
* ttm_bo_lock_delayed_workqueue
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 6c7352e13708..059a595e14e5 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -155,7 +155,7 @@ static inline void
ttm_bo_move_to_lru_tail_unlocked(struct ttm_buffer_object *bo)
{
spin_lock(&bo->bdev->lru_lock);
- ttm_bo_move_to_lru_tail(bo, NULL);
+ ttm_bo_move_to_lru_tail(bo);
spin_unlock(&bo->bdev->lru_lock);
}
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 425150f35fbe..95b3c04b1ab9 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -198,15 +198,6 @@ struct ttm_device_funcs {
int (*access_memory)(struct ttm_buffer_object *bo, unsigned long offset,
void *buf, int len, int write);
- /**
- * struct ttm_bo_driver member del_from_lru_notify
- *
- * @bo: the buffer object deleted from lru
- *
- * notify driver that a BO was deleted from LRU.
- */
- void (*del_from_lru_notify)(struct ttm_buffer_object *bo);
-
/**
* Notify the driver that we're about to release a BO
*
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 990ed0b289a2..a1f495b1dd55 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -202,7 +202,7 @@ struct ttm_resource_cursor {
* @first: first res in the bulk move range
* @last: last res in the bulk move range
*
- * Positions for a lru bulk move.
+ * Range of resources for a lru bulk move.
*/
struct ttm_lru_bulk_move_pos {
struct ttm_resource *first;
@@ -308,10 +308,13 @@ ttm_resource_manager_cleanup(struct ttm_resource_manager *man)
}
void ttm_lru_bulk_move_init(struct ttm_lru_bulk_move *bulk);
+void ttm_lru_bulk_move_add(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res);
+void ttm_lru_bulk_move_del(struct ttm_lru_bulk_move *bulk,
+ struct ttm_resource *res);
void ttm_lru_bulk_move_tail(struct ttm_lru_bulk_move *bulk);
-void ttm_resource_move_to_lru_tail(struct ttm_resource *res,
- struct ttm_lru_bulk_move *bulk);
+void ttm_resource_move_to_lru_tail(struct ttm_resource *res);
void ttm_resource_init(struct ttm_buffer_object *bo,
const struct ttm_place *place,
--
2.25.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 01/11] drm/ttm: fix resource manager size type and description
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (9 preceding siblings ...)
2022-02-14 9:34 ` [PATCH 11/11] drm/ttm: rework bulk move handling v2 Christian König
@ 2022-02-14 9:36 ` Christian König
2022-02-14 10:27 ` Matthew Auld
11 siblings, 0 replies; 25+ messages in thread
From: Christian König @ 2022-02-14 9:36 UTC (permalink / raw)
To: matthew.william.auld, daniel, thomas.hellstrom, felix.kuehling,
dri-devel
Hi guys,
crap I once more forgot the cover letter, but this set should be pretty
straight forward by now.
Please review and comment,
Christian.
Am 14.02.22 um 10:34 schrieb Christian König:
> That are not pages any more.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/ttm/ttm_resource.c | 6 +++---
> include/drm/ttm/ttm_resource.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 68344c90549b..ae40e144e728 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -153,19 +153,19 @@ void ttm_resource_set_bo(struct ttm_resource *res,
> *
> * @man: memory manager object to init
> * @bdev: ttm device this manager belongs to
> - * @p_size: size managed area in pages.
> + * @size: size of managed resources in arbitary units
> *
> * Initialise core parts of a manager object.
> */
> void ttm_resource_manager_init(struct ttm_resource_manager *man,
> struct ttm_device *bdev,
> - unsigned long p_size)
> + uint64_t size)
> {
> unsigned i;
>
> spin_lock_init(&man->move_lock);
> man->bdev = bdev;
> - man->size = p_size;
> + man->size = size;
>
> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> INIT_LIST_HEAD(&man->lru[i]);
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 69eea9d6399b..555a11fb8a7f 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -278,7 +278,7 @@ void ttm_resource_set_bo(struct ttm_resource *res,
>
> void ttm_resource_manager_init(struct ttm_resource_manager *man,
> struct ttm_device *bdev,
> - unsigned long p_size);
> + uint64_t size);
>
> int ttm_resource_manager_evict_all(struct ttm_device *bdev,
> struct ttm_resource_manager *man);
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 01/11] drm/ttm: fix resource manager size type and description
2022-02-14 9:34 [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
` (10 preceding siblings ...)
2022-02-14 9:36 ` [PATCH 01/11] drm/ttm: fix resource manager size type and description Christian König
@ 2022-02-14 10:27 ` Matthew Auld
11 siblings, 0 replies; 25+ messages in thread
From: Matthew Auld @ 2022-02-14 10:27 UTC (permalink / raw)
To: Christian König; +Cc: Thomas Hellström, felix.kuehling, ML dri-devel
On Mon, 14 Feb 2022 at 09:34, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> That are not pages any more.
"Leave the man->size units as driver defined."
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/ttm/ttm_resource.c | 6 +++---
> include/drm/ttm/ttm_resource.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 68344c90549b..ae40e144e728 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -153,19 +153,19 @@ void ttm_resource_set_bo(struct ttm_resource *res,
> *
> * @man: memory manager object to init
> * @bdev: ttm device this manager belongs to
> - * @p_size: size managed area in pages.
> + * @size: size of managed resources in arbitary units
s/arbitary/arbitrary/
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> *
> * Initialise core parts of a manager object.
> */
> void ttm_resource_manager_init(struct ttm_resource_manager *man,
> struct ttm_device *bdev,
> - unsigned long p_size)
> + uint64_t size)
> {
> unsigned i;
>
> spin_lock_init(&man->move_lock);
> man->bdev = bdev;
> - man->size = p_size;
> + man->size = size;
>
> for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> INIT_LIST_HEAD(&man->lru[i]);
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 69eea9d6399b..555a11fb8a7f 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -278,7 +278,7 @@ void ttm_resource_set_bo(struct ttm_resource *res,
>
> void ttm_resource_manager_init(struct ttm_resource_manager *man,
> struct ttm_device *bdev,
> - unsigned long p_size);
> + uint64_t size);
>
> int ttm_resource_manager_evict_all(struct ttm_device *bdev,
> struct ttm_resource_manager *man);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread