Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
On 2024-04-02 10:17, Christian König wrote: > Am 26.03.24 um 15:53 schrieb Alex Deucher: >> On Tue, Mar 26, 2024 at 10:01 AM Alex Deucher wrote: >>> On Tue, Mar 26, 2024 at 9:59 AM Paneer Selvam, Arunpravin >> @@ -501,6 +502,9 @@ static int amdgpu_vram_mgr_new(struct >> ttm_resource_manager *man, >> if (place->flags & TTM_PL_FLAG_CONTIGUOUS) >> vres->flags |= DRM_BUDDY_CONTIGUOUS_ALLOCATION; >> >> + if (bo->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED) >> + vres->flags |= DRM_BUDDY_CLEAR_ALLOCATION; > Is there any reason to not always do this? Here we are trying to keep a pool of cleared memory which are cleared on free path and that can given to the application which requires a zeroed memory. I think here if we set always clear the memory, we would go over the limit of cleared memory in that particular instance and the application should wait until the hardware clears the memory and this might impact the overall performance. >>> I'd like to have the driver always deliver cleared memory. >> Actually, I think we can just always set the flag in the allocation IOCTLs. > > We have use cases where that hurts as. Especially during boot when the > backing VRAM isn't cleared yet. > > That's one of the reasons why we never always cleared the memory. Any such performance gain was only valid in the first place if the kernel delivering non-cleared memory to user space was considered acceptable, which it quite clearly isn't. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Am 26.03.24 um 15:53 schrieb Alex Deucher:
On Tue, Mar 26, 2024 at 10:01 AM Alex Deucher wrote:
On Tue, Mar 26, 2024 at 9:59 AM Paneer Selvam, Arunpravin
wrote:
Hi Alex,
On 3/26/2024 7:08 PM, Alex Deucher wrote:
On Mon, Mar 18, 2024 at 5:47 PM Arunpravin Paneer Selvam
wrote:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if (!amdgpu_vram_mgr_is_cleared(block))
+
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Hi Alex,
On 3/26/2024 8:23 PM, Alex Deucher wrote:
On Tue, Mar 26, 2024 at 10:01 AM Alex Deucher wrote:
On Tue, Mar 26, 2024 at 9:59 AM Paneer Selvam, Arunpravin
wrote:
Hi Alex,
On 3/26/2024 7:08 PM, Alex Deucher wrote:
On Mon, Mar 18, 2024 at 5:47 PM Arunpravin Paneer Selvam
wrote:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
On Tue, Mar 26, 2024 at 10:01 AM Alex Deucher wrote:
>
> On Tue, Mar 26, 2024 at 9:59 AM Paneer Selvam, Arunpravin
> wrote:
> >
> > Hi Alex,
> >
> > On 3/26/2024 7:08 PM, Alex Deucher wrote:
> > > On Mon, Mar 18, 2024 at 5:47 PM Arunpravin Paneer Selvam
> > > wrote:
> > >> Add clear page support in vram memory region.
> > >>
> > >> v1(Christian):
> > >>- Dont handle clear page as TTM flag since when moving the BO back
> > >> in from GTT again we don't need that.
> > >>- Make a specialized version of amdgpu_fill_buffer() which only
> > >> clears the VRAM areas which are not already cleared
> > >>- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
> > >> amdgpu_object.c
> > >>
> > >> v2:
> > >>- Modify the function name amdgpu_ttm_* (Alex)
> > >>- Drop the delayed parameter (Christian)
> > >>- handle amdgpu_res_cleared() just above the size
> > >> calculation (Christian)
> > >>- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
> > >> in the free path to properly wait for fences etc.. (Christian)
> > >>
> > >> v3(Christian):
> > >>- Remove buffer clear code in VRAM manager instead change the
> > >> AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
> > >> the DRM_BUDDY_CLEARED flag.
> > >>- Remove ! from amdgpu_res_cleared() check.
> > >>
> > >> Signed-off-by: Arunpravin Paneer Selvam
> > >> Suggested-by: Christian König
> > >> Acked-by: Felix Kuehling
> > >> ---
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
> > >> .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
> > >> 6 files changed, 111 insertions(+), 13 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > >> index 8bc79924d171..c92d92b28a57 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > >> @@ -39,6 +39,7 @@
> > >> #include "amdgpu.h"
> > >> #include "amdgpu_trace.h"
> > >> #include "amdgpu_amdkfd.h"
> > >> +#include "amdgpu_vram_mgr.h"
> > >>
> > >> /**
> > >>* DOC: amdgpu_object
> > >> @@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> > >> if (!amdgpu_bo_support_uswc(bo->flags))
> > >> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> > >>
> > >> - if (adev->ras_enabled)
> > >> - bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
> > >> + bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
> > >>
> > >> bo->tbo.bdev = >mman.bdev;
> > >> if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
> > >> @@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> > >>
> > >> if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
> > >> bo->tbo.resource->mem_type == TTM_PL_VRAM) {
> > >> - struct dma_fence *fence;
> > >> + struct dma_fence *fence = NULL;
> > >>
> > >> - r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, ,
> > >> true);
> > >> + r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv,
> > >> );
> > >> if (unlikely(r))
> > >> goto fail_unreserve;
> > >>
> > >> - dma_resv_add_fence(bo->tbo.base.resv, fence,
> > >> - DMA_RESV_USAGE_KERNEL);
> > >> - dma_fence_put(fence);
> > >> + if (fence) {
> > >> + dma_resv_add_fence(bo->tbo.base.resv, fence,
> > >> + DMA_RESV_USAGE_KERNEL);
> > >> + dma_fence_put(fence);
> > >> + }
> > >> }
> > >> if (!bp->resv)
> > >> amdgpu_bo_unreserve(bo);
> > >> @@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct
> > >> ttm_buffer_object *bo)
> > >> if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
> > >> return;
> > >>
> > >> - r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv,
> > >> , true);
> > >> + r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
> > >> if (!WARN_ON(r)) {
> > >> + struct amdgpu_vram_mgr_resource *vres;
> > >> +
> > >> + vres = to_amdgpu_vram_mgr_resource(bo->resource);
> > >> + vres->flags |= DRM_BUDDY_CLEARED;
> > >> amdgpu_bo_fence(abo, fence, false);
> > >> dma_fence_put(fence);
> > >> }
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> > >> index 381101d2bf05..50fcd86e1033
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
On Tue, Mar 26, 2024 at 9:59 AM Paneer Selvam, Arunpravin
wrote:
>
> Hi Alex,
>
> On 3/26/2024 7:08 PM, Alex Deucher wrote:
> > On Mon, Mar 18, 2024 at 5:47 PM Arunpravin Paneer Selvam
> > wrote:
> >> Add clear page support in vram memory region.
> >>
> >> v1(Christian):
> >>- Dont handle clear page as TTM flag since when moving the BO back
> >> in from GTT again we don't need that.
> >>- Make a specialized version of amdgpu_fill_buffer() which only
> >> clears the VRAM areas which are not already cleared
> >>- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
> >> amdgpu_object.c
> >>
> >> v2:
> >>- Modify the function name amdgpu_ttm_* (Alex)
> >>- Drop the delayed parameter (Christian)
> >>- handle amdgpu_res_cleared() just above the size
> >> calculation (Christian)
> >>- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
> >> in the free path to properly wait for fences etc.. (Christian)
> >>
> >> v3(Christian):
> >>- Remove buffer clear code in VRAM manager instead change the
> >> AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
> >> the DRM_BUDDY_CLEARED flag.
> >>- Remove ! from amdgpu_res_cleared() check.
> >>
> >> Signed-off-by: Arunpravin Paneer Selvam
> >> Suggested-by: Christian König
> >> Acked-by: Felix Kuehling
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
> >> .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
> >> 6 files changed, 111 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> index 8bc79924d171..c92d92b28a57 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> @@ -39,6 +39,7 @@
> >> #include "amdgpu.h"
> >> #include "amdgpu_trace.h"
> >> #include "amdgpu_amdkfd.h"
> >> +#include "amdgpu_vram_mgr.h"
> >>
> >> /**
> >>* DOC: amdgpu_object
> >> @@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> >> if (!amdgpu_bo_support_uswc(bo->flags))
> >> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> >>
> >> - if (adev->ras_enabled)
> >> - bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
> >> + bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
> >>
> >> bo->tbo.bdev = >mman.bdev;
> >> if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
> >> @@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> >>
> >> if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
> >> bo->tbo.resource->mem_type == TTM_PL_VRAM) {
> >> - struct dma_fence *fence;
> >> + struct dma_fence *fence = NULL;
> >>
> >> - r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, ,
> >> true);
> >> + r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
> >> if (unlikely(r))
> >> goto fail_unreserve;
> >>
> >> - dma_resv_add_fence(bo->tbo.base.resv, fence,
> >> - DMA_RESV_USAGE_KERNEL);
> >> - dma_fence_put(fence);
> >> + if (fence) {
> >> + dma_resv_add_fence(bo->tbo.base.resv, fence,
> >> + DMA_RESV_USAGE_KERNEL);
> >> + dma_fence_put(fence);
> >> + }
> >> }
> >> if (!bp->resv)
> >> amdgpu_bo_unreserve(bo);
> >> @@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct
> >> ttm_buffer_object *bo)
> >> if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
> >> return;
> >>
> >> - r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, ,
> >> true);
> >> + r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
> >> if (!WARN_ON(r)) {
> >> + struct amdgpu_vram_mgr_resource *vres;
> >> +
> >> + vres = to_amdgpu_vram_mgr_resource(bo->resource);
> >> + vres->flags |= DRM_BUDDY_CLEARED;
> >> amdgpu_bo_fence(abo, fence, false);
> >> dma_fence_put(fence);
> >> }
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> >> index 381101d2bf05..50fcd86e1033 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> >> @@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
> >> amdgpu_res_cursor *cur, uint64_t size)
> >> }
> >> }
> >>
> >> +/**
> >> + *
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Hi Alex,
On 3/26/2024 7:08 PM, Alex Deucher wrote:
On Mon, Mar 18, 2024 at 5:47 PM Arunpravin Paneer Selvam
wrote:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if (!amdgpu_vram_mgr_is_cleared(block))
+ return false;
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
On Mon, Mar 18, 2024 at 5:47 PM Arunpravin Paneer Selvam
wrote:
>
> Add clear page support in vram memory region.
>
> v1(Christian):
> - Dont handle clear page as TTM flag since when moving the BO back
> in from GTT again we don't need that.
> - Make a specialized version of amdgpu_fill_buffer() which only
> clears the VRAM areas which are not already cleared
> - Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
> amdgpu_object.c
>
> v2:
> - Modify the function name amdgpu_ttm_* (Alex)
> - Drop the delayed parameter (Christian)
> - handle amdgpu_res_cleared() just above the size
> calculation (Christian)
> - Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
> in the free path to properly wait for fences etc.. (Christian)
>
> v3(Christian):
> - Remove buffer clear code in VRAM manager instead change the
> AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
> the DRM_BUDDY_CLEARED flag.
> - Remove ! from amdgpu_res_cleared() check.
>
> Signed-off-by: Arunpravin Paneer Selvam
> Suggested-by: Christian König
> Acked-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
> .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
> 6 files changed, 111 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 8bc79924d171..c92d92b28a57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -39,6 +39,7 @@
> #include "amdgpu.h"
> #include "amdgpu_trace.h"
> #include "amdgpu_amdkfd.h"
> +#include "amdgpu_vram_mgr.h"
>
> /**
> * DOC: amdgpu_object
> @@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> if (!amdgpu_bo_support_uswc(bo->flags))
> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>
> - if (adev->ras_enabled)
> - bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
> + bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
>
> bo->tbo.bdev = >mman.bdev;
> if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
> @@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>
> if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
> bo->tbo.resource->mem_type == TTM_PL_VRAM) {
> - struct dma_fence *fence;
> + struct dma_fence *fence = NULL;
>
> - r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, ,
> true);
> + r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
> if (unlikely(r))
> goto fail_unreserve;
>
> - dma_resv_add_fence(bo->tbo.base.resv, fence,
> - DMA_RESV_USAGE_KERNEL);
> - dma_fence_put(fence);
> + if (fence) {
> + dma_resv_add_fence(bo->tbo.base.resv, fence,
> + DMA_RESV_USAGE_KERNEL);
> + dma_fence_put(fence);
> + }
> }
> if (!bp->resv)
> amdgpu_bo_unreserve(bo);
> @@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
> *bo)
> if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
> return;
>
> - r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, ,
> true);
> + r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
> if (!WARN_ON(r)) {
> + struct amdgpu_vram_mgr_resource *vres;
> +
> + vres = to_amdgpu_vram_mgr_resource(bo->resource);
> + vres->flags |= DRM_BUDDY_CLEARED;
> amdgpu_bo_fence(abo, fence, false);
> dma_fence_put(fence);
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> index 381101d2bf05..50fcd86e1033 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
> @@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
> amdgpu_res_cursor *cur, uint64_t size)
> }
> }
>
> +/**
> + * amdgpu_res_cleared - check if blocks are cleared
> + *
> + * @cur: the cursor to extract the block
> + *
> + * Check if the @cur block is cleared
> + */
> +static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
> +{
> + struct drm_buddy_block *block;
> +
> + switch (cur->mem_type) {
> + case TTM_PL_VRAM:
> + block = cur->node;
> +
> + if (!amdgpu_vram_mgr_is_cleared(block))
> + return false;
> + break;
> +
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Hi Christian,
On 3/19/2024 7:17 PM, Christian König wrote:
Am 19.03.24 um 12:41 schrieb Paneer Selvam, Arunpravin:
Hi Christian,
On 3/19/2024 3:58 PM, Christian König wrote:
Am 18.03.24 um 22:40 schrieb Arunpravin Paneer Selvam:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the
buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
Just a few nit picks below, but in general already looks good to me.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61
++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, ,
true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct
ttm_buffer_object *bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv,
, true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
Those lines should probably be in the VRAM manager.
This flag is set based on the amdgpu_fill_buffer() function return
value, can we move the amdgpu_fill_buffer() function call and
DRM_BUDDY_CLEARED flag set lines to amdgpu_vram_mgr_del() in VRAM
manager and does it require to wipe the VRAM buffers here as well
without setting the DRM_BUDDY_CLEARED.
No that won't work. The call to amdgpu_fill_buffer() *must* be here
because that here is called before the resource is actually freed.
Only setting the vres->flags should probably be moved into
amdgpu_vram_mgr.c.
So instead of calling that manually here just make a function for it
to keep the code related to the buddy allocator in one place.
I got it, Thanks.
Regards,
Arun.
Regards,
Christian.
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Am 19.03.24 um 12:41 schrieb Paneer Selvam, Arunpravin:
Hi Christian,
On 3/19/2024 3:58 PM, Christian König wrote:
Am 18.03.24 um 22:40 schrieb Arunpravin Paneer Selvam:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the
buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
Just a few nit picks below, but in general already looks good to me.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61
++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, ,
true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct
ttm_buffer_object *bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv,
, true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
Those lines should probably be in the VRAM manager.
This flag is set based on the amdgpu_fill_buffer() function return
value, can we move the amdgpu_fill_buffer() function call and
DRM_BUDDY_CLEARED flag set lines to amdgpu_vram_mgr_del() in VRAM
manager and does it require to wipe the VRAM buffers here as well
without setting the DRM_BUDDY_CLEARED.
No that won't work. The call to amdgpu_fill_buffer() *must* be here
because that here is called before the resource is actually freed.
Only setting the vres->flags should probably be moved into
amdgpu_vram_mgr.c.
So instead of calling that manually here just make a function for it to
keep the code related to the buddy allocator in one place.
Regards,
Christian.
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Hi Christian,
On 3/19/2024 3:58 PM, Christian König wrote:
Am 18.03.24 um 22:40 schrieb Arunpravin Paneer Selvam:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
Just a few nit picks below, but in general already looks good to me.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, ,
true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct
ttm_buffer_object *bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv,
, true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
Those lines should probably be in the VRAM manager.
This flag is set based on the amdgpu_fill_buffer() function return
value, can we move the amdgpu_fill_buffer() function call and
DRM_BUDDY_CLEARED flag set lines to amdgpu_vram_mgr_del() in VRAM
manager and does it require to wipe the VRAM buffers here as well
without setting the DRM_BUDDY_CLEARED.
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if (!amdgpu_vram_mgr_is_cleared(block))
+ return false;
Re: [PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Am 18.03.24 um 22:40 schrieb Arunpravin Paneer Selvam:
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
Just a few nit picks below, but in general already looks good to me.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
Those lines should probably be in the VRAM manager.
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if (!amdgpu_vram_mgr_is_cleared(block))
+ return false;
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
#endif
diff --git
[PATCH v9 2/3] drm/amdgpu: Enable clear page functionality
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 22 ---
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 61 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 5 ++
6 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..c92d92b28a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -632,15 +632,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
- struct dma_fence *fence;
+ struct dma_fence *fence = NULL;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
- dma_resv_add_fence(bo->tbo.base.resv, fence,
- DMA_RESV_USAGE_KERNEL);
- dma_fence_put(fence);
+ if (fence) {
+ dma_resv_add_fence(bo->tbo.base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ dma_fence_put(fence);
+ }
}
if (!bp->resv)
amdgpu_bo_unreserve(bo);
@@ -1365,8 +1367,12 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ struct amdgpu_vram_mgr_resource *vres;
+
+ vres = to_amdgpu_vram_mgr_resource(bo->resource);
+ vres->flags |= DRM_BUDDY_CLEARED;
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if (!amdgpu_vram_mgr_is_cleared(block))
+ return false;
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 8722beba494e..bcbffe909b47 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++
