From: "Sharma, Shashank" <shashank.sharma@amd.com>
To: "Lazar, Lijo" <lijo.lazar@amd.com>, amd-gfx@lists.freedesktop.org
Cc: alexander.deucher@amd.com, amaranath.somalapuram@amd.com,
christian.koenig@amd.com
Subject: Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
Date: Tue, 27 Sep 2022 14:25:38 +0200 [thread overview]
Message-ID: <92ae3eb3-6a9b-6ad3-7970-dad0b82523fd@amd.com> (raw)
In-Reply-To: <60d98c36-165d-8e59-d110-a4f2ea2977f5@amd.com>
On 9/27/2022 2:20 PM, Lazar, Lijo wrote:
>
>
> On 9/27/2022 5:17 PM, Sharma, Shashank wrote:
>>
>>
>> On 9/27/2022 12:03 PM, Lazar, Lijo wrote:
>>>
>>>
>>> On 9/27/2022 3:10 AM, Shashank Sharma wrote:
>>>> This patch and switches the GPU workload based profile based
>>>> on the workload hint information saved in the workload context.
>>>> The workload profile is reset to NONE when the job is done.
>>>>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c | 4 ----
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 15 +++++++++++++++
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 3 +++
>>>> 4 files changed, 20 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> index b7bae833c804..de906a42144f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> @@ -237,6 +237,8 @@ static int amdgpu_cs_parser_init(struct
>>>> amdgpu_cs_parser *p, union drm_amdgpu_cs
>>>> goto free_all_kdata;
>>>> }
>>>> + p->job->workload_mode = p->ctx->workload_mode;
>>>> +
>>>> if (p->uf_entry.tv.bo)
>>>> p->job->uf_addr = uf_offset;
>>>> kvfree(chunk_array);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
>>>> index a11cf29bc388..625114804121 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx_workload.c
>>>> @@ -55,15 +55,11 @@ int amdgpu_set_workload_profile(struct
>>>> amdgpu_device *adev,
>>>> mutex_lock(&adev->pm.smu_workload_lock);
>>>> - if (adev->pm.workload_mode == hint)
>>>> - goto unlock;
>>>> -
>>>
>>> What is the expectation when a GFX job + VCN job together (or in
>>> general two jobs running in separate schedulers) and each prefers a
>>> different workload type? FW will switch as requested.
>>
>> Well, I guess the last switched mode will take over. Do note that like
>> most of the PM features, the real benefit of power profiles can be
>> seen with consistant and similar workloads running for some time (Like
>> gaming, video playback etc).
>>
>
> Yes, so the extra protection layer wrapping around this is really not
> helping (user doesn't know if the job is really run in the requested
> mode). I would suggest to avoid that and document the usage of this API
> as exclusive mode usage for some profiling use cases.
>
As I mentioned in the other comment, this extra protection is not for
not allowing it to change the mode, but from preventing PM reset from
job_cleanup thread, while another work is in progress.
- Shashank
> Thanks,
> Lijo
>
>> - Shashank
>>
>>>
>>> Thanks,
>>> Lijo
>>>
>>>> ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
>>>> if (!ret)
>>>> adev->pm.workload_mode = hint;
>>>> atomic_inc(&adev->pm.workload_switch_ref);
>>>> -unlock:
>>>> mutex_unlock(&adev->pm.smu_workload_lock);
>>>> return ret;
>>>> }
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> index c2fd6f3076a6..9300e86ee7c5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> @@ -30,6 +30,7 @@
>>>> #include "amdgpu.h"
>>>> #include "amdgpu_trace.h"
>>>> #include "amdgpu_reset.h"
>>>> +#include "amdgpu_ctx_workload.h"
>>>> static enum drm_gpu_sched_stat amdgpu_job_timedout(struct
>>>> drm_sched_job *s_job)
>>>> {
>>>> @@ -144,6 +145,14 @@ void amdgpu_job_free_resources(struct
>>>> amdgpu_job *job)
>>>> static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
>>>> {
>>>> struct amdgpu_job *job = to_amdgpu_job(s_job);
>>>> + struct amdgpu_ring *ring = to_amdgpu_ring(s_job->sched);
>>>> +
>>>> + if (job->workload_mode != AMDGPU_CTX_WORKLOAD_HINT_NONE) {
>>>> + if (amdgpu_clear_workload_profile(ring->adev,
>>>> job->workload_mode))
>>>> + DRM_WARN("Failed to come out of workload profile %s\n",
>>>> + amdgpu_workload_profile_name(job->workload_mode));
>>>> + job->workload_mode = AMDGPU_CTX_WORKLOAD_HINT_NONE;
>>>> + }
>>>> drm_sched_job_cleanup(s_job);
>>>> @@ -256,6 +265,12 @@ static struct dma_fence *amdgpu_job_run(struct
>>>> drm_sched_job *sched_job)
>>>> DRM_ERROR("Error scheduling IBs (%d)\n", r);
>>>> }
>>>> + if (job->workload_mode != AMDGPU_CTX_WORKLOAD_HINT_NONE) {
>>>> + if (amdgpu_set_workload_profile(ring->adev,
>>>> job->workload_mode))
>>>> + DRM_WARN("Failed to set workload profile to %s\n",
>>>> + amdgpu_workload_profile_name(job->workload_mode));
>>>> + }
>>>> +
>>>> job->job_run_counter++;
>>>> amdgpu_job_free_resources(job);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> index babc0af751c2..573e8692c814 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> @@ -68,6 +68,9 @@ struct amdgpu_job {
>>>> /* job_run_counter >= 1 means a resubmit job */
>>>> uint32_t job_run_counter;
>>>> + /* workload mode hint for pm */
>>>> + uint32_t workload_mode;
>>>> +
>>>> uint32_t num_ibs;
>>>> struct amdgpu_ib ibs[];
>>>> };
>>>>
next prev parent reply other threads:[~2022-09-27 12:25 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 21:40 [PATCH v3 0/5] GPU workload hints for better performance Shashank Sharma
2022-09-26 21:40 ` [PATCH v3 1/5] drm/amdgpu: add UAPI for workload hints to ctx ioctl Shashank Sharma
2022-09-27 6:07 ` Christian König
2022-09-27 14:28 ` Felix Kuehling
2023-03-21 3:05 ` Marek Olšák
2023-03-21 13:00 ` Sharma, Shashank
2023-03-21 13:54 ` Christian König
2023-03-22 14:05 ` Marek Olšák
2023-03-22 14:08 ` Christian König
2023-03-22 14:24 ` Marek Olšák
2023-03-22 14:29 ` Christian König
2023-03-22 14:36 ` Marek Olšák
2023-03-22 14:52 ` Alex Deucher
2023-03-22 15:11 ` Marek Olšák
2023-03-22 14:38 ` Sharma, Shashank
2022-09-26 21:40 ` [PATCH v3 2/5] drm/amdgpu: add new functions to set GPU power profile Shashank Sharma
2022-09-27 2:14 ` Quan, Evan
2022-09-27 7:29 ` Sharma, Shashank
2022-09-27 9:29 ` Quan, Evan
2022-09-27 10:00 ` Sharma, Shashank
2022-09-27 6:08 ` Christian König
2022-09-27 9:58 ` Lazar, Lijo
2022-09-27 11:41 ` Sharma, Shashank
2022-09-27 12:10 ` Lazar, Lijo
2022-09-27 12:23 ` Sharma, Shashank
2022-09-27 12:39 ` Lazar, Lijo
2022-09-27 12:53 ` Sharma, Shashank
2022-09-27 13:29 ` Lazar, Lijo
2022-09-27 13:47 ` Sharma, Shashank
2022-09-27 14:00 ` Lazar, Lijo
2022-09-27 14:20 ` Sharma, Shashank
2022-09-27 14:34 ` Lazar, Lijo
2022-09-27 14:50 ` Sharma, Shashank
2022-09-27 15:20 ` Felix Kuehling
2022-09-26 21:40 ` [PATCH v3 3/5] drm/amdgpu: set GPU workload via ctx IOCTL Shashank Sharma
2022-09-27 6:09 ` Christian König
2022-09-26 21:40 ` [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile Shashank Sharma
2022-09-27 6:11 ` Christian König
2022-09-27 10:03 ` Lazar, Lijo
2022-09-27 11:47 ` Sharma, Shashank
2022-09-27 12:20 ` Lazar, Lijo
2022-09-27 12:25 ` Sharma, Shashank [this message]
2022-09-27 16:33 ` Michel Dänzer
2022-09-27 17:06 ` Sharma, Shashank
2022-09-27 17:29 ` Michel Dänzer
2022-09-26 21:40 ` [PATCH v3 5/5] drm/amdgpu: switch workload context to/from compute Shashank Sharma
2022-09-27 6:12 ` Christian König
2022-09-27 14:48 ` Felix Kuehling
2022-09-27 14:58 ` Sharma, Shashank
2022-09-27 15:23 ` Felix Kuehling
2022-09-27 15:38 ` Sharma, Shashank
2022-09-27 20:40 ` Alex Deucher
2022-09-28 7:05 ` Lazar, Lijo
2022-09-28 8:56 ` Sharma, Shashank
2022-09-28 9:00 ` Sharma, Shashank
2022-09-28 21:51 ` Alex Deucher
2022-09-29 8:48 ` Sharma, Shashank
2022-09-29 11:10 ` Lazar, Lijo
2022-09-29 13:20 ` Sharma, Shashank
2022-09-29 13:37 ` Lazar, Lijo
2022-09-29 14:00 ` Sharma, Shashank
2022-09-29 14:14 ` Lazar, Lijo
2022-09-29 14:40 ` Sharma, Shashank
2022-09-29 18:32 ` Alex Deucher
2022-09-30 5:08 ` Lazar, Lijo
2022-09-30 8:37 ` Sharma, Shashank
2022-09-30 9:13 ` Lazar, Lijo
2022-09-30 9:22 ` Sharma, Shashank
2022-09-30 9:54 ` Lazar, Lijo
2022-09-30 10:09 ` Sharma, Shashank
2022-09-29 18:07 ` Felix Kuehling
2022-09-30 4:46 ` Lazar, Lijo
2022-09-27 16:24 ` [PATCH v3 0/5] GPU workload hints for better performance Michel Dänzer
2022-09-27 16:59 ` Sharma, Shashank
2022-09-27 17:13 ` Michel Dänzer
2022-09-27 17:25 ` Sharma, Shashank
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=92ae3eb3-6a9b-6ad3-7970-dad0b82523fd@amd.com \
--to=shashank.sharma@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amaranath.somalapuram@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=lijo.lazar@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).