Re: [PATCH 1/1] Revert "drm/amdkfd: Free queue after unmap queue success"
On 2022-06-27 15:05, Deucher, Alexander
wrote:
[Public]
-Original Message-
From: amd-gfx On Behalf Of Philip
Yang
Sent: Monday, June 27, 2022 1:32 PM
To: amd-gfx@lists.freedesktop.org
Cc: Yang, Philip
Subject: [PATCH 1/1] Revert "drm/amdkfd: Free queue after unmap queue
success"
This reverts commit 150c1266d78fbaa0fc5f89461daafae416db1c3e.
This causes KFDTest regression on gfx9, will submit new patch after fixing.
Which test? Also, missing your s-o-b. With that fixed:
Acked-by: Alex Deucher
I will update commit with KFDMemoryTest.MemoryRegister test
failed, add s-o-b then push.
Thanks,
Philip
---
.../drm/amd/amdkfd/kfd_device_queue_manager.c | 28 ---
.../amd/amdkfd/kfd_process_queue_manager.c| 2 +-
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 21e451acfa59..93a0b6995430 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1881,22 +1881,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
}
- if (q->properties.is_active) {
- if (!dqm->dev->shared_resources.enable_mes) {
- retval = execute_queues_cpsch(dqm,
-
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
- if (retval == -ETIME)
-qpd->reset_wavefronts = true;
- } else {
- retval = remove_queue_mes(dqm, q, qpd);
- }
-
- if (retval)
- goto failed_unmap_queue;
-
- decrement_queue_count(dqm, qpd, q);
- }
-
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
@@ -1910,6 +1894,17 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
list_del(&q->list);
qpd->queue_count--;
+ if (q->properties.is_active) {
+ if (!dqm->dev->shared_resources.enable_mes) {
+ decrement_queue_count(dqm, qpd, q);
+ retval = execute_queues_cpsch(dqm,
+
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
+ if (retval == -ETIME)
+qpd->reset_wavefronts = true;
+ } else {
+ retval = remove_queue_mes(dqm, q, qpd);
+ }
+ }
/*
* Unconditionally decrement this counter, regardless of the queue's
@@ -1926,7 +1921,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
return retval;
-failed_unmap_queue:
failed_try_destroy_debugged_queue:
dqm_unlock(dqm);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index a46e2a37b4a6..c9c205df4a14 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -422,6 +422,7 @@ int pqm_destroy_queue(struct
process_queue_manager *pqm, unsigned int qid)
}
if (pqn->q) {
+ kfd_procfs_del_queue(pqn->q);
dqm = pqn->q->device->dqm;
retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q);
if (retval) {
@@ -445,7 +446,6 @@ int pqm_destroy_queue(struct
process_queue_manager *pqm, unsigned int qid)
amdgpu_amdkfd_free_gtt_mem(dev->adev,
pqn->q->wptr_bo);
}
- kfd_procfs_del_queue(pqn->q);
uninit_queue(pqn->q);
}
--
2.35.1
[PATCH AUTOSEL 4.19 21/22] drm/amdgpu: Adjust logic around GTT size (v3)
From: Alex Deucher
[ Upstream commit f15345a377c6ea9c7cc74f079616af8856aff37f ]
Certain GL unit tests for large textures can cause problems
with the OOM killer since there is no way to link this memory
to a process. This was originally mitigated (but not necessarily
eliminated) by limiting the GTT size. The problem is this limit
is often too low for many modern games so just make the limit 1/2
of system memory. The OOM accounting needs to be addressed, but
we shouldn't prevent common 3D applications from being usable
just to potentially mitigate that corner case.
Set default GTT size to max(3G, 1/2 of system ram) by default.
v2: drop previous logic and default to 3/4 of ram
v3: default to half of ram to align with ttm
v4: fix spelling in comment (Kent)
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1942
Reviewed-by: Marek Olšák
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 50807d621eca..7018873324d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1782,18 +1782,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
(unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
- /* Compute GTT size, either bsaed on 3/4th the size of RAM size
+ /* Compute GTT size, either based on 1/2 the size of RAM size
* or whatever the user passed on module init */
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->gmc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ /* Certain GL unit tests for large textures can cause problems
+* with the OOM killer since there is no way to link this memory
+* to a process. This was originally mitigated (but not
necessarily
+* eliminated) by limiting the GTT size. The problem is this
limit
+* is often too low for many modern games so just make the
limit 1/2
+* of system memory which aligns with TTM. The OOM accounting
needs
+* to be addressed, but we shouldn't prevent common 3D
applications
+* from being usable just to potentially mitigate that corner
case.
+*/
+ gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
+ (u64)si.totalram * si.mem_unit / 2);
+ } else {
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
+ }
/* Initialize GTT memory pool */
r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
--
2.35.1
[PATCH AUTOSEL 5.4 25/27] drm/amdgpu: Adjust logic around GTT size (v3)
From: Alex Deucher
[ Upstream commit f15345a377c6ea9c7cc74f079616af8856aff37f ]
Certain GL unit tests for large textures can cause problems
with the OOM killer since there is no way to link this memory
to a process. This was originally mitigated (but not necessarily
eliminated) by limiting the GTT size. The problem is this limit
is often too low for many modern games so just make the limit 1/2
of system memory. The OOM accounting needs to be addressed, but
we shouldn't prevent common 3D applications from being usable
just to potentially mitigate that corner case.
Set default GTT size to max(3G, 1/2 of system ram) by default.
v2: drop previous logic and default to 3/4 of ram
v3: default to half of ram to align with ttm
v4: fix spelling in comment (Kent)
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1942
Reviewed-by: Marek Olšák
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 870dd78d5a21..e70b33f3f363 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1749,18 +1749,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
(unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
- /* Compute GTT size, either bsaed on 3/4th the size of RAM size
+ /* Compute GTT size, either based on 1/2 the size of RAM size
* or whatever the user passed on module init */
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->gmc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ /* Certain GL unit tests for large textures can cause problems
+* with the OOM killer since there is no way to link this memory
+* to a process. This was originally mitigated (but not
necessarily
+* eliminated) by limiting the GTT size. The problem is this
limit
+* is often too low for many modern games so just make the
limit 1/2
+* of system memory which aligns with TTM. The OOM accounting
needs
+* to be addressed, but we shouldn't prevent common 3D
applications
+* from being usable just to potentially mitigate that corner
case.
+*/
+ gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
+ (u64)si.totalram * si.mem_unit / 2);
+ } else {
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
+ }
/* Initialize GTT memory pool */
r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
--
2.35.1
[PATCH AUTOSEL 5.10 31/34] drm/amdgpu: Adjust logic around GTT size (v3)
From: Alex Deucher
[ Upstream commit f15345a377c6ea9c7cc74f079616af8856aff37f ]
Certain GL unit tests for large textures can cause problems
with the OOM killer since there is no way to link this memory
to a process. This was originally mitigated (but not necessarily
eliminated) by limiting the GTT size. The problem is this limit
is often too low for many modern games so just make the limit 1/2
of system memory. The OOM accounting needs to be addressed, but
we shouldn't prevent common 3D applications from being usable
just to potentially mitigate that corner case.
Set default GTT size to max(3G, 1/2 of system ram) by default.
v2: drop previous logic and default to 3/4 of ram
v3: default to half of ram to align with ttm
v4: fix spelling in comment (Kent)
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1942
Reviewed-by: Marek Olšák
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0b162928a248..b6d5987efc6a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1960,18 +1960,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
(unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
- /* Compute GTT size, either bsaed on 3/4th the size of RAM size
+ /* Compute GTT size, either based on 1/2 the size of RAM size
* or whatever the user passed on module init */
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->gmc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ /* Certain GL unit tests for large textures can cause problems
+* with the OOM killer since there is no way to link this memory
+* to a process. This was originally mitigated (but not
necessarily
+* eliminated) by limiting the GTT size. The problem is this
limit
+* is often too low for many modern games so just make the
limit 1/2
+* of system memory which aligns with TTM. The OOM accounting
needs
+* to be addressed, but we shouldn't prevent common 3D
applications
+* from being usable just to potentially mitigate that corner
case.
+*/
+ gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
+ (u64)si.totalram * si.mem_unit / 2);
+ } else {
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
+ }
/* Initialize GTT memory pool */
r = amdgpu_gtt_mgr_init(adev, gtt_size);
--
2.35.1
[PATCH AUTOSEL 5.15 38/41] drm/amdgpu: Adjust logic around GTT size (v3)
From: Alex Deucher
[ Upstream commit f15345a377c6ea9c7cc74f079616af8856aff37f ]
Certain GL unit tests for large textures can cause problems
with the OOM killer since there is no way to link this memory
to a process. This was originally mitigated (but not necessarily
eliminated) by limiting the GTT size. The problem is this limit
is often too low for many modern games so just make the limit 1/2
of system memory. The OOM accounting needs to be addressed, but
we shouldn't prevent common 3D applications from being usable
just to potentially mitigate that corner case.
Set default GTT size to max(3G, 1/2 of system ram) by default.
v2: drop previous logic and default to 3/4 of ram
v3: default to half of ram to align with ttm
v4: fix spelling in comment (Kent)
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1942
Reviewed-by: Marek Olšák
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 51c76d6322c9..d6c30eaf4fcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1747,18 +1747,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
(unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
- /* Compute GTT size, either bsaed on 3/4th the size of RAM size
+ /* Compute GTT size, either based on 1/2 the size of RAM size
* or whatever the user passed on module init */
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->gmc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ /* Certain GL unit tests for large textures can cause problems
+* with the OOM killer since there is no way to link this memory
+* to a process. This was originally mitigated (but not
necessarily
+* eliminated) by limiting the GTT size. The problem is this
limit
+* is often too low for many modern games so just make the
limit 1/2
+* of system memory which aligns with TTM. The OOM accounting
needs
+* to be addressed, but we shouldn't prevent common 3D
applications
+* from being usable just to potentially mitigate that corner
case.
+*/
+ gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
+ (u64)si.totalram * si.mem_unit / 2);
+ } else {
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
+ }
/* Initialize GTT memory pool */
r = amdgpu_gtt_mgr_init(adev, gtt_size);
--
2.35.1
[PATCH AUTOSEL 5.18 50/53] drm/amdgpu: Adjust logic around GTT size (v3)
From: Alex Deucher
[ Upstream commit f15345a377c6ea9c7cc74f079616af8856aff37f ]
Certain GL unit tests for large textures can cause problems
with the OOM killer since there is no way to link this memory
to a process. This was originally mitigated (but not necessarily
eliminated) by limiting the GTT size. The problem is this limit
is often too low for many modern games so just make the limit 1/2
of system memory. The OOM accounting needs to be addressed, but
we shouldn't prevent common 3D applications from being usable
just to potentially mitigate that corner case.
Set default GTT size to max(3G, 1/2 of system ram) by default.
v2: drop previous logic and default to 3/4 of ram
v3: default to half of ram to align with ttm
v4: fix spelling in comment (Kent)
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1942
Reviewed-by: Marek Olšák
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 4b9ee6e27f74..ef3ada98bdb6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1798,18 +1798,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
(unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
- /* Compute GTT size, either bsaed on 3/4th the size of RAM size
+ /* Compute GTT size, either based on 1/2 the size of RAM size
* or whatever the user passed on module init */
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->gmc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ /* Certain GL unit tests for large textures can cause problems
+* with the OOM killer since there is no way to link this memory
+* to a process. This was originally mitigated (but not
necessarily
+* eliminated) by limiting the GTT size. The problem is this
limit
+* is often too low for many modern games so just make the
limit 1/2
+* of system memory which aligns with TTM. The OOM accounting
needs
+* to be addressed, but we shouldn't prevent common 3D
applications
+* from being usable just to potentially mitigate that corner
case.
+*/
+ gtt_size = max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
+ (u64)si.totalram * si.mem_unit / 2);
+ } else {
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
+ }
/* Initialize GTT memory pool */
r = amdgpu_gtt_mgr_init(adev, gtt_size);
--
2.35.1
RE: [PATCH] drm/amdgpu: Remove useless amdgpu_display_freesync_ioctl() declaration
Reviewed-by: Guchun Chen Regards, Guchun -Original Message- From: amd-gfx On Behalf Of Leslie Shi Sent: Monday, June 27, 2022 1:02 PM To: amd-gfx@lists.freedesktop.org Cc: Shi, Leslie ; Chen, Guchun ; Zhang, Hawking Subject: [PATCH] drm/amdgpu: Remove useless amdgpu_display_freesync_ioctl() declaration Signed-off-by: Leslie Shi --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h index 7b6d83e2b13c..560352f7c317 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h @@ -35,8 +35,6 @@ #define amdgpu_display_add_encoder(adev, e, s, c) (adev)->mode_info.funcs->add_encoder((adev), (e), (s), (c)) #define amdgpu_display_add_connector(adev, ci, sd, ct, ib, coi, h, r) (adev)->mode_info.funcs->add_connector((adev), (ci), (sd), (ct), (ib), (coi), (h), (r)) -int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data, - struct drm_file *filp); void amdgpu_display_update_priority(struct amdgpu_device *adev); uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev, uint64_t bo_flags); -- 2.25.1
[PATCH 3/3] drm/amdgpu: add debugfs for kfd system and ttm mem used
This keeps track of kfd system mem used and kfd ttm mem used.
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 3 +++
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 19 +++
drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c | 2 ++
3 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index e6244182a3a4..53cdf7f00b3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -172,6 +172,9 @@ int amdgpu_queue_mask_bit_to_set_resource_bit(struct
amdgpu_device *adev,
struct amdgpu_amdkfd_fence *amdgpu_amdkfd_fence_create(u64 context,
struct mm_struct *mm,
struct svm_range_bo *svm_bo);
+#if defined(CONFIG_DEBUG_FS)
+int kfd_debugfs_kfd_mem_limits(struct seq_file *m, void *data);
+#endif
#if IS_ENABLED(CONFIG_HSA_AMD)
bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm);
struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 9719577ecc6d..c48557b683c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2935,3 +2935,22 @@ bool amdgpu_amdkfd_bo_mapped_to_dev(struct amdgpu_device
*adev, struct kgd_mem *
}
return false;
}
+
+#if defined(CONFIG_DEBUG_FS)
+
+int kfd_debugfs_kfd_mem_limits(struct seq_file *m, void *data)
+{
+
+ spin_lock(&kfd_mem_limit.mem_limit_lock);
+ seq_printf(m, "System mem used %lldM out of %lluM\n",
+ (kfd_mem_limit.system_mem_used >> 20),
+ (kfd_mem_limit.max_system_mem_limit >> 20));
+ seq_printf(m, "TTM mem used %lldM out of %lluM\n",
+ (kfd_mem_limit.ttm_mem_used >> 20),
+ (kfd_mem_limit.max_ttm_mem_limit >> 20));
+ spin_unlock(&kfd_mem_limit.mem_limit_lock);
+
+ return 0;
+}
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
index 581c3a30fee1..ad5a40a685ac 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
@@ -101,6 +101,8 @@ void kfd_debugfs_init(void)
kfd_debugfs_rls_by_device, &kfd_debugfs_fops);
debugfs_create_file("hang_hws", S_IFREG | 0200, debugfs_root,
kfd_debugfs_hang_hws_read,
&kfd_debugfs_hang_hws_fops);
+ debugfs_create_file("mem_limit", S_IFREG | 0200, debugfs_root,
+ kfd_debugfs_kfd_mem_limits, &kfd_debugfs_fops);
}
void kfd_debugfs_fini(void)
--
2.32.0
[PATCH 2/3] drm/amdkfd: track unified memory reservation with xnack off
[WHY]
Unified memory with xnack off should be tracked, as userptr mappings
and legacy allocations do. To avoid oversuscribe system memory when
xnack off.
[How]
Exposing functions reserve_mem_limit and unreserve_mem_limit to SVM
API and call them on every prange creation and free.
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 ++
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 25
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 58 +--
3 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index b25b41f50213..e6244182a3a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -305,6 +305,10 @@ bool amdgpu_amdkfd_bo_mapped_to_dev(struct amdgpu_device
*adev, struct kgd_mem *
void amdgpu_amdkfd_block_mmu_notifications(void *p);
int amdgpu_amdkfd_criu_resume(void *p);
bool amdgpu_amdkfd_ras_query_utcl2_poison_status(struct amdgpu_device *adev);
+int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
+ uint64_t size, u32 alloc_flag);
+void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev,
+ uint64_t size, u32 alloc_flag);
#if IS_ENABLED(CONFIG_HSA_AMD)
void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 9142f6cc3f4d..9719577ecc6d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -128,7 +128,7 @@ void amdgpu_amdkfd_reserve_system_mem(uint64_t size)
*
* Return: returns -ENOMEM in case of error, ZERO otherwise
*/
-static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
+int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
uint64_t reserved_for_pt =
@@ -168,7 +168,7 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
kfd_mem_limit.max_system_mem_limit && !no_system_mem_limit) ||
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
kfd_mem_limit.max_ttm_mem_limit) ||
- (adev->kfd.vram_used + vram_needed >
+ (adev && adev->kfd.vram_used + vram_needed >
adev->gmc.real_vram_size -
atomic64_read(&adev->vram_pin_size) -
reserved_for_pt)) {
@@ -179,7 +179,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
/* Update memory accounting by decreasing available system
* memory, TTM memory and GPU memory as computed above
*/
- adev->kfd.vram_used += vram_needed;
+ WARN_ONCE(vram_needed && !adev,
+ "adev reference can't be null when vram is used");
+ if (adev)
+ adev->kfd.vram_used += vram_needed;
kfd_mem_limit.system_mem_used += system_mem_needed;
kfd_mem_limit.ttm_mem_used += ttm_mem_needed;
@@ -188,7 +191,7 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
return ret;
}
-static void unreserve_mem_limit(struct amdgpu_device *adev,
+void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
spin_lock(&kfd_mem_limit.mem_limit_lock);
@@ -197,7 +200,10 @@ static void unreserve_mem_limit(struct amdgpu_device *adev,
kfd_mem_limit.system_mem_used -= size;
kfd_mem_limit.ttm_mem_used -= size;
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
- adev->kfd.vram_used -= ALIGN(size, VRAM_ALLOCATION_ALIGN);
+ WARN_ONCE(!adev,
+ "adev reference can't be null when alloc mem flags
vram is set");
+ if (adev)
+ adev->kfd.vram_used -= ALIGN(size,
VRAM_ALLOCATION_ALIGN);
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
kfd_mem_limit.system_mem_used -= size;
} else if (!(alloc_flag &
@@ -206,11 +212,8 @@ static void unreserve_mem_limit(struct amdgpu_device *adev,
pr_err("%s: Invalid BO type %#x\n", __func__, alloc_flag);
goto release;
}
-
- WARN_ONCE(adev->kfd.vram_used < 0,
+ WARN_ONCE(adev && adev->kfd.vram_used < 0,
"KFD VRAM memory accounting unbalanced");
- WARN_ONCE(kfd_mem_limit.ttm_mem_used < 0,
- "KFD TTM memory accounting unbalanced");
WARN_ONCE(kfd_mem_limit.system_mem_used < 0,
"KFD system memory accounting unbalanced");
@@ -224,7 +227,7 @@ void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo)
u32 alloc_flags = bo->kfd_bo->alloc_flags;
u64 size = amdgpu_bo_size(bo);
- unreserve_mem_limit(adev, size, alloc_flags);
+ amdgpu_amdkfd_unreserve_mem_limit
[PATCH 1/3] drm/amdgpu: remove acc_size from reserve/unreserve mem
TTM used to track the "acc_size" of all BOs internally. We needed to
keep track of it in our memory reservation to avoid TTM running out
of memory in its own accounting. However, that "acc_size" accounting
has since been removed from TTM. Therefore we don't really need to
track it any more.
Signed-off-by: Alex Sierra
Reviewed-by: Philip Yang
Reviewed-by: Felix Kuehling
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 60 ++-
1 file changed, 17 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 5ba9070d8722..9142f6cc3f4d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -114,21 +114,12 @@ void amdgpu_amdkfd_reserve_system_mem(uint64_t size)
* compromise that should work in most cases without reserving too
* much memory for page tables unnecessarily (factor 16K, >> 14).
*/
-#define ESTIMATE_PT_SIZE(mem_size) max(((mem_size) >> 14),
AMDGPU_VM_RESERVED_VRAM)
-
-static size_t amdgpu_amdkfd_acc_size(uint64_t size)
-{
- size >>= PAGE_SHIFT;
- size *= sizeof(dma_addr_t) + sizeof(void *);
- return __roundup_pow_of_two(sizeof(struct amdgpu_bo)) +
- __roundup_pow_of_two(sizeof(struct ttm_tt)) +
- PAGE_ALIGN(size);
-}
+#define ESTIMATE_PT_SIZE(mem_size) max(((mem_size) >> 14),
AMDGPU_VM_RESERVED_VRAM)
/**
* amdgpu_amdkfd_reserve_mem_limit() - Decrease available memory by size
- * of buffer including any reserved for control structures
+ * of buffer.
*
* @adev: Device to which allocated BO belongs to
* @size: Size of buffer, in bytes, encapsulated by B0. This should be
@@ -142,19 +133,16 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
{
uint64_t reserved_for_pt =
ESTIMATE_PT_SIZE(amdgpu_amdkfd_total_mem_size);
- size_t acc_size, system_mem_needed, ttm_mem_needed, vram_needed;
+ size_t system_mem_needed, ttm_mem_needed, vram_needed;
int ret = 0;
- acc_size = amdgpu_amdkfd_acc_size(size);
-
+ system_mem_needed = 0;
+ ttm_mem_needed = 0;
vram_needed = 0;
if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
- system_mem_needed = acc_size + size;
- ttm_mem_needed = acc_size + size;
+ system_mem_needed = size;
+ ttm_mem_needed = size;
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
- system_mem_needed = acc_size;
- ttm_mem_needed = acc_size;
-
/*
* Conservatively round up the allocation requirement to 2 MB
* to avoid fragmentation caused by 4K allocations in the tail
@@ -162,14 +150,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
*/
vram_needed = ALIGN(size, VRAM_ALLOCATION_ALIGN);
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
- system_mem_needed = acc_size + size;
- ttm_mem_needed = acc_size;
- } else if (alloc_flag &
- (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
- KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
- system_mem_needed = acc_size;
- ttm_mem_needed = acc_size;
- } else {
+ system_mem_needed = size;
+ } else if (!(alloc_flag &
+ (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
+KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP))) {
pr_err("%s: Invalid BO type %#x\n", __func__, alloc_flag);
return -ENOMEM;
}
@@ -207,28 +191,18 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
static void unreserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
- size_t acc_size;
-
- acc_size = amdgpu_amdkfd_acc_size(size);
-
spin_lock(&kfd_mem_limit.mem_limit_lock);
if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_GTT) {
- kfd_mem_limit.system_mem_used -= (acc_size + size);
- kfd_mem_limit.ttm_mem_used -= (acc_size + size);
+ kfd_mem_limit.system_mem_used -= size;
+ kfd_mem_limit.ttm_mem_used -= size;
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
- kfd_mem_limit.system_mem_used -= acc_size;
- kfd_mem_limit.ttm_mem_used -= acc_size;
adev->kfd.vram_used -= ALIGN(size, VRAM_ALLOCATION_ALIGN);
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
- kfd_mem_limit.system_mem_used -= (acc_size + size);
- kfd_mem_limit.ttm_mem_used -= acc_size;
- } else if (alloc_flag &
- (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
- KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
- kfd_mem_limit.system_mem_use
[PATCH v6 12/14] tools: update test_hmm script to support SP config
Add two more parameters to set spm_addr_dev0 & spm_addr_dev1
addresses. These two parameters configure the start SP
addresses for each device in test_hmm driver.
Consequently, this configures zone device type as coherent.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Popple
Signed-off-by: Christoph Hellwig
---
tools/testing/selftests/vm/test_hmm.sh | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/vm/test_hmm.sh
b/tools/testing/selftests/vm/test_hmm.sh
index 0647b525a625..539c9371e592 100755
--- a/tools/testing/selftests/vm/test_hmm.sh
+++ b/tools/testing/selftests/vm/test_hmm.sh
@@ -40,11 +40,26 @@ check_test_requirements()
load_driver()
{
- modprobe $DRIVER > /dev/null 2>&1
+ if [ $# -eq 0 ]; then
+ modprobe $DRIVER > /dev/null 2>&1
+ else
+ if [ $# -eq 2 ]; then
+ modprobe $DRIVER spm_addr_dev0=$1 spm_addr_dev1=$2
+ > /dev/null 2>&1
+ else
+ echo "Missing module parameters. Make sure pass"\
+ "spm_addr_dev0 and spm_addr_dev1"
+ usage
+ fi
+ fi
if [ $? == 0 ]; then
major=$(awk "\$2==\"HMM_DMIRROR\" {print \$1}" /proc/devices)
mknod /dev/hmm_dmirror0 c $major 0
mknod /dev/hmm_dmirror1 c $major 1
+ if [ $# -eq 2 ]; then
+ mknod /dev/hmm_dmirror2 c $major 2
+ mknod /dev/hmm_dmirror3 c $major 3
+ fi
fi
}
@@ -58,7 +73,7 @@ run_smoke()
{
echo "Running smoke test. Note, this test provides basic coverage."
- load_driver
+ load_driver $1 $2
$(dirname "${BASH_SOURCE[0]}")/hmm-tests
unload_driver
}
@@ -75,6 +90,9 @@ usage()
echo "# Smoke testing"
echo "./${TEST_NAME}.sh smoke"
echo
+ echo "# Smoke testing with SPM enabled"
+ echo "./${TEST_NAME}.sh smoke "
+ echo
exit 0
}
@@ -84,7 +102,7 @@ function run_test()
usage
else
if [ "$1" = "smoke" ]; then
- run_smoke
+ run_smoke $2 $3
else
usage
fi
--
2.32.0
[PATCH v6 10/14] lib: add support for device coherent type in test_hmm
Device Coherent type uses device memory that is coherently accesible by
the CPU. This could be shown as SP (special purpose) memory range
at the BIOS-e820 memory enumeration. If no SP memory is supported in
system, this could be faked by setting CONFIG_EFI_FAKE_MEMMAP.
Currently, test_hmm only supports two different SP ranges of at least
256MB size. This could be specified in the kernel parameter variable
efi_fake_mem. Ex. Two SP ranges of 1GB starting at 0x1 &
0x14000 physical address. Ex.
efi_fake_mem=1G@0x1:0x4,1G@0x14000:0x4
Private and coherent device mirror instances can be created in the same
probed. This is done by passing the module parameters spm_addr_dev0 &
spm_addr_dev1. In this case, it will create four instances of
device_mirror. The first two correspond to private device type, the
last two to coherent type. Then, they can be easily accessed from user
space through /dev/hmm_mirror. Usually num_device 0 and 1
are for private, and 2 and 3 for coherent types. If no module
parameters are passed, two instances of private type device_mirror will
be created only.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Poppple
---
lib/test_hmm.c | 253 +---
lib/test_hmm_uapi.h | 4 +
2 files changed, 196 insertions(+), 61 deletions(-)
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index afb30af9f3ff..7930853e7fc5 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -32,11 +32,22 @@
#include "test_hmm_uapi.h"
-#define DMIRROR_NDEVICES 2
+#define DMIRROR_NDEVICES 4
#define DMIRROR_RANGE_FAULT_TIMEOUT1000
#define DEVMEM_CHUNK_SIZE (256 * 1024 * 1024U)
#define DEVMEM_CHUNKS_RESERVE 16
+/*
+ * For device_private pages, dpage is just a dummy struct page
+ * representing a piece of device memory. dmirror_devmem_alloc_page
+ * allocates a real system memory page as backing storage to fake a
+ * real device. zone_device_data points to that backing page. But
+ * for device_coherent memory, the struct page represents real
+ * physical CPU-accessible memory that we can use directly.
+ */
+#define BACKING_PAGE(page) (is_device_private_page((page)) ? \
+ (page)->zone_device_data : (page))
+
static unsigned long spm_addr_dev0;
module_param(spm_addr_dev0, long, 0644);
MODULE_PARM_DESC(spm_addr_dev0,
@@ -125,6 +136,21 @@ static int dmirror_bounce_init(struct dmirror_bounce
*bounce,
return 0;
}
+static bool dmirror_is_private_zone(struct dmirror_device *mdevice)
+{
+ return (mdevice->zone_device_type ==
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE) ? true : false;
+}
+
+static enum migrate_vma_direction
+dmirror_select_device(struct dmirror *dmirror)
+{
+ return (dmirror->mdevice->zone_device_type ==
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE) ?
+ MIGRATE_VMA_SELECT_DEVICE_PRIVATE :
+ MIGRATE_VMA_SELECT_DEVICE_COHERENT;
+}
+
static void dmirror_bounce_fini(struct dmirror_bounce *bounce)
{
vfree(bounce->ptr);
@@ -575,16 +601,19 @@ static int dmirror_allocate_chunk(struct dmirror_device
*mdevice,
static struct page *dmirror_devmem_alloc_page(struct dmirror_device *mdevice)
{
struct page *dpage = NULL;
- struct page *rpage;
+ struct page *rpage = NULL;
/*
-* This is a fake device so we alloc real system memory to store
-* our device memory.
+* For ZONE_DEVICE private type, this is a fake device so we allocate
+* real system memory to store our device memory.
+* For ZONE_DEVICE coherent type we use the actual dpage to store the
+* data and ignore rpage.
*/
- rpage = alloc_page(GFP_HIGHUSER);
- if (!rpage)
- return NULL;
-
+ if (dmirror_is_private_zone(mdevice)) {
+ rpage = alloc_page(GFP_HIGHUSER);
+ if (!rpage)
+ return NULL;
+ }
spin_lock(&mdevice->lock);
if (mdevice->free_pages) {
@@ -603,7 +632,8 @@ static struct page *dmirror_devmem_alloc_page(struct
dmirror_device *mdevice)
return dpage;
error:
- __free_page(rpage);
+ if (rpage)
+ __free_page(rpage);
return NULL;
}
@@ -629,12 +659,16 @@ static void dmirror_migrate_alloc_and_copy(struct
migrate_vma *args,
* unallocated pte_none() or read-only zero page.
*/
spage = migrate_pfn_to_page(*src);
+ if (WARN(spage && is_zone_device_page(spage),
+"page already in device spage pfn: 0x%lx\n",
+page_to_pfn(spage)))
+ continue;
dpage = dmirror_devmem_alloc_page(mdevice);
if (!dpage)
continue;
- rpage = dpage->zone_device_data;
+ rpage = BACKING_PAGE(dpage);
[PATCH v6 13/14] tools: add hmm gup tests for device coherent type
The intention is to test hmm device coherent type under different get
user pages paths. Also, test gup with FOLL_LONGTERM flag set in
device coherent pages. These pages should get migrated back to system
memory.
Signed-off-by: Alex Sierra
Reviewed-by: Alistair Popple
---
tools/testing/selftests/vm/hmm-tests.c | 110 +
1 file changed, 110 insertions(+)
diff --git a/tools/testing/selftests/vm/hmm-tests.c
b/tools/testing/selftests/vm/hmm-tests.c
index 4b547188ec40..bb38b9777610 100644
--- a/tools/testing/selftests/vm/hmm-tests.c
+++ b/tools/testing/selftests/vm/hmm-tests.c
@@ -36,6 +36,7 @@
* in the usual include/uapi/... directory.
*/
#include "../../../../lib/test_hmm_uapi.h"
+#include "../../../../mm/gup_test.h"
struct hmm_buffer {
void*ptr;
@@ -59,6 +60,9 @@ enum {
#define NTIMES 10
#define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
+/* Just the flags we need, copied from mm.h: */
+#define FOLL_WRITE 0x01/* check pte is writable */
+#define FOLL_LONGTERM 0x1 /* mapping lifetime is indefinite */
FIXTURE(hmm)
{
@@ -1764,4 +1768,110 @@ TEST_F(hmm, exclusive_cow)
hmm_buffer_free(buffer);
}
+static int gup_test_exec(int gup_fd, unsigned long addr, int cmd,
+int npages, int size, int flags)
+{
+ struct gup_test gup = {
+ .nr_pages_per_call = npages,
+ .addr = addr,
+ .gup_flags = FOLL_WRITE | flags,
+ .size = size,
+ };
+
+ if (ioctl(gup_fd, cmd, &gup)) {
+ perror("ioctl on error\n");
+ return errno;
+ }
+
+ return 0;
+}
+
+/*
+ * Test get user device pages through gup_test. Setting PIN_LONGTERM flag.
+ * This should trigger a migration back to system memory for both, private
+ * and coherent type pages.
+ * This test makes use of gup_test module. Make sure GUP_TEST_CONFIG is added
+ * to your configuration before you run it.
+ */
+TEST_F(hmm, hmm_gup_test)
+{
+ struct hmm_buffer *buffer;
+ int gup_fd;
+ unsigned long npages;
+ unsigned long size;
+ unsigned long i;
+ int *ptr;
+ int ret;
+ unsigned char *m;
+
+ gup_fd = open("/sys/kernel/debug/gup_test", O_RDWR);
+ if (gup_fd == -1)
+ SKIP(return, "Skipping test, could not find gup_test driver");
+
+ npages = 4;
+ size = npages << self->page_shift;
+
+ buffer = malloc(sizeof(*buffer));
+ ASSERT_NE(buffer, NULL);
+
+ buffer->fd = -1;
+ buffer->size = size;
+ buffer->mirror = malloc(size);
+ ASSERT_NE(buffer->mirror, NULL);
+
+ buffer->ptr = mmap(NULL, size,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS,
+ buffer->fd, 0);
+ ASSERT_NE(buffer->ptr, MAP_FAILED);
+
+ /* Initialize buffer in system memory. */
+ for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i)
+ ptr[i] = i;
+
+ /* Migrate memory to device. */
+ ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(buffer->cpages, npages);
+ /* Check what the device read. */
+ for (i = 0, ptr = buffer->mirror; i < size / sizeof(*ptr); ++i)
+ ASSERT_EQ(ptr[i], i);
+
+ ASSERT_EQ(gup_test_exec(gup_fd,
+ (unsigned long)buffer->ptr,
+ GUP_BASIC_TEST, 1, self->page_size, 0), 0);
+ ASSERT_EQ(gup_test_exec(gup_fd,
+ (unsigned long)buffer->ptr + 1 *
self->page_size,
+ GUP_FAST_BENCHMARK, 1, self->page_size, 0), 0);
+ ASSERT_EQ(gup_test_exec(gup_fd,
+ (unsigned long)buffer->ptr + 2 *
self->page_size,
+ PIN_FAST_BENCHMARK, 1, self->page_size,
FOLL_LONGTERM), 0);
+ ASSERT_EQ(gup_test_exec(gup_fd,
+ (unsigned long)buffer->ptr + 3 *
self->page_size,
+ PIN_LONGTERM_BENCHMARK, 1, self->page_size, 0),
0);
+
+ /* Take snapshot to CPU pagetables */
+ ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_SNAPSHOT, buffer, npages);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(buffer->cpages, npages);
+ m = buffer->mirror;
+ if (hmm_is_coherent_type(variant->device_number)) {
+ ASSERT_EQ(HMM_DMIRROR_PROT_DEV_COHERENT_LOCAL |
HMM_DMIRROR_PROT_WRITE, m[0]);
+ ASSERT_EQ(HMM_DMIRROR_PROT_DEV_COHERENT_LOCAL |
HMM_DMIRROR_PROT_WRITE, m[1]);
+ } else {
+ ASSERT_EQ(HMM_DMIRROR_PROT_WRITE, m[0]);
+ ASSERT_EQ(HMM_DMIRROR_PROT_WRITE, m[1]);
+ }
+ ASSERT_EQ(HMM_DMIRROR_PROT_WRITE, m[2]);
+ ASSERT_EQ(HMM_DMIRROR_PROT_WRITE, m[3]);
+ /*
+* Check again the cont
[PATCH v6 11/14] tools: update hmm-test to support device coherent type
Test cases such as migrate_fault and migrate_multiple, were modified to
explicit migrate from device to sys memory without the need of page
faults, when using device coherent type.
Snapshot test case updated to read memory device type first and based
on that, get the proper returned results migrate_ping_pong test case
added to test explicit migration from device to sys memory for both
private and coherent zone types.
Helpers to migrate from device to sys memory and vicerversa
were also added.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Popple
Signed-off-by: Christoph Hellwig
---
tools/testing/selftests/vm/hmm-tests.c | 121 -
1 file changed, 100 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/vm/hmm-tests.c
b/tools/testing/selftests/vm/hmm-tests.c
index 203323967b50..4b547188ec40 100644
--- a/tools/testing/selftests/vm/hmm-tests.c
+++ b/tools/testing/selftests/vm/hmm-tests.c
@@ -46,6 +46,13 @@ struct hmm_buffer {
uint64_tfaults;
};
+enum {
+ HMM_PRIVATE_DEVICE_ONE,
+ HMM_PRIVATE_DEVICE_TWO,
+ HMM_COHERENCE_DEVICE_ONE,
+ HMM_COHERENCE_DEVICE_TWO,
+};
+
#define TWOMEG (1 << 21)
#define HMM_BUFFER_SIZE (1024 << 12)
#define HMM_PATH_MAX64
@@ -60,6 +67,21 @@ FIXTURE(hmm)
unsigned intpage_shift;
};
+FIXTURE_VARIANT(hmm)
+{
+ int device_number;
+};
+
+FIXTURE_VARIANT_ADD(hmm, hmm_device_private)
+{
+ .device_number = HMM_PRIVATE_DEVICE_ONE,
+};
+
+FIXTURE_VARIANT_ADD(hmm, hmm_device_coherent)
+{
+ .device_number = HMM_COHERENCE_DEVICE_ONE,
+};
+
FIXTURE(hmm2)
{
int fd0;
@@ -68,6 +90,24 @@ FIXTURE(hmm2)
unsigned intpage_shift;
};
+FIXTURE_VARIANT(hmm2)
+{
+ int device_number0;
+ int device_number1;
+};
+
+FIXTURE_VARIANT_ADD(hmm2, hmm2_device_private)
+{
+ .device_number0 = HMM_PRIVATE_DEVICE_ONE,
+ .device_number1 = HMM_PRIVATE_DEVICE_TWO,
+};
+
+FIXTURE_VARIANT_ADD(hmm2, hmm2_device_coherent)
+{
+ .device_number0 = HMM_COHERENCE_DEVICE_ONE,
+ .device_number1 = HMM_COHERENCE_DEVICE_TWO,
+};
+
static int hmm_open(int unit)
{
char pathname[HMM_PATH_MAX];
@@ -81,12 +121,19 @@ static int hmm_open(int unit)
return fd;
}
+static bool hmm_is_coherent_type(int dev_num)
+{
+ return (dev_num >= HMM_COHERENCE_DEVICE_ONE);
+}
+
FIXTURE_SETUP(hmm)
{
self->page_size = sysconf(_SC_PAGE_SIZE);
self->page_shift = ffs(self->page_size) - 1;
- self->fd = hmm_open(0);
+ self->fd = hmm_open(variant->device_number);
+ if (self->fd < 0 && hmm_is_coherent_type(variant->device_number))
+ SKIP(exit(0), "DEVICE_COHERENT not available");
ASSERT_GE(self->fd, 0);
}
@@ -95,9 +142,11 @@ FIXTURE_SETUP(hmm2)
self->page_size = sysconf(_SC_PAGE_SIZE);
self->page_shift = ffs(self->page_size) - 1;
- self->fd0 = hmm_open(0);
+ self->fd0 = hmm_open(variant->device_number0);
+ if (self->fd0 < 0 && hmm_is_coherent_type(variant->device_number0))
+ SKIP(exit(0), "DEVICE_COHERENT not available");
ASSERT_GE(self->fd0, 0);
- self->fd1 = hmm_open(1);
+ self->fd1 = hmm_open(variant->device_number1);
ASSERT_GE(self->fd1, 0);
}
@@ -211,6 +260,20 @@ static void hmm_nanosleep(unsigned int n)
nanosleep(&t, NULL);
}
+static int hmm_migrate_sys_to_dev(int fd,
+ struct hmm_buffer *buffer,
+ unsigned long npages)
+{
+ return hmm_dmirror_cmd(fd, HMM_DMIRROR_MIGRATE_TO_DEV, buffer, npages);
+}
+
+static int hmm_migrate_dev_to_sys(int fd,
+ struct hmm_buffer *buffer,
+ unsigned long npages)
+{
+ return hmm_dmirror_cmd(fd, HMM_DMIRROR_MIGRATE_TO_SYS, buffer, npages);
+}
+
/*
* Simple NULL test of device open/close.
*/
@@ -875,7 +938,7 @@ TEST_F(hmm, migrate)
ptr[i] = i;
/* Migrate memory to device. */
- ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+ ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, 0);
ASSERT_EQ(buffer->cpages, npages);
@@ -923,7 +986,7 @@ TEST_F(hmm, migrate_fault)
ptr[i] = i;
/* Migrate memory to device. */
- ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+ ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, 0);
ASSERT_EQ(buffer->cpages, npages);
@@ -936,7 +999,7 @@ TEST_F(hmm, migrate_fault)
ASSERT_EQ(ptr[i], i);
/* Migrate memory to the device again. */
- ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_MIGRATE, buffer, npages);
+ ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
ASSERT_EQ(ret, 0);
ASSERT_EQ(buffer->cpages,
[PATCH v6 14/14] tools: add selftests to hmm for COW in device memory
The objective is to test device migration mechanism in pages marked
as COW, for private and coherent device type. In case of writing to
COW private page(s), a page fault will migrate pages back to system
memory first. Then, these pages will be duplicated. In case of COW
device coherent type, pages are duplicated directly from device
memory.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
---
tools/testing/selftests/vm/hmm-tests.c | 80 ++
1 file changed, 80 insertions(+)
diff --git a/tools/testing/selftests/vm/hmm-tests.c
b/tools/testing/selftests/vm/hmm-tests.c
index bb38b9777610..716b62c05e3d 100644
--- a/tools/testing/selftests/vm/hmm-tests.c
+++ b/tools/testing/selftests/vm/hmm-tests.c
@@ -1874,4 +1874,84 @@ TEST_F(hmm, hmm_gup_test)
close(gup_fd);
hmm_buffer_free(buffer);
}
+
+/*
+ * Test copy-on-write in device pages.
+ * In case of writing to COW private page(s), a page fault will migrate pages
+ * back to system memory first. Then, these pages will be duplicated. In case
+ * of COW device coherent type, pages are duplicated directly from device
+ * memory.
+ */
+TEST_F(hmm, hmm_cow_in_device)
+{
+ struct hmm_buffer *buffer;
+ unsigned long npages;
+ unsigned long size;
+ unsigned long i;
+ int *ptr;
+ int ret;
+ unsigned char *m;
+ pid_t pid;
+ int status;
+
+ npages = 4;
+ size = npages << self->page_shift;
+
+ buffer = malloc(sizeof(*buffer));
+ ASSERT_NE(buffer, NULL);
+
+ buffer->fd = -1;
+ buffer->size = size;
+ buffer->mirror = malloc(size);
+ ASSERT_NE(buffer->mirror, NULL);
+
+ buffer->ptr = mmap(NULL, size,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS,
+ buffer->fd, 0);
+ ASSERT_NE(buffer->ptr, MAP_FAILED);
+
+ /* Initialize buffer in system memory. */
+ for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i)
+ ptr[i] = i;
+
+ /* Migrate memory to device. */
+
+ ret = hmm_migrate_sys_to_dev(self->fd, buffer, npages);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(buffer->cpages, npages);
+
+ pid = fork();
+ if (pid == -1)
+ ASSERT_EQ(pid, 0);
+ if (!pid) {
+ /* Child process waitd for SIGTERM from the parent. */
+ while (1) {
+ }
+ perror("Should not reach this\n");
+ exit(0);
+ }
+ /* Parent process writes to COW pages(s) and gets a
+* new copy in system. In case of device private pages,
+* this write causes a migration to system mem first.
+*/
+ for (i = 0, ptr = buffer->ptr; i < size / sizeof(*ptr); ++i)
+ ptr[i] = i;
+
+ /* Terminate child and wait */
+ EXPECT_EQ(0, kill(pid, SIGTERM));
+ EXPECT_EQ(pid, waitpid(pid, &status, 0));
+ EXPECT_NE(0, WIFSIGNALED(status));
+ EXPECT_EQ(SIGTERM, WTERMSIG(status));
+
+ /* Take snapshot to CPU pagetables */
+ ret = hmm_dmirror_cmd(self->fd, HMM_DMIRROR_SNAPSHOT, buffer, npages);
+ ASSERT_EQ(ret, 0);
+ ASSERT_EQ(buffer->cpages, npages);
+ m = buffer->mirror;
+ for (i = 0; i < npages; i++)
+ ASSERT_EQ(HMM_DMIRROR_PROT_WRITE, m[i]);
+
+ hmm_buffer_free(buffer);
+}
TEST_HARNESS_MAIN
--
2.32.0
[PATCH v6 08/14] lib: test_hmm add ioctl to get zone device type
new ioctl cmd added to query zone device type. This will be
used once the test_hmm adds zone device coherent type.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Poppple
Signed-off-by: Christoph Hellwig
---
lib/test_hmm.c | 11 +--
lib/test_hmm_uapi.h | 14 ++
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index cfe632047839..915ef6b5b0d4 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -87,6 +87,7 @@ struct dmirror_chunk {
struct dmirror_device {
struct cdev cdevice;
struct hmm_devmem *devmem;
+ unsigned intzone_device_type;
unsigned intdevmem_capacity;
unsigned intdevmem_count;
@@ -1260,14 +1261,20 @@ static void dmirror_device_remove(struct dmirror_device
*mdevice)
static int __init hmm_dmirror_init(void)
{
int ret;
- int id;
+ int id = 0;
+ int ndevices = 0;
ret = alloc_chrdev_region(&dmirror_dev, 0, DMIRROR_NDEVICES,
"HMM_DMIRROR");
if (ret)
goto err_unreg;
- for (id = 0; id < DMIRROR_NDEVICES; id++) {
+ memset(dmirror_devices, 0, DMIRROR_NDEVICES *
sizeof(dmirror_devices[0]));
+ dmirror_devices[ndevices++].zone_device_type =
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
+ dmirror_devices[ndevices++].zone_device_type =
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
+ for (id = 0; id < ndevices; id++) {
ret = dmirror_device_init(dmirror_devices + id, id);
if (ret)
goto err_chrdev;
diff --git a/lib/test_hmm_uapi.h b/lib/test_hmm_uapi.h
index f14dea5dcd06..0511af7464ee 100644
--- a/lib/test_hmm_uapi.h
+++ b/lib/test_hmm_uapi.h
@@ -31,10 +31,11 @@ struct hmm_dmirror_cmd {
/* Expose the address space of the calling process through hmm device file */
#define HMM_DMIRROR_READ _IOWR('H', 0x00, struct hmm_dmirror_cmd)
#define HMM_DMIRROR_WRITE _IOWR('H', 0x01, struct hmm_dmirror_cmd)
-#define HMM_DMIRROR_MIGRATE_IOWR('H', 0x02, struct hmm_dmirror_cmd)
-#define HMM_DMIRROR_SNAPSHOT _IOWR('H', 0x03, struct hmm_dmirror_cmd)
-#define HMM_DMIRROR_EXCLUSIVE _IOWR('H', 0x04, struct hmm_dmirror_cmd)
-#define HMM_DMIRROR_CHECK_EXCLUSIVE_IOWR('H', 0x05, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_MIGRATE_TO_DEV _IOWR('H', 0x02, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_MIGRATE_TO_SYS _IOWR('H', 0x03, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_SNAPSHOT _IOWR('H', 0x04, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_EXCLUSIVE _IOWR('H', 0x05, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_CHECK_EXCLUSIVE_IOWR('H', 0x06, struct hmm_dmirror_cmd)
/*
* Values returned in hmm_dmirror_cmd.ptr for HMM_DMIRROR_SNAPSHOT.
@@ -62,4 +63,9 @@ enum {
HMM_DMIRROR_PROT_DEV_PRIVATE_REMOTE = 0x30,
};
+enum {
+ /* 0 is reserved to catch uninitialized type fields */
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE = 1,
+};
+
#endif /* _LIB_TEST_HMM_UAPI_H */
--
2.32.0
[PATCH v6 07/14] drm/amdkfd: add SPM support for SVM
When CPU is connected throug XGMI, it has coherent
access to VRAM resource. In this case that resource
is taken from a table in the device gmc aperture base.
This resource is used along with the device type, which could
be DEVICE_PRIVATE or DEVICE_COHERENT to create the device
page map region.
Also, MIGRATE_VMA_SELECT_DEVICE_COHERENT flag is selected for
coherent type case during migration to device.
Signed-off-by: Alex Sierra
Reviewed-by: Felix Kuehling
Signed-off-by: Christoph Hellwig
---
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 34 +++-
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index e44376c2ecdc..f73e3e340413 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -671,13 +671,15 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct
svm_range *prange,
migrate.vma = vma;
migrate.start = start;
migrate.end = end;
- migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
migrate.pgmap_owner = SVM_ADEV_PGMAP_OWNER(adev);
+ if (adev->gmc.xgmi.connected_to_cpu)
+ migrate.flags = MIGRATE_VMA_SELECT_DEVICE_COHERENT;
+ else
+ migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
buf = kvcalloc(npages,
2 * sizeof(*migrate.src) + sizeof(uint64_t) +
sizeof(dma_addr_t),
GFP_KERNEL);
-
if (!buf)
goto out;
@@ -947,7 +949,7 @@ int svm_migrate_init(struct amdgpu_device *adev)
{
struct kfd_dev *kfddev = adev->kfd.dev;
struct dev_pagemap *pgmap;
- struct resource *res;
+ struct resource *res = NULL;
unsigned long size;
void *r;
@@ -962,28 +964,34 @@ int svm_migrate_init(struct amdgpu_device *adev)
* should remove reserved size
*/
size = ALIGN(adev->gmc.real_vram_size, 2ULL << 20);
- res = devm_request_free_mem_region(adev->dev, &iomem_resource, size);
- if (IS_ERR(res))
- return -ENOMEM;
+ if (adev->gmc.xgmi.connected_to_cpu) {
+ pgmap->range.start = adev->gmc.aper_base;
+ pgmap->range.end = adev->gmc.aper_base + adev->gmc.aper_size -
1;
+ pgmap->type = MEMORY_DEVICE_COHERENT;
+ } else {
+ res = devm_request_free_mem_region(adev->dev, &iomem_resource,
size);
+ if (IS_ERR(res))
+ return -ENOMEM;
+ pgmap->range.start = res->start;
+ pgmap->range.end = res->end;
+ pgmap->type = MEMORY_DEVICE_PRIVATE;
+ }
- pgmap->type = MEMORY_DEVICE_PRIVATE;
pgmap->nr_range = 1;
- pgmap->range.start = res->start;
- pgmap->range.end = res->end;
pgmap->ops = &svm_migrate_pgmap_ops;
pgmap->owner = SVM_ADEV_PGMAP_OWNER(adev);
- pgmap->flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
-
+ pgmap->flags = 0;
/* Device manager releases device-specific resources, memory region and
* pgmap when driver disconnects from device.
*/
r = devm_memremap_pages(adev->dev, pgmap);
if (IS_ERR(r)) {
pr_err("failed to register HMM device memory\n");
-
/* Disable SVM support capability */
pgmap->type = 0;
- devm_release_mem_region(adev->dev, res->start,
resource_size(res));
+ if (pgmap->type == MEMORY_DEVICE_PRIVATE)
+ devm_release_mem_region(adev->dev, res->start,
+ res->end - res->start + 1);
return PTR_ERR(r);
}
--
2.32.0
[PATCH v6 06/14] mm: add device coherent checker to is_pinnable_page
is_device_coherent checker was added to is_pinnable_page and renamed
to is_longterm_pinnable_page. The reason is that device coherent
pages are not supported for longterm pinning.
Signed-off-by: Alex Sierra
---
include/linux/memremap.h | 25 +
include/linux/mm.h | 24
mm/gup.c | 5 ++---
mm/gup_test.c| 4 ++--
mm/hugetlb.c | 2 +-
5 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 9f752ebed613..6fc0ced64b2d 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -169,6 +169,31 @@ static inline bool is_pci_p2pdma_page(const struct page
*page)
page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
}
+/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin pages */
+#ifdef CONFIG_MIGRATION
+static inline bool is_longterm_pinnable_page(struct page *page)
+{
+#ifdef CONFIG_CMA
+ int mt = get_pageblock_migratetype(page);
+
+ if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
+ return false;
+#endif
+ return !(is_device_coherent_page(page) ||
+is_zone_movable_page(page) ||
+is_zero_pfn(page_to_pfn(page)));
+}
+#else
+static inline bool is_longterm_pinnable_page(struct page *page)
+{
+ return true;
+}
+#endif
+static inline bool folio_is_longterm_pinnable(struct folio *folio)
+{
+ return is_longterm_pinnable_page(&folio->page);
+}
+
#ifdef CONFIG_ZONE_DEVICE
void *memremap_pages(struct dev_pagemap *pgmap, int nid);
void memunmap_pages(struct dev_pagemap *pgmap);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index f6f5d48c1934..b91a4a1f260b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1590,30 +1590,6 @@ static inline bool page_needs_cow_for_dma(struct
vm_area_struct *vma,
return page_maybe_dma_pinned(page);
}
-/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin pages */
-#ifdef CONFIG_MIGRATION
-static inline bool is_pinnable_page(struct page *page)
-{
-#ifdef CONFIG_CMA
- int mt = get_pageblock_migratetype(page);
-
- if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
- return false;
-#endif
- return !is_zone_movable_page(page) || is_zero_pfn(page_to_pfn(page));
-}
-#else
-static inline bool is_pinnable_page(struct page *page)
-{
- return true;
-}
-#endif
-
-static inline bool folio_is_pinnable(struct folio *folio)
-{
- return is_pinnable_page(&folio->page);
-}
-
static inline void set_page_zone(struct page *page, enum zone_type zone)
{
page->flags &= ~(ZONES_MASK << ZONES_PGSHIFT);
diff --git a/mm/gup.c b/mm/gup.c
index c29a7b5fbbfd..ada73b775a82 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -133,8 +133,7 @@ struct folio *try_grab_folio(struct page *page, int refs,
unsigned int flags)
* path.
*/
if (unlikely((flags & FOLL_LONGTERM) &&
-(!is_pinnable_page(page) ||
-is_device_coherent_page(page
+ !is_longterm_pinnable_page(page)))
return NULL;
/*
@@ -1931,7 +1930,7 @@ static long check_and_migrate_movable_pages(unsigned long
nr_pages,
continue;
}
- if (folio_is_pinnable(folio))
+ if (folio_is_longterm_pinnable(folio))
continue;
/*
* Try to move out any movable page before pinning the range.
diff --git a/mm/gup_test.c b/mm/gup_test.c
index d974dec19e1c..9d705ba6737e 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -1,5 +1,5 @@
#include
-#include
+#include
#include
#include
#include
@@ -53,7 +53,7 @@ static void verify_dma_pinned(unsigned int cmd, struct page
**pages,
dump_page(page, "gup_test failure");
break;
} else if (cmd == PIN_LONGTERM_BENCHMARK &&
- WARN(!is_pinnable_page(page),
+ WARN(!is_longterm_pinnable_page(page),
"pages[%lu] is NOT pinnable but pinned\n",
i)) {
dump_page(page, "gup_test failure");
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a57e1be41401..368fd33787b0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1135,7 +1135,7 @@ static struct page *dequeue_huge_page_node_exact(struct
hstate *h, int nid)
lockdep_assert_held(&hugetlb_lock);
list_for_each_entry(page, &h->hugepage_freelists[nid], lru) {
- if (pin && !is_pinnable_page(page))
+ if (pin && !is_longterm_pinnable_page(page))
continue;
if (PageHWPoison(page))
--
2.32.0
[PATCH v6 09/14] lib: test_hmm add module param for zone device type
In order to configure device coherent in test_hmm, two module parameters
should be passed, which correspond to the SP start address of each
device (2) spm_addr_dev0 & spm_addr_dev1. If no parameters are passed,
private device type is configured.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Poppple
Signed-off-by: Christoph Hellwig
---
lib/test_hmm.c | 73 -
lib/test_hmm_uapi.h | 1 +
2 files changed, 53 insertions(+), 21 deletions(-)
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 915ef6b5b0d4..afb30af9f3ff 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -37,6 +37,16 @@
#define DEVMEM_CHUNK_SIZE (256 * 1024 * 1024U)
#define DEVMEM_CHUNKS_RESERVE 16
+static unsigned long spm_addr_dev0;
+module_param(spm_addr_dev0, long, 0644);
+MODULE_PARM_DESC(spm_addr_dev0,
+ "Specify start address for SPM (special purpose memory) used
for device 0. By setting this Coherent device type will be used. Make sure
spm_addr_dev1 is set too. Minimum SPM size should be DEVMEM_CHUNK_SIZE.");
+
+static unsigned long spm_addr_dev1;
+module_param(spm_addr_dev1, long, 0644);
+MODULE_PARM_DESC(spm_addr_dev1,
+ "Specify start address for SPM (special purpose memory) used
for device 1. By setting this Coherent device type will be used. Make sure
spm_addr_dev0 is set too. Minimum SPM size should be DEVMEM_CHUNK_SIZE.");
+
static const struct dev_pagemap_ops dmirror_devmem_ops;
static const struct mmu_interval_notifier_ops dmirror_min_ops;
static dev_t dmirror_dev;
@@ -455,28 +465,44 @@ static int dmirror_write(struct dmirror *dmirror, struct
hmm_dmirror_cmd *cmd)
return ret;
}
-static bool dmirror_allocate_chunk(struct dmirror_device *mdevice,
+static int dmirror_allocate_chunk(struct dmirror_device *mdevice,
struct page **ppage)
{
struct dmirror_chunk *devmem;
- struct resource *res;
+ struct resource *res = NULL;
unsigned long pfn;
unsigned long pfn_first;
unsigned long pfn_last;
void *ptr;
+ int ret = -ENOMEM;
devmem = kzalloc(sizeof(*devmem), GFP_KERNEL);
if (!devmem)
- return false;
+ return ret;
- res = request_free_mem_region(&iomem_resource, DEVMEM_CHUNK_SIZE,
- "hmm_dmirror");
- if (IS_ERR(res))
+ switch (mdevice->zone_device_type) {
+ case HMM_DMIRROR_MEMORY_DEVICE_PRIVATE:
+ res = request_free_mem_region(&iomem_resource,
DEVMEM_CHUNK_SIZE,
+ "hmm_dmirror");
+ if (IS_ERR_OR_NULL(res))
+ goto err_devmem;
+ devmem->pagemap.range.start = res->start;
+ devmem->pagemap.range.end = res->end;
+ devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
+ break;
+ case HMM_DMIRROR_MEMORY_DEVICE_COHERENT:
+ devmem->pagemap.range.start = (MINOR(mdevice->cdevice.dev) - 2)
?
+ spm_addr_dev0 :
+ spm_addr_dev1;
+ devmem->pagemap.range.end = devmem->pagemap.range.start +
+ DEVMEM_CHUNK_SIZE - 1;
+ devmem->pagemap.type = MEMORY_DEVICE_COHERENT;
+ break;
+ default:
+ ret = -EINVAL;
goto err_devmem;
+ }
- devmem->pagemap.type = MEMORY_DEVICE_PRIVATE;
- devmem->pagemap.range.start = res->start;
- devmem->pagemap.range.end = res->end;
devmem->pagemap.nr_range = 1;
devmem->pagemap.ops = &dmirror_devmem_ops;
devmem->pagemap.owner = mdevice;
@@ -497,10 +523,14 @@ static bool dmirror_allocate_chunk(struct dmirror_device
*mdevice,
mdevice->devmem_capacity = new_capacity;
mdevice->devmem_chunks = new_chunks;
}
-
ptr = memremap_pages(&devmem->pagemap, numa_node_id());
- if (IS_ERR(ptr))
+ if (IS_ERR_OR_NULL(ptr)) {
+ if (ptr)
+ ret = PTR_ERR(ptr);
+ else
+ ret = -EFAULT;
goto err_release;
+ }
devmem->mdevice = mdevice;
pfn_first = devmem->pagemap.range.start >> PAGE_SHIFT;
@@ -529,15 +559,17 @@ static bool dmirror_allocate_chunk(struct dmirror_device
*mdevice,
}
spin_unlock(&mdevice->lock);
- return true;
+ return 0;
err_release:
mutex_unlock(&mdevice->devmem_lock);
- release_mem_region(devmem->pagemap.range.start,
range_len(&devmem->pagemap.range));
+ if (res && devmem->pagemap.type == MEMORY_DEVICE_PRIVATE)
+ release_mem_region(devmem->pagemap.range.start,
+ range_len(&devmem->pagemap.range));
[PATCH v6 03/14] mm: add device coherent vma selection for memory migration
This case is used to migrate pages from device memory, back to system
memory. Device coherent type memory is cache coherent from device and CPU
point of view.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Poppple
Signed-off-by: Christoph Hellwig
---
include/linux/migrate.h | 1 +
mm/migrate_device.c | 12 +---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 069a89e847f3..b84908debe5c 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -148,6 +148,7 @@ static inline unsigned long migrate_pfn(unsigned long pfn)
enum migrate_vma_direction {
MIGRATE_VMA_SELECT_SYSTEM = 1 << 0,
MIGRATE_VMA_SELECT_DEVICE_PRIVATE = 1 << 1,
+ MIGRATE_VMA_SELECT_DEVICE_COHERENT = 1 << 2,
};
struct migrate_vma {
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index a4847ad65da3..18bc6483f63a 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -148,15 +148,21 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
if (is_writable_device_private_entry(entry))
mpfn |= MIGRATE_PFN_WRITE;
} else {
- if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
- goto next;
pfn = pte_pfn(pte);
- if (is_zero_pfn(pfn)) {
+ if (is_zero_pfn(pfn) &&
+ (migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) {
mpfn = MIGRATE_PFN_MIGRATE;
migrate->cpages++;
goto next;
}
page = vm_normal_page(migrate->vma, addr, pte);
+ if (page && !is_zone_device_page(page) &&
+ !(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
+ goto next;
+ else if (page && is_device_coherent_page(page) &&
+ (!(migrate->flags &
MIGRATE_VMA_SELECT_DEVICE_COHERENT) ||
+page->pgmap->owner != migrate->pgmap_owner))
+ goto next;
mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
}
--
2.32.0
[PATCH v6 04/14] mm: remove the vma check in migrate_vma_setup()
From: Alistair Popple
migrate_vma_setup() checks that a valid vma is passed so that the page
tables can be walked to find the pfns associated with a given address
range. However in some cases the pfns are already known, such as when
migrating device coherent pages during pin_user_pages() meaning a valid
vma isn't required.
Signed-off-by: Alistair Popple
Acked-by: Felix Kuehling
Signed-off-by: Christoph Hellwig
---
mm/migrate_device.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 18bc6483f63a..cf9668376c5a 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -486,24 +486,24 @@ int migrate_vma_setup(struct migrate_vma *args)
args->start &= PAGE_MASK;
args->end &= PAGE_MASK;
- if (!args->vma || is_vm_hugetlb_page(args->vma) ||
- (args->vma->vm_flags & VM_SPECIAL) || vma_is_dax(args->vma))
- return -EINVAL;
- if (nr_pages <= 0)
- return -EINVAL;
- if (args->start < args->vma->vm_start ||
- args->start >= args->vma->vm_end)
- return -EINVAL;
- if (args->end <= args->vma->vm_start || args->end > args->vma->vm_end)
- return -EINVAL;
if (!args->src || !args->dst)
return -EINVAL;
-
- memset(args->src, 0, sizeof(*args->src) * nr_pages);
- args->cpages = 0;
- args->npages = 0;
-
- migrate_vma_collect(args);
+ if (args->vma) {
+ if (is_vm_hugetlb_page(args->vma) ||
+ (args->vma->vm_flags & VM_SPECIAL) || vma_is_dax(args->vma))
+ return -EINVAL;
+ if (args->start < args->vma->vm_start ||
+ args->start >= args->vma->vm_end)
+ return -EINVAL;
+ if (args->end <= args->vma->vm_start ||
+ args->end > args->vma->vm_end)
+ return -EINVAL;
+ memset(args->src, 0, sizeof(*args->src) * nr_pages);
+ args->cpages = 0;
+ args->npages = 0;
+
+ migrate_vma_collect(args);
+ }
if (args->cpages)
migrate_vma_unmap(args);
@@ -685,7 +685,7 @@ void migrate_vma_pages(struct migrate_vma *migrate)
continue;
}
- if (!page) {
+ if (!page && migrate->vma) {
if (!(migrate->src[i] & MIGRATE_PFN_MIGRATE))
continue;
if (!notified) {
--
2.32.0
[PATCH v6 05/14] mm/gup: migrate device coherent pages when pinning instead of failing
From: Alistair Popple
Currently any attempts to pin a device coherent page will fail. This is
because device coherent pages need to be managed by a device driver, and
pinning them would prevent a driver from migrating them off the device.
However this is no reason to fail pinning of these pages. These are
coherent and accessible from the CPU so can be migrated just like
pinning ZONE_MOVABLE pages. So instead of failing all attempts to pin
them first try migrating them out of ZONE_DEVICE.
Signed-off-by: Alistair Popple
Acked-by: Felix Kuehling
[hch: rebased to the split device memory checks,
moved migrate_device_page to migrate_device.c]
Signed-off-by: Christoph Hellwig
---
mm/gup.c| 50 +-
mm/internal.h | 1 +
mm/migrate_device.c | 53 +
3 files changed, 98 insertions(+), 6 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 48b45bcc8501..c29a7b5fbbfd 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -133,7 +133,8 @@ struct folio *try_grab_folio(struct page *page, int refs,
unsigned int flags)
* path.
*/
if (unlikely((flags & FOLL_LONGTERM) &&
-!is_pinnable_page(page)))
+(!is_pinnable_page(page) ||
+is_device_coherent_page(page
return NULL;
/*
@@ -1895,9 +1896,43 @@ static long check_and_migrate_movable_pages(unsigned
long nr_pages,
continue;
prev_folio = folio;
- if (folio_is_pinnable(folio))
+ /*
+* Device private pages will get faulted in during gup so it
+* shouldn't be possible to see one here.
+*/
+ if (WARN_ON_ONCE(folio_is_device_private(folio))) {
+ ret = -EFAULT;
+ goto unpin_pages;
+ }
+
+ /*
+* Device coherent pages are managed by a driver and should not
+* be pinned indefinitely as it prevents the driver moving the
+* page. So when trying to pin with FOLL_LONGTERM instead try
+* to migrate the page out of device memory.
+*/
+ if (folio_is_device_coherent(folio)) {
+ WARN_ON_ONCE(PageCompound(&folio->page));
+
+ /*
+* Migration will fail if the page is pinned, so convert
+* the pin on the source page to a normal reference.
+*/
+ if (gup_flags & FOLL_PIN) {
+ get_page(&folio->page);
+ unpin_user_page(&folio->page);
+ }
+
+ pages[i] = migrate_device_page(&folio->page, gup_flags);
+ if (!pages[i]) {
+ ret = -EBUSY;
+ goto unpin_pages;
+ }
continue;
+ }
+ if (folio_is_pinnable(folio))
+ continue;
/*
* Try to move out any movable page before pinning the range.
*/
@@ -1933,10 +1968,13 @@ static long check_and_migrate_movable_pages(unsigned
long nr_pages,
return nr_pages;
unpin_pages:
- if (gup_flags & FOLL_PIN) {
- unpin_user_pages(pages, nr_pages);
- } else {
- for (i = 0; i < nr_pages; i++)
+ for (i = 0; i < nr_pages; i++) {
+ if (!pages[i])
+ continue;
+
+ if (gup_flags & FOLL_PIN)
+ unpin_user_page(pages[i]);
+ else
put_page(pages[i]);
}
diff --git a/mm/internal.h b/mm/internal.h
index c0f8fbe0445b..eeab4ee7a4a3 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -853,6 +853,7 @@ int numa_migrate_prep(struct page *page, struct
vm_area_struct *vma,
unsigned long addr, int page_nid, int *flags);
void free_zone_device_page(struct page *page);
+struct page *migrate_device_page(struct page *page, unsigned int gup_flags);
/*
* mm/gup.c
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index cf9668376c5a..5decd26dd551 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -794,3 +794,56 @@ void migrate_vma_finalize(struct migrate_vma *migrate)
}
}
EXPORT_SYMBOL(migrate_vma_finalize);
+
+/*
+ * Migrate a device coherent page back to normal memory. The caller should
have
+ * a reference on page which will be copied to the new page if migration is
+ * successful or dropped on failure.
+ */
+struct page *migrate_device_page(struct page *page, unsigned int gup_flags)
+{
+ unsigned long src_pfn, dst_pfn = 0;
+ struct migrate_vma ar
[PATCH v6 02/14] mm: handling Non-LRU pages returned by vm_normal_pages
With DEVICE_COHERENT, we'll soon have vm_normal_pages() return
device-managed anonymous pages that are not LRU pages. Although they
behave like normal pages for purposes of mapping in CPU page, and for
COW. They do not support LRU lists, NUMA migration or THP.
We also introduced a FOLL_LRU flag that adds the same behaviour to
follow_page and related APIs, to allow callers to specify that they
expect to put pages on an LRU list.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Popple
---
fs/proc/task_mmu.c | 2 +-
include/linux/mm.h | 3 ++-
mm/gup.c | 6 +-
mm/huge_memory.c | 2 +-
mm/khugepaged.c| 9 ++---
mm/ksm.c | 6 +++---
mm/madvise.c | 4 ++--
mm/memory.c| 9 -
mm/mempolicy.c | 2 +-
mm/migrate.c | 4 ++--
mm/mlock.c | 2 +-
mm/mprotect.c | 2 +-
12 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2d04e3470d4c..2dd8c8a66924 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1792,7 +1792,7 @@ static struct page *can_gather_numa_stats(pte_t pte,
struct vm_area_struct *vma,
return NULL;
page = vm_normal_page(vma, addr, pte);
- if (!page)
+ if (!page || is_zone_device_page(page))
return NULL;
if (PageReserved(page))
diff --git a/include/linux/mm.h b/include/linux/mm.h
index cf3d0d673f6b..f6f5d48c1934 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -601,7 +601,7 @@ struct vm_operations_struct {
#endif
/*
* Called by vm_normal_page() for special PTEs to find the
-* page for @addr. This is useful if the default behavior
+* page for @addr. This is useful if the default behavior
* (using pte_page()) would not find the correct page.
*/
struct page *(*find_special_page)(struct vm_area_struct *vma,
@@ -2934,6 +2934,7 @@ struct page *follow_page(struct vm_area_struct *vma,
unsigned long address,
#define FOLL_NUMA 0x200 /* force NUMA hinting page fault */
#define FOLL_MIGRATION 0x400 /* wait for page to replace migration entry */
#define FOLL_TRIED 0x800 /* a retry, previous pass started an IO */
+#define FOLL_LRU0x1000 /* return only LRU (anon or page cache) */
#define FOLL_REMOTE0x2000 /* we are working on non-current tsk/mm */
#define FOLL_COW 0x4000 /* internal GUP flag */
#define FOLL_ANON 0x8000 /* don't do file mappings */
diff --git a/mm/gup.c b/mm/gup.c
index 551264407624..48b45bcc8501 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -532,7 +532,11 @@ static struct page *follow_page_pte(struct vm_area_struct
*vma,
}
page = vm_normal_page(vma, address, pte);
- if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN))) {
+ if ((flags & FOLL_LRU) && ((page && is_zone_device_page(page)) ||
+ (!page && pte_devmap(pte {
+ page = ERR_PTR(-EEXIST);
+ goto out;
+ } else if (!page && pte_devmap(pte) && (flags & (FOLL_GET | FOLL_PIN)))
{
/*
* Only return device mapping pages in the FOLL_GET or FOLL_PIN
* case since they are only valid while holding the pgmap
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 834f288b3769..d242184ab169 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2906,7 +2906,7 @@ static int split_huge_pages_pid(int pid, unsigned long
vaddr_start,
}
/* FOLL_DUMP to ignore special (like zero) pages */
- page = follow_page(vma, addr, FOLL_GET | FOLL_DUMP);
+ page = follow_page(vma, addr, FOLL_GET | FOLL_DUMP | FOLL_LRU);
if (IS_ERR(page))
continue;
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 16be62d493cd..671ac7800e53 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -618,7 +618,7 @@ static int __collapse_huge_page_isolate(struct
vm_area_struct *vma,
goto out;
}
page = vm_normal_page(vma, address, pteval);
- if (unlikely(!page)) {
+ if (unlikely(!page) || unlikely(is_zone_device_page(page))) {
result = SCAN_PAGE_NULL;
goto out;
}
@@ -1267,7 +1267,7 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
writable = true;
page = vm_normal_page(vma, _address, pteval);
- if (unlikely(!page)) {
+ if (unlikely(!page) || unlikely(is_zone_device_page(page))) {
result = SCAN_PAGE_NULL;
goto out_unmap;
}
@@ -1479,7 +1479,8 @@ void collapse_pte_mapped_thp(struct mm_struct *mm,
unsigned long addr)
goto abort;
page = vm_normal_page(vma, addr, *pte);
-
+ if (WARN_ON
[PATCH v6 01/14] mm: add zone device coherent type memory support
Device memory that is cache coherent from device and CPU point of view.
This is used on platforms that have an advanced system bus (like CAPI
or CXL). Any page of a process can be migrated to such memory. However,
no one should be allowed to pin such memory so that it can always be
evicted.
Signed-off-by: Alex Sierra
Acked-by: Felix Kuehling
Reviewed-by: Alistair Popple
[hch: rebased ontop of the refcount changes,
removed is_dev_private_or_coherent_page]
Signed-off-by: Christoph Hellwig
---
include/linux/memremap.h | 19 +++
mm/memcontrol.c | 7 ---
mm/memory-failure.c | 8 ++--
mm/memremap.c| 10 ++
mm/migrate_device.c | 16 +++-
mm/rmap.c| 5 +++--
6 files changed, 49 insertions(+), 16 deletions(-)
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 8af304f6b504..9f752ebed613 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -41,6 +41,13 @@ struct vmem_altmap {
* A more complete discussion of unaddressable memory may be found in
* include/linux/hmm.h and Documentation/vm/hmm.rst.
*
+ * MEMORY_DEVICE_COHERENT:
+ * Device memory that is cache coherent from device and CPU point of view. This
+ * is used on platforms that have an advanced system bus (like CAPI or CXL). A
+ * driver can hotplug the device memory using ZONE_DEVICE and with that memory
+ * type. Any page of a process can be migrated to such memory. However no one
+ * should be allowed to pin such memory so that it can always be evicted.
+ *
* MEMORY_DEVICE_FS_DAX:
* Host memory that has similar access semantics as System RAM i.e. DMA
* coherent and supports page pinning. In support of coordinating page
@@ -61,6 +68,7 @@ struct vmem_altmap {
enum memory_type {
/* 0 is reserved to catch uninitialized type fields */
MEMORY_DEVICE_PRIVATE = 1,
+ MEMORY_DEVICE_COHERENT,
MEMORY_DEVICE_FS_DAX,
MEMORY_DEVICE_GENERIC,
MEMORY_DEVICE_PCI_P2PDMA,
@@ -143,6 +151,17 @@ static inline bool folio_is_device_private(const struct
folio *folio)
return is_device_private_page(&folio->page);
}
+static inline bool is_device_coherent_page(const struct page *page)
+{
+ return is_zone_device_page(page) &&
+ page->pgmap->type == MEMORY_DEVICE_COHERENT;
+}
+
+static inline bool folio_is_device_coherent(const struct folio *folio)
+{
+ return is_device_coherent_page(&folio->page);
+}
+
static inline bool is_pci_p2pdma_page(const struct page *page)
{
return IS_ENABLED(CONFIG_PCI_P2PDMA) &&
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 618c366a2f07..5d37a85c67da 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5665,8 +5665,8 @@ static int mem_cgroup_move_account(struct page *page,
* 2(MC_TARGET_SWAP): if the swap entry corresponding to this pte is a
* target for charge migration. if @target is not NULL, the entry is stored
* in target->ent.
- * 3(MC_TARGET_DEVICE): like MC_TARGET_PAGE but page is
MEMORY_DEVICE_PRIVATE
- * (so ZONE_DEVICE page and thus not on the lru).
+ * 3(MC_TARGET_DEVICE): like MC_TARGET_PAGE but page is device memory and
+ * thus not on the lru.
* For now we such page is charge like a regular page would be as for all
* intent and purposes it is just special memory taking the place of a
* regular page.
@@ -5704,7 +5704,8 @@ static enum mc_target_type get_mctgt_type(struct
vm_area_struct *vma,
*/
if (page_memcg(page) == mc.from) {
ret = MC_TARGET_PAGE;
- if (is_device_private_page(page))
+ if (is_device_private_page(page) ||
+ is_device_coherent_page(page))
ret = MC_TARGET_DEVICE;
if (target)
target->page = page;
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index da39ec8afca8..79f175eeb190 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1685,12 +1685,16 @@ static int memory_failure_dev_pagemap(unsigned long
pfn, int flags,
goto unlock;
}
- if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
+ switch (pgmap->type) {
+ case MEMORY_DEVICE_PRIVATE:
+ case MEMORY_DEVICE_COHERENT:
/*
-* TODO: Handle HMM pages which may need coordination
+* TODO: Handle device pages which may need coordination
* with device-side memory.
*/
goto unlock;
+ default:
+ break;
}
/*
diff --git a/mm/memremap.c b/mm/memremap.c
index b870a659eee6..0f8f08f8a991 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -315,6 +315,16 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
return ERR_PTR(-EINVAL);
}
break;
[PATCH v6 00/14] Add MEMORY_DEVICE_COHERENT for coherent device memory mapping
This is our MEMORY_DEVICE_COHERENT patch series rebased and updated for current 5.19.0-rc4 Changes since the last version: - Fixed problems with migration during long-term pinning in get_user_pages - Open coded vm_normal_lru_pages as suggested in previous code review - Update hmm_gup_test with more get_user_pages calls, include hmm_cow_in_device in hmm-test. This patch series introduces MEMORY_DEVICE_COHERENT, a type of memory owned by a device that can be mapped into CPU page tables like MEMORY_DEVICE_GENERIC and can also be migrated like MEMORY_DEVICE_PRIVATE. This patch series is mostly self-contained except for a few places where it needs to update other subsystems to handle the new memory type. System stability and performance are not affected according to our ongoing testing, including xfstests. How it works: The system BIOS advertises the GPU device memory (aka VRAM) as SPM (special purpose memory) in the UEFI system address map. The amdgpu driver registers the memory with devmap as MEMORY_DEVICE_COHERENT using devm_memremap_pages. The initial user for this hardware page migration capability is the Frontier supercomputer project. This functionality is not AMD-specific. We expect other GPU vendors to find this functionality useful, and possibly other hardware types in the future. Our test nodes in the lab are similar to the Frontier configuration, with .5 TB of system memory plus 256 GB of device memory split across 4 GPUs, all in a single coherent address space. Page migration is expected to improve application efficiency significantly. We will report empirical results as they become available. Coherent device type pages at gup are now migrated back to system memory if they are being pinned long-term (FOLL_LONGTERM). The reason is, that long-term pinning would interfere with the device memory manager owning the device-coherent pages (e.g. evictions in TTM). These series incorporate Alistair Popple patches to do this migration from pin_user_pages() calls. hmm_gup_test has been added to hmm-test to test different get user pages calls. This series includes handling of device-managed anonymous pages returned by vm_normal_pages. Although they behave like normal pages for purposes of mapping in CPU page tables and for COW, they do not support LRU lists, NUMA migration or THP. We also introduced a FOLL_LRU flag that adds the same behaviour to follow_page and related APIs, to allow callers to specify that they expect to put pages on an LRU list. v2: - Rebase to latest 5.18-rc7. - Drop patch "mm: add device coherent checker to remove migration pte" and modify try_to_migrate_one, to let DEVICE_COHERENT pages fall through to normal page path. Based on Alistair Popple's comment. - Fix comment formatting. - Reword comment in vm_normal_page about pte_devmap(). - Merge "drm/amdkfd: coherent type as sys mem on migration to ram" to "drm/amdkfd: add SPM support for SVM". v3: - Rebase to latest 5.18.0. - Patch "mm: handling Non-LRU pages returned by vm_normal_pages" reordered. - Add WARN_ON_ONCE for thp device coherent case. v4: - Rebase to latest 5.18.0 - Fix consitency between pages with FOLL_LRU flag set and pte_devmap at follow_page_pte. v5: - Remove unused zone_device_type from lib/test_hmm and selftest/vm/hmm-test.c. v6: - Rebase to 5.19.0-rc4 - Rename is_pinnable_page to is_longterm_pinnable_page and add a coherent device checker. - Add a new gup test to hmm-test to cover fast pinnable case with FOLL_LONGTERM Alex Sierra (12): mm: add zone device coherent type memory support mm: handling Non-LRU pages returned by vm_normal_pages mm: add device coherent vma selection for memory migration mm: add device coherent checker to is_pinnable_page drm/amdkfd: add SPM support for SVM lib: test_hmm add ioctl to get zone device type lib: test_hmm add module param for zone device type lib: add support for device coherent type in test_hmm tools: update hmm-test to support device coherent type tools: update test_hmm script to support SP config tools: add hmm gup tests for device coherent type tools: add selftests to hmm for COW in device memory Alistair Popple (2): mm: remove the vma check in migrate_vma_setup() mm/gup: migrate device coherent pages when pinning instead of failing drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 34 ++- fs/proc/task_mmu.c | 2 +- include/linux/memremap.h | 44 +++ include/linux/migrate.h | 1 + include/linux/mm.h | 27 +- lib/test_hmm.c | 337 +-- lib/test_hmm_uapi.h | 19 +- mm/gup.c | 55 +++- mm/gup_test.c| 4 +- mm/huge_memory.c | 2 +- mm/hugetlb.c | 2 +- mm/internal.h| 1 + mm/khugepaged.c | 9 +- mm/ksm.c
Re: [PATCH] drm/amd/display: change to_dal_irq_source_dnc32() storage class specifier to static
Applied. Thanks!
Alex
On Mon, Jun 27, 2022 at 9:20 AM Aurabindo Pillai
wrote:
>
>
>
> On 2022-06-26 10:46, Tom Rix wrote:
> > sparse reports
> > drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn32/irq_service_dcn32.c:39:20:
> > warning: symbol 'to_dal_irq_source_dcn32' was not declared. Should it be
> > static?
> >
> > to_dal_irq_source_dnc32() is only referenced in irq_service_dnc32.c, so
> > change its
> > storage class specifier to static.
> >
> > Fixes: 0efd4374f6b4 ("drm/amd/display: add dcn32 IRQ changes")
> > Signed-off-by: Tom Rix
> > ---
> > drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
> > b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
> > index 3a213ca2f077..b1012fa1977b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
> > +++ b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
> > @@ -36,7 +36,7 @@
> >
> > #define DCN_BASE__INST0_SEG2 0x34C0
> >
> > -enum dc_irq_source to_dal_irq_source_dcn32(
> > +static enum dc_irq_source to_dal_irq_source_dcn32(
> > struct irq_service *irq_service,
> > uint32_t src_id,
> > uint32_t ext_id)
>
> Reviewed-by: Aurabindo Pillai
Re: [PATCH] drm/amd/display: Remove unused globals FORCE_RATE and FORCE_LANE_COUNT
Applied. Thanks!
Alex
On Mon, Jun 27, 2022 at 9:20 AM Aurabindo Pillai
wrote:
>
>
>
> On 2022-06-26 10:20, Tom Rix wrote:
> > sparse reports
> > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3885:6: warning:
> > symbol 'FORCE_RATE' was not declared. Should it be static?
> > drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3886:10:
> > warning: symbol 'FORCE_LANE_COUNT' was not declared. Should it be static?
> >
> > Neither of thse variables is used in dc_link_dp.c. Reviewing the commit
> > listed in
> > the fixes tag shows neither was used in the original patch. So remove them.
> >
> > Fixes: 265280b99822 ("drm/amd/display: add CLKMGR changes for DCN32/321")
> > Signed-off-by: Tom Rix
> > ---
> > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > index be1dcb0a2a06..f3421f2bd52e 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > @@ -3882,9 +3882,6 @@ static bool decide_mst_link_settings(const struct
> > dc_link *link, struct dc_link_
> > return true;
> > }
> >
> > -bool FORCE_RATE = false;
> > -uint32_t FORCE_LANE_COUNT = 0;
> > -
> > void decide_link_settings(struct dc_stream_state *stream,
> > struct dc_link_settings *link_setting)
> > {
>
>
> Reviewed-by: Aurabindo Pillai
Re: [PATCH 2/2] drm/amdgpu: fix documentation warning
Ping?
On Thu, Jun 23, 2022 at 12:41 PM Alex Deucher wrote:
>
> Fixes this issue:
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:5094: warning: expecting prototype
> for amdgpu_device_gpu_recover_imp(). Prototype was for
> amdgpu_device_gpu_recover() instead
>
> Fixes: cf727044144d ("drm/amdgpu: Rename amdgpu_device_gpu_recover_imp back
> to amdgpu_device_gpu_recover")
> Reported-by: Stephen Rothwell
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f2a4c268ac72..6c0fbc662b3a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5079,7 +5079,7 @@ static inline void
> amdggpu_device_stop_pedning_resets(struct amdgpu_device *adev
>
>
> /**
> - * amdgpu_device_gpu_recover_imp - reset the asic and recover scheduler
> + * amdgpu_device_gpu_recover - reset the asic and recover scheduler
> *
> * @adev: amdgpu_device pointer
> * @job: which job trigger hang
> --
> 2.35.3
>
Re: [PATCH] drm/amd/display: Removed unused variable ret
Applied. Thanks!
Alex
On Fri, Jun 24, 2022 at 9:42 PM Souptick Joarder wrote:
>
> From: "Souptick Joarder (HPE)"
>
> Kernel test robot throws below warning ->
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:
> In function 'dc_link_reduce_mst_payload':
>drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:3782:32:
> warning: variable 'ret' set but not used [-Wunused-but-set-variable]
> 3782 | enum act_return_status ret;
>
> Removed the unused ret variable.
>
> Reported-by: kernel test robot
> Signed-off-by: Souptick Joarder (HPE)
> ---
> drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 55a8f58ee239..445357623d8b 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -3706,7 +3706,6 @@ enum dc_status dc_link_reduce_mst_payload(struct
> pipe_ctx *pipe_ctx, uint32_t bw
> struct fixed31_32 pbn_per_slot;
> struct dp_mst_stream_allocation_table proposed_table = {0};
> uint8_t i;
> - enum act_return_status ret;
> const struct link_hwss *link_hwss = get_link_hwss(link,
> &pipe_ctx->link_res);
> DC_LOGGER_INIT(link->ctx->logger);
>
> @@ -3777,7 +3776,7 @@ enum dc_status dc_link_reduce_mst_payload(struct
> pipe_ctx *pipe_ctx, uint32_t bw
> &link->mst_stream_alloc_table);
>
> /* poll for immediate branch device ACT handled */
> - ret = dm_helpers_dp_mst_poll_for_allocation_change_trigger(
> + dm_helpers_dp_mst_poll_for_allocation_change_trigger(
> stream->ctx,
> stream);
>
> --
> 2.25.1
>
Re: [PATCH] Revert "drm/amdgpu: remove ctx->lock"
On Tue, Jun 21, 2022 at 10:42 AM Luben Tuikov wrote:
>
> This reverts commit e68efb27647f2106d6b545667f35b2ea39746b57.
>
> We see that the bo validate list gets corrupted, in
> amdgpu_cs_list_validate(), the lobj->tv.bo is NULL. Then getting usermm on
> the next line, references a NULL bo and we get a koops.
>
> Bisecting leads to the commit being reverted as the cause of the list
> corruption. See the link below for details of the corruption failure.
>
> Cc: Christian König
> Cc: Andrey Grodzovsky
> Cc: Alex Deucher
> Cc: Vitaly Prosyak
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2048#note_1427539
Looks like this bug is also relevant:
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=216143
> Signed-off-by: Luben Tuikov
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 16 +---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> 3 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 36ac1f1d11e6b4..e619031753b927 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -128,6 +128,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
> *p, union drm_amdgpu_cs
> goto free_chunk;
> }
>
> + mutex_lock(&p->ctx->lock);
> +
> /* skip guilty context job */
> if (atomic_read(&p->ctx->guilty) == 1) {
> ret = -ECANCELED;
> @@ -585,16 +587,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser
> *p,
> }
> }
>
> - /* Move fence waiting after getting reservation lock of
> -* PD root. Then there is no need on a ctx mutex lock.
> -*/
> - r = amdgpu_ctx_wait_prev_fence(p->ctx, p->entity);
> - if (unlikely(r != 0)) {
> - if (r != -ERESTARTSYS)
> - DRM_ERROR("amdgpu_ctx_wait_prev_fence failed.\n");
> - goto error_validate;
> - }
> -
> amdgpu_cs_get_threshold_for_moves(p->adev, &p->bytes_moved_threshold,
> &p->bytes_moved_vis_threshold);
> p->bytes_moved = 0;
> @@ -722,6 +714,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
> *parser, int error,
> dma_fence_put(parser->fence);
>
> if (parser->ctx) {
> + mutex_unlock(&parser->ctx->lock);
> amdgpu_ctx_put(parser->ctx);
> }
> if (parser->bo_list)
> @@ -965,7 +958,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
> if (parser->job->uf_addr && ring->funcs->no_user_fence)
> return -EINVAL;
>
> - return 0;
> + return amdgpu_ctx_wait_prev_fence(parser->ctx, parser->entity);
> }
>
> static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,
> @@ -1384,6 +1377,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> goto out;
>
> r = amdgpu_cs_submit(&parser, cs);
> +
> out:
> amdgpu_cs_parser_fini(&parser, r, reserved_buffers);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 53f9268366f29e..2ef5296216d64d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -286,6 +286,7 @@ static int amdgpu_ctx_init(struct amdgpu_ctx_mgr *mgr,
> int32_t priority,
> kref_init(&ctx->refcount);
> ctx->mgr = mgr;
> spin_lock_init(&ctx->ring_lock);
> + mutex_init(&ctx->lock);
>
> ctx->reset_counter = atomic_read(&mgr->adev->gpu_reset_counter);
> ctx->reset_counter_query = ctx->reset_counter;
> @@ -400,6 +401,7 @@ static void amdgpu_ctx_fini(struct kref *ref)
> drm_dev_exit(idx);
> }
>
> + mutex_destroy(&ctx->lock);
> kfree(ctx);
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 0fa0e56daf67e9..cc7c8afff4144c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -53,6 +53,7 @@ struct amdgpu_ctx {
> boolpreamble_presented;
> int32_t init_priority;
> int32_t override_priority;
> + struct mutexlock;
> atomic_tguilty;
> unsigned long ras_counter_ce;
> unsigned long ras_counter_ue;
>
> base-commit: f4b3c543a2a759ce657de4b6b7e88eaddee85ec2
> --
> 2.36.1.74.g277cf0bc36
>
[PATCH] drm/amdgpu/display: add missing FP_START/END checks dcn32_clk_mgr.c
Properly handle FP code in dcn32_clk_mgr.c.
Fixes: 265280b99822 ("drm/amd/display: add CLKMGR changes for DCN32/321")
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
index 4e8059f20007..72bbe7f18f5d 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c
@@ -288,8 +288,10 @@ void dcn32_init_clocks(struct clk_mgr *clk_mgr_base)
/* Get UCLK, update bounding box */
clk_mgr_base->funcs->get_memclk_states_from_smu(clk_mgr_base);
+ DC_FP_START();
/* WM range table */
dcn32_build_wm_range_table(clk_mgr);
+ DC_FP_END();
}
static void dcn32_update_clocks_update_dtb_dto(struct clk_mgr_internal
*clk_mgr,
@@ -724,9 +726,11 @@ static void dcn32_get_memclk_states_from_smu(struct
clk_mgr *clk_mgr_base)
&num_levels);
clk_mgr_base->bw_params->clk_table.num_entries = num_levels ?
num_levels : 1;
+ DC_FP_START();
/* Refresh bounding box */
clk_mgr_base->ctx->dc->res_pool->funcs->update_bw_bounding_box(
clk_mgr->base.ctx->dc, clk_mgr_base->bw_params);
+ DC_FP_END();
}
static bool dcn32_are_clock_states_equal(struct dc_clocks *a,
--
2.35.3
Re: rcu_sched detected expedited stalls in amdgpu after suspend
On Mon, Jun 27, 2022 at 03:22:24PM -0400, Alex Xu (Hello71) wrote:
> Hi,
>
> Since Linux 5.19-ish, I consistently get these types of errors when
> resuming from S3:
>
> [15652.909157] rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks:
> { 11-... } 7 jiffies s: 9981 root: 0x800/.
> [15652.909162] rcu: blocking rcu_node structures (internal RCU debug):
> [15652.909163] Task dump for CPU 11:
> [15652.909164] task:kworker/u24:65 state:R running task stack:0
> pid:210218 ppid: 2 flags:0x4008
> [15652.909167] Workqueue: events_unbound async_run_entry_fn
> [15652.909172] Call Trace:
> [15652.909173]
> [15652.909174] ? atom_get_src_int+0x38e/0x680
> [15652.909179] ? atom_op_test+0x67/0x190
> [15652.909181] ? amdgpu_atom_execute_table_locked+0x19a/0x300
> [15652.909184] ? atom_op_calltable+0xb1/0x110
> [15652.909186] ? amdgpu_atom_execute_table_locked+0x19a/0x300
> [15652.909189] ? atom_op_calltable+0xb1/0x110
> [15652.909191] ? amdgpu_atom_execute_table_locked+0x19a/0x300
> [15652.909193] ? __switch_to+0x137/0x440
> [15652.909195] ? amdgpu_atom_asic_init+0xe0/0x100
> [15652.909198] ? pci_bus_read_config_dword+0x36/0x50
> [15652.909201] ? amdgpu_device_resume+0x10b/0x3e0
> [15652.909203] ? amdgpu_pmops_resume+0x32/0x60
> [15652.909204] ? pci_pm_suspend+0x2b0/0x2b0
> [15652.909206] ? dpm_run_callback+0x35/0x1f0
> [15652.909209] ? device_resume+0x1ca/0x220
> [15652.909211] ? async_resume+0x19/0xe0
> [15652.909213] ? async_run_entry_fn+0x33/0x120
> [15652.909215] ? process_one_work+0x1d6/0x350
> [15652.909218] ? worker_thread+0x24d/0x480
> [15652.909220] ? kthread+0x137/0x150
> [15652.909221] ? worker_clr_flags+0x40/0x40
> [15652.909224] ? kthread_blkcg+0x30/0x30
> [15652.909226] ? ret_from_fork+0x22/0x30
> [15652.909227]
> [15653.015808] rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks:
> { 11-... } 7 jiffies s: 9985 root: 0x800/.
> [15653.015812] rcu: blocking rcu_node structures (internal RCU debug):
> [15653.015813] Task dump for CPU 11:
> [15653.015813] task:kworker/u24:65 state:R running task stack:0
> pid:210218 ppid: 2 flags:0x4008
> [15653.015816] Workqueue: events_unbound async_run_entry_fn
> [15653.015820] Call Trace:
> [15653.015820]
> [15653.015821] ? amdgpu_cgs_read_register+0x10/0x10
> [15653.015825] ? smu7_copy_bytes_to_smc+0xd4/0x200
> [15653.015828] ? polaris10_program_memory_timing_parameters+0x195/0x1b0
> [15653.015831] ? sysvec_apic_timer_interrupt+0xa/0x80
> [15653.015834] ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
> [15653.015836] ? amdgpu_cgs_destroy_device+0x10/0x10
> [15653.015839] ? sysvec_apic_timer_interrupt+0xa/0x80
> [15653.015841] ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
> [15653.015843] ? amdgpu_cgs_destroy_device+0x10/0x10
> [15653.015846] ? amdgpu_device_rreg+0x8f/0xd0
> [15653.015847] ? phm_wait_for_register_unequal+0x99/0xd0
> [15653.015850] ? smu7_send_msg_to_smc+0x95/0x130
> [15653.015853] ? smum_send_msg_to_smc+0x5d/0xa0
> [15653.015854] ? amdgpu_cgs_read_ind_register+0xa0/0xa0
> [15653.015857] ? smu7_enable_dpm_tasks+0x241f/0x28c0
> [15653.015859] ? hwmgr_resume+0x31/0x70
> [15653.015861] ? amdgpu_device_resume+0x1fa/0x3e0
> [15653.015863] ? amdgpu_pmops_resume+0x32/0x60
> [15653.015864] ? pci_pm_suspend+0x2b0/0x2b0
> [15653.015866] ? dpm_run_callback+0x35/0x1f0
> [15653.015868] ? device_resume+0x1ca/0x220
> [15653.015870] ? async_resume+0x19/0xe0
> [15653.015872] ? async_run_entry_fn+0x33/0x120
> [15653.015874] ? process_one_work+0x1d6/0x350
> [15653.015877] ? worker_thread+0x24d/0x480
> [15653.015878] ? kthread+0x137/0x150
> [15653.015880] ? worker_clr_flags+0x40/0x40
> [15653.015882] ? kthread_blkcg+0x30/0x30
> [15653.015884] ? ret_from_fork+0x22/0x30
> [15653.015886]
>
> I have not noticed any resulting problems. I am reporting this in the
> hope that it is easy to fix the issue and remove the error messages
> which may obscure some future problem.
The usual way this happens is for a task to be spinning. In this case,
that might be due to excessive lock contention on async_lock within the
async_run_entry_fn() function. Or perhaps the problem is in one of the
functions preceded by "?" above.
One way to debug this is to place trace_printk()s or similar in the
functions called out above to track it down.
I bet that the reason this showed up in v5.19 is this guy:
28b3ae426598 ("rcu: Introduce CONFIG_RCU_EXP_CPU_STALL_TIMEOUT")
So do you have CONFIG_RCU_EXP_CPU_STALL_TIMEOUT set to some small
number of milliseconds? If so, you can override this by adjusting the
CONFIG_RCU_EXP_CPU_STALL_TIMEOUT Kconfig option or by booting with a
longer timeout via the rcupdate.rcu_exp_cpu_stall_timeout= kernel boot
parameter.
But if you are running on an Android platform, Uladzislau will be
interested in addressing the underlying issue, so I have added him on CC.
Thanx, Paul
[linux-next:master] BUILD REGRESSION aab35c3d5112df6e329a1a5a5a1881e5c4ca3821
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: aab35c3d5112df6e329a1a5a5a1881e5c4ca3821 Add linux-next specific files for 20220627 Error/Warning: (recently discovered and may have been fixed) arch/powerpc/kernel/interrupt.c:542:55: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body] arch/powerpc/kernel/interrupt.c:542:55: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1025:33: warning: variable 'pre_connection_type' set but not used [-Wunused-but-set-variable] drivers/ntb/test/ntb_perf.c:1145: undefined reference to `__umoddi3' drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: no previous prototype for 'pci_read' [-Wmissing-prototypes] drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: no previous prototype for 'pci_write' [-Wmissing-prototypes] vmlinux.o: warning: objtool: __ct_user_enter+0x8c: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_idle_enter+0x19: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_idle_exit+0x3e: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_irq_enter+0x6a: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_irq_exit+0x6a: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_kernel_enter.constprop.0+0x2a: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_kernel_enter_state+0x2d: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_kernel_exit.constprop.0+0x53: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_kernel_exit_state+0x2d: call to ftrace_likely_update() leaves .noinstr.text section vmlinux.o: warning: objtool: ct_nmi_enter+0x4b: call to ftrace_likely_update() leaves .noinstr.text section Unverified Error/Warning (likely false positive, please contact us if interested): drivers/acpi/scan.c:615:35: warning: Uninitialized variable: acpi_device_bus_id->bus_id [uninitvar] drivers/phy/qualcomm/phy-qcom-qmp-combo.c:1701:19: warning: Value stored to 'qmp' during its initialization is never read [clang-analyzer-deadcode.DeadStores] drivers/soc/mediatek/mtk-mutex.c:799:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/staging/media/zoran/zr36016.c:430:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/staging/media/zoran/zr36050.c:829:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/staging/media/zoran/zr36060.c:869:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/thunderbolt/tmu.c:758:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 sound/soc/sof/intel/mtl.c:547:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link.c:warning:variable-pre_connection_type-set-but-not-used | |-- drivers-pci-endpoint-functions-pci-epf-vntb.c:warning:no-previous-prototype-for-pci_read | `-- drivers-pci-endpoint-functions-pci-epf-vntb.c:warning:no-previous-prototype-for-pci_write |-- arc-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link.c:warning:variable-pre_connection_type-set-but-not-used | |-- drivers-pci-endpoint-functions-pci-epf-vntb.c:warning:no-previous-prototype-for-pci_read | |-- drivers-pci-endpoint-functions-pci-epf-vntb.c:warning:no-previous-prototype-for-pci_write | |-- drivers-soc-mediatek-mtk-mutex.c:internal-compiler-error:in-arc_ifcvt-at-config-arc-arc.c | |-- drivers-staging-media-zoran-zr36016.c:internal-compiler-error:in-arc_ifcvt-at-config-arc-arc.c | |-- drivers-staging-media-zoran-zr36050.c:internal-compiler-error:in-arc_ifcvt-at-config-arc-arc.c | |-- drivers-staging-media-zoran-zr36060.c:internal-compiler-error:in-arc_ifcvt-at-config-arc-arc.c | |-- drivers-thunderbolt-tmu.c:internal-compiler-error:in-arc_ifcvt-at-config-arc-arc.c | `-- sound-soc-sof-intel-mtl.c:internal-compiler-error:in-arc_ifcvt-at-config-arc-arc.c |-- arm-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link.c:warning:variable-pre_connection_type-set-but-not-used | |-- drivers-pci-endpoint-functions-pci-epf-vntb.c:warning:no-previous-prototype-for-pci_read | `-- drivers-pci-endpoint-functions-pci-epf-vntb.c:warning:no-previous-prototype-for-pci_write |-- arm64-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link.c:warning:variable-pre_connection_type-set-but-not-used | |-- drivers-pci-endpoint
rcu_sched detected expedited stalls in amdgpu after suspend
Hi,
Since Linux 5.19-ish, I consistently get these types of errors when
resuming from S3:
[15652.909157] rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: {
11-... } 7 jiffies s: 9981 root: 0x800/.
[15652.909162] rcu: blocking rcu_node structures (internal RCU debug):
[15652.909163] Task dump for CPU 11:
[15652.909164] task:kworker/u24:65 state:R running task stack:0
pid:210218 ppid: 2 flags:0x4008
[15652.909167] Workqueue: events_unbound async_run_entry_fn
[15652.909172] Call Trace:
[15652.909173]
[15652.909174] ? atom_get_src_int+0x38e/0x680
[15652.909179] ? atom_op_test+0x67/0x190
[15652.909181] ? amdgpu_atom_execute_table_locked+0x19a/0x300
[15652.909184] ? atom_op_calltable+0xb1/0x110
[15652.909186] ? amdgpu_atom_execute_table_locked+0x19a/0x300
[15652.909189] ? atom_op_calltable+0xb1/0x110
[15652.909191] ? amdgpu_atom_execute_table_locked+0x19a/0x300
[15652.909193] ? __switch_to+0x137/0x440
[15652.909195] ? amdgpu_atom_asic_init+0xe0/0x100
[15652.909198] ? pci_bus_read_config_dword+0x36/0x50
[15652.909201] ? amdgpu_device_resume+0x10b/0x3e0
[15652.909203] ? amdgpu_pmops_resume+0x32/0x60
[15652.909204] ? pci_pm_suspend+0x2b0/0x2b0
[15652.909206] ? dpm_run_callback+0x35/0x1f0
[15652.909209] ? device_resume+0x1ca/0x220
[15652.909211] ? async_resume+0x19/0xe0
[15652.909213] ? async_run_entry_fn+0x33/0x120
[15652.909215] ? process_one_work+0x1d6/0x350
[15652.909218] ? worker_thread+0x24d/0x480
[15652.909220] ? kthread+0x137/0x150
[15652.909221] ? worker_clr_flags+0x40/0x40
[15652.909224] ? kthread_blkcg+0x30/0x30
[15652.909226] ? ret_from_fork+0x22/0x30
[15652.909227]
[15653.015808] rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: {
11-... } 7 jiffies s: 9985 root: 0x800/.
[15653.015812] rcu: blocking rcu_node structures (internal RCU debug):
[15653.015813] Task dump for CPU 11:
[15653.015813] task:kworker/u24:65 state:R running task stack:0
pid:210218 ppid: 2 flags:0x4008
[15653.015816] Workqueue: events_unbound async_run_entry_fn
[15653.015820] Call Trace:
[15653.015820]
[15653.015821] ? amdgpu_cgs_read_register+0x10/0x10
[15653.015825] ? smu7_copy_bytes_to_smc+0xd4/0x200
[15653.015828] ? polaris10_program_memory_timing_parameters+0x195/0x1b0
[15653.015831] ? sysvec_apic_timer_interrupt+0xa/0x80
[15653.015834] ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
[15653.015836] ? amdgpu_cgs_destroy_device+0x10/0x10
[15653.015839] ? sysvec_apic_timer_interrupt+0xa/0x80
[15653.015841] ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
[15653.015843] ? amdgpu_cgs_destroy_device+0x10/0x10
[15653.015846] ? amdgpu_device_rreg+0x8f/0xd0
[15653.015847] ? phm_wait_for_register_unequal+0x99/0xd0
[15653.015850] ? smu7_send_msg_to_smc+0x95/0x130
[15653.015853] ? smum_send_msg_to_smc+0x5d/0xa0
[15653.015854] ? amdgpu_cgs_read_ind_register+0xa0/0xa0
[15653.015857] ? smu7_enable_dpm_tasks+0x241f/0x28c0
[15653.015859] ? hwmgr_resume+0x31/0x70
[15653.015861] ? amdgpu_device_resume+0x1fa/0x3e0
[15653.015863] ? amdgpu_pmops_resume+0x32/0x60
[15653.015864] ? pci_pm_suspend+0x2b0/0x2b0
[15653.015866] ? dpm_run_callback+0x35/0x1f0
[15653.015868] ? device_resume+0x1ca/0x220
[15653.015870] ? async_resume+0x19/0xe0
[15653.015872] ? async_run_entry_fn+0x33/0x120
[15653.015874] ? process_one_work+0x1d6/0x350
[15653.015877] ? worker_thread+0x24d/0x480
[15653.015878] ? kthread+0x137/0x150
[15653.015880] ? worker_clr_flags+0x40/0x40
[15653.015882] ? kthread_blkcg+0x30/0x30
[15653.015884] ? ret_from_fork+0x22/0x30
[15653.015886]
I have not noticed any resulting problems. I am reporting this in the
hope that it is easy to fix the issue and remove the error messages
which may obscure some future problem.
Thanks,
Alex.
RE: [PATCH 1/1] Revert "drm/amdkfd: Free queue after unmap queue success"
[Public]
> -Original Message-
> From: amd-gfx On Behalf Of Philip
> Yang
> Sent: Monday, June 27, 2022 1:32 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Yang, Philip
> Subject: [PATCH 1/1] Revert "drm/amdkfd: Free queue after unmap queue
> success"
>
> This reverts commit 150c1266d78fbaa0fc5f89461daafae416db1c3e.
>
> This causes KFDTest regression on gfx9, will submit new patch after fixing.
Which test? Also, missing your s-o-b. With that fixed:
Acked-by: Alex Deucher
> ---
> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 28 ---
> .../amd/amdkfd/kfd_process_queue_manager.c| 2 +-
> 2 files changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 21e451acfa59..93a0b6995430 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1881,22 +1881,6 @@ static int destroy_queue_cpsch(struct
> device_queue_manager *dqm,
>
> }
>
> - if (q->properties.is_active) {
> - if (!dqm->dev->shared_resources.enable_mes) {
> - retval = execute_queues_cpsch(dqm,
> -
> KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> - if (retval == -ETIME)
> - qpd->reset_wavefronts = true;
> - } else {
> - retval = remove_queue_mes(dqm, q, qpd);
> - }
> -
> - if (retval)
> - goto failed_unmap_queue;
> -
> - decrement_queue_count(dqm, qpd, q);
> - }
> -
> mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> q->properties.type)];
>
> @@ -1910,6 +1894,17 @@ static int destroy_queue_cpsch(struct
> device_queue_manager *dqm,
>
> list_del(&q->list);
> qpd->queue_count--;
> + if (q->properties.is_active) {
> + if (!dqm->dev->shared_resources.enable_mes) {
> + decrement_queue_count(dqm, qpd, q);
> + retval = execute_queues_cpsch(dqm,
> +
> KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> + if (retval == -ETIME)
> + qpd->reset_wavefronts = true;
> + } else {
> + retval = remove_queue_mes(dqm, q, qpd);
> + }
> + }
>
> /*
>* Unconditionally decrement this counter, regardless of the queue's
> @@ -1926,7 +1921,6 @@ static int destroy_queue_cpsch(struct
> device_queue_manager *dqm,
>
> return retval;
>
> -failed_unmap_queue:
> failed_try_destroy_debugged_queue:
>
> dqm_unlock(dqm);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index a46e2a37b4a6..c9c205df4a14 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -422,6 +422,7 @@ int pqm_destroy_queue(struct
> process_queue_manager *pqm, unsigned int qid)
> }
>
> if (pqn->q) {
> + kfd_procfs_del_queue(pqn->q);
> dqm = pqn->q->device->dqm;
> retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q);
> if (retval) {
> @@ -445,7 +446,6 @@ int pqm_destroy_queue(struct
> process_queue_manager *pqm, unsigned int qid)
> amdgpu_amdkfd_free_gtt_mem(dev->adev,
> pqn->q->wptr_bo);
>
> }
> - kfd_procfs_del_queue(pqn->q);
> uninit_queue(pqn->q);
> }
>
> --
> 2.35.1
RE: [PATCH] drm: amd: amdgpu: fix checkpatch warnings
[AMD Official Use Only - General]
> -Original Message-
> From: Deucher, Alexander
> Sent: Monday, June 27, 2022 2:18 PM
> To: Vijendar Mukunda ; broo...@kernel.org;
> alsa-de...@alsa-project.org; dri-de...@lists.freedesktop.org; amd-
> g...@lists.freedesktop.org
> Cc: Dommati, Sunil-kumar ; David Airlie
> ; Hiregoudar, Basavaraj ;
> Pan, Xinhui ; open list ker...@vger.kernel.org>; Kai-Heng Feng ;
> Daniel Vetter ; Mukunda, Vijendar
> ; Koenig, Christian
>
> Subject: RE: [PATCH] drm: amd: amdgpu: fix checkpatch warnings
>
> [AMD Official Use Only - General]
>
> > -Original Message-
> > From: amd-gfx On Behalf Of
> > Vijendar Mukunda
> > Sent: Monday, June 27, 2022 8:59 AM
> > To: broo...@kernel.org; alsa-de...@alsa-project.org; dri-
> > de...@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> > Cc: Dommati, Sunil-kumar ; David Airlie
> > ; Hiregoudar, Basavaraj
> > ; Pan, Xinhui ;
> open
> > list ; Kai-Heng Feng
> > ; Daniel Vetter ;
> > Mukunda, Vijendar ; Deucher, Alexander
> > ; Koenig, Christian
> >
> > Subject: [PATCH] drm: amd: amdgpu: fix checkpatch warnings
> >
> > From: vijendar
> >
> > Fixed below checkpatch warnings and errors
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:131: CHECK: Comparison to
> NULL
> > could be written "apd"
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:150: CHECK: Comparison to
> NULL
> > could be written "apd"
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:196: CHECK: Prefer kernel
> type
> > 'u64' over 'uint64_t'
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:224: CHECK: Please don't use
> > multiple blank lines
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:226: CHECK: Comparison to
> NULL
> > could be written "!adev->acp.acp_genpd"
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:233: CHECK: Please don't use
> > multiple blank lines
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:239: CHECK: Alignment
> should
> > match open parenthesis
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:241: CHECK: Comparison to
> NULL
> > could be written "!adev->acp.acp_cell"
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:247: CHECK: Comparison to
> NULL
> > could be written "!adev->acp.acp_res"
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:253: CHECK: Comparison to
> NULL
> > could be written "!i2s_pdata"
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:350: CHECK: Alignment
> should
> > match open parenthesis
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:550: ERROR: that open brace
> {
> > should be on the previous line
> >
> > Signed-off-by: Vijendar Mukunda
>
> Reviewed-by: Alex Deucher
>
I'm also fine to have this go through the audio tree if it is a pre-requisite
for your jadeite audio series.
Alex
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 27
> > +
> > 1 file changed, 10 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > index cc9c9f8b23b2..ba1605ff521f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> > @@ -128,7 +128,7 @@ static int acp_poweroff(struct generic_pm_domain
> > *genpd)
> > struct amdgpu_device *adev;
> >
> > apd = container_of(genpd, struct acp_pm_domain, gpd);
> > - if (apd != NULL) {
> > + if (apd) {
> > adev = apd->adev;
> > /* call smu to POWER GATE ACP block
> > * smu will
> > @@ -147,7 +147,7 @@ static int acp_poweron(struct generic_pm_domain
> > *genpd)
> > struct amdgpu_device *adev;
> >
> > apd = container_of(genpd, struct acp_pm_domain, gpd);
> > - if (apd != NULL) {
> > + if (apd) {
> > adev = apd->adev;
> > /* call smu to UNGATE ACP block
> > * smu will
> > @@ -193,7 +193,7 @@ static int acp_genpd_remove_device(struct device
> > *dev, void *data) static int acp_hw_init(void *handle) {
> > int r;
> > - uint64_t acp_base;
> > + u64 acp_base;
> > u32 val = 0;
> > u32 count = 0;
> > struct i2s_platform_data *i2s_pdata = NULL; @@ -220,37 +220,32
> @@
> > static int acp_hw_init(void *handle)
> > return -EINVAL;
> >
> > acp_base = adev->rmmio_base;
> > -
> > -
> > adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain),
> > GFP_KERNEL);
> > - if (adev->acp.acp_genpd == NULL)
> > + if (!adev->acp.acp_genpd)
> > return -ENOMEM;
> >
> > adev->acp.acp_genpd->gpd.name = "ACP_AUDIO";
> > adev->acp.acp_genpd->gpd.power_off = acp_poweroff;
> > adev->acp.acp_genpd->gpd.power_on = acp_poweron;
> > -
> > -
> > adev->acp.acp_genpd->adev = adev;
> >
> > pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
> >
> > - adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> > - GFP_KERNEL);
> > + adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> > +GFP_KERNEL);
> >
> > - if (adev->acp.acp_cell == NULL) {
> > + if (!adev->acp.acp_cell) {
> > r = -ENOME
RE: [PATCH] drm: amd: amdgpu: fix checkpatch warnings
[AMD Official Use Only - General]
> -Original Message-
> From: amd-gfx On Behalf Of
> Vijendar Mukunda
> Sent: Monday, June 27, 2022 8:59 AM
> To: broo...@kernel.org; alsa-de...@alsa-project.org; dri-
> de...@lists.freedesktop.org; amd-gfx@lists.freedesktop.org
> Cc: Dommati, Sunil-kumar ; David Airlie
> ; Hiregoudar, Basavaraj ;
> Pan, Xinhui ; open list ker...@vger.kernel.org>; Kai-Heng Feng ;
> Daniel Vetter ; Mukunda, Vijendar
> ; Deucher, Alexander
> ; Koenig, Christian
>
> Subject: [PATCH] drm: amd: amdgpu: fix checkpatch warnings
>
> From: vijendar
>
> Fixed below checkpatch warnings and errors
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:131: CHECK: Comparison to
> NULL could be written "apd"
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:150: CHECK: Comparison to
> NULL could be written "apd"
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:196: CHECK: Prefer kernel
> type 'u64' over 'uint64_t'
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:224: CHECK: Please don't use
> multiple blank lines
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:226: CHECK: Comparison to
> NULL could be written "!adev->acp.acp_genpd"
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:233: CHECK: Please don't use
> multiple blank lines
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:239: CHECK: Alignment should
> match open parenthesis
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:241: CHECK: Comparison to
> NULL could be written "!adev->acp.acp_cell"
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:247: CHECK: Comparison to
> NULL could be written "!adev->acp.acp_res"
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:253: CHECK: Comparison to
> NULL could be written "!i2s_pdata"
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:350: CHECK: Alignment should
> match open parenthesis
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:550: ERROR: that open brace {
> should be on the previous line
>
> Signed-off-by: Vijendar Mukunda
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 27 +
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index cc9c9f8b23b2..ba1605ff521f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -128,7 +128,7 @@ static int acp_poweroff(struct generic_pm_domain
> *genpd)
> struct amdgpu_device *adev;
>
> apd = container_of(genpd, struct acp_pm_domain, gpd);
> - if (apd != NULL) {
> + if (apd) {
> adev = apd->adev;
> /* call smu to POWER GATE ACP block
>* smu will
> @@ -147,7 +147,7 @@ static int acp_poweron(struct generic_pm_domain
> *genpd)
> struct amdgpu_device *adev;
>
> apd = container_of(genpd, struct acp_pm_domain, gpd);
> - if (apd != NULL) {
> + if (apd) {
> adev = apd->adev;
> /* call smu to UNGATE ACP block
>* smu will
> @@ -193,7 +193,7 @@ static int acp_genpd_remove_device(struct device
> *dev, void *data) static int acp_hw_init(void *handle) {
> int r;
> - uint64_t acp_base;
> + u64 acp_base;
> u32 val = 0;
> u32 count = 0;
> struct i2s_platform_data *i2s_pdata = NULL; @@ -220,37 +220,32
> @@ static int acp_hw_init(void *handle)
> return -EINVAL;
>
> acp_base = adev->rmmio_base;
> -
> -
> adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain),
> GFP_KERNEL);
> - if (adev->acp.acp_genpd == NULL)
> + if (!adev->acp.acp_genpd)
> return -ENOMEM;
>
> adev->acp.acp_genpd->gpd.name = "ACP_AUDIO";
> adev->acp.acp_genpd->gpd.power_off = acp_poweroff;
> adev->acp.acp_genpd->gpd.power_on = acp_poweron;
> -
> -
> adev->acp.acp_genpd->adev = adev;
>
> pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
>
> - adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> - GFP_KERNEL);
> + adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
> +GFP_KERNEL);
>
> - if (adev->acp.acp_cell == NULL) {
> + if (!adev->acp.acp_cell) {
> r = -ENOMEM;
> goto failure;
> }
>
> adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
> - if (adev->acp.acp_res == NULL) {
> + if (!adev->acp.acp_res) {
> r = -ENOMEM;
> goto failure;
> }
>
> i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
> - if (i2s_pdata == NULL) {
> + if (!i2s_pdata) {
> r = -ENOMEM;
> goto failure;
> }
> @@ -346,8 +341,7 @@ static int acp_hw_init(void *handle)
> adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
> adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
>
> - r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
> -
[PATCH 1/1] Revert "drm/amdkfd: Free queue after unmap queue success"
This reverts commit 150c1266d78fbaa0fc5f89461daafae416db1c3e.
This causes KFDTest regression on gfx9, will submit new patch after
fixing.
---
.../drm/amd/amdkfd/kfd_device_queue_manager.c | 28 ---
.../amd/amdkfd/kfd_process_queue_manager.c| 2 +-
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 21e451acfa59..93a0b6995430 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1881,22 +1881,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
}
- if (q->properties.is_active) {
- if (!dqm->dev->shared_resources.enable_mes) {
- retval = execute_queues_cpsch(dqm,
-
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
- if (retval == -ETIME)
- qpd->reset_wavefronts = true;
- } else {
- retval = remove_queue_mes(dqm, q, qpd);
- }
-
- if (retval)
- goto failed_unmap_queue;
-
- decrement_queue_count(dqm, qpd, q);
- }
-
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
@@ -1910,6 +1894,17 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
list_del(&q->list);
qpd->queue_count--;
+ if (q->properties.is_active) {
+ if (!dqm->dev->shared_resources.enable_mes) {
+ decrement_queue_count(dqm, qpd, q);
+ retval = execute_queues_cpsch(dqm,
+
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
+ if (retval == -ETIME)
+ qpd->reset_wavefronts = true;
+ } else {
+ retval = remove_queue_mes(dqm, q, qpd);
+ }
+ }
/*
* Unconditionally decrement this counter, regardless of the queue's
@@ -1926,7 +1921,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
return retval;
-failed_unmap_queue:
failed_try_destroy_debugged_queue:
dqm_unlock(dqm);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index a46e2a37b4a6..c9c205df4a14 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -422,6 +422,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm,
unsigned int qid)
}
if (pqn->q) {
+ kfd_procfs_del_queue(pqn->q);
dqm = pqn->q->device->dqm;
retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q);
if (retval) {
@@ -445,7 +446,6 @@ int pqm_destroy_queue(struct process_queue_manager *pqm,
unsigned int qid)
amdgpu_amdkfd_free_gtt_mem(dev->adev,
pqn->q->wptr_bo);
}
- kfd_procfs_del_queue(pqn->q);
uninit_queue(pqn->q);
}
--
2.35.1
Re: Annoying AMDGPU boot-time warning due to simplefb / amdgpu resource clash
On Mon, Jun 27, 2022 at 1:02 AM Javier Martinez Canillas wrote: > > The flag was dropped because it was causing drivers that requested their > memory resource with pci_request_region() to fail with -EBUSY (e.g: the > vmwgfx driver): > > https://www.spinics.net/lists/dri-devel/msg329672.html See, *that* link would have been useful in the commit. Rather than the useless link it has. Anyway, removing the busy bit just made things worse. > > If simplefb is actually still using that frame buffer, it's a problem. > > If it isn't, then maybe that resource should have been released? > > It's supposed to be released once amdgpu asks for conflicting framebuffers > to be removed calling drm_aperture_remove_conflicting_pci_framebuffers(). That most definitely doesn't happen. This is on a running system: [torvalds@ryzen linux]$ cat /proc/iomem | grep BOOTFB - : BOOTFB so I suspect that the BUSY bit was never the problem - even for vmwgfx). The problem was that simplefb doesn't remove its resource. Guys, the *reason* for resource management is to catch people that trample over each other's resources. You literally basically disabled the code that checked for it by removing the BUSY flag, and just continued to have conflicting resources. That isn't a "fix", that is literally "we are ignoring and breaking the whole reason that the resource tree exists, but we'll still use it for no good reason". Yeah, yeah, most modern drivers ignore the IO resource tree, because they end up working on another resource level entirely: they work on not the IO resources, but on the "driver level" instead, and just attach to PCI devices. So these days, few enough drivers even care about the IO resource tree, and it's mostly used for (a) legacy devices (think ISA) and (b) the actual bus resource handling (so the PCI code itself uses it to sort out resource use and avoid conflicts, but PCI drivers themselves generally then don't care, because the bus has "taken care of it". So that's why the amdgpu driver itself doesn't care about resource allocations, and we only get a warning for that memory type case, not for any deeper resource case. And apparently the vmwgfx driver still uses that legacy "let's claim all PCI resources in the resource tree" instead of just claiming the device itself. Which is why it hit this whole BOOTFB resource thing even harder. But the real bug is that BOOTFB seems to claim this resource even after it is done with it and other drivers want to take over. Not the BUSY bit. Linus
Re: [PATCH 1/3] drm/amdkfd: add new flags for svm
On Mon, Jun 27, 2022 at 12:01 PM Eric Huang wrote:
>
> No. There is only internal link for now, because it is under review.
> Once it is submitted, external link should be in gerritgit for libhsakmt.
We need to have that available at the same time so that the kernel
side can be properly reviewed here.
Alex
>
> Regards,
> Eric
>
> On 2022-06-27 11:58, Alex Deucher wrote:
> > On Mon, Jun 27, 2022 at 11:36 AM Eric Huang
> > wrote:
> >> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgerrit-git.amd.com%2Fc%2Fcompute%2Fec%2Flibhsakmt%2F%2B%2F697296&data=05%7C01%7Cjinhuieric.huang%40amd.com%7C61498029cd6743a4519108da5855f02e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637919423397667222%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wPlHRSmOyzO%2B2vbwN9IK4qR5dhk%2BaOw2rt3JEdjizRU%3D&reserved=0
> > Got an external link?
> >
> > Alex
> >
> >> Regards,
> >> Eric
> >>
> >> On 2022-06-27 11:33, Alex Deucher wrote:
> >>> On Fri, Jun 24, 2022 at 12:03 PM Eric Huang
> >>> wrote:
> It is to add new options for always keeping gpu mapping
> and custom of coarse grain allocation intead of fine
> grain as default.
>
> Signed-off-by: Eric Huang
> >>> Can you provide a link to the proposed userspace for this?
> >>>
> >>> Alex
> >>>
> ---
> include/uapi/linux/kfd_ioctl.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/kfd_ioctl.h
> b/include/uapi/linux/kfd_ioctl.h
> index fd49dde4d5f4..9dbf215675a0 100644
> --- a/include/uapi/linux/kfd_ioctl.h
> +++ b/include/uapi/linux/kfd_ioctl.h
> @@ -1076,6 +1076,10 @@ struct kfd_ioctl_cross_memory_copy_args {
> #define KFD_IOCTL_SVM_FLAG_GPU_EXEC0x0010
> /* GPUs mostly read, may allow similar optimizations as RO, but
> writes fault */
> #define KFD_IOCTL_SVM_FLAG_GPU_READ_MOSTLY 0x0020
> +/* Keep GPU memory mapping always valid as if XNACK is disable */
> +#define KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED 0x0040
> +/* Allow set custom flags instead of defaults */
> +#define KFD_IOCTL_SVM_FLAG_CUSTOM 0x8000
>
> /**
> * kfd_ioctl_svm_op - SVM ioctl operations
> --
> 2.25.1
>
>
Re: [PATCH 1/3] drm/amdkfd: add new flags for svm
No. There is only internal link for now, because it is under review.
Once it is submitted, external link should be in gerritgit for libhsakmt.
Regards,
Eric
On 2022-06-27 11:58, Alex Deucher wrote:
On Mon, Jun 27, 2022 at 11:36 AM Eric Huang wrote:
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgerrit-git.amd.com%2Fc%2Fcompute%2Fec%2Flibhsakmt%2F%2B%2F697296&data=05%7C01%7Cjinhuieric.huang%40amd.com%7C61498029cd6743a4519108da5855f02e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637919423397667222%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=wPlHRSmOyzO%2B2vbwN9IK4qR5dhk%2BaOw2rt3JEdjizRU%3D&reserved=0
Got an external link?
Alex
Regards,
Eric
On 2022-06-27 11:33, Alex Deucher wrote:
On Fri, Jun 24, 2022 at 12:03 PM Eric Huang wrote:
It is to add new options for always keeping gpu mapping
and custom of coarse grain allocation intead of fine
grain as default.
Signed-off-by: Eric Huang
Can you provide a link to the proposed userspace for this?
Alex
---
include/uapi/linux/kfd_ioctl.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index fd49dde4d5f4..9dbf215675a0 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -1076,6 +1076,10 @@ struct kfd_ioctl_cross_memory_copy_args {
#define KFD_IOCTL_SVM_FLAG_GPU_EXEC0x0010
/* GPUs mostly read, may allow similar optimizations as RO, but writes fault
*/
#define KFD_IOCTL_SVM_FLAG_GPU_READ_MOSTLY 0x0020
+/* Keep GPU memory mapping always valid as if XNACK is disable */
+#define KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED 0x0040
+/* Allow set custom flags instead of defaults */
+#define KFD_IOCTL_SVM_FLAG_CUSTOM 0x8000
/**
* kfd_ioctl_svm_op - SVM ioctl operations
--
2.25.1
Re: [PATCH 1/3] drm/amdkfd: add new flags for svm
On Mon, Jun 27, 2022 at 11:36 AM Eric Huang wrote:
>
> http://gerrit-git.amd.com/c/compute/ec/libhsakmt/+/697296
Got an external link?
Alex
>
> Regards,
> Eric
>
> On 2022-06-27 11:33, Alex Deucher wrote:
> > On Fri, Jun 24, 2022 at 12:03 PM Eric Huang
> > wrote:
> >> It is to add new options for always keeping gpu mapping
> >> and custom of coarse grain allocation intead of fine
> >> grain as default.
> >>
> >> Signed-off-by: Eric Huang
> > Can you provide a link to the proposed userspace for this?
> >
> > Alex
> >
> >> ---
> >> include/uapi/linux/kfd_ioctl.h | 4
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/kfd_ioctl.h
> >> b/include/uapi/linux/kfd_ioctl.h
> >> index fd49dde4d5f4..9dbf215675a0 100644
> >> --- a/include/uapi/linux/kfd_ioctl.h
> >> +++ b/include/uapi/linux/kfd_ioctl.h
> >> @@ -1076,6 +1076,10 @@ struct kfd_ioctl_cross_memory_copy_args {
> >> #define KFD_IOCTL_SVM_FLAG_GPU_EXEC0x0010
> >> /* GPUs mostly read, may allow similar optimizations as RO, but writes
> >> fault */
> >> #define KFD_IOCTL_SVM_FLAG_GPU_READ_MOSTLY 0x0020
> >> +/* Keep GPU memory mapping always valid as if XNACK is disable */
> >> +#define KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED 0x0040
> >> +/* Allow set custom flags instead of defaults */
> >> +#define KFD_IOCTL_SVM_FLAG_CUSTOM 0x8000
> >>
> >> /**
> >>* kfd_ioctl_svm_op - SVM ioctl operations
> >> --
> >> 2.25.1
> >>
>
Re: [PATCH 1/3] drm/amdkfd: add new flags for svm
http://gerrit-git.amd.com/c/compute/ec/libhsakmt/+/697296
Regards,
Eric
On 2022-06-27 11:33, Alex Deucher wrote:
On Fri, Jun 24, 2022 at 12:03 PM Eric Huang wrote:
It is to add new options for always keeping gpu mapping
and custom of coarse grain allocation intead of fine
grain as default.
Signed-off-by: Eric Huang
Can you provide a link to the proposed userspace for this?
Alex
---
include/uapi/linux/kfd_ioctl.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index fd49dde4d5f4..9dbf215675a0 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -1076,6 +1076,10 @@ struct kfd_ioctl_cross_memory_copy_args {
#define KFD_IOCTL_SVM_FLAG_GPU_EXEC0x0010
/* GPUs mostly read, may allow similar optimizations as RO, but writes fault
*/
#define KFD_IOCTL_SVM_FLAG_GPU_READ_MOSTLY 0x0020
+/* Keep GPU memory mapping always valid as if XNACK is disable */
+#define KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED 0x0040
+/* Allow set custom flags instead of defaults */
+#define KFD_IOCTL_SVM_FLAG_CUSTOM 0x8000
/**
* kfd_ioctl_svm_op - SVM ioctl operations
--
2.25.1
Re: [PATCH 1/3] drm/amdkfd: add new flags for svm
On Fri, Jun 24, 2022 at 12:03 PM Eric Huang wrote:
>
> It is to add new options for always keeping gpu mapping
> and custom of coarse grain allocation intead of fine
> grain as default.
>
> Signed-off-by: Eric Huang
Can you provide a link to the proposed userspace for this?
Alex
> ---
> include/uapi/linux/kfd_ioctl.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
> index fd49dde4d5f4..9dbf215675a0 100644
> --- a/include/uapi/linux/kfd_ioctl.h
> +++ b/include/uapi/linux/kfd_ioctl.h
> @@ -1076,6 +1076,10 @@ struct kfd_ioctl_cross_memory_copy_args {
> #define KFD_IOCTL_SVM_FLAG_GPU_EXEC0x0010
> /* GPUs mostly read, may allow similar optimizations as RO, but writes fault
> */
> #define KFD_IOCTL_SVM_FLAG_GPU_READ_MOSTLY 0x0020
> +/* Keep GPU memory mapping always valid as if XNACK is disable */
> +#define KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED 0x0040
> +/* Allow set custom flags instead of defaults */
> +#define KFD_IOCTL_SVM_FLAG_CUSTOM 0x8000
>
> /**
> * kfd_ioctl_svm_op - SVM ioctl operations
> --
> 2.25.1
>
Re: [PATCH] drm/amd/display: Re-org and cleanup the redundant code
On 2022-06-26 06:15, Chandan Vurdigere Nataraj wrote:
[Why]
Redundant if-else cases for repeater and non-repeater checks
[How]
Without changing the core logic, rearranged the code by removing
redundant checks
Signed-off-by: Chandan Vurdigere Nataraj
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 4027f439a5a4..e3254ac05191 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -907,25 +907,17 @@ enum dc_status dp_get_lane_status_and_lane_adjust(
ln_align->raw = dpcd_buf[2];
if (is_repeater(link, offset)) {
+
With this extra newline dropped,
Reviewed-by: Leo Li
Thanks!
DC_LOG_HW_LINK_TRAINING("%s:\n LTTPR Repeater ID: %d\n"
" 0x%X Lane01Status = %x\n 0x%X Lane23Status = %x\n
",
__func__,
offset,
lane01_status_address, dpcd_buf[0],
lane01_status_address + 1, dpcd_buf[1]);
- } else {
- DC_LOG_HW_LINK_TRAINING("%s:\n 0x%X Lane01Status = %x\n 0x%X
Lane23Status = %x\n ",
- __func__,
- lane01_status_address, dpcd_buf[0],
- lane01_status_address + 1, dpcd_buf[1]);
- }
- lane01_adjust_address = DP_ADJUST_REQUEST_LANE0_1;
- if (is_repeater(link, offset))
lane01_adjust_address = DP_ADJUST_REQUEST_LANE0_1_PHY_REPEATER1
+
((DP_REPEATER_CONFIGURATION_AND_STATUS_SIZE) *
(offset - 1));
- if (is_repeater(link, offset)) {
DC_LOG_HW_LINK_TRAINING("%s:\n LTTPR Repeater ID: %d\n"
" 0x%X Lane01AdjustRequest = %x\n 0x%X
Lane23AdjustRequest = %x\n",
__func__,
@@ -935,6 +927,14 @@ enum dc_status dp_get_lane_status_and_lane_adjust(
lane01_adjust_address + 1,
dpcd_buf[lane_adjust_offset + 1]);
} else {
+
+ DC_LOG_HW_LINK_TRAINING("%s:\n 0x%X Lane01Status = %x\n 0x%X
Lane23Status = %x\n ",
+ __func__,
+ lane01_status_address, dpcd_buf[0],
+ lane01_status_address + 1, dpcd_buf[1]);
+
+ lane01_adjust_address = DP_ADJUST_REQUEST_LANE0_1;
+
DC_LOG_HW_LINK_TRAINING("%s:\n 0x%X Lane01AdjustRequest = %x\n 0x%X
Lane23AdjustRequest = %x\n",
__func__,
lane01_adjust_address,
[PATCH] drm/amd/display: expose additional modifier for DCN32/321
[Why&How]
Some userspace expect a backwards compatible modifier on DCN32/321. For
hardware with num_pipes more than 16, we expose the most efficient
modifier first. As a fall back method, we need to expose slightly inefficient
modifier AMD_FMT_MOD_TILE_GFX9_64K_R_X after the best option.
Also set the number of packers to fixed value as required per hardware
documentation. This value is cached during hardware initialization and
can be read through the base driver.
Fixes: 0a2c19562ffe ('Revert "drm/amd/display: ignore modifiers when checking
for format support"')
Signed-off-by: Aurabindo Pillai
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 +--
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 1a512d78673a..0f5bfe5df627 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -743,8 +743,7 @@ static int convert_tiling_flags_to_modifier(struct
amdgpu_framebuffer *afb)
switch (version) {
case AMD_FMT_MOD_TILE_VER_GFX11:
pipe_xor_bits = min(block_size_bits - 8, pipes);
- packers = min(block_size_bits - 8 -
pipe_xor_bits,
-
ilog2(adev->gfx.config.gb_addr_config_fields.num_pkrs));
+ packers =
ilog2(adev->gfx.config.gb_addr_config_fields.num_pkrs);
break;
case AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS:
pipe_xor_bits = min(block_size_bits - 8, pipes);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c9145864ed2b..bea9cee37f65 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5203,6 +5203,7 @@ add_gfx11_modifiers(struct amdgpu_device *adev,
int pkrs = 0;
u32 gb_addr_config;
unsigned swizzle_r_x;
+ uint64_t modifier_r_x_best;
uint64_t modifier_r_x;
uint64_t modifier_dcc_best;
uint64_t modifier_dcc_4k;
@@ -5223,10 +5224,12 @@ add_gfx11_modifiers(struct amdgpu_device *adev,
modifier_r_x = AMD_FMT_MOD |
AMD_FMT_MOD_SET(TILE_VERSION, AMD_FMT_MOD_TILE_VER_GFX11) |
- AMD_FMT_MOD_SET(TILE, swizzle_r_x) |
AMD_FMT_MOD_SET(PIPE_XOR_BITS, pipe_xor_bits) |
AMD_FMT_MOD_SET(PACKERS, pkrs);
+ modifier_r_x_best = modifier_r_x | AMD_FMT_MOD_SET(TILE,
AMD_FMT_MOD_TILE_GFX11_256K_R_X);
+ modifier_r_x = modifier_r_x | AMD_FMT_MOD_SET(TILE,
AMD_FMT_MOD_TILE_GFX9_64K_R_X);
+
/* DCC_CONSTANT_ENCODE is not set because it can't vary with gfx11
(it's implied to be 1). */
modifier_dcc_best = modifier_r_x |
AMD_FMT_MOD_SET(DCC, 1) |
@@ -5247,6 +5250,9 @@ add_gfx11_modifiers(struct amdgpu_device *adev,
add_modifier(mods, size, capacity, modifier_dcc_best |
AMD_FMT_MOD_SET(DCC_RETILE, 1));
add_modifier(mods, size, capacity, modifier_dcc_4k |
AMD_FMT_MOD_SET(DCC_RETILE, 1));
+ if (num_pipes > 16)
+ add_modifier(mods, size, capacity, modifier_r_x_best);
+
add_modifier(mods, size, capacity, modifier_r_x);
add_modifier(mods, size, capacity, AMD_FMT_MOD |
--
2.36.1
Re: [PATCH] drm/amd/display: Remove unused globals FORCE_RATE and FORCE_LANE_COUNT
On 2022-06-26 10:20, Tom Rix wrote:
sparse reports
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3885:6: warning:
symbol 'FORCE_RATE' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3886:10: warning:
symbol 'FORCE_LANE_COUNT' was not declared. Should it be static?
Neither of thse variables is used in dc_link_dp.c. Reviewing the commit listed
in
the fixes tag shows neither was used in the original patch. So remove them.
Fixes: 265280b99822 ("drm/amd/display: add CLKMGR changes for DCN32/321")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index be1dcb0a2a06..f3421f2bd52e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -3882,9 +3882,6 @@ static bool decide_mst_link_settings(const struct dc_link
*link, struct dc_link_
return true;
}
-bool FORCE_RATE = false;
-uint32_t FORCE_LANE_COUNT = 0;
-
void decide_link_settings(struct dc_stream_state *stream,
struct dc_link_settings *link_setting)
{
Reviewed-by: Aurabindo Pillai
Re: [PATCH] drm/amd/display: change to_dal_irq_source_dnc32() storage class specifier to static
On 2022-06-26 10:46, Tom Rix wrote:
sparse reports
drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn32/irq_service_dcn32.c:39:20:
warning: symbol 'to_dal_irq_source_dcn32' was not declared. Should it be static?
to_dal_irq_source_dnc32() is only referenced in irq_service_dnc32.c, so change
its
storage class specifier to static.
Fixes: 0efd4374f6b4 ("drm/amd/display: add dcn32 IRQ changes")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
index 3a213ca2f077..b1012fa1977b 100644
--- a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
+++ b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
@@ -36,7 +36,7 @@
#define DCN_BASE__INST0_SEG2 0x34C0
-enum dc_irq_source to_dal_irq_source_dcn32(
+static enum dc_irq_source to_dal_irq_source_dcn32(
struct irq_service *irq_service,
uint32_t src_id,
uint32_t ext_id)
Reviewed-by: Aurabindo Pillai
[PATCH] drm: amd: amdgpu: fix checkpatch warnings
From: vijendar
Fixed below checkpatch warnings and errors
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:131: CHECK: Comparison to NULL could be
written "apd"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:150: CHECK: Comparison to NULL could be
written "apd"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:196: CHECK: Prefer kernel type 'u64'
over 'uint64_t'
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:224: CHECK: Please don't use multiple
blank lines
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:226: CHECK: Comparison to NULL could be
written "!adev->acp.acp_genpd"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:233: CHECK: Please don't use multiple
blank lines
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:239: CHECK: Alignment should match open
parenthesis
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:241: CHECK: Comparison to NULL could be
written "!adev->acp.acp_cell"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:247: CHECK: Comparison to NULL could be
written "!adev->acp.acp_res"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:253: CHECK: Comparison to NULL could be
written "!i2s_pdata"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:350: CHECK: Alignment should match open
parenthesis
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:550: ERROR: that open brace { should be
on the previous line
Signed-off-by: Vijendar Mukunda
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 27 +
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index cc9c9f8b23b2..ba1605ff521f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -128,7 +128,7 @@ static int acp_poweroff(struct generic_pm_domain *genpd)
struct amdgpu_device *adev;
apd = container_of(genpd, struct acp_pm_domain, gpd);
- if (apd != NULL) {
+ if (apd) {
adev = apd->adev;
/* call smu to POWER GATE ACP block
* smu will
@@ -147,7 +147,7 @@ static int acp_poweron(struct generic_pm_domain *genpd)
struct amdgpu_device *adev;
apd = container_of(genpd, struct acp_pm_domain, gpd);
- if (apd != NULL) {
+ if (apd) {
adev = apd->adev;
/* call smu to UNGATE ACP block
* smu will
@@ -193,7 +193,7 @@ static int acp_genpd_remove_device(struct device *dev, void
*data)
static int acp_hw_init(void *handle)
{
int r;
- uint64_t acp_base;
+ u64 acp_base;
u32 val = 0;
u32 count = 0;
struct i2s_platform_data *i2s_pdata = NULL;
@@ -220,37 +220,32 @@ static int acp_hw_init(void *handle)
return -EINVAL;
acp_base = adev->rmmio_base;
-
-
adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL);
- if (adev->acp.acp_genpd == NULL)
+ if (!adev->acp.acp_genpd)
return -ENOMEM;
adev->acp.acp_genpd->gpd.name = "ACP_AUDIO";
adev->acp.acp_genpd->gpd.power_off = acp_poweroff;
adev->acp.acp_genpd->gpd.power_on = acp_poweron;
-
-
adev->acp.acp_genpd->adev = adev;
pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
- adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
- GFP_KERNEL);
+ adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
GFP_KERNEL);
- if (adev->acp.acp_cell == NULL) {
+ if (!adev->acp.acp_cell) {
r = -ENOMEM;
goto failure;
}
adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
- if (adev->acp.acp_res == NULL) {
+ if (!adev->acp.acp_res) {
r = -ENOMEM;
goto failure;
}
i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
- if (i2s_pdata == NULL) {
+ if (!i2s_pdata) {
r = -ENOMEM;
goto failure;
}
@@ -346,8 +341,7 @@ static int acp_hw_init(void *handle)
adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
- r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
- ACP_DEVS);
+ r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
ACP_DEVS);
if (r)
goto failure;
@@ -546,8 +540,7 @@ static const struct amd_ip_funcs acp_ip_funcs = {
.set_powergating_state = acp_set_powergating_state,
};
-const struct amdgpu_ip_block_version acp_ip_block =
-{
+const struct amdgpu_ip_block_version acp_ip_block = {
.type = AMD_IP_BLOCK_TYPE_ACP,
.major = 2,
.minor = 2,
--
2.25.1
[PATCH] drm/amd/display: Removed unused variable ret
From: "Souptick Joarder (HPE)"
Kernel test robot throws below warning ->
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:
In function 'dc_link_reduce_mst_payload':
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:3782:32:
warning: variable 'ret' set but not used [-Wunused-but-set-variable]
3782 | enum act_return_status ret;
Removed the unused ret variable.
Reported-by: kernel test robot
Signed-off-by: Souptick Joarder (HPE)
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 55a8f58ee239..445357623d8b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -3706,7 +3706,6 @@ enum dc_status dc_link_reduce_mst_payload(struct pipe_ctx
*pipe_ctx, uint32_t bw
struct fixed31_32 pbn_per_slot;
struct dp_mst_stream_allocation_table proposed_table = {0};
uint8_t i;
- enum act_return_status ret;
const struct link_hwss *link_hwss = get_link_hwss(link,
&pipe_ctx->link_res);
DC_LOGGER_INIT(link->ctx->logger);
@@ -3777,7 +3776,7 @@ enum dc_status dc_link_reduce_mst_payload(struct pipe_ctx
*pipe_ctx, uint32_t bw
&link->mst_stream_alloc_table);
/* poll for immediate branch device ACT handled */
- ret = dm_helpers_dp_mst_poll_for_allocation_change_trigger(
+ dm_helpers_dp_mst_poll_for_allocation_change_trigger(
stream->ctx,
stream);
--
2.25.1
Annoying AMDGPU boot-time warning due to simplefb / amdgpu resource clash
So this has been going on for a while, and it's quite annoying.
At bootup, my main desktop (Threadripper 3970X with radeon graphics)
now complains about
resource sanity check: requesting [mem 0xd000-0xdfff], which
spans more than BOOTFB [mem 0xd000-0xd02f]
and then gives me a nasty callchain that is basically the amdgpu probe
sequence ending in amdgpu_bo_init() doing the
arch_io_reserve_memtype_wc() which is then what complains.
That "BOOTFB" resource is from sysfb_simplefb.c, and I think what
started triggering this is commit c96898342c38 ("drivers/firmware:
Don't mark as busy the simple-framebuffer IO resource").
Because it turns out that that removed the IORESOURCE_BUSY, which in
turn is what makes the resource conflict code complain about it now,
because
/*
* if a resource is "BUSY", it's not a hardware resource
* but a driver mapping of such a resource; we don't want
* to warn for those; some drivers legitimately map only
* partial hardware resources. (example: vesafb)
*/
so the issue is that now the resource code - correctly - says "hey,
you have *two* conflicting driver mappings".
And that commit claims it did it because "which can lead to drivers
requesting the same memory resource to fail", but - once again - the
link in the commit that might actually tell more is just one of those
useless patch submission links again.
So who knows why that commit was actually done, but it's causing annoyance.
If simplefb is actually still using that frame buffer, it's a problem.
If it isn't, then maybe that resource should have been released?
I really think that commit c96898342c38 is buggy. It talks about "let
drivers to request it as busy instead", but then it registers a
resource that isn't actually a proper real resource. It's just a
random incomplete chunk of the actual real thing, so it will still
interfere with resource allocation, and in fact now interferes even
with that "set memtype" because of this valid warning.
Linus
[PATCH] drm/amd/display: change to_dal_irq_source_dnc32() storage class specifier to static
sparse reports
drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn32/irq_service_dcn32.c:39:20:
warning: symbol 'to_dal_irq_source_dcn32' was not declared. Should it be static?
to_dal_irq_source_dnc32() is only referenced in irq_service_dnc32.c, so change
its
storage class specifier to static.
Fixes: 0efd4374f6b4 ("drm/amd/display: add dcn32 IRQ changes")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
index 3a213ca2f077..b1012fa1977b 100644
--- a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
+++ b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
@@ -36,7 +36,7 @@
#define DCN_BASE__INST0_SEG2 0x34C0
-enum dc_irq_source to_dal_irq_source_dcn32(
+static enum dc_irq_source to_dal_irq_source_dcn32(
struct irq_service *irq_service,
uint32_t src_id,
uint32_t ext_id)
--
2.27.0
[PATCH] drm/amd/display: Remove unused globals FORCE_RATE and FORCE_LANE_COUNT
sparse reports
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3885:6: warning:
symbol 'FORCE_RATE' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3886:10: warning:
symbol 'FORCE_LANE_COUNT' was not declared. Should it be static?
Neither of thse variables is used in dc_link_dp.c. Reviewing the commit listed
in
the fixes tag shows neither was used in the original patch. So remove them.
Fixes: 265280b99822 ("drm/amd/display: add CLKMGR changes for DCN32/321")
Signed-off-by: Tom Rix
---
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index be1dcb0a2a06..f3421f2bd52e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -3882,9 +3882,6 @@ static bool decide_mst_link_settings(const struct dc_link
*link, struct dc_link_
return true;
}
-bool FORCE_RATE = false;
-uint32_t FORCE_LANE_COUNT = 0;
-
void decide_link_settings(struct dc_stream_state *stream,
struct dc_link_settings *link_setting)
{
--
2.27.0
Re: [RFC PATCH 4/5] drm/drm_color_mgmt: add 3D LUT to color mgmt properties
On Sun, Jun 19, 2022 at 09:31:03PM -0100, Melissa Wen wrote: > Add 3D LUT for gammar correction using a 3D lookup table. The position > in the color correction pipeline where 3D LUT is applied depends on hw > design, being after CTM or gamma. If just after CTM, a shaper lut must > be set to shape the content for a non-linear space. That details should > be handled by the driver according to its color capabilities. I also cooked up a WIP 3D LUT support some time ago for Intel hw: https://github.com/vsyrjala/linux/commits/3dlut But that dried up due to having no userspace for it. I also cooked up some basic igts for it: https://patchwork.freedesktop.org/series/90165/ > + * “LUT3D”: > + * Blob property to set the 3D LUT mapping pixel data after the color > + * transformation matrix and before gamma 1D lut correction. On Intel hw the 3DLUT is after the gamma LUT in the pipeline, which is where I placed it in my branch. There is now some discussion happening about exposing some kind of color pipeline description/configuration properties: https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/11 -- Ville Syrjälä Intel
Re: Annoying AMDGPU boot-time warning due to simplefb / amdgpu resource clash
Hi
Am 26.06.22 um 20:54 schrieb Linus Torvalds:
So this has been going on for a while, and it's quite annoying.
At bootup, my main desktop (Threadripper 3970X with radeon graphics)
now complains about
resource sanity check: requesting [mem 0xd000-0xdfff], which
spans more than BOOTFB [mem 0xd000-0xd02f]
and then gives me a nasty callchain that is basically the amdgpu probe
sequence ending in amdgpu_bo_init() doing the
arch_io_reserve_memtype_wc() which is then what complains.
That "BOOTFB" resource is from sysfb_simplefb.c, and I think what
started triggering this is commit c96898342c38 ("drivers/firmware:
Don't mark as busy the simple-framebuffer IO resource").
Because it turns out that that removed the IORESOURCE_BUSY, which in
turn is what makes the resource conflict code complain about it now,
because
/*
* if a resource is "BUSY", it's not a hardware resource
* but a driver mapping of such a resource; we don't want
* to warn for those; some drivers legitimately map only
* partial hardware resources. (example: vesafb)
*/
so the issue is that now the resource code - correctly - says "hey,
you have *two* conflicting driver mappings".
And that commit claims it did it because "which can lead to drivers
requesting the same memory resource to fail", but - once again - the
link in the commit that might actually tell more is just one of those
useless patch submission links again.
So who knows why that commit was actually done, but it's causing annoyance.
If simplefb is actually still using that frame buffer, it's a problem.
If it isn't, then maybe that resource should have been released?
As Javier said, that resource is the framebuffer that's set up by the
firmware. It should be gone after the call to
drm_aperture_remove_conflicting_pci_framebuffers(). [1] The call to
amdgpu_bo_init() runs afterwards, so that removal apparently failed.
Is the BOOTFB entry still listed in /proc/iomem after the system
finished booting?
Attached is a (totally untested) patch to manually point amdgpu to the
right location. Does it fix the problem?
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v5.18.7/source/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c#L2077
I really think that commit c96898342c38 is buggy. It talks about "let
drivers to request it as busy instead", but then it registers a
resource that isn't actually a proper real resource. It's just a
random incomplete chunk of the actual real thing, so it will still
interfere with resource allocation, and in fact now interferes even
with that "set memtype" because of this valid warning.
Linus
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
From c37f0fa8e763c471ddaccc08da9ec9bb1a715451 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann
Date: Mon, 27 Jun 2022 10:51:44 +0200
Subject: [PATCH] drm/amdgpu: Remove firmware framebuffer without PCI helper
The DRM aperture helper for PCI devices fails to remove the firmware
framebuffer's device. Manually tell it where to look.
Reported-by: Linus Torvalds
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 46ef57b07c15..e00318ff66ff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2073,7 +2073,8 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
/* Get rid of things like offb */
- ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &amdgpu_kms_driver);
+ ret = drm_aperture_remove_conflicting_framebuffers(base, size, is_fw_fb,
+ &amdgpu_kms_driver);
if (ret)
return ret;
--
2.36.1
OpenPGP_signature
Description: OpenPGP digital signature
Re: Annoying AMDGPU boot-time warning due to simplefb / amdgpu resource clash
Hello Linus,
On 6/26/22 20:54, Linus Torvalds wrote:
> So this has been going on for a while, and it's quite annoying.
>
> At bootup, my main desktop (Threadripper 3970X with radeon graphics)
> now complains about
>
> resource sanity check: requesting [mem 0xd000-0xdfff], which
> spans more than BOOTFB [mem 0xd000-0xd02f]
>
> and then gives me a nasty callchain that is basically the amdgpu probe
> sequence ending in amdgpu_bo_init() doing the
> arch_io_reserve_memtype_wc() which is then what complains.
>
> That "BOOTFB" resource is from sysfb_simplefb.c, and I think what
> started triggering this is commit c96898342c38 ("drivers/firmware:
> Don't mark as busy the simple-framebuffer IO resource").
>
> Because it turns out that that removed the IORESOURCE_BUSY, which in
> turn is what makes the resource conflict code complain about it now,
> because
>
> /*
> * if a resource is "BUSY", it's not a hardware resource
> * but a driver mapping of such a resource; we don't want
> * to warn for those; some drivers legitimately map only
> * partial hardware resources. (example: vesafb)
> */
>
> so the issue is that now the resource code - correctly - says "hey,
> you have *two* conflicting driver mappings".
>
> And that commit claims it did it because "which can lead to drivers
> requesting the same memory resource to fail", but - once again - the
> link in the commit that might actually tell more is just one of those
> useless patch submission links again.
>
> So who knows why that commit was actually done, but it's causing annoyance.
>
The flag was dropped because it was causing drivers that requested their
memory resource with pci_request_region() to fail with -EBUSY (e.g: the
vmwgfx driver):
https://www.spinics.net/lists/dri-devel/msg329672.html
> If simplefb is actually still using that frame buffer, it's a problem.
> If it isn't, then maybe that resource should have been released?
>
It's supposed to be released once amdgpu asks for conflicting framebuffers
to be removed calling drm_aperture_remove_conflicting_pci_framebuffers().
I'm not familiar with the amdgpu driver, but maybe that call has to be done
earlier before the arch_io_reserve_memtype_wc() ?
> I really think that commit c96898342c38 is buggy. It talks about "let
> drivers to request it as busy instead", but then it registers a
> resource that isn't actually a proper real resource. It's just a
> random incomplete chunk of the actual real thing, so it will still
> interfere with resource allocation, and in fact now interferes even
> with that "set memtype" because of this valid warning.
>
That registered resource is what the firmware provides for drivers to use
the system framebuffer for scan-out. It's not the real thing, that's true
since a native driver would kick it out (leading to a resource release)
and register the real aperture.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
RE: [PATCH] drm/amdgpu: Remove useless amdgpu_display_freesync_ioctl() declaration
[AMD Official Use Only - General] Reviewed-by: Hawking Zhang Regards, Hawking -Original Message- From: amd-gfx On Behalf Of Leslie Shi Sent: Monday, June 27, 2022 13:02 To: amd-gfx@lists.freedesktop.org Cc: Shi, Leslie ; Chen, Guchun ; Zhang, Hawking Subject: [PATCH] drm/amdgpu: Remove useless amdgpu_display_freesync_ioctl() declaration Signed-off-by: Leslie Shi --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h index 7b6d83e2b13c..560352f7c317 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.h @@ -35,8 +35,6 @@ #define amdgpu_display_add_encoder(adev, e, s, c) (adev)->mode_info.funcs->add_encoder((adev), (e), (s), (c)) #define amdgpu_display_add_connector(adev, ci, sd, ct, ib, coi, h, r) (adev)->mode_info.funcs->add_connector((adev), (ci), (sd), (ct), (ib), (coi), (h), (r)) -int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data, - struct drm_file *filp); void amdgpu_display_update_priority(struct amdgpu_device *adev); uint32_t amdgpu_display_supported_domains(struct amdgpu_device *adev, uint64_t bo_flags); -- 2.25.1
