Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD
I was able to bisect it to this commit: $git bisect good 6643ba1ff05d252e451bada9443759edb95eab3b is the first bad commit commit 6643ba1ff05d252e451bada9443759edb95eab3b Author: Luben Tuikov Date: Mon Feb 10 18:16:45 2020 -0500 drm/amdgpu: Move to a per-IB secure flag (TMZ) Move from a per-CS secure flag (TMZ) to a per-IB secure flag. Signed-off-by: Luben Tuikov Reviewed-by: Huang Rui drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 -- drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 23 --- drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 9 - drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 23 +++ drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c| 3 +-- drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c| 3 +-- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 3 +-- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 ++-- include/uapi/drm/amdgpu_drm.h| 7 --- 10 files changed, 44 insertions(+), 52 deletions(-) It's a bit baffling and perhaps there is a clash in the new flag, or libdrm needs to also be updated. Will look at it more tomorrow. My bisect log can be found below. Regards, Luben git bisect start # good: [31866a9d7d40245316ad7c17b87961f68321cab8] drm/amd/display: Move drm_dp_mst_atomic_check() to the front of dc_validate_global_state() git bisect good 31866a9d7d40245316ad7c17b87961f68321cab8 # bad: [7fd3b632e17e55c5ffd008f9f025754e7daa1b66] drm/amdgpu: fix colliding of preemption git bisect bad 7fd3b632e17e55c5ffd008f9f025754e7daa1b66 # good: [41d073f29e59abdfb0d415033772c01c321086c9] drm/amdgpu/vcn2.5: fix warning git bisect good 41d073f29e59abdfb0d415033772c01c321086c9 # good: [71da21488b65ade2b789416088b9f2493ad3e056] drm/amd/display: fix dtm unloading git bisect good 71da21488b65ade2b789416088b9f2493ad3e056 # bad: [e3ca25cd2e75824e4dd9e6bb16013ab5f3ec63a6] drm/ttm: individualize resv objects before calling release_notify git bisect bad e3ca25cd2e75824e4dd9e6bb16013ab5f3ec63a6 # good: [7e3452a6536ee7136a4d79f2369f15d5ce96583c] drm/amdgpu: return -EFAULT if copy_to_user() fails git bisect good 7e3452a6536ee7136a4d79f2369f15d5ce96583c # bad: [9b7ac0fb3bbfd6dd001423da497aafec3e8a5131] drm/amdgpu: log on non-zero error conter per IP before GPU reset git bisect bad 9b7ac0fb3bbfd6dd001423da497aafec3e8a5131 # bad: [6643ba1ff05d252e451bada9443759edb95eab3b] drm/amdgpu: Move to a per-IB secure flag (TMZ) git bisect bad 6643ba1ff05d252e451bada9443759edb95eab3b # good: [3387f56e37b2fa8b0fbb3a538bc08daae923bb5f] drm/amd/powerplay: correct the way for checking SMU_FEATURE_BACO_BIT support git bisect good 3387f56e37b2fa8b0fbb3a538bc08daae923bb5f # first bad commit: [6643ba1ff05d252e451bada9443759edb95eab3b] drm/amdgpu: Move to a per-IB secure flag (TMZ) On 2020-02-19 8:02 p.m., Luben Tuikov wrote: > New developments: > > Running "amdgpu_test -s 1 -t 4" causes timeouts and koops. Attached > is the system log, tested Navi 10: > > [ 144.484547] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting > for fences timed out! > [ 149.604641] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_0.0.0 > timeout, signaled seq=1459, emitted seq=1462 > [ 149.604779] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process > information: process amdgpu_test pid 2696 thread amdgpu_test pid 2696 > [ 149.604788] amdgpu :0b:00.0: GPU reset begin! > ... > > The kernel is at 7fd3b632e17e55c5ffd008f9f025754e7daa1b66 plus > the patch of the original post of this thread (thus the "-dirty"). > > Running the same test on the previous version of the kernel I was running, > at 31866a9d7d40245316ad7c17b87961f68321cab8, succeeds as follows: > > Suite: Basic Tests > Test: Command submission Test (GFX) ...passed > > Run Summary:Type TotalRan Passed Failed Inactive > suites 11 0n/a 00 >tests 63 1 1 00 > asserts 526725 526725 526725 0 n/a > > Elapsed time =0.027 seconds > > Regards, > Luben > > On 2020-02-19 4:40 p.m., Luben Tuikov wrote: >> On 2020-02-19 9:44 a.m., Christian König wrote: >>> Well it should apply on top of amd-staging-drm-next. But I haven't >>> fetched that today yet. >>> >>> Give me a minute to rebase. >> >> This patch seems to have fixed the regression we saw yesterday. >> It applies to amd-staging-drm-next with a small jitter: >> >> $patch -p1 < /tmp/\[PATCH\]\ drm_amdgpu\:\ add\ VM\ update\ fences\ back\ >> to\ the\ root\ PD.eml >> patching file amdgpu_vm.c >> Hunk #2 succeeded at 1599 (offset -20 lines). >> >> I've been running 'glxgears' on the root window and 'pinion' >> and no problems--clean log. >> >> Tested-by: Luben Tuikov >> >> Regards, >> Luben >> >>> >>> Christian. >>> >>> Am 19.02.20 um 15:27 schrieb Tom St Denis: This doesn't apply on top of 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace
[AMD Official Use Only - Internal Distribution Only]
Christian is right here, that will cause many problems for simply using VMID in
kernel.
We already have an pair interface for RGP, I think you can use it instead of
involving additional kernel change.
amdgpu_vm_reserve_vmid/ amdgpu_vm_unreserve_vmid.
-David
-Original Message-
From: amd-gfx On Behalf Of Christian
König
Sent: Wednesday, February 19, 2020 7:03 PM
To: He, Jacob ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace
Am 19.02.20 um 11:15 schrieb Jacob He:
> [WHY]
> When SPM trace enabled, SPM_VMID should be updated with the current
> vmid.
>
> [HOW]
> Add a chunk id, AMDGPU_CHUNK_ID_SPM_TRACE, so that UMD can tell us
> which job should update SPM_VMID.
> Right before a job is submitted to GPU, set the SPM_VMID accordingly.
>
> [Limitation]
> Running more than one SPM trace enabled processes simultaneously is
> not supported.
Well there are multiple problems with that patch.
First of all you need to better describe what SPM tracing is in the commit
message.
Then the updating of mmRLC_SPM_MC_CNTL must be executed asynchronously on the
ring. Otherwise we might corrupt an already executing SPM trace.
And you also need to make sure to disable the tracing again or otherwise we run
into a bunch of trouble when the VMID is reused.
You also need to make sure that IBs using the SPM trace are serialized with
each other, e.g. hack into amdgpu_ids.c file and make sure that only one VMID
at a time can have that attribute.
Regards,
Christian.
>
> Change-Id: Ic932ef6ac9dbf244f03aaee90550e8ff3a675666
> Signed-off-by: Jacob He
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 7 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 10 +++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h | 1 +
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 15 ++-
> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 15 ++-
> 8 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index f9fa6e104fef..3f32c4db5232 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -113,6 +113,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
> *p, union drm_amdgpu_cs
> uint32_t uf_offset = 0;
> int i;
> int ret;
> + bool update_spm_vmid = false;
>
> if (cs->in.num_chunks == 0)
> return 0;
> @@ -221,6 +222,10 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
> *p, union drm_amdgpu_cs
> case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
> break;
>
> + case AMDGPU_CHUNK_ID_SPM_TRACE:
> + update_spm_vmid = true;
> + break;
> +
> default:
> ret = -EINVAL;
> goto free_partial_kdata;
> @@ -231,6 +236,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
> *p, union drm_amdgpu_cs
> if (ret)
> goto free_all_kdata;
>
> + p->job->need_update_spm_vmid = update_spm_vmid;
> +
> if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
> ret = -ECANCELED;
> goto free_all_kdata;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index cae81914c821..36faab12b585 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -156,9 +156,13 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> unsigned num_ibs,
> return -EINVAL;
> }
>
> - if (vm && !job->vmid) {
> - dev_err(adev->dev, "VM IB without ID\n");
> - return -EINVAL;
> + if (vm) {
> + if (!job->vmid) {
> + dev_err(adev->dev, "VM IB without ID\n");
> + return -EINVAL;
> + } else if (adev->gfx.rlc.funcs->update_spm_vmid &&
> job->need_update_spm_vmid) {
> + adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid);
> + }
> }
>
> alloc_size = ring->funcs->emit_frame_size + num_ibs * diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index 2e2110dddb76..4582536961c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -52,6 +52,7 @@ struct amdgpu_job {
> boolvm_needs_flush;
> uint64_tvm_pd_addr;
> unsignedvmid;
> + boolneed_update_spm_vmid;
> unsignedpasid;
> uint32_tgds_base, gds_size;
>
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD
New developments:
Running "amdgpu_test -s 1 -t 4" causes timeouts and koops. Attached
is the system log, tested Navi 10:
[ 144.484547] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for
fences timed out!
[ 149.604641] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx_0.0.0
timeout, signaled seq=1459, emitted seq=1462
[ 149.604779] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
process amdgpu_test pid 2696 thread amdgpu_test pid 2696
[ 149.604788] amdgpu :0b:00.0: GPU reset begin!
...
The kernel is at 7fd3b632e17e55c5ffd008f9f025754e7daa1b66 plus
the patch of the original post of this thread (thus the "-dirty").
Running the same test on the previous version of the kernel I was running,
at 31866a9d7d40245316ad7c17b87961f68321cab8, succeeds as follows:
Suite: Basic Tests
Test: Command submission Test (GFX) ...passed
Run Summary:Type TotalRan Passed Failed Inactive
suites 11 0n/a 00
tests 63 1 1 00
asserts 526725 526725 526725 0 n/a
Elapsed time =0.027 seconds
Regards,
Luben
On 2020-02-19 4:40 p.m., Luben Tuikov wrote:
> On 2020-02-19 9:44 a.m., Christian König wrote:
>> Well it should apply on top of amd-staging-drm-next. But I haven't
>> fetched that today yet.
>>
>> Give me a minute to rebase.
>
> This patch seems to have fixed the regression we saw yesterday.
> It applies to amd-staging-drm-next with a small jitter:
>
> $patch -p1 < /tmp/\[PATCH\]\ drm_amdgpu\:\ add\ VM\ update\ fences\ back\ to\
> the\ root\ PD.eml
> patching file amdgpu_vm.c
> Hunk #2 succeeded at 1599 (offset -20 lines).
>
> I've been running 'glxgears' on the root window and 'pinion'
> and no problems--clean log.
>
> Tested-by: Luben Tuikov
>
> Regards,
> Luben
>
>>
>> Christian.
>>
>> Am 19.02.20 um 15:27 schrieb Tom St Denis:
>>> This doesn't apply on top of 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
>>> which is the tip of drm-next
>>>
>>>
>>> Tom
>>>
>>> On 2020-02-19 9:20 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e7ab0c1e2793..dd63ccdbad2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -585,8 +585,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1619,6 +1619,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, resv, sync_mode);
if (r)
goto error_unlock;
>>
>
[0.00] Linux version 5.4.0-rc7-luben-work-02847-g7fd3b632e17e-dirty
(ltuikov@localhost.localdomain) (gcc version 9.2.1 20190827 (Red Hat 9.2.1-1)
(GCC)) #4 SMP Wed Feb 19 16:04:37 EST 2020
[0.00] Command line:
BOOT_IMAGE=(hd0,msdos1)/vmlinuz-5.4.0-rc7-luben-work-02847-g7fd3b632e17e-dirty
root=/dev/mapper/fedora_localhost--live-root ro
resume=/dev/mapper/fedora_localhost--live-swap
rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap
[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point
registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[0.00] x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256
[0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes,
using 'compacted' format.
[0.00] BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009ebff] usable
[0.00] BIOS-e820: [mem 0x0009ec00-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e-0x000f] reserved
[0.00] BIOS-e820: [mem
Re: [PATCH] drm/amdgpu: Add a GEM_CREATE mask and bugfix
On 2020-02-19 3:20 a.m., Christian König wrote:
> Am 18.02.20 um 22:46 schrieb Luben Tuikov:
>> On 2020-02-17 10:08 a.m., Christian König wrote:
>>> Am 17.02.20 um 15:44 schrieb Alex Deucher:
On Fri, Feb 14, 2020 at 7:17 PM Luben Tuikov wrote:
> Add a AMDGPU_GEM_CREATE_MASK and use it to check
> for valid/invalid GEM create flags coming in from
> userspace.
>
> Fix a bug in checking whether TMZ is supported at
> GEM create time.
>
> Signed-off-by: Luben Tuikov
> ---
>drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 ++-
>include/uapi/drm/amdgpu_drm.h | 2 ++
>2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index b51a060c637d..74bb79e64fa3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -221,21 +221,14 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev,
> void *data,
> int r;
>
> /* reject invalid gem flags */
> - if (flags & ~(AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> - AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> - AMDGPU_GEM_CREATE_VRAM_CLEARED |
> - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
> - AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> - AMDGPU_GEM_CREATE_ENCRYPTED))
> -
I'd rather keep the list explicit so no one ends up forgetting to
update the mask the next time new flags are added.
>>> Additional to that this patch is bogus.
>> So the only "additional" thing you're contributing to the review of
>> this patch is calling it "bogus". Characterizing the patch with an adjective
>> as "bogus" is very disturbing. What's next?
>
> Well the patch is technical incorrect and breaks the code in a very
> subtle and hard to detect manner. Alex didn't noticed that either.
>
> I'm not a native speaker of English, but as far as I have learned
> "bogus" is the right adjective for that.
>
>>> We intentionally filter out the flags here which userspace is allowed to
>>> specify in the GEM interface, but after this patch we would allow all
>>> flags to be specified.
>> I thought that that could be the case.
>
> Then why did you send out a patch which is seriously broken like that?
>
> I mean if I hadn't noticed it by chance we would have committed this and
> added a potentially security problematic bug to the IOCTL interface.
>
>> But in your review why not
>> recommend we have a mask to check user-settable flags? So the
>> actual flags are collected and visible in one place and well
>> understood that that is what is being checked and well-defined
>> by a macro with an appropriate name?
>
> See the flags tested here are the flags currently accepted by the
> amdgpu_gem_create_ioctl() function. It doesn't say anything about what
> the kernel would accept in the future.
>
> So when we move that into the UAPI header we give userspace a
> technically incorrect value.
>
>> Why did NO ONE comment on the bug fix below? No one.
>
> Because you mixed up a style change into some bug fix. That people go
> for the problematic parts during code review is not really surprising at
> all.
I wouldn't have expected a petulant-childlike response.
I would've expected a leadership-like position, something like:
Split your patch into two. The second part is okay with the addition
of DRM_PRINT_ONCE, and the first part admits bits which we don't
want user space to control.
Regards,
Luben
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Luben
>>
>>> Christian.
>>>
>>>
>>>
Alex
> + if (flags & ~AMDGPU_GEM_CREATE_MASK)
> return -EINVAL;
>
> /* reject invalid gem domains */
> if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
> return -EINVAL;
>
> - if (amdgpu_is_tmz(adev) && (flags & AMDGPU_GEM_CREATE_ENCRYPTED))
> {
> + if (!amdgpu_is_tmz(adev) && flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
> DRM_ERROR("Cannot allocate secure buffer since TMZ is
> disabled\n");
> return -EINVAL;
> }
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index eaf94a421901..c8463cdf4448 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -141,6 +141,8 @@ extern "C" {
> */
>#define AMDGPU_GEM_CREATE_ENCRYPTED(1 << 10)
>
> +#define AMDGPU_GEM_CREATE_MASK ((1 << 11)-1)
> +
>struct drm_amdgpu_gem_create_in {
> /** the requested memory size */
> __u64 bo_size;
> --
> 2.25.0.232.gd8437c57fa
>
>
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD
On 2020-02-19 9:44 a.m., Christian König wrote:
> Well it should apply on top of amd-staging-drm-next. But I haven't
> fetched that today yet.
>
> Give me a minute to rebase.
This patch seems to have fixed the regression we saw yesterday.
It applies to amd-staging-drm-next with a small jitter:
$patch -p1 < /tmp/\[PATCH\]\ drm_amdgpu\:\ add\ VM\ update\ fences\ back\ to\
the\ root\ PD.eml
patching file amdgpu_vm.c
Hunk #2 succeeded at 1599 (offset -20 lines).
I've been running 'glxgears' on the root window and 'pinion'
and no problems--clean log.
Tested-by: Luben Tuikov
Regards,
Luben
>
> Christian.
>
> Am 19.02.20 um 15:27 schrieb Tom St Denis:
>> This doesn't apply on top of 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
>> which is the tip of drm-next
>>
>>
>> Tom
>>
>> On 2020-02-19 9:20 a.m., Christian König wrote:
>>> Add update fences to the root PD while mapping BOs.
>>>
>>> Otherwise PDs freed during the mapping won't wait for
>>> updates to finish and can cause corruptions.
>>>
>>> Signed-off-by: Christian König
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index e7ab0c1e2793..dd63ccdbad2a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -585,8 +585,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
>>> {
>>> entry->priority = 0;
>>> entry->tv.bo = >root.base.bo->tbo;
>>> - /* One for TTM and one for the CS job */
>>> - entry->tv.num_shared = 2;
>>> + /* Two for VM updates, one for TTM and one for the CS job */
>>> + entry->tv.num_shared = 4;
>>> entry->user_pages = NULL;
>>> list_add(>tv.head, validated);
>>> }
>>> @@ -1619,6 +1619,16 @@ static int amdgpu_vm_bo_update_mapping(struct
>>> amdgpu_device *adev,
>>> goto error_unlock;
>>> }
>>> + if (flags & AMDGPU_PTE_VALID) {
>>> + struct amdgpu_bo *root = vm->root.base.bo;
>>> +
>>> + if (!dma_fence_is_signaled(vm->last_direct))
>>> + amdgpu_bo_fence(root, vm->last_direct, true);
>>> +
>>> + if (!dma_fence_is_signaled(vm->last_delayed))
>>> + amdgpu_bo_fence(root, vm->last_delayed, true);
>>> + }
>>> +
>>> r = vm->update_funcs->prepare(, resv, sync_mode);
>>> if (r)
>>> goto error_unlock;
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdgpu/powerplay: Refactor SMU message handling for safety
Move the responsibility for reading argument registers into the
smu_send_smc_msg* implementations, so that adding a message-sending lock
to protect the SMU registers will result in the lock still being held
when the argument is read.
For code compatibility on hardware I don't have the funds to buy, I kept
the old interface around with `_unsafe` suffixes.
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c| 47 +++-
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 18 +--
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h| 3 +-
drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 7 +-
drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h | 3 +-
drivers/gpu/drm/amd/powerplay/navi10_ppt.c| 76 +++-
drivers/gpu/drm/amd/powerplay/renoir_ppt.c| 22 ++--
drivers/gpu/drm/amd/powerplay/smu_internal.h | 15 ++-
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 114 +-
drivers/gpu/drm/amd/powerplay/smu_v12_0.c | 65 ++
drivers/gpu/drm/amd/powerplay/vega20_ppt.c| 42 +++
11 files changed, 213 insertions(+), 199 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 4720aa58a150..759f7446529f 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -121,20 +121,20 @@ static int smu_feature_update_enable_state(struct
smu_context *smu,
if (enabled) {
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_EnableSmuFeaturesLow,
- feature_low);
+ feature_low, NULL);
if (ret)
return ret;
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_EnableSmuFeaturesHigh,
- feature_high);
+ feature_high, NULL);
if (ret)
return ret;
} else {
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_DisableSmuFeaturesLow,
- feature_low);
+ feature_low, NULL);
if (ret)
return ret;
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_DisableSmuFeaturesHigh,
- feature_high);
+ feature_high, NULL);
if (ret)
return ret;
}
@@ -195,21 +195,13 @@ int smu_get_smc_version(struct smu_context *smu, uint32_t
*if_version, uint32_t
return -EINVAL;
if (if_version) {
- ret = smu_send_smc_msg(smu, SMU_MSG_GetDriverIfVersion);
- if (ret)
- return ret;
-
- ret = smu_read_smc_arg(smu, if_version);
+ ret = smu_send_smc_msg(smu, SMU_MSG_GetDriverIfVersion,
if_version);
if (ret)
return ret;
}
if (smu_version) {
- ret = smu_send_smc_msg(smu, SMU_MSG_GetSmuVersion);
- if (ret)
- return ret;
-
- ret = smu_read_smc_arg(smu, smu_version);
+ ret = smu_send_smc_msg(smu, SMU_MSG_GetSmuVersion, smu_version);
if (ret)
return ret;
}
@@ -251,7 +243,7 @@ int smu_set_hard_freq_range(struct smu_context *smu, enum
smu_clk_type clk_type,
if (max > 0) {
param = (uint32_t)((clk_id << 16) | (max & 0x));
ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetHardMaxByFreq,
- param);
+ param, NULL);
if (ret)
return ret;
}
@@ -259,7 +251,7 @@ int smu_set_hard_freq_range(struct smu_context *smu, enum
smu_clk_type clk_type,
if (min > 0) {
param = (uint32_t)((clk_id << 16) | (min & 0x));
ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetHardMinByFreq,
- param);
+ param, NULL);
if (ret)
return ret;
}
@@ -335,12 +327,8 @@ int smu_get_dpm_freq_by_index(struct smu_context *smu,
enum smu_clk_type clk_typ
param = (uint32_t)(((clk_id & 0x) << 16) | (level & 0x));
- ret = smu_send_smc_msg_with_param(smu,SMU_MSG_GetDpmFreqByIndex,
- param);
- if (ret)
- return ret;
-
- ret = smu_read_smc_arg(smu, );
+ ret = smu_send_smc_msg_with_param(smu, SMU_MSG_GetDpmFreqByIndex,
+ param, );
if (ret)
return ret;
@@ -542,7 +530,8
[PATCH 0/2] Implement SMU message register protection
Hey Alex, I took a crack at the implementation I was talking about here, where we can protect the read argument register reads as well. I only transitioned the actual implementation for hardware that I have funds for/access to, and left an '_unsafe' path for the other implementations since I cannot test. What do you think of this implementation? Matt Coffin (2): drm/amdgpu/powerplay: Refactor SMU message handling for safety drm/amdgpu/smu: Add message sending lock drivers/gpu/drm/amd/powerplay/amdgpu_smu.c| 47 +++ drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 18 +-- .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h| 3 +- drivers/gpu/drm/amd/powerplay/inc/smu_v11_0.h | 7 +- drivers/gpu/drm/amd/powerplay/inc/smu_v12_0.h | 3 +- drivers/gpu/drm/amd/powerplay/navi10_ppt.c| 76 ++- drivers/gpu/drm/amd/powerplay/renoir_ppt.c| 22 ++-- drivers/gpu/drm/amd/powerplay/smu_internal.h | 15 ++- drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 118 +- drivers/gpu/drm/amd/powerplay/smu_v12_0.c | 65 ++ drivers/gpu/drm/amd/powerplay/vega20_ppt.c| 42 +++ 11 files changed, 216 insertions(+), 200 deletions(-) -- 2.25.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu/smu: Add message sending lock
This adds a message lock to the smu_send_smc_msg* implementations to
protect against concurrent access to the mmu registers used to
communicate with the SMU
---
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 220bf0b10971..19bebba3e0a4 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -102,11 +102,12 @@ smu_v11_0_send_msg_with_param(struct smu_context *smu,
if (index < 0)
return index;
+ mutex_lock(>message_lock);
ret = smu_v11_0_wait_for_response(smu);
if (ret) {
pr_err("Msg issuing pre-check failed and "
"SMU may be not in the right state!\n");
- return ret;
+ goto out;
}
WREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_90, 0);
@@ -119,18 +120,19 @@ smu_v11_0_send_msg_with_param(struct smu_context *smu,
if (ret) {
pr_err("failed send message: %10s (%d) \tparam: 0x%08x response
%#x\n",
smu_get_message_name(smu, msg), index, param, ret);
- return ret;
+ goto out;
}
if (read_arg) {
ret = smu_v11_0_read_arg(smu, read_arg);
if (ret) {
pr_err("failed to read message arg: %10s (%d) \tparam:
0x%08x response %#x\n",
smu_get_message_name(smu, msg), index, param,
ret);
- return ret;
+ goto out;
}
}
-
- return 0;
+out:
+ mutex_unlock(>message_lock);
+ return ret;
}
int smu_v11_0_init_microcode(struct smu_context *smu)
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v6] drm/amdgpu: Remove kfd eviction fence before release bo
On 2020-02-19 7:46, xinhui pan wrote:
No need to trigger eviction as the memory mapping will not be used
anymore.
All pt/pd bos share same resv, hence the same shared eviction fence.
Everytime page table is freed, the fence will be signled and that cuases
kfd unexcepted evictions.
CC: Christian König
CC: Felix Kuehling
CC: Alex Deucher
Acked-by: Christian König
Signed-off-by: xinhui pan
Reviewed-by: Felix Kuehling
---
change from v5:
use trylock instead, and add warn_on_once to give a limitd warning.
change from v4:
based on new ttm code.
change from v3:
fix a coding error
change from v2:
based on Chris' drm/ttm: rework BO delayed delete patchset.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 1 +
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 38 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 5 +++
3 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 9e8db702d878..0ee8aae6c519 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -96,6 +96,7 @@ struct amdgpu_amdkfd_fence *amdgpu_amdkfd_fence_create(u64
context,
struct mm_struct *mm);
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);
+int amdgpu_amdkfd_remove_fence_on_pt_pd_bos(struct amdgpu_bo *bo);
struct amdkfd_process_info {
/* List head of all VMs that belong to a KFD process */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index ef721cb65868..898851bec377 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -276,6 +276,42 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
amdgpu_bo *bo,
return 0;
}
+int amdgpu_amdkfd_remove_fence_on_pt_pd_bos(struct amdgpu_bo *bo)
+{
+ struct amdgpu_bo *root = bo;
+ struct amdgpu_vm_bo_base *vm_bo;
+ struct amdgpu_vm *vm;
+ struct amdkfd_process_info *info;
+ struct amdgpu_amdkfd_fence *ef;
+ int ret;
+
+ /* we can always get vm_bo from root PD bo.*/
+ while (root->parent)
+ root = root->parent;
+
+ vm_bo = root->vm_bo;
+ if (!vm_bo)
+ return 0;
+
+ vm = vm_bo->vm;
+ if (!vm)
+ return 0;
+
+ info = vm->process_info;
+ if (!info || !info->eviction_fence)
+ return 0;
+
+ ef = container_of(dma_fence_get(>eviction_fence->base),
+ struct amdgpu_amdkfd_fence, base);
+
+ BUG_ON(!dma_resv_trylock(bo->tbo.base.resv));
+ ret = amdgpu_amdkfd_remove_eviction_fence(bo, ef);
+ dma_resv_unlock(bo->tbo.base.resv);
+
+ dma_fence_put(>base);
+ return ret;
+}
+
static int amdgpu_amdkfd_bo_validate(struct amdgpu_bo *bo, uint32_t domain,
bool wait)
{
@@ -1045,6 +1081,8 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device
*adev,
list_del(>vm_list_node);
mutex_unlock(_info->lock);
+ vm->process_info = NULL;
+
/* Release per-process resources when last compute VM is destroyed */
if (!process_info->n_vms) {
WARN_ON(!list_empty(_info->kfd_bo_list));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6f60a581e3ba..5766d20f29d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1307,6 +1307,11 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (abo->kfd_bo)
amdgpu_amdkfd_unreserve_memory_limit(abo);
+ /* We only remove the fence if the resv has individualized. */
+ WARN_ON_ONCE(bo->base.resv != >base._resv);
+ if (bo->base.resv == >base._resv)
+ amdgpu_amdkfd_remove_fence_on_pt_pd_bos(abo);
+
if (bo->mem.mem_type != TTM_PL_VRAM || !bo->mem.mm_node ||
!(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE))
return;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/discovery: make the discovery code less chatty
Make the IP block base output debug only.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index f95092741c38..27d8ae19a7a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -307,7 +307,7 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device
*adev)
for (hw_ip = 0; hw_ip < MAX_HWIP; hw_ip++) {
if (hw_id_map[hw_ip] == le16_to_cpu(ip->hw_id))
{
- DRM_INFO("set register base offset for
%s\n",
+ DRM_DEBUG("set register base offset for
%s\n",
hw_id_names[le16_to_cpu(ip->hw_id)]);
adev->reg_offset[hw_ip][ip->number_instance] =
ip->base_address;
--
2.24.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
On Wed, Feb 19, 2020 at 11:28:48AM -0500, Kenny Ho wrote: > On Wed, Feb 19, 2020 at 11:18 AM Johannes Weiner wrote: > > > > Yes, I'd go with absolute units when it comes to memory, because it's > > not a renewable resource like CPU and IO, and so we do have cliff > > behavior around the edge where you transition from ok to not-enough. > > > > memory.low is a bit in flux right now, so if anything is unclear > > around its semantics, please feel free to reach out. > > I am not familiar with the discussion, would you point me to a > relevant thread please? Here is a cleanup patch, not yet merged, that documents the exact semantics and behavioral considerations: https://lore.kernel.org/linux-mm/20191213192158.188939-3-han...@cmpxchg.org/ But the high-level idea is this: you assign each cgroup or cgroup subtree a chunk of the resource that it's guaranteed to be able to consume. It *can* consume beyond that threshold if available, but that overage may get reclaimed again if somebody else needs it instead. This allows you to do a ballpark distribution of the resource between different workloads, while the kernel retains the ability to optimize allocation of spare resources - because in practice, workload demand varies over time, workloads disappear and new ones start up etc. > In addition, is there some kind of order of preference for > implementing low vs high vs max? If you implement only one allocation model, the preference would be on memory.low. Limits are rigid and per definition waste resources, so in practice we're moving away from them. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[pull] amdgpu 5.6 fixes
Hi Dave, Daniel, Fixes for 5.6. The following changes since commit 6f4134b30b6ee33e2fd4d602099e6c5e60d0351a: Merge tag 'drm-intel-next-fixes-2020-02-13' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2020-02-14 13:04:46 +1000) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/amd-drm-fixes-5.6-2020-02-19 for you to fetch changes up to 6c62ce8073daf27ae3fd03b6929d6cea3887eeb2: drm/amdgpu/display: clean up hdcp workqueue handling (2020-02-19 11:03:24 -0500) amd-drm-fixes-5.6-2020-02-19: amdgpu: - HDCP fixes - xclk fix for raven - GFXOFF fixes Alex Deucher (4): drm/amdgpu/soc15: fix xclk for raven drm/amdgpu/gfx9: disable gfxoff when reading rlc clock drm/amdgpu/gfx10: disable gfxoff when reading rlc clock drm/amdgpu/display: clean up hdcp workqueue handling Bhawanpreet Lakha (2): drm/amd/display: fix backwards byte order in rx_caps. drm/amd/display: fix dtm unloading Evan Quan (1): drm/amd/powerplay: always refetch the enabled features status on dpm enablement changzhu (1): drm/amdgpu: add is_raven_kicker judgement for raven1 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 26 +- drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 2 ++ drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 13 +-- drivers/gpu/drm/amd/amdgpu/soc15.c | 7 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 + .../drm/amd/display/modules/hdcp/hdcp2_execution.c | 4 ++-- drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 ++--- 7 files changed, 55 insertions(+), 13 deletions(-) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD v2
For amd-staging-drm-next you need the first version of the patch.
For drm-misc-next or drm-next you need the second version of the patch.
We probably need to merge the patch through drm-misc-next anyway since
there is also the patch which causes the problems.
Christian.
Am 19.02.20 um 16:47 schrieb Tom St Denis:
The tip of origin/amd-staging-drm-next for me is:
commit 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
Refs: {origin/amd-staging-drm-next}, v5.4-rc7-2847-g7fd3b632e17e
Author: Monk Liu
AuthorDate: Thu Feb 6 23:55:58 2020 +0800
Commit: Monk Liu
CommitDate: Wed Feb 19 13:33:02 2020 +0800
drm/amdgpu: fix colliding of preemption
what:
some os preemption path is messed up with world switch preemption
fix:
cleanup those logics so os preemption not mixed with world switch
this patch is a general fix for issues comes from SRIOV MCBP, but
there is still UMD side issues not resovlved yet, so this patch
cannot fix all world switch bug.
Signed-off-by: Monk Liu
Acked-by: Hawking Zhang
Which I had fetched just an hour ago.
On 2020-02-19 10:41 a.m., Christian König wrote:
Well what branch are you trying to merge that into?
The conflict resolution should be simple, just keep the
vm->update_funcs->prepare(...) line as it is in your branch.
When you get those errors then something went wrong in your rebase.
Christian.
Am 19.02.20 um 16:14 schrieb Tom St Denis:
Doesn't build even with conflict resolved:
[root@raven linux]# make
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c: In function
‘amdgpu_vm_bo_update_mapping’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: error: ‘owner’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: note: each
undeclared identifier is reported only once for each function it
appears in
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:48: error: ‘exclusive’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
make[4]: *** [scripts/Makefile.build:266:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o] Error 1
make[3]: *** [scripts/Makefile.build:509:
drivers/gpu/drm/amd/amdgpu] Error 2
make[2]: *** [scripts/Makefile.build:509: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:509: drivers/gpu] Error 2
make: *** [Makefile:1649: drivers] Error 2
Should I just move to drm-misc-next?
tom
On 2020-02-19 10:02 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to
the resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int
amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
On Wed, Feb 19, 2020 at 11:18 AM Johannes Weiner wrote: > > Yes, I'd go with absolute units when it comes to memory, because it's > not a renewable resource like CPU and IO, and so we do have cliff > behavior around the edge where you transition from ok to not-enough. > > memory.low is a bit in flux right now, so if anything is unclear > around its semantics, please feel free to reach out. I am not familiar with the discussion, would you point me to a relevant thread please? In addition, is there some kind of order of preference for implementing low vs high vs max? Regards, Kenny ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/3] drm/amdgpu: Enter low power state if CRTC active.
On Mon, Dec 16, 2019 at 12:18 PM Alex Deucher wrote:
>
> From: Andrey Grodzovsky
>
> CRTC in DPMS state off calls for low power state entry.
> Support both atomic mode setting and pre-atomic mode setting.
>
> v2: move comment
>
> Signed-off-by: Andrey Grodzovsky
> Signed-off-by: Alex Deucher
Ping?
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 45 +
> 1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 61dc26515c7e..e7f7463a0cbe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1296,24 +1296,55 @@ static int amdgpu_pmops_runtime_idle(struct device
> *dev)
> {
> struct drm_device *drm_dev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_dev->dev_private;
> - struct drm_crtc *crtc;
> + /* we don't want the main rpm_idle to call suspend - we want to
> autosuspend */
> + int ret = 1;
>
> if (!adev->runpm) {
> pm_runtime_forbid(dev);
> return -EBUSY;
> }
>
> - list_for_each_entry(crtc, _dev->mode_config.crtc_list, head) {
> - if (crtc->enabled) {
> - DRM_DEBUG_DRIVER("failing to power off - crtc
> active\n");
> - return -EBUSY;
> + if (amdgpu_device_has_dc_support(adev)) {
> + struct drm_crtc *crtc;
> +
> + drm_modeset_lock_all(drm_dev);
> +
> + drm_for_each_crtc(crtc, drm_dev) {
> + if (crtc->state->active) {
> + ret = -EBUSY;
> + break;
> + }
> }
> +
> + drm_modeset_unlock_all(drm_dev);
> +
> + } else {
> + struct drm_connector *list_connector;
> + struct drm_connector_list_iter iter;
> +
> + mutex_lock(_dev->mode_config.mutex);
> + drm_modeset_lock(_dev->mode_config.connection_mutex,
> NULL);
> +
> + drm_connector_list_iter_begin(drm_dev, );
> + drm_for_each_connector_iter(list_connector, ) {
> + if (list_connector->dpms == DRM_MODE_DPMS_ON) {
> + ret = -EBUSY;
> + break;
> + }
> + }
> +
> + drm_connector_list_iter_end();
> +
> + drm_modeset_unlock(_dev->mode_config.connection_mutex);
> + mutex_unlock(_dev->mode_config.mutex);
> }
>
> + if (ret == -EBUSY)
> + DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
> +
> pm_runtime_mark_last_busy(dev);
> pm_runtime_autosuspend(dev);
> - /* we don't want the main rpm_idle to call suspend - we want to
> autosuspend */
> - return 1;
> + return ret;
> }
>
> long amdgpu_drm_ioctl(struct file *filp,
> --
> 2.23.0
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
On Fri, Feb 14, 2020 at 03:28:40PM -0500, Kenny Ho wrote: > On Fri, Feb 14, 2020 at 2:17 PM Tejun Heo wrote: > > Also, a rather trivial high level question. Is drm a good controller > > name given that other controller names are like cpu, memory, io? > > There was a discussion about naming early in the RFC (I believe > RFCv2), the consensuses then was to use drmcg to align with the drm > subsystem. I have no problem renaming it to gpucg or something > similar if that is the last thing that's blocking acceptance. For > now, I would like to get some clarity on the implementation before > having more code churn. As far as precedence goes, we named the other controllers after the resources they control rather than the subsystem: cpu instead of scheduler, memory instead of mm, io instead of block layer etc. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
On Fri, Feb 14, 2020 at 02:17:54PM -0500, Tejun Heo wrote: > Hello, Kenny, Daniel. > > (cc'ing Johannes) > > On Fri, Feb 14, 2020 at 01:51:32PM -0500, Kenny Ho wrote: > > On Fri, Feb 14, 2020 at 1:34 PM Daniel Vetter wrote: > > > > > > I think guidance from Tejun in previos discussions was pretty clear that > > > he expects cgroups to be both a) standardized and c) sufficient clear > > > meaning that end-users have a clear understanding of what happens when > > > they change the resource allocation. > > > > > > I'm not sure lgpu here, at least as specified, passes either. > > > > I disagree (at least on the characterization of the feedback > > provided.) I believe this series satisfied the sprite of Tejun's > > guidance so far (the weight knob for lgpu, for example, was > > specifically implemented base on his input.) But, I will let Tejun > > speak for himself after he considered the implementation in detail. > > I have to agree with Daniel here. My apologies if I weren't clear > enough. Here's one interface I can think of: > > * compute weight: The same format as io.weight. Proportional control >of gpu compute. > > * memory low: Please see how the system memory.low behaves. For gpus, >it'll need per-device entries. > > Note that for both, there one number to configure and conceptually > it's pretty clear to everybody what that number means, which is not to > say that it's clear to implement but it's much better to deal with > that on this side of the interface than the other. > > cc'ing Johannes. Do you have anything on mind regarding how gpu memory > configuration should look like? e.g. should it go w/ weights rather > than absoulte units (I don't think so given that it'll most likely > need limits at some point too but still and there are benefits from > staying consistent with system memory). Yes, I'd go with absolute units when it comes to memory, because it's not a renewable resource like CPU and IO, and so we do have cliff behavior around the edge where you transition from ok to not-enough. memory.low is a bit in flux right now, so if anything is unclear around its semantics, please feel free to reach out. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/display: clean up hdcp workqueue handling
Thanks.
Reviewed-by: Bhawanpreet Lakha
On 2020-02-19 9:24 a.m., Alex Deucher wrote:
Use the existence of the workqueue itself to determine when to
enable HDCP features rather than sprinkling asic checks all over
the code. Also add a check for the existence of the hdcp
workqueue in the irq handling on the off chance we get and HPD
RX interrupt with the CP bit set. This avoids a crash if
the driver doesn't support HDCP for a particular asic.
Fixes: 96a3b32e67236f ("drm/amd/display: only enable HDCP for DCN+")
Bug:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D206519data=02%7C01%7Cbhawanpreet.lakha%40amd.com%7C0bb0c7154e6946e09b0c08d7b547766c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637177190908488608sdata=AQqpGAH86XJj%2FtdLOWgWQchwffQYW2ob5aSXzJiaQJo%3Dreserved=0
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
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 f7d541030698..4cb3eb7c6745 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1961,7 +1961,7 @@ static void handle_hpd_irq(void *param)
mutex_lock(>hpd_lock);
#ifdef CONFIG_DRM_AMD_DC_HDCP
- if (adev->asic_type >= CHIP_RAVEN)
+ if (adev->dm.hdcp_workqueue)
hdcp_reset_display(adev->dm.hdcp_workqueue,
aconnector->dc_link->link_index);
#endif
if (aconnector->fake_enable)
@@ -2138,8 +2138,10 @@ static void handle_hpd_rx_irq(void *param)
}
}
#ifdef CONFIG_DRM_AMD_DC_HDCP
- if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ)
- hdcp_handle_cpirq(adev->dm.hdcp_workqueue,
aconnector->base.index);
+ if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
+ if (adev->dm.hdcp_workqueue)
+ hdcp_handle_cpirq(adev->dm.hdcp_workqueue,
aconnector->base.index);
+ }
#endif
if ((dc_link->cur_link_settings.lane_count != LANE_COUNT_UNKNOWN) ||
(dc_link->type == dc_connection_mst_branch))
@@ -5836,7 +5838,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
drm_connector_attach_vrr_capable_property(
>base);
#ifdef CONFIG_DRM_AMD_DC_HDCP
- if (adev->asic_type >= CHIP_RAVEN)
+ if (adev->dm.hdcp_workqueue)
drm_connector_attach_content_protection_property(>base, true);
#endif
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD v2
The tip of origin/amd-staging-drm-next for me is:
commit 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
Refs: {origin/amd-staging-drm-next}, v5.4-rc7-2847-g7fd3b632e17e
Author: Monk Liu
AuthorDate: Thu Feb 6 23:55:58 2020 +0800
Commit: Monk Liu
CommitDate: Wed Feb 19 13:33:02 2020 +0800
drm/amdgpu: fix colliding of preemption
what:
some os preemption path is messed up with world switch preemption
fix:
cleanup those logics so os preemption not mixed with world switch
this patch is a general fix for issues comes from SRIOV MCBP, but
there is still UMD side issues not resovlved yet, so this patch
cannot fix all world switch bug.
Signed-off-by: Monk Liu
Acked-by: Hawking Zhang
Which I had fetched just an hour ago.
On 2020-02-19 10:41 a.m., Christian König wrote:
Well what branch are you trying to merge that into?
The conflict resolution should be simple, just keep the
vm->update_funcs->prepare(...) line as it is in your branch.
When you get those errors then something went wrong in your rebase.
Christian.
Am 19.02.20 um 16:14 schrieb Tom St Denis:
Doesn't build even with conflict resolved:
[root@raven linux]# make
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c: In function
‘amdgpu_vm_bo_update_mapping’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: error: ‘owner’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: note: each undeclared
identifier is reported only once for each function it appears in
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:48: error: ‘exclusive’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
make[4]: *** [scripts/Makefile.build:266:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o] Error 1
make[3]: *** [scripts/Makefile.build:509: drivers/gpu/drm/amd/amdgpu]
Error 2
make[2]: *** [scripts/Makefile.build:509: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:509: drivers/gpu] Error 2
make: *** [Makefile:1649: drivers] Error 2
Should I just move to drm-misc-next?
tom
On 2020-02-19 10:02 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to
the resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD v2
Well what branch are you trying to merge that into?
The conflict resolution should be simple, just keep the
vm->update_funcs->prepare(...) line as it is in your branch.
When you get those errors then something went wrong in your rebase.
Christian.
Am 19.02.20 um 16:14 schrieb Tom St Denis:
Doesn't build even with conflict resolved:
[root@raven linux]# make
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c: In function
‘amdgpu_vm_bo_update_mapping’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: error: ‘owner’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: note: each undeclared
identifier is reported only once for each function it appears in
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:48: error: ‘exclusive’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
make[4]: *** [scripts/Makefile.build:266:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o] Error 1
make[3]: *** [scripts/Makefile.build:509: drivers/gpu/drm/amd/amdgpu]
Error 2
make[2]: *** [scripts/Makefile.build:509: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:509: drivers/gpu] Error 2
make: *** [Makefile:1649: drivers] Error 2
Should I just move to drm-misc-next?
tom
On 2020-02-19 10:02 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to the
resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD v2
Doesn't build even with conflict resolved:
[root@raven linux]# make
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c: In function
‘amdgpu_vm_bo_update_mapping’:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: error: ‘owner’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:41: note: each undeclared
identifier is reported only once for each function it appears in
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1612:48: error: ‘exclusive’
undeclared (first use in this function)
1612 | r = vm->update_funcs->prepare(, owner, exclusive);
| ^
make[4]: *** [scripts/Makefile.build:266:
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.o] Error 1
make[3]: *** [scripts/Makefile.build:509: drivers/gpu/drm/amd/amdgpu]
Error 2
make[2]: *** [scripts/Makefile.build:509: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:509: drivers/gpu] Error 2
make: *** [Makefile:1649: drivers] Error 2
Should I just move to drm-misc-next?
tom
On 2020-02-19 10:02 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to the resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/amdgpu: disable GFXOFF around debugfs access to MMIO
I got some messages after a while:
[ 741.788564] Failed to send Message 8.
[ 746.671509] Failed to send Message 8.
[ 748.749673] Failed to send Message 2b.
[ 759.245414] Failed to send Message 7.
[ 763.216902] Failed to send Message 2a.
Are there any additional locks that should be held? Because some
commands like --top or --waves can do a lot of distinct read operations
(causing a lot of enable/disable calls).
I'm going to sit on this a bit since I don't think the patch is ready
for pushing out.
Tom
On 2020-02-19 10:07 a.m., Alex Deucher wrote:
On Wed, Feb 19, 2020 at 10:04 AM Tom St Denis wrote:
Signed-off-by: Tom St Denis
Please add a patch description. With that fixed:
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 7379910790c9..66f763300c96 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -169,6 +169,7 @@ static int amdgpu_debugfs_process_reg_op(bool read, struct
file *f,
if (pm_pg_lock)
mutex_lock(>pm.mutex);
+ amdgpu_gfx_off_ctrl(adev, false);
while (size) {
uint32_t value;
@@ -192,6 +193,8 @@ static int amdgpu_debugfs_process_reg_op(bool read, struct
file *f,
}
end:
+ amdgpu_gfx_off_ctrl(adev, true);
+
if (use_bank) {
amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
mutex_unlock(>grbm_idx_mutex);
--
2.24.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Ctom.stdenis%40amd.com%7Cf3c4cd3120fa476bbca808d7b54d8076%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637177216782684720sdata=m5eyco1nGc5XD%2Ford7Q3goA48G5NhR%2BqdSYQjnfd7%2Fg%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/amdgpu: disable GFXOFF around debugfs access to MMIO
On Wed, Feb 19, 2020 at 10:04 AM Tom St Denis wrote:
>
> Signed-off-by: Tom St Denis
Please add a patch description. With that fixed:
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 7379910790c9..66f763300c96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -169,6 +169,7 @@ static int amdgpu_debugfs_process_reg_op(bool read,
> struct file *f,
> if (pm_pg_lock)
> mutex_lock(>pm.mutex);
>
> + amdgpu_gfx_off_ctrl(adev, false);
> while (size) {
> uint32_t value;
>
> @@ -192,6 +193,8 @@ static int amdgpu_debugfs_process_reg_op(bool read,
> struct file *f,
> }
>
> end:
> + amdgpu_gfx_off_ctrl(adev, true);
> +
> if (use_bank) {
> amdgpu_gfx_select_se_sh(adev, 0x, 0x,
> 0x);
> mutex_unlock(>grbm_idx_mutex);
> --
> 2.24.1
>
> ___
> 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: add VM update fences back to the root PD v2
Ignore that my brain wasn't engaged in the process. It's clear where
you wanted the prepare call.
Tom
On 2020-02-19 10:06 a.m., Tom St Denis wrote:
I get this conflict on top of drm-next
++<<< HEAD
+ r = vm->update_funcs->prepare(, resv, sync_mode);
++===
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
+ r = vm->update_funcs->prepare(, owner, exclusive);
++>>> drm/amdgpu: add VM update fences back to the root PD v2
Should I keep the prepare call before or after your block?
Tom
On 2020-02-19 10:02 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to the
resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD v2
I get this conflict on top of drm-next
++<<< HEAD
+ r = vm->update_funcs->prepare(, resv, sync_mode);
++===
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
+ r = vm->update_funcs->prepare(, owner, exclusive);
++>>> drm/amdgpu: add VM update fences back to the root PD v2
Should I keep the prepare call before or after your block?
Tom
On 2020-02-19 10:02 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to the resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/amdgpu: disable GFXOFF around debugfs access to MMIO
Signed-off-by: Tom St Denis
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 7379910790c9..66f763300c96 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -169,6 +169,7 @@ static int amdgpu_debugfs_process_reg_op(bool read, struct
file *f,
if (pm_pg_lock)
mutex_lock(>pm.mutex);
+ amdgpu_gfx_off_ctrl(adev, false);
while (size) {
uint32_t value;
@@ -192,6 +193,8 @@ static int amdgpu_debugfs_process_reg_op(bool read, struct
file *f,
}
end:
+ amdgpu_gfx_off_ctrl(adev, true);
+
if (use_bank) {
amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
mutex_unlock(>grbm_idx_mutex);
--
2.24.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: add VM update fences back to the root PD v2
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
v2: rebased on drm-misc-next
Signed-off-by: Christian König
Fixes: 90b69cdc5f159 drm/amdgpu: stop adding VM updates fences to the resv obj
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d16231d6a790..ef73fa94f357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -588,8 +588,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1591,6 +1591,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, owner, exclusive);
if (r)
goto error_unlock;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD
Well it should apply on top of amd-staging-drm-next. But I haven't
fetched that today yet.
Give me a minute to rebase.
Christian.
Am 19.02.20 um 15:27 schrieb Tom St Denis:
This doesn't apply on top of 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
which is the tip of drm-next
Tom
On 2020-02-19 9:20 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e7ab0c1e2793..dd63ccdbad2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -585,8 +585,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1619,6 +1619,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, resv, sync_mode);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add VM update fences back to the root PD
This doesn't apply on top of 7fd3b632e17e55c5ffd008f9f025754e7daa1b66
which is the tip of drm-next
Tom
On 2020-02-19 9:20 a.m., Christian König wrote:
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e7ab0c1e2793..dd63ccdbad2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -585,8 +585,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1619,6 +1619,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, resv, sync_mode);
if (r)
goto error_unlock;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/display: clean up hdcp workqueue handling
Use the existence of the workqueue itself to determine when to
enable HDCP features rather than sprinkling asic checks all over
the code. Also add a check for the existence of the hdcp
workqueue in the irq handling on the off chance we get and HPD
RX interrupt with the CP bit set. This avoids a crash if
the driver doesn't support HDCP for a particular asic.
Fixes: 96a3b32e67236f ("drm/amd/display: only enable HDCP for DCN+")
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=206519
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
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 f7d541030698..4cb3eb7c6745 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1961,7 +1961,7 @@ static void handle_hpd_irq(void *param)
mutex_lock(>hpd_lock);
#ifdef CONFIG_DRM_AMD_DC_HDCP
- if (adev->asic_type >= CHIP_RAVEN)
+ if (adev->dm.hdcp_workqueue)
hdcp_reset_display(adev->dm.hdcp_workqueue,
aconnector->dc_link->link_index);
#endif
if (aconnector->fake_enable)
@@ -2138,8 +2138,10 @@ static void handle_hpd_rx_irq(void *param)
}
}
#ifdef CONFIG_DRM_AMD_DC_HDCP
- if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ)
- hdcp_handle_cpirq(adev->dm.hdcp_workqueue,
aconnector->base.index);
+ if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
+ if (adev->dm.hdcp_workqueue)
+ hdcp_handle_cpirq(adev->dm.hdcp_workqueue,
aconnector->base.index);
+ }
#endif
if ((dc_link->cur_link_settings.lane_count != LANE_COUNT_UNKNOWN) ||
(dc_link->type == dc_connection_mst_branch))
@@ -5836,7 +5838,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
drm_connector_attach_vrr_capable_property(
>base);
#ifdef CONFIG_DRM_AMD_DC_HDCP
- if (adev->asic_type >= CHIP_RAVEN)
+ if (adev->dm.hdcp_workqueue)
drm_connector_attach_content_protection_property(>base, true);
#endif
}
--
2.24.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: add VM update fences back to the root PD
Add update fences to the root PD while mapping BOs.
Otherwise PDs freed during the mapping won't wait for
updates to finish and can cause corruptions.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e7ab0c1e2793..dd63ccdbad2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -585,8 +585,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
{
entry->priority = 0;
entry->tv.bo = >root.base.bo->tbo;
- /* One for TTM and one for the CS job */
- entry->tv.num_shared = 2;
+ /* Two for VM updates, one for TTM and one for the CS job */
+ entry->tv.num_shared = 4;
entry->user_pages = NULL;
list_add(>tv.head, validated);
}
@@ -1619,6 +1619,16 @@ static int amdgpu_vm_bo_update_mapping(struct
amdgpu_device *adev,
goto error_unlock;
}
+ if (flags & AMDGPU_PTE_VALID) {
+ struct amdgpu_bo *root = vm->root.base.bo;
+
+ if (!dma_fence_is_signaled(vm->last_direct))
+ amdgpu_bo_fence(root, vm->last_direct, true);
+
+ if (!dma_fence_is_signaled(vm->last_delayed))
+ amdgpu_bo_fence(root, vm->last_delayed, true);
+ }
+
r = vm->update_funcs->prepare(, resv, sync_mode);
if (r)
goto error_unlock;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3
Store ttm bo->offset in struct nouveau_bo instead.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 +++---
drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 +++---
drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_abi16.c | 8
drivers/gpu/drm/nouveau/nouveau_bo.c| 8
drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++
drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +-
15 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 1f08de4241e0..d06a93f2b38a 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -845,7 +845,7 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
fb = nouveau_framebuffer(crtc->primary->fb);
}
- nv_crtc->fb.offset = fb->nvbo->bo.offset;
+ nv_crtc->fb.offset = fb->nvbo->offset;
if (nv_crtc->lut.depth != drm_fb->format->depth) {
nv_crtc->lut.depth = drm_fb->format->depth;
@@ -1013,7 +1013,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct
drm_file *file_priv,
nv04_cursor_upload(dev, cursor, nv_crtc->cursor.nvbo);
nouveau_bo_unmap(cursor);
- nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->bo.offset;
+ nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->offset;
nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.offset);
nv_crtc->cursor.show(nv_crtc, true);
out:
@@ -1191,7 +1191,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct
drm_framebuffer *fb,
/* Initialize a page flip struct */
*s = (struct nv04_page_flip_state)
{ { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
- new_bo->bo.offset };
+ new_bo->offset };
/* Keep vblanks on during flip, for the target crtc of this flip */
drm_crtc_vblank_get(crtc);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 44ee82d0c9b6..89a4ddfcc55f 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -151,7 +151,7 @@ nv04_display_init(struct drm_device *dev, bool resume, bool
runtime)
continue;
if (nv_crtc->cursor.set_offset)
- nv_crtc->cursor.set_offset(nv_crtc,
nv_crtc->cursor.nvbo->bo.offset);
+ nv_crtc->cursor.set_offset(nv_crtc,
nv_crtc->cursor.nvbo->offset);
nv_crtc->cursor.set_pos(nv_crtc, nv_crtc->cursor_saved_x,
nv_crtc->cursor_saved_y);
}
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index a3a0a73ae8ab..9529bd9053e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -150,7 +150,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
nvif_mask(dev, NV_PCRTC_ENGINE_CTRL + soff2, NV_CRTC_FSEL_OVERLAY, 0);
nvif_wr32(dev, NV_PVIDEO_BASE(flip), 0);
- nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset);
+ nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_SIZE_IN(flip), src_h << 16 | src_w);
nvif_wr32(dev, NV_PVIDEO_POINT_IN(flip), src_y << 16 | src_x);
nvif_wr32(dev, NV_PVIDEO_DS_DX(flip), (src_w << 20) / crtc_w);
@@ -172,7 +172,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
if (format & NV_PVIDEO_FORMAT_PLANAR) {
nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
- nv_fb->nvbo->bo.offset + fb->offsets[1]);
+ nv_fb->nvbo->offset + fb->offsets[1]);
}
nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_STOP, 0);
@@ -396,7 +396,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
for (i = 0; i < 2; i++) {
nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i,
- nv_fb->nvbo->bo.offset);
+ nv_fb->nvbo->offset);
nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i,
fb->pitches[0]);
nvif_wr32(dev, NV_PVIDEO_BUFF0_OFFSET + 4 * i,
[PATCH 5/8] drm/qxl: don't use ttm bo->offset
This patch removes slot->gpu_offset which is not required as
VRAM and PRIV slot are in separate PCI bar
This patch also removes unused qxl_bo_gpu_offset()
Signed-off-by: Nirmoy Das
Acked-by: Christian König
Acked-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_drv.h| 6 ++
drivers/gpu/drm/qxl/qxl_kms.c| 5 ++---
drivers/gpu/drm/qxl/qxl_object.h | 5 -
drivers/gpu/drm/qxl/qxl_ttm.c| 9 -
4 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 27e45a2d6b52..df581f0e6699 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -134,7 +134,6 @@ struct qxl_memslot {
uint64_tstart_phys_addr;
uint64_tsize;
uint64_thigh_bits;
- uint64_tgpu_offset;
};
enum {
@@ -311,10 +310,9 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct
qxl_bo *bo,
(bo->tbo.mem.mem_type == TTM_PL_VRAM)
? >main_slot : >surfaces_slot;
- WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
+ /* TODO - need to hold one of the locks to read bo->tbo.mem.start */
- /* TODO - need to hold one of the locks to read tbo.offset */
- return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
+ return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) + offset);
}
/* qxl_display.c */
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 70b20ee4741a..7a5bf544f34d 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -86,11 +86,10 @@ static void setup_slot(struct qxl_device *qdev,
high_bits <<= (64 - (qdev->rom->slot_gen_bits +
qdev->rom->slot_id_bits));
slot->high_bits = high_bits;
- DRM_INFO("slot %d (%s): base 0x%08lx, size 0x%08lx, gpu_offset 0x%lx\n",
+ DRM_INFO("slot %d (%s): base 0x%08lx, size 0x%08lx\n",
slot->index, slot->name,
(unsigned long)slot->start_phys_addr,
-(unsigned long)slot->size,
-(unsigned long)slot->gpu_offset);
+(unsigned long)slot->size);
}
void qxl_reinit_memslots(struct qxl_device *qdev)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..21fa81048f4f 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -48,11 +48,6 @@ static inline void qxl_bo_unreserve(struct qxl_bo *bo)
ttm_bo_unreserve(>tbo);
}
-static inline u64 qxl_bo_gpu_offset(struct qxl_bo *bo)
-{
- return bo->tbo.offset;
-}
-
static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
{
return bo->tbo.num_pages << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 62a5e424971b..635d000e7934 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -51,11 +51,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device
*bdev)
static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
struct ttm_mem_type_manager *man)
{
- struct qxl_device *qdev = qxl_get_qdev(bdev);
- unsigned int gpu_offset_shift =
- 64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits + 8);
- struct qxl_memslot *slot;
-
switch (type) {
case TTM_PL_SYSTEM:
/* System memory */
@@ -66,11 +61,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
case TTM_PL_PRIV:
/* "On-card" video ram */
- slot = (type == TTM_PL_VRAM) ?
- >main_slot : >surfaces_slot;
- slot->gpu_offset = (uint64_t)type << gpu_offset_shift;
man->func = _bo_manager_func;
- man->gpu_offset = slot->gpu_offset;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_MASK_CACHING;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 8/8] drm/ttm: do not keep GPU dependent addresses
GPU address handling is device specific and should be handle by its device
driver.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/ttm/ttm_bo.c| 7 ---
include/drm/ttm/ttm_bo_api.h| 2 --
include/drm/ttm/ttm_bo_driver.h | 1 -
3 files changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 151edfd8de77..d5885cd609a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -85,7 +85,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev,
struct drm_printer *p
drm_printf(p, "has_type: %d\n", man->has_type);
drm_printf(p, "use_type: %d\n", man->use_type);
drm_printf(p, "flags: 0x%08X\n", man->flags);
- drm_printf(p, "gpu_offset: 0x%08llX\n", man->gpu_offset);
drm_printf(p, "size: %llu\n", man->size);
drm_printf(p, "available_caching: 0x%08X\n",
man->available_caching);
drm_printf(p, "default_caching: 0x%08X\n", man->default_caching);
@@ -345,12 +344,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
moved:
bo->evicted = false;
- if (bo->mem.mm_node)
- bo->offset = (bo->mem.start << PAGE_SHIFT) +
- bdev->man[bo->mem.mem_type].gpu_offset;
- else
- bo->offset = 0;
-
ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
return 0;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index b9bc1b00142e..d6f39ee5bf5d 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -213,8 +213,6 @@ struct ttm_buffer_object {
* either of these locks held.
*/
- uint64_t offset; /* GPU address space is independent of CPU word size */
-
struct sg_table *sg;
};
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index c9e0fd09f4b2..c8ce6c181abe 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -177,7 +177,6 @@ struct ttm_mem_type_manager {
bool has_type;
bool use_type;
uint32_t flags;
- uint64_t gpu_offset; /* GPU address space is independent of CPU word
size */
uint64_t size;
uint32_t available_caching;
uint32_t default_caching;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3 0/8] do not store GPU address in TTM
With this patch series I am trying to remove GPU address dependency in TTM and moving GPU address calculation to individual drm drivers. I tested this patch series on qxl, bochs and amdgpu. Christian tested it on radeon HW. It would be nice if someone test this for nouveau and vmgfx. v2: * set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL v3: * catch return value of drm_gem_vram_offset() in drm/bochs * introduce drm_gem_vram_pg_offset() in vram helper * improve nbo->offset calculation for nouveau Nirmoy Das (8): drm/amdgpu: move ttm bo->offset to amdgpu_bo drm/radeon: don't use ttm bo->offset drm/vmwgfx: don't use ttm bo->offset drm/nouveau: don't use ttm bo->offset drm/qxl: don't use ttm bo->offset drm/vram-helper: don't use ttm bo->offset drm/bochs: use drm_gem_vram_offset to get bo offset drm/ttm: do not keep GPU dependent addresses drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 29 - drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 + drivers/gpu/drm/bochs/bochs_kms.c | 2 +- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/crtc.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 6 ++--- drivers/gpu/drm/nouveau/dispnv50/base507c.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/core507d.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 2 +- drivers/gpu/drm/nouveau/nouveau_abi16.c | 8 +++--- drivers/gpu/drm/nouveau/nouveau_bo.c| 1 + drivers/gpu/drm/nouveau/nouveau_bo.h| 3 +++ drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +- drivers/gpu/drm/nouveau/nouveau_dmem.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 2 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 10 +++ drivers/gpu/drm/qxl/qxl_drv.h | 6 ++--- drivers/gpu/drm/qxl/qxl_kms.c | 5 ++-- drivers/gpu/drm/qxl/qxl_object.h| 5 drivers/gpu/drm/qxl/qxl_ttm.c | 9 --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_object.h | 16 +++- drivers/gpu/drm/radeon/radeon_ttm.c | 4 +-- drivers/gpu/drm/ttm/ttm_bo.c| 7 - drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c| 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 -- include/drm/ttm/ttm_bo_api.h| 2 -- include/drm/ttm/ttm_bo_driver.h | 1 - 35 files changed, 99 insertions(+), 76 deletions(-) -- 2.25.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v2
Calculate GEM VRAM bo's offset within vram-helper without depending on
bo->offset
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/drm_gem_vram_helper.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 92a11bb42365..3edf5f241c15 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -198,6 +198,21 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object
*gbo)
}
EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
+/**
+ * drm_gem_vram_pg_offset() - Returns a GEM VRAM object's page offset
+ * @gbo: the GEM VRAM object
+ *
+ * Returns:
+ * The buffer object's page offset, or
+ * 0 with a warning when memory manager node of the buffer object is NULL
+ * */
+static s64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
+{
+ if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
+ return 0;
+ return gbo->bo.mem.start;
+}
+
/**
* drm_gem_vram_offset() - \
Returns a GEM VRAM object's offset in video memory
@@ -214,7 +229,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
{
if (WARN_ON_ONCE(!gbo->pin_count))
return (s64)-ENODEV;
- return gbo->bo.offset;
+ return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
}
EXPORT_SYMBOL(drm_gem_vram_offset);
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset v2
Switch over to GEM VRAM's implementation to retrieve bo->offset
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/bochs/bochs_kms.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
b/drivers/gpu/drm/bochs/bochs_kms.c
index 8066d7d370d5..18d2ec34534d 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -29,16 +29,21 @@ static void bochs_plane_update(struct bochs_device *bochs,
struct drm_plane_state *state)
{
struct drm_gem_vram_object *gbo;
+ s64 gpu_addr;
if (!state->fb || !bochs->stride)
return;
gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
+ gpu_addr = drm_gem_vram_offset(gbo);
+ if (WARN_ON_ONCE(gpu_addr < 0))
+ return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
+
bochs_hw_setbase(bochs,
state->crtc_x,
state->crtc_y,
state->fb->pitches[0],
-state->fb->offsets[0] + gbo->bo.offset);
+state->fb->offsets[0] + gpu_addr);
bochs_hw_setformat(bochs, state->fb->format);
}
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/8] drm/amdgpu: move ttm bo->offset to amdgpu_bo
GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu driver.
Signed-off-by: Nirmoy Das
Acked-by: Huang Rui
Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 29 --
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h| 1 +
4 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index e3f16b49e970..04e78f783638 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -917,7 +917,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32
domain,
bo->pin_count++;
if (max_offset != 0) {
- u64 domain_start =
bo->tbo.bdev->man[mem_type].gpu_offset;
+ u64 domain_start = amdgpu_ttm_domain_start(adev,
mem_type);
WARN_ON_ONCE(max_offset <
(amdgpu_bo_gpu_offset(bo) - domain_start));
}
@@ -1445,7 +1445,25 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
!(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
- return amdgpu_gmc_sign_extend(bo->tbo.offset);
+ return amdgpu_bo_gpu_offset_no_check(bo);
+}
+
+/**
+ * amdgpu_bo_gpu_offset_no_check - return GPU offset of bo
+ * @bo:amdgpu object for which we query the offset
+ *
+ * Returns:
+ * current GPU offset of the object without raising warnings.
+ */
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
+{
+ struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ uint64_t offset;
+
+offset = (bo->tbo.mem.start << PAGE_SHIFT) +
+amdgpu_ttm_domain_start(adev, bo->tbo.mem.mem_type);
+
+ return amdgpu_gmc_sign_extend(offset);
}
/**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 36dec51d1ef1..1d86b4c7a1f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -279,6 +279,7 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence
*fence,
bool shared);
int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
int amdgpu_bo_validate(struct amdgpu_bo *bo);
int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
struct dma_fence **fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3ab46d4647e4..e329a108e760 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -97,7 +97,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_TT:
/* GTT memory */
man->func = _gtt_mgr_func;
- man->gpu_offset = adev->gmc.gart_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -105,7 +104,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _vram_mgr_func;
- man->gpu_offset = adev->gmc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
@@ -116,7 +114,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case AMDGPU_PL_OA:
/* On-chip GDS memory*/
man->func = _bo_manager_func;
- man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
man->available_caching = TTM_PL_FLAG_UNCACHED;
man->default_caching = TTM_PL_FLAG_UNCACHED;
@@ -264,7 +261,7 @@ static uint64_t amdgpu_mm_node_addr(struct
ttm_buffer_object *bo,
if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
addr = mm_node->start << PAGE_SHIFT;
- addr += bo->bdev->man[mem->mem_type].gpu_offset;
+ addr += amdgpu_ttm_domain_start(amdgpu_ttm_adev(bo->bdev),
mem->mem_type);
}
return addr;
}
@@ -751,6 +748,27 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct
ttm_buffer_object *bo,
(offset >> PAGE_SHIFT);
}
+/**
+ * amdgpu_ttm_domain_start - Returns GPU start address
+ * @adev: amdgpu device object
+
[PATCH 2/8] drm/radeon: don't use ttm bo->offset
Calculate GPU offset in radeon_bo_gpu_offset without depending on
bo->offset
Signed-off-by: Nirmoy Das
Reviewed-and-tested-by: Christian König
---
drivers/gpu/drm/radeon/radeon.h| 1 +
drivers/gpu/drm/radeon/radeon_object.h | 16 +++-
drivers/gpu/drm/radeon/radeon_ttm.c| 4 +---
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 30e32adc1fc6..b7c3fb2bfb54 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2828,6 +2828,7 @@ extern void radeon_ttm_set_active_vram_size(struct
radeon_device *rdev, u64 size
extern void radeon_program_register_sequence(struct radeon_device *rdev,
const u32 *registers,
const u32 array_size);
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
/*
* vm
diff --git a/drivers/gpu/drm/radeon/radeon_object.h
b/drivers/gpu/drm/radeon/radeon_object.h
index d23f2ed4126e..4d37571c7ff5 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
*/
static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
{
- return bo->tbo.offset;
+ struct radeon_device *rdev;
+ u64 start = 0;
+
+ rdev = radeon_get_rdev(bo->tbo.bdev);
+
+ switch(bo->tbo.mem.mem_type) {
+ case TTM_PL_TT:
+ start = rdev->mc.gtt_start;
+ break;
+ case TTM_PL_VRAM:
+ start = rdev->mc.vram_start;
+ break;
+ }
+
+ return (bo->tbo.mem.start << PAGE_SHIFT) + start;
}
static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index badf1b6d1549..1c8303468e8f 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -56,7 +56,7 @@
static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
-static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
{
struct radeon_mman *mman;
struct radeon_device *rdev;
@@ -82,7 +82,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
break;
case TTM_PL_TT:
man->func = _bo_manager_func;
- man->gpu_offset = rdev->mc.gtt_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -104,7 +103,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
- man->gpu_offset = rdev->mc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/8] drm/vmwgfx: don't use ttm bo->offset
Calculate GPU offset within vmwgfx driver itself without depending on
bo->offset
Signed-off-by: Nirmoy Das
Acked-by: Christian König
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
4 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 8b71bf6b58ef..1e59c019affa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -258,7 +258,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private
*dev_priv,
ret = ttm_bo_validate(bo, , );
/* For some reason we didn't end up at the start of vram */
- WARN_ON(ret == 0 && bo->offset != 0);
+ WARN_ON(ret == 0 && bo->mem.start != 0);
if (!ret)
vmw_bo_pin_reserved(buf, true);
@@ -317,7 +317,7 @@ void vmw_bo_get_guest_ptr(const struct ttm_buffer_object
*bo,
{
if (bo->mem.mem_type == TTM_PL_VRAM) {
ptr->gmrId = SVGA_GMR_FRAMEBUFFER;
- ptr->offset = bo->offset;
+ ptr->offset = bo->mem.start << PAGE_SHIFT;
} else {
ptr->gmrId = bo->mem.start;
ptr->offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 73489a45decb..72c2cf4053df 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3313,7 +3313,7 @@ static void vmw_apply_relocations(struct vmw_sw_context
*sw_context)
bo = >vbo->base;
switch (bo->mem.mem_type) {
case TTM_PL_VRAM:
- reloc->location->offset += bo->offset;
+ reloc->location->offset += bo->mem.start << PAGE_SHIFT;
reloc->location->gmrId = SVGA_GMR_FRAMEBUFFER;
break;
case VMW_PL_GMR:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
index e5252ef3812f..1cdc445b24c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -612,7 +612,7 @@ static int vmw_fifo_emit_dummy_legacy_query(struct
vmw_private *dev_priv,
if (bo->mem.mem_type == TTM_PL_VRAM) {
cmd->body.guestResult.gmrId = SVGA_GMR_FRAMEBUFFER;
- cmd->body.guestResult.offset = bo->offset;
+ cmd->body.guestResult.offset = bo->mem.start << PAGE_SHIFT;
} else {
cmd->body.guestResult.gmrId = bo->mem.start;
cmd->body.guestResult.offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 3f3b2c7a208a..e7134aebeb81 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -750,7 +750,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _bo_manager_func;
- man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
man->default_caching = TTM_PL_FLAG_CACHED;
@@ -763,7 +762,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev,
uint32_t type,
* slots as well as the bo size.
*/
man->func = _gmrid_manager_func;
- man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_CACHED;
man->default_caching = TTM_PL_FLAG_CACHED;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[RFC PATCH v6] drm/amdgpu: Remove kfd eviction fence before release bo
No need to trigger eviction as the memory mapping will not be used
anymore.
All pt/pd bos share same resv, hence the same shared eviction fence.
Everytime page table is freed, the fence will be signled and that cuases
kfd unexcepted evictions.
CC: Christian König
CC: Felix Kuehling
CC: Alex Deucher
Acked-by: Christian König
Signed-off-by: xinhui pan
---
change from v5:
use trylock instead, and add warn_on_once to give a limitd warning.
change from v4:
based on new ttm code.
change from v3:
fix a coding error
change from v2:
based on Chris' drm/ttm: rework BO delayed delete patchset.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 1 +
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 38 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 5 +++
3 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 9e8db702d878..0ee8aae6c519 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -96,6 +96,7 @@ struct amdgpu_amdkfd_fence *amdgpu_amdkfd_fence_create(u64
context,
struct mm_struct *mm);
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);
+int amdgpu_amdkfd_remove_fence_on_pt_pd_bos(struct amdgpu_bo *bo);
struct amdkfd_process_info {
/* List head of all VMs that belong to a KFD process */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index ef721cb65868..898851bec377 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -276,6 +276,42 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
amdgpu_bo *bo,
return 0;
}
+int amdgpu_amdkfd_remove_fence_on_pt_pd_bos(struct amdgpu_bo *bo)
+{
+ struct amdgpu_bo *root = bo;
+ struct amdgpu_vm_bo_base *vm_bo;
+ struct amdgpu_vm *vm;
+ struct amdkfd_process_info *info;
+ struct amdgpu_amdkfd_fence *ef;
+ int ret;
+
+ /* we can always get vm_bo from root PD bo.*/
+ while (root->parent)
+ root = root->parent;
+
+ vm_bo = root->vm_bo;
+ if (!vm_bo)
+ return 0;
+
+ vm = vm_bo->vm;
+ if (!vm)
+ return 0;
+
+ info = vm->process_info;
+ if (!info || !info->eviction_fence)
+ return 0;
+
+ ef = container_of(dma_fence_get(>eviction_fence->base),
+ struct amdgpu_amdkfd_fence, base);
+
+ BUG_ON(!dma_resv_trylock(bo->tbo.base.resv));
+ ret = amdgpu_amdkfd_remove_eviction_fence(bo, ef);
+ dma_resv_unlock(bo->tbo.base.resv);
+
+ dma_fence_put(>base);
+ return ret;
+}
+
static int amdgpu_amdkfd_bo_validate(struct amdgpu_bo *bo, uint32_t domain,
bool wait)
{
@@ -1045,6 +1081,8 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device
*adev,
list_del(>vm_list_node);
mutex_unlock(_info->lock);
+ vm->process_info = NULL;
+
/* Release per-process resources when last compute VM is destroyed */
if (!process_info->n_vms) {
WARN_ON(!list_empty(_info->kfd_bo_list));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6f60a581e3ba..5766d20f29d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1307,6 +1307,11 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (abo->kfd_bo)
amdgpu_amdkfd_unreserve_memory_limit(abo);
+ /* We only remove the fence if the resv has individualized. */
+ WARN_ON_ONCE(bo->base.resv != >base._resv);
+ if (bo->base.resv == >base._resv)
+ amdgpu_amdkfd_remove_fence_on_pt_pd_bos(abo);
+
if (bo->mem.mem_type != TTM_PL_VRAM || !bo->mem.mm_node ||
!(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE))
return;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Nouveau] [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses
On 2/18/20 8:06 PM, Daniel Vetter wrote:
On Tue, Feb 18, 2020 at 07:37:44PM +0100, Christian König wrote:
Am 18.02.20 um 19:28 schrieb Thomas Zimmermann:
Hi
Am 18.02.20 um 19:23 schrieb Christian König:
Am 18.02.20 um 19:16 schrieb Thomas Zimmermann:
Hi
Am 18.02.20 um 18:13 schrieb Nirmoy:
On 2/18/20 1:44 PM, Christian König wrote:
Am 18.02.20 um 13:40 schrieb Thomas Zimmermann:
Hi
Am 17.02.20 um 16:04 schrieb Nirmoy Das:
GPU address handling is device specific and should be handle by its
device
driver.
Signed-off-by: Nirmoy Das
---
drivers/gpu/drm/ttm/ttm_bo.c | 7 ---
include/drm/ttm/ttm_bo_api.h | 2 --
include/drm/ttm/ttm_bo_driver.h | 1 -
3 files changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c
index 151edfd8de77..d5885cd609a3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -85,7 +85,6 @@ static void ttm_mem_type_debug(struct
ttm_bo_device *bdev, struct drm_printer *p
drm_printf(p, " has_type: %d\n", man->has_type);
drm_printf(p, " use_type: %d\n", man->use_type);
drm_printf(p, " flags: 0x%08X\n", man->flags);
- drm_printf(p, " gpu_offset: 0x%08llX\n", man->gpu_offset);
drm_printf(p, " size: %llu\n", man->size);
drm_printf(p, " available_caching: 0x%08X\n",
man->available_caching);
drm_printf(p, " default_caching: 0x%08X\n",
man->default_caching);
@@ -345,12 +344,6 @@ static int ttm_bo_handle_move_mem(struct
ttm_buffer_object *bo,
moved:
bo->evicted = false;
- if (bo->mem.mm_node)
- bo->offset = (bo->mem.start << PAGE_SHIFT) +
- bdev->man[bo->mem.mem_type].gpu_offset;
- else
- bo->offset = 0;
-
After moving this into users, the else branch has been lost. Is
'bo->mem.mm_node' always true?
At least for the amdgpu and radeon use cases, yes.
But that is a rather good question I mean for it is illegal to get the
GPU BO address if it is inaccessible (e.g. in the system domain).
Could be that some driver relied on the behavior to get 0 for the
system domain here.
I wonder how to verify that ?
If I understand correctly:
1 qxl uses bo->offset only in qxl_bo_physical_address() which is not in
system domain.
2 unfortunately I can't say the same for bochs but it works with this
patch series so I think bochs is fine as well.
3 vmwgfx uses bo->offset only when bo->mem.mem_type == TTM_PL_VRAM so
vmwgfx should be fine.
4 amdgpu and radeon runs with 'bo->mem.mm_node' always true
I am not sure about nouveau as bo->offset is being used in many places.
I could probably mirror the removed logic to nouveau as
I suggest to introduce a ttm helper that contains the original branching
and use it everywhere. Something like
s64 ttm_bo_offset(struct ttm_buffer_object *bo)
{
if (WARN_ON_ONCE(!bo->mem.mm_node))
return 0;
return bo->mem.start << PAGE_SHIFT;
}
Could be static inline. The warning should point to broken drivers. This
also gets rid of the ugly shift in the drivers.
Big NAK on this. That is exactly what we should NOT do.
See the shift belongs into the driver, because it is driver dependent if
we work with page or byte offsets.
For amdgpu we for example want to work with byte offsets and TTM should
not make any assumption about what bo->mem.start actually contains.
OK. What about something like ttm_bo_pg_offset()? Same code without the
shift. Would also make it clear that it's a page offset.
That is a rather good idea. We could name that ttm_bo_man_offset() and put
it into ttm_bo_manager.c next to the manager which allocates them.
It's just that this manager is not used by everybody, so amdgpu, radeon and
nouveau would still need a separate function.
Let me pile on with my bikeshed color choice :-)
I'd do this a wrapper, but for drivers individually. And only for those we
haven't audited yet, or where we think the WARN_ON actually provides
value. So maybe in vram helpers, since that might be used by some drivers
that get stuff wrong. Or maybe noveau isn't audited yet.
I like this idea more. I will modify and resend patches with above
suggestions.
That also sidesteps the "what should we call this, drivers are different?"
problem.
Anyway feel free to ignore me, really just a bikeshed at this point.
Cheers, Daniel
Christian.
Best regards
Thomas
Regards,
Christian.
Best regards
Thomas
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index f8015e0318d7..5a6a2af91318 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1317,6 +1317,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object
*bo, bool evict,
list_for_each_entry(vma, >vma_list, head) {
nouveau_vma_map(vma, mem);
}
+ if (bo->mem.mm_node)
+ nvbo->offset =
Re: [PATCH umr v2] fix field names for INDIRECT_BUFFER_CONST/CIK for gfx9/gfx10
[AMD Official Use Only - Internal Distribution Only]
Thanks Tom.
BR,
Xiaojie
From: StDenis, Tom
Sent: Wednesday, February 19, 2020 8:01 PM
To: Yuan, Xiaojie; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH umr v2] fix field names for INDIRECT_BUFFER_CONST/CIK for
gfx9/gfx10
Hmm it doesn't apply on top of the tip of master. I'll just manually
apply it.
Tom
On 2020-02-19 6:56 a.m., Xiaojie Yuan wrote:
> field names for INDIRECT_BUFFER_CONST/CIK of gfx9/gfx10 are the same.
> fields like OFFLOAD_POLLING and VALID are defined in mec's
> INDIRECT_BUFFER packet, so not applicable here.
>
> v2: fix umr_pm4_decode_opcodes.c as well
>
> Signed-off-by: Xiaojie Yuan
> ---
> src/lib/ring_decode.c| 23 +++
> src/lib/umr_pm4_decode_opcodes.c | 20 ++--
> 2 files changed, 13 insertions(+), 30 deletions(-)
>
> diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
> index 250dfd7..fa44f27 100644
> --- a/src/lib/ring_decode.c
> +++ b/src/lib/ring_decode.c
> @@ -617,22 +617,13 @@ static void print_decode_pm4_pkt3(struct umr_asic
> *asic, struct umr_ring_decoder
> case 2: printf("IB_SIZE:%s%lu%s, VMID:
> %s%lu%s", BLUE, BITS(ib, 0, 20), RST, BLUE, BITS(ib, 24, 28), RST);
> decoder->pm4.next_ib_state.ib_size =
> BITS(ib, 0, 20) * 4;
> decoder->pm4.next_ib_state.ib_vmid =
> decoder->next_ib_info.vmid ? decoder->next_ib_info.vmid : BITS(ib, 24, 28);
> - if (decoder->pm4.cur_opcode == 0x33) {
> - if (asic->family >= FAMILY_NV) {
> - printf(", CHAIN:
> %s%u%s, PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV:
> %s%u%s",
> -BLUE,
> (unsigned)BITS(ib, 20, 21), RST,
> -BLUE,
> (unsigned)BITS(ib, 21, 22), RST,
> -BLUE,
> (unsigned)BITS(ib, 28, 30), RST,
> -BLUE,
> (unsigned)BITS(ib, 30, 31), RST,
> -BLUE,
> (unsigned)BITS(ib, 31, 32), RST);
> - } else if (asic->family >=
> FAMILY_AI) {
> - printf(", CHAIN:
> %s%u%s, OFFLOAD_POLLING: %s%u%s, VALID: %s%u%s, CACHE_POLICY: %s%u%s PRIV:
> %s%u%s",
> -BLUE,
> (unsigned)BITS(ib, 20, 21), RST,
> -BLUE,
> (unsigned)BITS(ib, 21, 22), RST,
> -BLUE,
> (unsigned)BITS(ib, 23, 24), RST,
> -BLUE,
> (unsigned)BITS(ib, 28, 30), RST,
> -BLUE,
> (unsigned)BITS(ib, 31, 32), RST);
> - }
> + if (asic->family >= FAMILY_AI) {
> + printf(", CHAIN: %s%u%s,
> PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
> +BLUE,
> (unsigned)BITS(ib, 20, 21), RST,
> +BLUE,
> (unsigned)BITS(ib, 21, 22), RST,
> +BLUE,
> (unsigned)BITS(ib, 28, 30), RST,
> +BLUE,
> (unsigned)BITS(ib, 30, 31), RST,
> +BLUE,
> (unsigned)BITS(ib, 31, 32), RST);
> }
> if (!asic->options.no_follow_ib) {
> if (umr_read_vram(asic,
> decoder->pm4.next_ib_state.ib_vmid,
> diff --git a/src/lib/umr_pm4_decode_opcodes.c
> b/src/lib/umr_pm4_decode_opcodes.c
> index d7c1495..a823ecf 100644
> --- a/src/lib/umr_pm4_decode_opcodes.c
> +++ b/src/lib/umr_pm4_decode_opcodes.c
> @@ -429,20 +429,12 @@ static void decode_pkt3(struct umr_asic *asic, struct
> umr_pm4_stream_decode_ui *
> ui->add_field(ui, ib_addr + 8, ib_vmid, "IB_BASE_HI",
> BITS(stream->words[1], 0, 16), NULL, 16);
> ui->add_field(ui, ib_addr + 12, ib_vmid, "IB_SIZE",
> BITS(stream->words[2], 0, 20), NULL, 10);
> ui->add_field(ui, ib_addr + 12, ib_vmid, "IB_VMID",
> BITS(stream->words[2], 24, 28), NULL, 10);
> - if (stream->opcode == 0x33) {
> -
Re: [PATCH umr v2] fix field names for INDIRECT_BUFFER_CONST/CIK for gfx9/gfx10
Hmm it doesn't apply on top of the tip of master. I'll just manually
apply it.
Tom
On 2020-02-19 6:56 a.m., Xiaojie Yuan wrote:
field names for INDIRECT_BUFFER_CONST/CIK of gfx9/gfx10 are the same.
fields like OFFLOAD_POLLING and VALID are defined in mec's
INDIRECT_BUFFER packet, so not applicable here.
v2: fix umr_pm4_decode_opcodes.c as well
Signed-off-by: Xiaojie Yuan
---
src/lib/ring_decode.c| 23 +++
src/lib/umr_pm4_decode_opcodes.c | 20 ++--
2 files changed, 13 insertions(+), 30 deletions(-)
diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index 250dfd7..fa44f27 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -617,22 +617,13 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic,
struct umr_ring_decoder
case 2: printf("IB_SIZE:%s%lu%s, VMID:
%s%lu%s", BLUE, BITS(ib, 0, 20), RST, BLUE, BITS(ib, 24, 28), RST);
decoder->pm4.next_ib_state.ib_size =
BITS(ib, 0, 20) * 4;
decoder->pm4.next_ib_state.ib_vmid =
decoder->next_ib_info.vmid ? decoder->next_ib_info.vmid : BITS(ib, 24, 28);
- if (decoder->pm4.cur_opcode == 0x33) {
- if (asic->family >= FAMILY_NV) {
- printf(", CHAIN: %s%u%s,
PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 30, 31), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- } else if (asic->family >=
FAMILY_AI) {
- printf(", CHAIN: %s%u%s,
OFFLOAD_POLLING: %s%u%s, VALID: %s%u%s, CACHE_POLICY: %s%u%s PRIV: %s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 23, 24), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- }
+ if (asic->family >= FAMILY_AI) {
+ printf(", CHAIN: %s%u%s, PRE_ENA:
%s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
+ BLUE,
(unsigned)BITS(ib, 20, 21), RST,
+ BLUE,
(unsigned)BITS(ib, 21, 22), RST,
+ BLUE,
(unsigned)BITS(ib, 28, 30), RST,
+ BLUE,
(unsigned)BITS(ib, 30, 31), RST,
+ BLUE,
(unsigned)BITS(ib, 31, 32), RST);
}
if (!asic->options.no_follow_ib) {
if (umr_read_vram(asic,
decoder->pm4.next_ib_state.ib_vmid,
diff --git a/src/lib/umr_pm4_decode_opcodes.c b/src/lib/umr_pm4_decode_opcodes.c
index d7c1495..a823ecf 100644
--- a/src/lib/umr_pm4_decode_opcodes.c
+++ b/src/lib/umr_pm4_decode_opcodes.c
@@ -429,20 +429,12 @@ static void decode_pkt3(struct umr_asic *asic, struct
umr_pm4_stream_decode_ui *
ui->add_field(ui, ib_addr + 8, ib_vmid, "IB_BASE_HI",
BITS(stream->words[1], 0, 16), NULL, 16);
ui->add_field(ui, ib_addr + 12, ib_vmid, "IB_SIZE",
BITS(stream->words[2], 0, 20), NULL, 10);
ui->add_field(ui, ib_addr + 12, ib_vmid, "IB_VMID",
BITS(stream->words[2], 24, 28), NULL, 10);
- if (stream->opcode == 0x33) {
- if (asic->family >= FAMILY_NV) {
- ui->add_field(ui, ib_addr + 12, ib_vmid,
"CHAIN", BITS(stream->words[2], 20, 21), NULL, 10);
- ui->add_field(ui, ib_addr + 12, ib_vmid,
"PRE_ENA", BITS(stream->words[2], 21, 22), NULL, 10);
- ui->add_field(ui, ib_addr + 12, ib_vmid,
"CACHE_POLICY", BITS(stream->words[2],
[PATCH umr v2] fix field names for INDIRECT_BUFFER_CONST/CIK for gfx9/gfx10
field names for INDIRECT_BUFFER_CONST/CIK of gfx9/gfx10 are the same.
fields like OFFLOAD_POLLING and VALID are defined in mec's
INDIRECT_BUFFER packet, so not applicable here.
v2: fix umr_pm4_decode_opcodes.c as well
Signed-off-by: Xiaojie Yuan
---
src/lib/ring_decode.c| 23 +++
src/lib/umr_pm4_decode_opcodes.c | 20 ++--
2 files changed, 13 insertions(+), 30 deletions(-)
diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index 250dfd7..fa44f27 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -617,22 +617,13 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic,
struct umr_ring_decoder
case 2: printf("IB_SIZE:%s%lu%s, VMID:
%s%lu%s", BLUE, BITS(ib, 0, 20), RST, BLUE, BITS(ib, 24, 28), RST);
decoder->pm4.next_ib_state.ib_size =
BITS(ib, 0, 20) * 4;
decoder->pm4.next_ib_state.ib_vmid =
decoder->next_ib_info.vmid ? decoder->next_ib_info.vmid : BITS(ib, 24, 28);
- if (decoder->pm4.cur_opcode == 0x33) {
- if (asic->family >= FAMILY_NV) {
- printf(", CHAIN:
%s%u%s, PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 30, 31), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- } else if (asic->family >=
FAMILY_AI) {
- printf(", CHAIN:
%s%u%s, OFFLOAD_POLLING: %s%u%s, VALID: %s%u%s, CACHE_POLICY: %s%u%s PRIV:
%s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 23, 24), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- }
+ if (asic->family >= FAMILY_AI) {
+ printf(", CHAIN: %s%u%s,
PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
+ BLUE,
(unsigned)BITS(ib, 20, 21), RST,
+ BLUE,
(unsigned)BITS(ib, 21, 22), RST,
+ BLUE,
(unsigned)BITS(ib, 28, 30), RST,
+ BLUE,
(unsigned)BITS(ib, 30, 31), RST,
+ BLUE,
(unsigned)BITS(ib, 31, 32), RST);
}
if (!asic->options.no_follow_ib) {
if (umr_read_vram(asic,
decoder->pm4.next_ib_state.ib_vmid,
diff --git a/src/lib/umr_pm4_decode_opcodes.c b/src/lib/umr_pm4_decode_opcodes.c
index d7c1495..a823ecf 100644
--- a/src/lib/umr_pm4_decode_opcodes.c
+++ b/src/lib/umr_pm4_decode_opcodes.c
@@ -429,20 +429,12 @@ static void decode_pkt3(struct umr_asic *asic, struct
umr_pm4_stream_decode_ui *
ui->add_field(ui, ib_addr + 8, ib_vmid, "IB_BASE_HI",
BITS(stream->words[1], 0, 16), NULL, 16);
ui->add_field(ui, ib_addr + 12, ib_vmid, "IB_SIZE",
BITS(stream->words[2], 0, 20), NULL, 10);
ui->add_field(ui, ib_addr + 12, ib_vmid, "IB_VMID",
BITS(stream->words[2], 24, 28), NULL, 10);
- if (stream->opcode == 0x33) {
- if (asic->family >= FAMILY_NV) {
- ui->add_field(ui, ib_addr + 12,
ib_vmid, "CHAIN", BITS(stream->words[2], 20, 21), NULL, 10);
- ui->add_field(ui, ib_addr + 12,
ib_vmid, "PRE_ENA", BITS(stream->words[2], 21, 22), NULL, 10);
- ui->add_field(ui, ib_addr + 12,
ib_vmid, "CACHE_POLICY", BITS(stream->words[2], 28, 30), NULL, 10);
- ui->add_field(ui, ib_addr + 12,
ib_vmid, "PRE_RESUME",
Re: [PATCH umr] fix field names for INDIRECT_BUFFER_CONST/CIK for gfx9/gfx10
[AMD Official Use Only - Internal Distribution Only]
Sure, I'll send v2 soon.
BR,
Xiaojie
From: StDenis, Tom
Sent: Wednesday, February 19, 2020 7:51 PM
To: Yuan, Xiaojie; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH umr] fix field names for INDIRECT_BUFFER_CONST/CIK for
gfx9/gfx10
Yup, my bad. We also need to fix the streaming version (line 432 of
src/lib/umr_pm4_decode_opcodes.c). Would you like to incorporate this
into this patch? Otherwise I can do it separately.
Thanks,
Tom
On 2020-02-19 6:26 a.m., Xiaojie Yuan wrote:
> field names for INDIRECT_BUFFER_CONST/CIK of gfx9/gfx10 are the same.
> fields like OFFLOAD_POLLING and VALID are defined in mec's
> INDIRECT_BUFFER packet, so not applicable here.
>
> Signed-off-by: Xiaojie Yuan
> ---
> src/lib/ring_decode.c | 23 +++
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
> index 250dfd7..fa44f27 100644
> --- a/src/lib/ring_decode.c
> +++ b/src/lib/ring_decode.c
> @@ -617,22 +617,13 @@ static void print_decode_pm4_pkt3(struct umr_asic
> *asic, struct umr_ring_decoder
> case 2: printf("IB_SIZE:%s%lu%s, VMID:
> %s%lu%s", BLUE, BITS(ib, 0, 20), RST, BLUE, BITS(ib, 24, 28), RST);
> decoder->pm4.next_ib_state.ib_size =
> BITS(ib, 0, 20) * 4;
> decoder->pm4.next_ib_state.ib_vmid =
> decoder->next_ib_info.vmid ? decoder->next_ib_info.vmid : BITS(ib, 24, 28);
> - if (decoder->pm4.cur_opcode == 0x33) {
> - if (asic->family >= FAMILY_NV) {
> - printf(", CHAIN:
> %s%u%s, PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV:
> %s%u%s",
> -BLUE,
> (unsigned)BITS(ib, 20, 21), RST,
> -BLUE,
> (unsigned)BITS(ib, 21, 22), RST,
> -BLUE,
> (unsigned)BITS(ib, 28, 30), RST,
> -BLUE,
> (unsigned)BITS(ib, 30, 31), RST,
> -BLUE,
> (unsigned)BITS(ib, 31, 32), RST);
> - } else if (asic->family >=
> FAMILY_AI) {
> - printf(", CHAIN:
> %s%u%s, OFFLOAD_POLLING: %s%u%s, VALID: %s%u%s, CACHE_POLICY: %s%u%s PRIV:
> %s%u%s",
> -BLUE,
> (unsigned)BITS(ib, 20, 21), RST,
> -BLUE,
> (unsigned)BITS(ib, 21, 22), RST,
> -BLUE,
> (unsigned)BITS(ib, 23, 24), RST,
> -BLUE,
> (unsigned)BITS(ib, 28, 30), RST,
> -BLUE,
> (unsigned)BITS(ib, 31, 32), RST);
> - }
> + if (asic->family >= FAMILY_AI) {
> + printf(", CHAIN: %s%u%s,
> PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
> +BLUE,
> (unsigned)BITS(ib, 20, 21), RST,
> +BLUE,
> (unsigned)BITS(ib, 21, 22), RST,
> +BLUE,
> (unsigned)BITS(ib, 28, 30), RST,
> +BLUE,
> (unsigned)BITS(ib, 30, 31), RST,
> +BLUE,
> (unsigned)BITS(ib, 31, 32), RST);
> }
> if (!asic->options.no_follow_ib) {
> if (umr_read_vram(asic,
> decoder->pm4.next_ib_state.ib_vmid,
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH umr] fix field names for INDIRECT_BUFFER_CONST/CIK for gfx9/gfx10
Yup, my bad. We also need to fix the streaming version (line 432 of
src/lib/umr_pm4_decode_opcodes.c). Would you like to incorporate this
into this patch? Otherwise I can do it separately.
Thanks,
Tom
On 2020-02-19 6:26 a.m., Xiaojie Yuan wrote:
field names for INDIRECT_BUFFER_CONST/CIK of gfx9/gfx10 are the same.
fields like OFFLOAD_POLLING and VALID are defined in mec's
INDIRECT_BUFFER packet, so not applicable here.
Signed-off-by: Xiaojie Yuan
---
src/lib/ring_decode.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index 250dfd7..fa44f27 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -617,22 +617,13 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic,
struct umr_ring_decoder
case 2: printf("IB_SIZE:%s%lu%s, VMID:
%s%lu%s", BLUE, BITS(ib, 0, 20), RST, BLUE, BITS(ib, 24, 28), RST);
decoder->pm4.next_ib_state.ib_size =
BITS(ib, 0, 20) * 4;
decoder->pm4.next_ib_state.ib_vmid =
decoder->next_ib_info.vmid ? decoder->next_ib_info.vmid : BITS(ib, 24, 28);
- if (decoder->pm4.cur_opcode == 0x33) {
- if (asic->family >= FAMILY_NV) {
- printf(", CHAIN: %s%u%s,
PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 30, 31), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- } else if (asic->family >=
FAMILY_AI) {
- printf(", CHAIN: %s%u%s,
OFFLOAD_POLLING: %s%u%s, VALID: %s%u%s, CACHE_POLICY: %s%u%s PRIV: %s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 23, 24), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- }
+ if (asic->family >= FAMILY_AI) {
+ printf(", CHAIN: %s%u%s, PRE_ENA:
%s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
+ BLUE,
(unsigned)BITS(ib, 20, 21), RST,
+ BLUE,
(unsigned)BITS(ib, 21, 22), RST,
+ BLUE,
(unsigned)BITS(ib, 28, 30), RST,
+ BLUE,
(unsigned)BITS(ib, 30, 31), RST,
+ BLUE,
(unsigned)BITS(ib, 31, 32), RST);
}
if (!asic->options.no_follow_ib) {
if (umr_read_vram(asic,
decoder->pm4.next_ib_state.ib_vmid,
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
回复: [PATCH] drm/amdgpu: fix a bug NULL pointer dereference
> + if (!entity->rq) > + return 0; > + Yes, supposedly we shouldn't get 'entity->rq == NULL' case , that looks the true bug -邮件原件- 发件人: amd-gfx 代表 Christian K?nig 发送时间: 2020年2月19日 18:50 收件人: Zhang, Hawking ; Li, Dennis ; amd-gfx@lists.freedesktop.org; Deucher, Alexander ; Zhou1, Tao ; Chen, Guchun 主题: Re: [PATCH] drm/amdgpu: fix a bug NULL pointer dereference Well of hand this patch looks like a clear NAK to me. Returning without raising an error is certainly the wrong thing to do here because we just drop the necessary page table updates. How does the entity->rq ends up as NULL in the first place? Regards, Christian. Am 19.02.20 um 07:26 schrieb Zhang, Hawking: > [AMD Official Use Only - Internal Distribution Only] > > Reviewed-by: Hawking Zhang > > Regards, > Hawking > -Original Message- > From: Dennis Li > Sent: Wednesday, February 19, 2020 12:05 > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander > ; Zhou1, Tao ; Zhang, > Hawking ; Chen, Guchun > Cc: Li, Dennis > Subject: [PATCH] drm/amdgpu: fix a bug NULL pointer dereference > > check whether the queue of entity is null to avoid null pointer dereference. > > Change-Id: I08d56774012cf229ba2fe7a011c1359e8d1e2781 > Signed-off-by: Dennis Li > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > index 4cc7881f438c..67cca463ddcc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c > @@ -95,6 +95,9 @@ static int amdgpu_vm_sdma_commit(struct > amdgpu_vm_update_params *p, > int r; > > entity = p->direct ? >vm->direct : >vm->delayed; > + if (!entity->rq) > + return 0; > + > ring = container_of(entity->rq->sched, struct amdgpu_ring, sched); > > WARN_ON(ib->length_dw == 0); > -- > 2.17.1 > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cmo > nk.liu%40amd.com%7C28e7260af3a24eec758f08d7b52975e3%7C3dd8961fe4884e60 > 8e11a82d994e183d%7C0%7C0%7C637177062003213431sdata=vMXmhwTlN8lAav > uqhYhpmKLM6V%2F%2B2%2FubFBbsk%2BGY%2Bjw%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cmonk.liu%40amd.com%7C28e7260af3a24eec758f08d7b52975e3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637177062003213431sdata=vMXmhwTlN8lAavuqhYhpmKLM6V%2F%2B2%2FubFBbsk%2BGY%2Bjw%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add a GEM_CREATE mask and bugfix
On Tue, Feb 18, 2020 at 04:46:21PM -0500, Luben Tuikov wrote: > On 2020-02-17 10:08 a.m., Christian König wrote: > > Am 17.02.20 um 15:44 schrieb Alex Deucher: > >> On Fri, Feb 14, 2020 at 7:17 PM Luben Tuikov wrote: > >>> Add a AMDGPU_GEM_CREATE_MASK and use it to check > >>> for valid/invalid GEM create flags coming in from > >>> userspace. > >>> > >>> Fix a bug in checking whether TMZ is supported at > >>> GEM create time. > >>> > >>> Signed-off-by: Luben Tuikov > >>> --- > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 ++- > >>> include/uapi/drm/amdgpu_drm.h | 2 ++ > >>> 2 files changed, 4 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> index b51a060c637d..74bb79e64fa3 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> @@ -221,21 +221,14 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, > >>> void *data, > >>> int r; > >>> > >>> /* reject invalid gem flags */ > >>> - if (flags & ~(AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED | > >>> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS | > >>> - AMDGPU_GEM_CREATE_CPU_GTT_USWC | > >>> - AMDGPU_GEM_CREATE_VRAM_CLEARED | > >>> - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | > >>> - AMDGPU_GEM_CREATE_EXPLICIT_SYNC | > >>> - AMDGPU_GEM_CREATE_ENCRYPTED)) > >>> - > >> I'd rather keep the list explicit so no one ends up forgetting to > >> update the mask the next time new flags are added. > > > > Additional to that this patch is bogus. > > So the only "additional" thing you're contributing to the review of > this patch is calling it "bogus". Characterizing the patch with an adjective > as "bogus" is very disturbing. What's next? > > > > > We intentionally filter out the flags here which userspace is allowed to > > specify in the GEM interface, but after this patch we would allow all > > flags to be specified. > > I thought that that could be the case. But in your review why not > recommend we have a mask to check user-settable flags? So the > actual flags are collected and visible in one place and well > understood that that is what is being checked and well-defined > by a macro with an appropriate name? > > Why did NO ONE comment on the bug fix below? No one. > I missed the bugfix patch before, sorry. (There are many patches from amd-gfx one day, if you didn't cc me, the mail may be missed). So I found the issue after sync up with drm-next and then give the fix. Hope you can understand and don't take it to heart. Thanks, Ray ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH umr] fix field names for INDIRECT_BUFFER_CONST/CIK for gfx9/gfx10
field names for INDIRECT_BUFFER_CONST/CIK of gfx9/gfx10 are the same.
fields like OFFLOAD_POLLING and VALID are defined in mec's
INDIRECT_BUFFER packet, so not applicable here.
Signed-off-by: Xiaojie Yuan
---
src/lib/ring_decode.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index 250dfd7..fa44f27 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -617,22 +617,13 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic,
struct umr_ring_decoder
case 2: printf("IB_SIZE:%s%lu%s, VMID:
%s%lu%s", BLUE, BITS(ib, 0, 20), RST, BLUE, BITS(ib, 24, 28), RST);
decoder->pm4.next_ib_state.ib_size =
BITS(ib, 0, 20) * 4;
decoder->pm4.next_ib_state.ib_vmid =
decoder->next_ib_info.vmid ? decoder->next_ib_info.vmid : BITS(ib, 24, 28);
- if (decoder->pm4.cur_opcode == 0x33) {
- if (asic->family >= FAMILY_NV) {
- printf(", CHAIN:
%s%u%s, PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 30, 31), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- } else if (asic->family >=
FAMILY_AI) {
- printf(", CHAIN:
%s%u%s, OFFLOAD_POLLING: %s%u%s, VALID: %s%u%s, CACHE_POLICY: %s%u%s PRIV:
%s%u%s",
- BLUE,
(unsigned)BITS(ib, 20, 21), RST,
- BLUE,
(unsigned)BITS(ib, 21, 22), RST,
- BLUE,
(unsigned)BITS(ib, 23, 24), RST,
- BLUE,
(unsigned)BITS(ib, 28, 30), RST,
- BLUE,
(unsigned)BITS(ib, 31, 32), RST);
- }
+ if (asic->family >= FAMILY_AI) {
+ printf(", CHAIN: %s%u%s,
PRE_ENA: %s%u%s, CACHE_POLICY: %s%u%s, PRE_RESUME: %s%u%s PRIV: %s%u%s",
+ BLUE,
(unsigned)BITS(ib, 20, 21), RST,
+ BLUE,
(unsigned)BITS(ib, 21, 22), RST,
+ BLUE,
(unsigned)BITS(ib, 28, 30), RST,
+ BLUE,
(unsigned)BITS(ib, 30, 31), RST,
+ BLUE,
(unsigned)BITS(ib, 31, 32), RST);
}
if (!asic->options.no_follow_ib) {
if (umr_read_vram(asic,
decoder->pm4.next_ib_state.ib_vmid,
--
2.20.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace
Am 19.02.20 um 11:15 schrieb Jacob He:
[WHY]
When SPM trace enabled, SPM_VMID should be updated with the current
vmid.
[HOW]
Add a chunk id, AMDGPU_CHUNK_ID_SPM_TRACE, so that UMD can tell us
which job should update SPM_VMID.
Right before a job is submitted to GPU, set the SPM_VMID accordingly.
[Limitation]
Running more than one SPM trace enabled processes simultaneously is
not supported.
Well there are multiple problems with that patch.
First of all you need to better describe what SPM tracing is in the
commit message.
Then the updating of mmRLC_SPM_MC_CNTL must be executed asynchronously
on the ring. Otherwise we might corrupt an already executing SPM trace.
And you also need to make sure to disable the tracing again or otherwise
we run into a bunch of trouble when the VMID is reused.
You also need to make sure that IBs using the SPM trace are serialized
with each other, e.g. hack into amdgpu_ids.c file and make sure that
only one VMID at a time can have that attribute.
Regards,
Christian.
Change-Id: Ic932ef6ac9dbf244f03aaee90550e8ff3a675666
Signed-off-by: Jacob He
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 7 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 10 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h | 1 +
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 15 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 15 ++-
8 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f9fa6e104fef..3f32c4db5232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -113,6 +113,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
uint32_t uf_offset = 0;
int i;
int ret;
+ bool update_spm_vmid = false;
if (cs->in.num_chunks == 0)
return 0;
@@ -221,6 +222,10 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
break;
+ case AMDGPU_CHUNK_ID_SPM_TRACE:
+ update_spm_vmid = true;
+ break;
+
default:
ret = -EINVAL;
goto free_partial_kdata;
@@ -231,6 +236,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
if (ret)
goto free_all_kdata;
+ p->job->need_update_spm_vmid = update_spm_vmid;
+
if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index cae81914c821..36faab12b585 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -156,9 +156,13 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
return -EINVAL;
}
- if (vm && !job->vmid) {
- dev_err(adev->dev, "VM IB without ID\n");
- return -EINVAL;
+ if (vm) {
+ if (!job->vmid) {
+ dev_err(adev->dev, "VM IB without ID\n");
+ return -EINVAL;
+ } else if (adev->gfx.rlc.funcs->update_spm_vmid &&
job->need_update_spm_vmid) {
+ adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid);
+ }
}
alloc_size = ring->funcs->emit_frame_size + num_ibs *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..4582536961c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -52,6 +52,7 @@ struct amdgpu_job {
boolvm_needs_flush;
uint64_tvm_pd_addr;
unsignedvmid;
+ boolneed_update_spm_vmid;
unsignedpasid;
uint32_tgds_base, gds_size;
uint32_tgws_base, gws_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
index d3d4707f2168..52509c254cbd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
@@ -126,6 +126,7 @@ struct amdgpu_rlc_funcs {
void (*stop)(struct amdgpu_device *adev);
void (*reset)(struct amdgpu_device *adev);
void (*start)(struct amdgpu_device *adev);
+ void (*update_spm_vmid)(struct amdgpu_device *adev, unsigned vmid);
};
struct amdgpu_rlc {
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index
Re: [PATCH] drm/amdgpu: fix a bug NULL pointer dereference
Well of hand this patch looks like a clear NAK to me. Returning without raising an error is certainly the wrong thing to do here because we just drop the necessary page table updates. How does the entity->rq ends up as NULL in the first place? Regards, Christian. Am 19.02.20 um 07:26 schrieb Zhang, Hawking: [AMD Official Use Only - Internal Distribution Only] Reviewed-by: Hawking Zhang Regards, Hawking -Original Message- From: Dennis Li Sent: Wednesday, February 19, 2020 12:05 To: amd-gfx@lists.freedesktop.org; Deucher, Alexander ; Zhou1, Tao ; Zhang, Hawking ; Chen, Guchun Cc: Li, Dennis Subject: [PATCH] drm/amdgpu: fix a bug NULL pointer dereference check whether the queue of entity is null to avoid null pointer dereference. Change-Id: I08d56774012cf229ba2fe7a011c1359e8d1e2781 Signed-off-by: Dennis Li diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c index 4cc7881f438c..67cca463ddcc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c @@ -95,6 +95,9 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p, int r; entity = p->direct ? >vm->direct : >vm->delayed; + if (!entity->rq) + return 0; + ring = container_of(entity->rq->sched, struct amdgpu_ring, sched); WARN_ON(ib->length_dw == 0); -- 2.17.1 ___ 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/smu: add an update table lock
Thanks. I went through that bug report. And it seems weird the table lock works
but msg lock does not considering if it was really caused by some race
conditions.
Considering the issue was found on multi monitors setup. Maybe mclk dpm is
related.
Is it possible to try with single monitor only? Or trying disabling mclk dpm?
-Original Message-
From: Alex Deucher
Sent: Tuesday, February 18, 2020 10:35 PM
To: Quan, Evan
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander
Subject: Re: [PATCH] drm/amdgpu/smu: add an update table lock
On Mon, Feb 17, 2020 at 10:01 PM Quan, Evan wrote:
>
> Hi Alex,
>
> Did you seen any issue caused by this?
Seems to help on:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F1047data=02%7C01%7CEvan.Quan%40amd.com%7C1266ea24bc2f4fff2cfb08d7b47fc095%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637176333114972316sdata=Lpts%2FYe%2Bq64ppyuzNIGWFYiGEXqzQVdAO2CiP6mSfFc%3Dreserved=0
I haven't been able to prove to myself that the existing high level locking
covers every case.
Alex
>
> Regards,
> Evan
> -Original Message-
> From: amd-gfx On Behalf Of
> Alex Deucher
> Sent: Tuesday, February 18, 2020 5:38 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander
> Subject: [PATCH] drm/amdgpu/smu: add an update table lock
>
> The driver uses a staging buffer to update tables in the SMU.
> Add a lock to make sure we don't try and do this concurrently by
> accident.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 7 ++-
> drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 9f2428fd98f6..437a3e7b36b4 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -530,6 +530,7 @@ int smu_update_table(struct smu_context *smu, enum
> smu_table_id table_index, int
>
> table_size = smu_table->tables[table_index].size;
>
> + mutex_lock(>update_table_lock);
> if (drv2smu) {
> memcpy(table->cpu_addr, table_data, table_size);
> /*
> @@ -544,13 +545,16 @@ int smu_update_table(struct smu_context *smu, enum
> smu_table_id table_index, int
> SMU_MSG_TransferTableSmu2Dram,
> table_id | ((argument & 0x) <<
> 16));
> if (ret)
> - return ret;
> + goto unlock;
>
> if (!drv2smu) {
> amdgpu_asic_flush_hdp(adev, NULL);
> memcpy(table_data, table->cpu_addr, table_size);
> }
>
> +unlock:
> + mutex_unlock(>update_table_lock);
> +
> return ret;
> }
>
> @@ -900,6 +904,7 @@ static int smu_sw_init(void *handle)
>
> mutex_init(>sensor_lock);
> mutex_init(>metrics_lock);
> + mutex_init(>update_table_lock);
>
> smu->watermarks_bitmap = 0;
> smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index 97b6714e83e6..506288072e8e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -362,6 +362,7 @@ struct smu_context
> struct mutexmutex;
> struct mutexsensor_lock;
> struct mutexmetrics_lock;
> + struct mutexupdate_table_lock;
> uint64_t pool_size;
>
> struct smu_table_contextsmu_table;
> --
> 2.24.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CEv
> an.Quan%40amd.com%7C1266ea24bc2f4fff2cfb08d7b47fc095%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C637176333114972316sdata=JiABwHLa0eLLp
> yiwKXU4nSU28OXBuxTnRbisgoC4uK0%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Add a chunk ID for spm trace
[WHY]
When SPM trace enabled, SPM_VMID should be updated with the current
vmid.
[HOW]
Add a chunk id, AMDGPU_CHUNK_ID_SPM_TRACE, so that UMD can tell us
which job should update SPM_VMID.
Right before a job is submitted to GPU, set the SPM_VMID accordingly.
[Limitation]
Running more than one SPM trace enabled processes simultaneously is
not supported.
Change-Id: Ic932ef6ac9dbf244f03aaee90550e8ff3a675666
Signed-off-by: Jacob He
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 7 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 10 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h | 1 +
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 15 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 15 ++-
8 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f9fa6e104fef..3f32c4db5232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -113,6 +113,7 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
uint32_t uf_offset = 0;
int i;
int ret;
+ bool update_spm_vmid = false;
if (cs->in.num_chunks == 0)
return 0;
@@ -221,6 +222,10 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
break;
+ case AMDGPU_CHUNK_ID_SPM_TRACE:
+ update_spm_vmid = true;
+ break;
+
default:
ret = -EINVAL;
goto free_partial_kdata;
@@ -231,6 +236,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, union drm_amdgpu_cs
if (ret)
goto free_all_kdata;
+ p->job->need_update_spm_vmid = update_spm_vmid;
+
if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index cae81914c821..36faab12b585 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -156,9 +156,13 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
return -EINVAL;
}
- if (vm && !job->vmid) {
- dev_err(adev->dev, "VM IB without ID\n");
- return -EINVAL;
+ if (vm) {
+ if (!job->vmid) {
+ dev_err(adev->dev, "VM IB without ID\n");
+ return -EINVAL;
+ } else if (adev->gfx.rlc.funcs->update_spm_vmid &&
job->need_update_spm_vmid) {
+ adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid);
+ }
}
alloc_size = ring->funcs->emit_frame_size + num_ibs *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..4582536961c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -52,6 +52,7 @@ struct amdgpu_job {
boolvm_needs_flush;
uint64_tvm_pd_addr;
unsignedvmid;
+ boolneed_update_spm_vmid;
unsignedpasid;
uint32_tgds_base, gds_size;
uint32_tgws_base, gws_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
index d3d4707f2168..52509c254cbd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
@@ -126,6 +126,7 @@ struct amdgpu_rlc_funcs {
void (*stop)(struct amdgpu_device *adev);
void (*reset)(struct amdgpu_device *adev);
void (*start)(struct amdgpu_device *adev);
+ void (*update_spm_vmid)(struct amdgpu_device *adev, unsigned vmid);
};
struct amdgpu_rlc {
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 5e9fb0976c6c..91eb788d6229 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4214,6 +4214,18 @@ static int gfx_v10_0_update_gfx_clock_gating(struct
amdgpu_device *adev,
return 0;
}
+static void gfx_v10_0_update_spm_vmid(struct amdgpu_device *adev, unsigned
vmid)
+{
+ u32 data;
+
+ data = RREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL);
+
+ data &= ~RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK;
+ data |= (vmid & RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK) <<
RLC_SPM_MC_CNTL__RLC_SPM_VMID__SHIFT;
+
+ WREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL, data);
+}
+
static const struct amdgpu_rlc_funcs
Re: 回复: 回复: [PATCH 3/3] drm/amdgpu: fix colliding of preemption
Hi Monk, oh, I've miss interpreted Hans response. It indeed sounds like that could work. We don't even need full preemption under VF, it would also make things easier if we just have the same CSA handling for both. Regards, Christian. Am 19.02.20 um 06:04 schrieb Liu, Monk: Hi Hans For CE/DE meta data, OS preemption would write those credential to a dynamically allocated CSA buffer (one per process), but for SRIOV we only write them to static CSA (one per VF) In windows KMD, the OS Preemption is forced to disabled when SRIOV is enabled Based on above two reason I stated that if we are under SRIOV, we should not allow an OS preemption triggered. Are you saying that we can still support OS preemption even for VF ? Thanks -邮件原件- 发件人: Fernlund, Hans 发送时间: 2020年2月18日 22:49 收件人: Liu, Monk ; Koenig, Christian ; Zhang, Hawking ; amd-gfx@lists.freedesktop.org 主题: RE: 回复: [PATCH 3/3] drm/amdgpu: fix colliding of preemption [AMD Official Use Only - Internal Distribution Only] At CP level, MCBP and SRIOV are not mutually exclusive. They are used together in some environments. /Hans -Original Message- From: Liu, Monk Sent: Tuesday, February 18, 2020 7:53 AM To: Koenig, Christian ; Zhang, Hawking ; amd-gfx@lists.freedesktop.org; Fernlund, Hans Subject: 回复: 回复: [PATCH 3/3] drm/amdgpu: fix colliding of preemption If OS preemption can work with SRIOV world switch I won't provide such patch, I would already clean up those MCBP checking ... To your question: I don't know details, but my guess is that CP use the same interface with KMD (e.g. CE/DE meta) to handle two MCBP type: world switch and OS preempt, But some implementation is different inside CP firmware by the given interface, that need @Fernlund, Hans to provide the details if you want an expertise, he is the author of CP preemption Thanks -邮件原件- 发件人: Christian König 发送时间: 2020年2月18日 20:40 收件人: Liu, Monk ; Zhang, Hawking ; amd-gfx@lists.freedesktop.org 主题: Re: 回复: [PATCH 3/3] drm/amdgpu: fix colliding of preemption Hawking is right here. We could just check amdgpu_mcbp during device initialization and forcefully clear it under SRIOV. But why is MCBP and SRIOV mutual exclusive? We are certainly getting the requirement for this sooner or later. Regards, Christian. Am 18.02.20 um 13:29 schrieb Liu, Monk: Even not talking about CE/DE meta and SDMA CS, we still cannot share amdgpu_mcbp with SRIOV case, e.g.: In some place we use "if (amdgpu_mcbp || amdgpu_sriov_vf()" to check, and we do the same thing under that condition, But we cannot do that thing by "if (amdgpu_mcbp)" and set "amdgpu_mcbp" to true under SRIOV case. Because that will let OS preemption work, which will trigger mismatch/crush in CPG side during the MCBP. (not sure if CWSR has such colliding) So we always need to disable OS preemption behavior for SRIOV (don't to preempt an IB by touch vmid_preempt register, and no IB resume happen) Thanks -邮件原件- 发件人: Zhang, Hawking 发送时间: 2020年2月18日 19:48 收件人: Zhang, Hawking ; Liu, Monk ; amd-gfx@lists.freedesktop.org 抄送: Liu, Monk 主题: RE: [PATCH 3/3] drm/amdgpu: fix colliding of preemption [AMD Official Use Only - Internal Distribution Only] Ahhh Send it too quickly. Of course, we still need to apply vf check for ce/de-meta, but I think in such way, we can dramatically reduce the amdgpu_sirov_vf check in every mcbp code path. Regards, Hawking -Original Message- From: amd-gfx On Behalf Of Zhang, Hawking Sent: Tuesday, February 18, 2020 19:32 To: Liu, Monk ; amd-gfx@lists.freedesktop.org Cc: Liu, Monk Subject: RE: [PATCH 3/3] drm/amdgpu: fix colliding of preemption [AMD Official Use Only - Internal Distribution Only] It's some kind of annoying to check vf in every place that is required for mcbp until amdgpu_mcbp is enabled by default. What's more, when amdgpu_mcbp is enabled by default, there will be many unnecessary vf check that can be removed as most of mcbp function actually can be shared between world switch preemption and os preemption. I'd prefer to enable amdgpu_mcbp for sriov in amdgpu_device_check_arguments to reduce the vf specific check everywhere. Regards, Hawking -Original Message- From: amd-gfx On Behalf Of Monk Liu Sent: Tuesday, February 18, 2020 10:54 To: amd-gfx@lists.freedesktop.org Cc: Liu, Monk Subject: [PATCH 3/3] drm/amdgpu: fix colliding of preemption what: some os preemption path is messed up with world switch preemption fix: cleanup those logics so os preemption not mixed with world switch this patch is a general fix for issues comes from SRIOV MCBP, but there is still UMD side issues not resovlved yet, so this patch cannot fix all world switch bug. Signed-off-by: Monk Liu --- drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 3 ++- drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 8 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
Re: [RFC PATCH v5] drm/amdgpu: Remove kfd eviction fence before release bo
Am 19.02.20 um 02:54 schrieb Pan, Xinhui:
2020年2月19日 07:10，Kuehling, Felix 写道：
Hi Xinhui,
Two suggestions inline. Looks good to me otherwise.
On 2020-02-17 10:36 p.m., xinhui pan wrote:
No need to trigger eviction as the memory mapping will not be used
anymore.
All pt/pd bos share same resv, hence the same shared eviction fence.
Everytime page table is freed, the fence will be signled and that cuases
kfd unexcepted evictions.
Signed-off-by: xinhui pan
CC: Christian König
CC: Felix Kuehling
CC: Alex Deucher
---
change from v4:
based on new ttm code.
change from v3:
fix a coding error
change from v2:
based on Chris' drm/ttm: rework BO delayed delete patchset.
---
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 1 +
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 37 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 4 ++
3 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 9e8db702d878..0ee8aae6c519 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -96,6 +96,7 @@ struct amdgpu_amdkfd_fence *amdgpu_amdkfd_fence_create(u64
context,
struct mm_struct *mm);
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);
+int amdgpu_amdkfd_remove_fence_on_pt_pd_bos(struct amdgpu_bo *bo);
struct amdkfd_process_info {
/* List head of all VMs that belong to a KFD process */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index ef721cb65868..6aa20aa82bd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -276,6 +276,41 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct
amdgpu_bo *bo,
return 0;
}
+int amdgpu_amdkfd_remove_fence_on_pt_pd_bos(struct amdgpu_bo *bo)
+{
+ struct amdgpu_bo *root = bo;
+ struct amdgpu_vm_bo_base *vm_bo;
+ struct amdgpu_vm *vm;
+ struct amdkfd_process_info *info;
+ struct amdgpu_amdkfd_fence *ef;
+ int ret;
+
+ while (root->parent)
+ root = root->parent;
This should not be necessary. Every page table BO has a pointer to a vm_bo that
has a pointer to the vm. So you don't need to find the root.
This should do the trick:
if (!bo->vm_bo || !bo->vm_bo->vm)
return 0;
vm = bo->vm_bo->vm;
well,when free page tables, it clears bo->vm_bo first then release pt/pd bo.
Also we can change the sequence like I do in V2, looks like hit some weird
issues.
+
+ vm_bo = root->vm_bo;
+ if (!vm_bo)
+ return 0;
+
+ vm = vm_bo->vm;
+ if (!vm)
+ return 0;
+
+ info = vm->process_info;
+ if (!info || !info->eviction_fence)
+ return 0;
+
+ ef = container_of(dma_fence_get(>eviction_fence->base),
+ struct amdgpu_amdkfd_fence, base);
+
+ dma_resv_lock(bo->tbo.base.resv, NULL);
Now that Felix mentioned it this should be a dma_resv_trylock().
dma_resv_lock() can intentionally fail randomly for testing purposes,
but trylock() will always succeed because we are the only one
referencing the BO at the moment.
Regards,
Christian.
+ ret = amdgpu_amdkfd_remove_eviction_fence(bo, ef);
+ dma_resv_unlock(bo->tbo.base.resv);
+
+ dma_fence_put(>base);
+ return ret;
+}
+
static int amdgpu_amdkfd_bo_validate(struct amdgpu_bo *bo, uint32_t domain,
bool wait)
{
@@ -1045,6 +1080,8 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device
*adev,
list_del(>vm_list_node);
mutex_unlock(_info->lock);
+ vm->process_info = NULL;
+
/* Release per-process resources when last compute VM is destroyed */
if (!process_info->n_vms) {
WARN_ON(!list_empty(_info->kfd_bo_list));
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6f60a581e3ba..16586651020f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1307,6 +1307,10 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (abo->kfd_bo)
amdgpu_amdkfd_unreserve_memory_limit(abo);
+ /* We only remove the fence if the resv has individualized. */
+ if (bo->base.resv == >base._resv)
Should this be a WARN_ON? We expect this condition to be always true. If it's
not, there should be a noisy warning that something is wrong.
good point.
thanks
xinhui
Regards,
Felix
+ amdgpu_amdkfd_remove_fence_on_pt_pd_bos(abo);
+
if (bo->mem.mem_type != TTM_PL_VRAM || !bo->mem.mm_node ||
!(abo->flags &
Re: [PATCH] drm/amdgpu: Add a GEM_CREATE mask and bugfix
Am 18.02.20 um 22:46 schrieb Luben Tuikov:
On 2020-02-17 10:08 a.m., Christian König wrote:
Am 17.02.20 um 15:44 schrieb Alex Deucher:
On Fri, Feb 14, 2020 at 7:17 PM Luben Tuikov wrote:
Add a AMDGPU_GEM_CREATE_MASK and use it to check
for valid/invalid GEM create flags coming in from
userspace.
Fix a bug in checking whether TMZ is supported at
GEM create time.
Signed-off-by: Luben Tuikov
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 ++-
include/uapi/drm/amdgpu_drm.h | 2 ++
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index b51a060c637d..74bb79e64fa3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -221,21 +221,14 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void
*data,
int r;
/* reject invalid gem flags */
- if (flags & ~(AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
- AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
- AMDGPU_GEM_CREATE_CPU_GTT_USWC |
- AMDGPU_GEM_CREATE_VRAM_CLEARED |
- AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
- AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
- AMDGPU_GEM_CREATE_ENCRYPTED))
-
I'd rather keep the list explicit so no one ends up forgetting to
update the mask the next time new flags are added.
Additional to that this patch is bogus.
So the only "additional" thing you're contributing to the review of
this patch is calling it "bogus". Characterizing the patch with an adjective
as "bogus" is very disturbing. What's next?
Well the patch is technical incorrect and breaks the code in a very
subtle and hard to detect manner. Alex didn't noticed that either.
I'm not a native speaker of English, but as far as I have learned
"bogus" is the right adjective for that.
We intentionally filter out the flags here which userspace is allowed to
specify in the GEM interface, but after this patch we would allow all
flags to be specified.
I thought that that could be the case.
Then why did you send out a patch which is seriously broken like that?
I mean if I hadn't noticed it by chance we would have committed this and
added a potentially security problematic bug to the IOCTL interface.
But in your review why not
recommend we have a mask to check user-settable flags? So the
actual flags are collected and visible in one place and well
understood that that is what is being checked and well-defined
by a macro with an appropriate name?
See the flags tested here are the flags currently accepted by the
amdgpu_gem_create_ioctl() function. It doesn't say anything about what
the kernel would accept in the future.
So when we move that into the UAPI header we give userspace a
technically incorrect value.
Why did NO ONE comment on the bug fix below? No one.
Because you mixed up a style change into some bug fix. That people go
for the problematic parts during code review is not really surprising at
all.
Regards,
Christian.
Regards,
Luben
Christian.
Alex
+ if (flags & ~AMDGPU_GEM_CREATE_MASK)
return -EINVAL;
/* reject invalid gem domains */
if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
return -EINVAL;
- if (amdgpu_is_tmz(adev) && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
+ if (!amdgpu_is_tmz(adev) && flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
DRM_ERROR("Cannot allocate secure buffer since TMZ is
disabled\n");
return -EINVAL;
}
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index eaf94a421901..c8463cdf4448 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -141,6 +141,8 @@ extern "C" {
*/
#define AMDGPU_GEM_CREATE_ENCRYPTED(1 << 10)
+#define AMDGPU_GEM_CREATE_MASK ((1 << 11)-1)
+
struct drm_amdgpu_gem_create_in {
/** the requested memory size */
__u64 bo_size;
--
2.25.0.232.gd8437c57fa
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cluben.tuikov%40amd.com%7Cb1fdc3970a224fc61fdc08d7b3bb4613%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175489240077250sdata=rE8A6jKAIX081ZhxxcMc%2BpGdXvsLUdrAW4AkLsTpNxg%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cluben.tuikov%40amd.com%7Cb1fdc3970a224fc61fdc08d7b3bb4613%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175489240077250sdata=rE8A6jKAIX081ZhxxcMc%2BpGdXvsLUdrAW4AkLsTpNxg%3Dreserved=0
