[PATCH] drm/amdgpu: Add a chunk ID for spm trace
Christian König
christian.koenig at amd.com
Fri Feb 21 15:33:25 UTC 2020
I would just do this as part of the vm_flush() callback on the ring.
E.g. check if the VMID you want to flush is reserved and if yes enable SPM.
Maybe pass along a flag or something in the job to make things easier.
Christian.
Am 21.02.20 um 16:31 schrieb Deucher, Alexander:
>
> [AMD Public Use]
>
>
> We already have the RESERVE_VMID ioctl interface, can't we just use
> that internally in the kernel to update the rlc register via the ring
> when we schedule the relevant IB? E.g., add a new ring callback to
> set SPM state and then set it to the reserved vmid before we schedule
> the ib, and then reset it to 0 after the IB in amdgpu_ib_schedule().
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 4b2342d11520..e0db9362c6ee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -185,6 +185,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> unsigned num_ibs,
> if (ring->funcs->insert_start)
> ring->funcs->insert_start(ring);
>
> + if (ring->funcs->setup_spm)
> + ring->funcs->setup_spm(ring, job);
> +
> if (job) {
> r = amdgpu_vm_flush(ring, job, need_pipe_sync);
> if (r) {
> @@ -273,6 +276,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
> unsigned num_ibs,
> return r;
> }
>
> + if (ring->funcs->setup_spm)
> + ring->funcs->setup_spm(ring, NULL);
> +
> if (ring->funcs->insert_end)
> ring->funcs->insert_end(ring);
>
>
>
> Alex
> ------------------------------------------------------------------------
> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of
> Christian König <ckoenig.leichtzumerken at gmail.com>
> *Sent:* Friday, February 21, 2020 5:28 AM
> *To:* Zhou, David(ChunMing) <David1.Zhou at amd.com>; He, Jacob
> <Jacob.He at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>;
> amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
> *Subject:* Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace
> That would probably be a no-go, but we could enhance the kernel driver
> to update the RLC_SPM_VMID register with the reserved VMID.
>
> Handling that in userspace is most likely not working anyway, since
> the RLC registers are usually not accessible by userspace.
>
> Regards,
> Christian.
>
> Am 20.02.20 um 16:15 schrieb Zhou, David(ChunMing):
>>
>> [AMD Official Use Only - Internal Distribution Only]
>>
>> You can enhance amdgpu_vm_ioctl In amdgpu_vm.c to return vmid to
>> userspace.
>>
>> -David
>>
>> *From:* He, Jacob <Jacob.He at amd.com> <mailto:Jacob.He at amd.com>
>> *Sent:* Thursday, February 20, 2020 10:46 PM
>> *To:* Zhou, David(ChunMing) <David1.Zhou at amd.com>
>> <mailto:David1.Zhou at amd.com>; Koenig, Christian
>> <Christian.Koenig at amd.com> <mailto:Christian.Koenig at amd.com>;
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>> *Subject:* RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace
>>
>> amdgpu_vm_reserve_vmid doesn’t return the reserved vmid back to user
>> space. There is no chance for user mode driver to update RLC_SPM_VMID.
>>
>> Thanks
>>
>> Jacob
>>
>> *From: *He, Jacob <mailto:Jacob.He at amd.com>
>> *Sent: *Thursday, February 20, 2020 6:20 PM
>> *To: *Zhou, David(ChunMing) <mailto:David1.Zhou at amd.com>; Koenig,
>> Christian <mailto:Christian.Koenig at amd.com>;
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>> *Subject: *RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace
>>
>> Looks like amdgpu_vm_reserve_vmid could work, let me have a try to
>> update the RLC_SPM_VMID with pm4 packets in UMD.
>>
>> Thanks
>>
>> Jacob
>>
>> *From: *Zhou, David(ChunMing) <mailto:David1.Zhou at amd.com>
>> *Sent: *Thursday, February 20, 2020 10:13 AM
>> *To: *Koenig, Christian <mailto:Christian.Koenig at amd.com>; He, Jacob
>> <mailto:Jacob.He at amd.com>; amd-gfx at lists.freedesktop.org
>> <mailto:amd-gfx at lists.freedesktop.org>
>> *Subject: *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 <amd-gfx-bounces at lists.freedesktop.org
>> <mailto:amd-gfx-bounces at lists.freedesktop.org>> On Behalf Of
>> Christian König
>> Sent: Wednesday, February 19, 2020 7:03 PM
>> To: He, Jacob <Jacob.He at amd.com <mailto:Jacob.He at amd.com>>;
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at 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 <jacob.he at amd.com <mailto:jacob.he at amd.com>>
>> > ---
>> > 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 {
>> > bool vm_needs_flush;
>> > uint64_t vm_pd_addr;
>> > unsigned vmid;
>> > + bool need_update_spm_vmid;
>> > unsigned pasid;
>> > uint32_t gds_base, gds_size;
>> > uint32_t gws_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 gfx_v10_0_rlc_funcs = {
>> > .is_rlc_enabled = gfx_v10_0_is_rlc_enabled,
>> > .set_safe_mode = gfx_v10_0_set_safe_mode, @@ -4224,7 +4236,8 @@
>> > static const struct amdgpu_rlc_funcs gfx_v10_0_rlc_funcs = {
>> > .resume = gfx_v10_0_rlc_resume,
>> > .stop = gfx_v10_0_rlc_stop,
>> > .reset = gfx_v10_0_rlc_reset,
>> > - .start = gfx_v10_0_rlc_start
>> > + .start = gfx_v10_0_rlc_start,
>> > + .update_spm_vmid = gfx_v10_0_update_spm_vmid
>> > };
>> >
>> > static int gfx_v10_0_set_powergating_state(void *handle, diff --git
>> > a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> > index 8f20a5dd44fe..b24fc55cf13a 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> > @@ -4221,7 +4221,8 @@ static const struct amdgpu_rlc_funcs
>> gfx_v7_0_rlc_funcs = {
>> > .resume = gfx_v7_0_rlc_resume,
>> > .stop = gfx_v7_0_rlc_stop,
>> > .reset = gfx_v7_0_rlc_reset,
>> > - .start = gfx_v7_0_rlc_start
>> > + .start = gfx_v7_0_rlc_start,
>> > + .update_spm_vmid = NULL
>> > };
>> >
>> > static int gfx_v7_0_early_init(void *handle) diff --git
>> > a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> > index fa245973de12..66640d2b6b37 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> > @@ -5600,7 +5600,8 @@ static const struct amdgpu_rlc_funcs
>> iceland_rlc_funcs = {
>> > .resume = gfx_v8_0_rlc_resume,
>> > .stop = gfx_v8_0_rlc_stop,
>> > .reset = gfx_v8_0_rlc_reset,
>> > - .start = gfx_v8_0_rlc_start
>> > + .start = gfx_v8_0_rlc_start,
>> > + .update_spm_vmid = NULL
>> > };
>> >
>> > static void gfx_v8_0_update_medium_grain_clock_gating(struct
>> > amdgpu_device *adev, diff --git
>> > a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> > index 9b7ff783e9a5..df872f949f68 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> > @@ -4704,6 +4704,18 @@ static int
>> gfx_v9_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>> > return 0;
>> > }
>> >
>> > +static void gfx_v9_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 gfx_v9_0_rlc_funcs = {
>> > .is_rlc_enabled = gfx_v9_0_is_rlc_enabled,
>> > .set_safe_mode = gfx_v9_0_set_safe_mode, @@ -4715,7 +4727,8 @@
>> > static const struct amdgpu_rlc_funcs gfx_v9_0_rlc_funcs = {
>> > .resume = gfx_v9_0_rlc_resume,
>> > .stop = gfx_v9_0_rlc_stop,
>> > .reset = gfx_v9_0_rlc_reset,
>> > - .start = gfx_v9_0_rlc_start
>> > + .start = gfx_v9_0_rlc_start,
>> > + .update_spm_vmid = gfx_v9_0_update_spm_vmid
>> > };
>> >
>> > static int gfx_v9_0_set_powergating_state(void *handle,
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cdavid1.zhou%40amd.com%7C354be34ff18e4f424f6708d7b52b43b0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637177069753914395&sdata=9rSL4kgPJweuZ4EJpdqtqTxyCVGEkmsg6aUzbtvGFrs%3D&reserved=0
>> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Calexander.deucher%40amd.com%7Ce68811adba0647c651a308d7b6b8d4c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637178777321057824&sdata=uRP0wQyfVKk0QoGlWM36nbeOY67nIjfD6vou2A%2BybEc%3D&reserved=0>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Calexander.deucher%40amd.com%7Ce68811adba0647c651a308d7b6b8d4c5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637178777321067823&sdata=7o2j5dTvK%2B5Gihwd5IB%2Blp1Bi0ZOx%2B%2F%2Bmi8wpRh4NBs%3D&reserved=0>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200221/a19c949e/attachment-0001.htm>
More information about the amd-gfx
mailing list