Re: [PATCH] drm/amdgpu: add VISIBLE info in amdgpu_bo_print_info
[Public]
Thanks Christian for the review. I'll remove the leading blanks before
submitting the patch.
Pierre-Eric
From: Koenig, Christian
Sent: Wednesday, June 21, 2023 5:00 PM
To: Pelloux-Prayer, Pierre-Eric ;
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: add VISIBLE info in amdgpu_bo_print_info
Am 21.06.23 um 16:35 schrieb Pierre-Eric Pelloux-Prayer:
> This allows tools to distinguish between VRAM and visible VRAM.
>
> Use the opportunity to fix locking before accessing bo.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 33 ++
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index ff73cc11d47e..f12f019d7f99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1583,18 +1583,27 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo
> *bo, struct seq_file *m)
>unsigned int pin_count;
>u64 size;
>
> - domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
> - switch (domain) {
> - case AMDGPU_GEM_DOMAIN_VRAM:
> - placement = "VRAM";
> - break;
> - case AMDGPU_GEM_DOMAIN_GTT:
> - placement = " GTT";
> - break;
> - case AMDGPU_GEM_DOMAIN_CPU:
> - default:
> - placement = " CPU";
> - break;
> + if (dma_resv_trylock(bo->tbo.base.resv)) {
> + unsigned int domain;
> + domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
> + switch (domain) {
> + case AMDGPU_GEM_DOMAIN_VRAM:
> + if (amdgpu_bo_in_cpu_visible_vram(bo))
> + placement = "VRAM VISIBLE";
> + else
> + placement = "VRAM";
> + break;
> + case AMDGPU_GEM_DOMAIN_GTT:
> + placement = " GTT";
We can probably drop the leading blank here and
> + break;
> + case AMDGPU_GEM_DOMAIN_CPU:
> + default:
> + placement = " CPU";
here when we don't keep the strings at the same length anyway.
With that fixed the change is Reviewed-by: Christian König
Regards,
Christian.
> + break;
> + }
> + dma_resv_unlock(bo->tbo.base.resv);
> + } else {
> + placement = "UNKNOWN";
>}
>
>size = amdgpu_bo_size(bo);
Re: [PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)
Hi Alex, On 23/10/2019 14:50, Deucher, Alexander wrote: >> -Original Message- >> From: amd-gfx On Behalf Of >> Christian König >> Sent: Wednesday, October 23, 2019 3:33 AM >> To: Pelloux-prayer, Pierre-eric ; amd- >> g...@lists.freedesktop.org >> Subject: Re: [PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2) >> >> Am 22.10.19 um 19:22 schrieb Pelloux-prayer, Pierre-eric: >>> This seems to help with >> https://bugs.freedesktop.org/show_bug.cgi?id=111481. >>> >>> v2: insert a NOP instead of skipping all 0-sized IBs to avoid breaking older >> hw >>> >>> Signed-off-by: Pierre-Eric Pelloux-Prayer > pra...@amd.com> >> >> Reviewed-by: Christian König > > Do nop packets have any alignment requirements on SDMA? Some of the other > packets do. There's no alignment requirements for nop packets. Pierre-Eric > > Alex > >> >>> --- >>> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c >> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c >>> index fb48622c2abd..6e1b25bd1fe7 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c >>> @@ -309,6 +309,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct >> amdgpu_device *adev, uint32_t vmid, >>> >>> job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo); >>> job->vm_needs_flush = true; >>> + job->ibs->ptr[job->ibs->length_dw++] = ring->funcs->nop; >>> amdgpu_ring_pad_ib(ring, >ibs[0]); >>> r = amdgpu_job_submit(job, >mman.entity, >>> AMDGPU_FENCE_OWNER_UNDEFINED, ); >> >> ___ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: call amdgpu_vm_prt_fini before deleting the root PD
amdgpu_vm_prt_fini uses "vm->root.base.bo" so it must still be valid when
we call it.
Fixes: b65709a92156 ("drm/amdgpu: reserve the root PD while freeing PASIDs")
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d9bece987e60..c8ce42200059 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2975,6 +2975,16 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
vm->pasid = 0;
}
+ list_for_each_entry_safe(mapping, tmp, >freed, list) {
+ if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
+ amdgpu_vm_prt_fini(adev, vm);
+ prt_fini_needed = false;
+ }
+
+ list_del(>list);
+ amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
+ }
+
amdgpu_vm_free_pts(adev, vm, NULL);
amdgpu_bo_unreserve(root);
amdgpu_bo_unref();
@@ -2994,15 +3004,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
list_del(>list);
kfree(mapping);
}
- list_for_each_entry_safe(mapping, tmp, >freed, list) {
- if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
- amdgpu_vm_prt_fini(adev, vm);
- prt_fini_needed = false;
- }
-
- list_del(>list);
- amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
- }
dma_fence_put(vm->last_update);
for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
--
2.23.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)
This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481. v2: insert a NOP instead of skipping all 0-sized IBs to avoid breaking older hw Signed-off-by: Pierre-Eric Pelloux-Prayer --- drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c index fb48622c2abd..6e1b25bd1fe7 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c @@ -309,6 +309,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid, job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo); job->vm_needs_flush = true; + job->ibs->ptr[job->ibs->length_dw++] = ring->funcs->nop; amdgpu_ring_pad_ib(ring, >ibs[0]); r = amdgpu_job_submit(job, >mman.entity, AMDGPU_FENCE_OWNER_UNDEFINED, ); -- 2.23.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
On 03/10/2019 15:12, Deucher, Alexander wrote:
> Does some variant of the patch on this thread help?
> https://patchwork.freedesktop.org/patch/333068/
Hi Alex,
The added condition in this patch is:
!(adev->asic_type >= CHIP_NAVI10 && adev->asic_type <= CHIP_NAVI12) ||
which will evaluate to "false ||" on Navi10 so I don't think it'll help.
I'll send an updated version of my patch that will only modify
gmc_v10_0_flush_gpu_tlb
to not send a 0-sized IB.
Thanks,
Pierre-Eric
>
> Alex
>
> ------
> *From:* amd-gfx on behalf of
> Pelloux-prayer, Pierre-eric
> *Sent:* Thursday, October 3, 2019 4:25 AM
> *To:* Koenig, Christian ;
> amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
>
>
> On 03/10/2019 10:09, Christian König wrote:
>> Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
>>> This can be safely skipped entirely.
>>> This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>>
>> NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some NOP
>> in the submitted IBs.
>
> Is there any interest in executing an empty (or only filled with NOPs) IB?
>
> Anyway I can modify the patch to do this.
>
> Thanks,
> Pierre-Eric
>
>>
>> Christian.
>>
>>>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> index 60655834d649..aa163e679f1f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> @@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>>> unsigned num_ibs,
>>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE
>>> ib must be inserted anyway */
>>> continue;
>>> + if (ib->length_dw == 0) {
>>> + /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
>>> + continue;
>>> + }
>>> +
>>> amdgpu_ring_emit_ib(ring, job, ib, status);
>>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>>> }
>>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
On 03/10/2019 10:09, Christian König wrote:
> Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
>> This can be safely skipped entirely.
>> This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>
> NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some NOP
> in the submitted IBs.
Is there any interest in executing an empty (or only filled with NOPs) IB?
Anyway I can modify the patch to do this.
Thanks,
Pierre-Eric
>
> Christian.
>
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 60655834d649..aa163e679f1f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE
>> ib must be inserted anyway */
>> continue;
>> + if (ib->length_dw == 0) {
>> + /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
>> + continue;
>> + }
>> +
>> amdgpu_ring_emit_ib(ring, job, ib, status);
>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: do not execute 0-sized IBs
This can be safely skipped entirely.
This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 60655834d649..aa163e679f1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
CE ib must be inserted anyway */
continue;
+ if (ib->length_dw == 0) {
+ /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
+ continue;
+ }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
--
2.23.0.rc1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: fix gfx9 soft recovery
The SOC15_REG_OFFSET() macro wasn't used, making the soft recovery fail. v2: use WREG32_SOC15 instead of WREG32 + SOC15_REG_OFFSET Signed-off-by: Pierre-Eric Pelloux-Prayer Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index bcd0301eee1e..1a2963e4bc68 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -5375,7 +5375,7 @@ static void gfx_v9_0_ring_soft_recovery(struct amdgpu_ring *ring, unsigned vmid) value = REG_SET_FIELD(value, SQ_CMD, MODE, 0x01); value = REG_SET_FIELD(value, SQ_CMD, CHECK_VMID, 1); value = REG_SET_FIELD(value, SQ_CMD, VM_ID, vmid); - WREG32(mmSQ_CMD, value); + WREG32_SOC15(GC, 0, mmSQ_CMD, value); } static void gfx_v9_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev, -- 2.23.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: fix gfx9 soft recovery
The SOC15_REG_OFFSET() macro wasn't used, making the soft recovery fail. Signed-off-by: Pierre-Eric Pelloux-Prayer --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index bcd0301eee1e..ff87f6ea5cd6 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -5375,7 +5375,7 @@ static void gfx_v9_0_ring_soft_recovery(struct amdgpu_ring *ring, unsigned vmid) value = REG_SET_FIELD(value, SQ_CMD, MODE, 0x01); value = REG_SET_FIELD(value, SQ_CMD, CHECK_VMID, 1); value = REG_SET_FIELD(value, SQ_CMD, VM_ID, vmid); - WREG32(mmSQ_CMD, value); + WREG32(SOC15_REG_OFFSET(GC, 0, mmSQ_CMD), value); } static void gfx_v9_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev, -- 2.23.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 10/10] drm/amdgpu: stop removing BOs from the LRU v3
Hi Christian,
The series is:
Tested-by: Pierre-Eric Pelloux-Prayer
Pierre-Eric
On 29/05/2019 14:27, Christian König wrote:
> This avoids OOM situations when we have lots of threads
> submitting at the same time.
>
> v3: apply this to the whole driver, not just CS
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c| 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c| 4 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 20f2955d2a55..3e2da24cd17a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser
> *p,
> }
>
> r = ttm_eu_reserve_buffers(>ticket, >validated, true,
> -, true);
> +, false);
> if (unlikely(r != 0)) {
> if (r != -ERESTARTSYS)
> DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> index 06f83cac0d3a..f660628e6af9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> @@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev,
> struct amdgpu_vm *vm,
> list_add(_tv.head, );
> amdgpu_vm_get_pd_bo(vm, , );
>
> - r = ttm_eu_reserve_buffers(, , true, NULL, true);
> + r = ttm_eu_reserve_buffers(, , true, NULL, false);
> if (r) {
> DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r);
> return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index d513a5ad03dd..ed25a4e14404 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>
> amdgpu_vm_get_pd_bo(vm, , _pd);
>
> - r = ttm_eu_reserve_buffers(, , false, , true);
> + r = ttm_eu_reserve_buffers(, , false, , false);
> if (r) {
> dev_err(adev->dev, "leaking bo va because "
> "we fail to reserve bo (%d)\n", r);
> @@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void
> *data,
>
> amdgpu_vm_get_pd_bo(>vm, , _pd);
>
> - r = ttm_eu_reserve_buffers(, , true, , true);
> + r = ttm_eu_reserve_buffers(, , true, , false);
> if (r)
> goto error_unref;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index c430e8259038..d60593cc436e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -155,7 +155,7 @@ static inline int amdgpu_bo_reserve(struct amdgpu_bo *bo,
> bool no_intr)
> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> int r;
>
> - r = ttm_bo_reserve(>tbo, !no_intr, false, NULL);
> + r = __ttm_bo_reserve(>tbo, !no_intr, false, NULL);
> if (unlikely(r != 0)) {
> if (r != -ERESTARTSYS)
> dev_err(adev->dev, "%p reserve failed\n", bo);
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
