Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
On 2022-09-27 19:06, Sharma, Shashank wrote: > On 9/27/2022 6:33 PM, Michel Dänzer wrote: >> On 2022-09-27 13:47, 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 > Signed-off-by: Shashank Sharma > --- > 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(>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). >> >> Not sure how that's supposed to work on a general purpose system, where >> there are always expected to be multiple processes (one of which being the >> display server) using the GPU for different workloads. >> >> Even in special cases there may be multiple different kinds of workloads >> constantly being used at the same time, e.g. a fullscreen game with live >> streaming / recording using VCN. >> > It looks like we can accommodate that now, see the recent discussion with > Felix in the patch 5, where we see that "amdgpu_dpm_switch_power_profile > enables and disables individual profiles, Disabling the 3D profile doesn't > disable the compute profile at the same time" > > So I think we won't be overwriting but would be enabling/disabling individual > profiles for compute/3D/MM etc. Of course I will have to update the patch > series accordingly. > >> >> Have you guys considered letting the display server (DRM master) choose the >> profile instead? >> > This seems to be very good input, in case of a further conflict in decision > making, we might > > as well add this intelligence in DRM master. Would you mind explaining this a > bit more on how do you think it should be done ? I don't have any specific ideas offhand; it was just an idea that happened to come to my mind, not sure it's a good one at all. Anyway, I think one important thing is that the same circumstances consistently result in the same profile being chosen. If it depends on luck / timing, that's a troubleshooting nightmare. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
On 9/27/2022 6:33 PM, Michel Dänzer wrote: On 2022-09-27 13:47, 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 Signed-off-by: Shashank Sharma --- 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(>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). Not sure how that's supposed to work on a general purpose system, where there are always expected to be multiple processes (one of which being the display server) using the GPU for different workloads. Even in special cases there may be multiple different kinds of workloads constantly being used at the same time, e.g. a fullscreen game with live streaming / recording using VCN. It looks like we can accommodate that now, see the recent discussion with Felix in the patch 5, where we see that "amdgpu_dpm_switch_power_profile enables and disables individual profiles, Disabling the 3D profile doesn't disable the compute profile at the same time" So I think we won't be overwriting but would be enabling/disabling individual profiles for compute/3D/MM etc. Of course I will have to update the patch series accordingly. Have you guys considered letting the display server (DRM master) choose the profile instead? This seems to be very good input, in case of a further conflict in decision making, we might as well add this intelligence in DRM master. Would you mind explaining this a bit more on how do you think it should be done ? - Shashank
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
On 2022-09-27 13:47, 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 >>> Signed-off-by: Shashank Sharma >>> --- >>> 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(>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). Not sure how that's supposed to work on a general purpose system, where there are always expected to be multiple processes (one of which being the display server) using the GPU for different workloads. Even in special cases there may be multiple different kinds of workloads constantly being used at the same time, e.g. a fullscreen game with live streaming / recording using VCN. Have you guys considered letting the display server (DRM master) choose the profile instead? -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
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
Signed-off-by: Shashank Sharma
---
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(>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(>pm.workload_switch_ref);
-unlock:
mutex_unlock(>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;
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
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
Signed-off-by: Shashank Sharma
---
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(>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.
Thanks,
Lijo
- Shashank
Thanks,
Lijo
ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
if (!ret)
adev->pm.workload_mode = hint;
atomic_inc(>pm.workload_switch_ref);
-unlock:
mutex_unlock(>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[];
};
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
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
Signed-off-by: Shashank Sharma
---
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(>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).
- Shashank
Thanks,
Lijo
ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
if (!ret)
adev->pm.workload_mode = hint;
atomic_inc(>pm.workload_switch_ref);
-unlock:
mutex_unlock(>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[];
};
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
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
Signed-off-by: Shashank Sharma
---
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(>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.
Thanks,
Lijo
ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
if (!ret)
adev->pm.workload_mode = hint;
atomic_inc(>pm.workload_switch_ref);
-unlock:
mutex_unlock(>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_tjob_run_counter;
+ /* workload mode hint for pm */
+ uint32_tworkload_mode;
+
uint32_tnum_ibs;
struct amdgpu_ibibs[];
};
Re: [PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
Am 26.09.22 um 23:40 schrieb Shashank Sharma:
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
Signed-off-by: Shashank Sharma
Reviewed-by: Christian König
---
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(>pm.smu_workload_lock);
- if (adev->pm.workload_mode == hint)
- goto unlock;
-
ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
if (!ret)
adev->pm.workload_mode = hint;
atomic_inc(>pm.workload_switch_ref);
-unlock:
mutex_unlock(>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_tjob_run_counter;
+ /* workload mode hint for pm */
+ uint32_tworkload_mode;
+
uint32_tnum_ibs;
struct amdgpu_ibibs[];
};
[PATCH v3 4/5] drm/amdgpu: switch GPU workload profile
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
Signed-off-by: Shashank Sharma
---
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(>pm.smu_workload_lock);
- if (adev->pm.workload_mode == hint)
- goto unlock;
-
ret = amdgpu_dpm_switch_power_profile(adev, profile, 1);
if (!ret)
adev->pm.workload_mode = hint;
atomic_inc(>pm.workload_switch_ref);
-unlock:
mutex_unlock(>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_tjob_run_counter;
+ /* workload mode hint for pm */
+ uint32_tworkload_mode;
+
uint32_tnum_ibs;
struct amdgpu_ibibs[];
};
--
2.34.1
