[PATCH 3/7] drm/amd/pm: drop unneeded smu->metrics_lock
Lazar, Lijo
lijo.lazar at amd.com
Mon Jan 10 07:03:15 UTC 2022
On 1/7/2022 7:49 AM, Quan, Evan wrote:
> [AMD Official Use Only]
>
>
>
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Thursday, January 6, 2022 4:29 PM
>> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
>> Subject: Re: [PATCH 3/7] drm/amd/pm: drop unneeded smu->metrics_lock
>>
>>
>>
>> On 1/6/2022 12:32 PM, Quan, Evan wrote:
>>> [AMD Official Use Only]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>>>> Sent: Thursday, January 6, 2022 2:17 PM
>>>> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
>>>> Subject: Re: [PATCH 3/7] drm/amd/pm: drop unneeded smu-
>>> metrics_lock
>>>>
>>>>
>>>>
>>>> On 1/6/2022 11:27 AM, Evan Quan wrote:
>>>>> As all those related APIs are already well protected by
>>>>> adev->pm.mutex and smu->message_lock.
>>>>>
>>>>
>>>> This one may be widely used. Instead of relying on pm.mutex it's
>>>> better to keep metrics lock so that multiple clients can read data
>>>> without waiting on other APIs that use pm.mutex.
>>> [Quan, Evan] If I understand it correctly, what you wanted to express is to
>> use fine-grained lock instead of cross-grained one to avoid chasing for the
>> same lock.
>>> Yes, that was what we did before and that's why we have so many types of
>> locks. Below are my considerations for this:
>>> 1. We actually do not have such issue that many APIs/clients chasing for the
>> same lock. Thus fine-grained locks cannot bring much benefits.
>>> Take the metrics_lock here for example. The data protected by
>> metrics_lock are for those pm sysfs interfaces. Those sysfs interface are not
>> so frequently called. And almost all the time, they are called one after one.
>> So, it's rarely they will chase for the same lock.
>>>
>>
>> It's not just sysfs, there are other interfaces like sensors, hwmons etc.
>> Basically, metrics table provides data like GFX activity or throttler status that
>> may be continuously monitored by app layer. So other APIs could suffer. My
>> thought is to just keep metrics under a separate lock and not tie with
>> pm.mutex.
> [Quan, Evan] I doubt about the guess that "other APIs could suffer if metrics data is continuously polled by app layer".
> Since according to my previous test(watch -n 0.1 "cat /sys/kernel/debug/dri/0/amdgpu_pm_info") which polled the metrics data every 0.1 second, the smu busy percent was almost not affected.
May be try close to real cases like running some videos/changing
resolution and run radeon profile or some monitoring app in the background?
> So, even the metric data is polled every second by app layer, that should not enforce much loading on the CPUs and SMU cores.
>
> Also, keeping a separate lock for metrics data does not help much with the issue mentioned here(even if there is such).
> As they(app layer and other APIs) will still chase for another lock(message_lock which is kept) for interaction with SMU.
>
Yes, only the subsequent steps related to reading table data from VRAM.
Other APIs don't need to wait for this. Only thinking about profiling
cases, may be I'm overthinking.
Thanks,
Lijo
> BR
> Evan
>>
>> Thanks,
>> Lijo
>>
>>> 2. Cross-grained lock can simplify our implementations. It's hard to believe,
>> there is 10+(actually 13 as I counted) different types of locks used in our
>> existing power code.
>>> By the cross-grained lock, we can simplify the code and protect us from
>> some unintentional dead-locks(I actually run into that several times and it's
>> really tricky).
>>>
>>> BR
>>> Evan
>>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>> Signed-off-by: Evan Quan <evan.quan at amd.com>
>>>>> Change-Id: Ic75326ba7b4b67be8762d5407d02f6c514e1ad35
>>>>> ---
>>>>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 1 -
>>>>> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 1 -
>>>>> .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 14 +--
>>>>> .../amd/pm/swsmu/smu11/cyan_skillfish_ppt.c | 10 +-
>>>>> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 112 +++++--------
>> ----
>>>> -
>>>>> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 27 ++---
>>>>> .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 28 ++---
>>>>> .../gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 14 +--
>>>>> .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 23 ++--
>>>>> .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 10 +-
>>>>> drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 21 +---
>>>>> drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h | 4 -
>>>>> 12 files changed, 70 insertions(+), 195 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> index ecbc768dfe2f..f0136bf36533 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> @@ -956,7 +956,6 @@ static int smu_sw_init(void *handle)
>>>>> bitmap_zero(smu->smu_feature.allowed, SMU_FEATURE_MAX);
>>>>>
>>>>> mutex_init(&smu->sensor_lock);
>>>>> - mutex_init(&smu->metrics_lock);
>>>>> mutex_init(&smu->message_lock);
>>>>>
>>>>> INIT_WORK(&smu->throttling_logging_work,
>>>>> smu_throttling_logging_work_fn); diff --git
>>>>> a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
>>>>> index c3efe4fea5e0..63ed807c96f5 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
>>>>> @@ -487,7 +487,6 @@ struct smu_context
>>>>> const struct cmn2asic_mapping *pwr_src_map;
>>>>> const struct cmn2asic_mapping *workload_map;
>>>>> struct mutex sensor_lock;
>>>>> - struct mutex metrics_lock;
>>>>> struct mutex message_lock;
>>>>> uint64_t pool_size;
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>>>> index addb0472d040..3f7c1f23475b 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>>>> @@ -602,15 +602,11 @@ static int
>>>>> arcturus_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table-
>>>>> metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -693,8 +689,6 @@ static int arcturus_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> diff --git
>> a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
>>>>> index 2238ee19c222..7ae6b1bd648a 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/cyan_skillfish_ppt.c
>>>>> @@ -150,13 +150,9 @@ cyan_skillfish_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table-
>>>>> metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu, NULL, false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu, NULL, false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -200,8 +196,6 @@ cyan_skillfish_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>>>> index fe17b3c1ece7..fdb059e7c6ba 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>>>> @@ -546,15 +546,11 @@ static int
>>>> navi10_get_legacy_smu_metrics_data(struct smu_context *smu,
>>>>> (SmuMetrics_legacy_t *)smu_table->metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -624,8 +620,6 @@ static int
>>>> navi10_get_legacy_smu_metrics_data(struct smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -638,15 +632,11 @@ static int navi10_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> (SmuMetrics_t *)smu_table->metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -719,8 +709,6 @@ static int navi10_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -733,15 +721,11 @@ static int
>>>> navi12_get_legacy_smu_metrics_data(struct smu_context *smu,
>>>>> (SmuMetrics_NV12_legacy_t *)smu_table->metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -811,8 +795,6 @@ static int
>>>> navi12_get_legacy_smu_metrics_data(struct smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -825,15 +807,11 @@ static int navi12_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> (SmuMetrics_NV12_t *)smu_table->metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -906,8 +884,6 @@ static int navi12_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -2708,20 +2684,14 @@ static ssize_t
>>>> navi10_get_legacy_gpu_metrics(struct smu_context *smu,
>>>>> SmuMetrics_legacy_t metrics;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - true);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + true);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> memcpy(&metrics, smu_table->metrics_table,
>>>>> sizeof(SmuMetrics_legacy_t));
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>>>>>
>>>>> gpu_metrics->temperature_edge = metrics.TemperatureEdge; @@
>>>>> -2899,20 +2869,14 @@ static ssize_t navi10_get_gpu_metrics(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_t metrics;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - true);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + true);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> memcpy(&metrics, smu_table->metrics_table,
>>>> sizeof(SmuMetrics_t));
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>>>>>
>>>>> gpu_metrics->temperature_edge = metrics.TemperatureEdge; @@
>>>>> -2977,20 +2941,14 @@ static ssize_t
>>>>> navi12_get_legacy_gpu_metrics(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_NV12_legacy_t metrics;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - true);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + true);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> memcpy(&metrics, smu_table->metrics_table,
>>>>> sizeof(SmuMetrics_NV12_legacy_t));
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>>>>>
>>>>> gpu_metrics->temperature_edge = metrics.TemperatureEdge; @@
>>>>> -3058,20 +3016,14 @@ static ssize_t navi12_get_gpu_metrics(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_NV12_t metrics;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - true);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + true);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> memcpy(&metrics, smu_table->metrics_table,
>>>>> sizeof(SmuMetrics_NV12_t));
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>>>>>
>>>>> gpu_metrics->temperature_edge = metrics.TemperatureEdge; diff --
>>>> git
>>>>> a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>>> index 93caaf45a2db..2241250c2d2a 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>>>> @@ -525,15 +525,11 @@ static int
>>>> sienna_cichlid_get_smu_metrics_data(struct smu_context *smu,
>>>>> uint16_t average_gfx_activity;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -633,8 +629,6 @@ static int
>>>> sienna_cichlid_get_smu_metrics_data(struct smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>>
>>>>> }
>>>>> @@ -3564,14 +3558,11 @@ static ssize_t
>>>> sienna_cichlid_get_gpu_metrics(struct smu_context *smu,
>>>>> uint16_t average_gfx_activity;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - &metrics_external,
>>>>> - true);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + &metrics_external,
>>>>> + true);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> smu_cmn_init_soft_gpu_metrics(gpu_metrics, 1, 3);
>>>>>
>>>>> @@ -3661,8 +3652,6 @@ static ssize_t
>>>> sienna_cichlid_get_gpu_metrics(struct smu_context *smu,
>>>>>
>>>> smu_v11_0_get_current_pcie_link_speed(smu);
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>>>>>
>>>>> *table = (void *)gpu_metrics;
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> index 5cb07ed227fb..c736adca6fbb 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c
>>>>> @@ -273,15 +273,11 @@ static int
>>>> vangogh_get_legacy_smu_metrics_data(struct smu_context *smu,
>>>>> SmuMetrics_legacy_t *metrics = (SmuMetrics_legacy_t
>>>> *)smu_table->metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -335,8 +331,6 @@ static int
>>>> vangogh_get_legacy_smu_metrics_data(struct smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -348,15 +342,11 @@ static int
>> vangogh_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table-
>>>>> metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -410,8 +400,6 @@ static int vangogh_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>>>>> index 25c4b135f830..d75508085578 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c
>>>>> @@ -1128,15 +1128,11 @@ static int
>>>>> renoir_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table-
>>>>> metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_AVERAGE_GFXCLK:
>>>>> @@ -1201,8 +1197,6 @@ static int renoir_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
>>>>> index f065d95b117a..014fb88daa04 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
>>>>> @@ -571,15 +571,11 @@ static int
>>>> aldebaran_get_smu_metrics_data(struct smu_context *smu,
>>>>> SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table-
>>>>> metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - NULL,
>>>>> - false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu,
>>>>> + NULL,
>>>>> + false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_CURR_GFXCLK:
>>>>> @@ -653,8 +649,6 @@ static int
>> aldebaran_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -1592,17 +1586,14 @@ static void aldebaran_get_unique_id(struct
>>>> smu_context *smu)
>>>>> uint32_t upper32 = 0, lower32 = 0;
>>>>> int ret;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu, NULL, false);
>>>>> + ret = smu_cmn_get_metrics_table(smu, NULL, false);
>>>>> if (ret)
>>>>> - goto out_unlock;
>>>>> + goto out;
>>>>>
>>>>> upper32 = metrics->PublicSerialNumUpper32;
>>>>> lower32 = metrics->PublicSerialNumLower32;
>>>>>
>>>>> -out_unlock:
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> +out:
>>>>> adev->unique_id = ((uint64_t)upper32 << 32) | lower32;
>>>>> if (adev->serial[0] == '\0')
>>>>> sprintf(adev->serial, "%016llx", adev->unique_id); diff --git
>>>>> a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>>>>> index caf1775d48ef..451d30dcc639 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c
>>>>> @@ -310,13 +310,9 @@ static int
>>>> yellow_carp_get_smu_metrics_data(struct smu_context *smu,
>>>>> SmuMetrics_t *metrics = (SmuMetrics_t *)smu_table-
>>>>> metrics_table;
>>>>> int ret = 0;
>>>>>
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> -
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu, NULL, false);
>>>>> - if (ret) {
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> + ret = smu_cmn_get_metrics_table(smu, NULL, false);
>>>>> + if (ret)
>>>>> return ret;
>>>>> - }
>>>>>
>>>>> switch (member) {
>>>>> case METRICS_AVERAGE_GFXCLK:
>>>>> @@ -387,8 +383,6 @@ static int
>>>>> yellow_carp_get_smu_metrics_data(struct
>>>> smu_context *smu,
>>>>> break;
>>>>> }
>>>>>
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> return ret;
>>>>> }
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>>>>> index 735e1a1e365d..d78e4f689a2a 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
>>>>> @@ -964,9 +964,9 @@ int smu_cmn_write_pptable(struct smu_context
>>>> *smu)
>>>>> true);
>>>>> }
>>>>>
>>>>> -int smu_cmn_get_metrics_table_locked(struct smu_context *smu,
>>>>> - void *metrics_table,
>>>>> - bool bypass_cache)
>>>>> +int smu_cmn_get_metrics_table(struct smu_context *smu,
>>>>> + void *metrics_table,
>>>>> + bool bypass_cache)
>>>>> {
>>>>> struct smu_table_context *smu_table= &smu->smu_table;
>>>>> uint32_t table_size =
>>>>> @@ -994,21 +994,6 @@ int smu_cmn_get_metrics_table_locked(struct
>>>> smu_context *smu,
>>>>> return 0;
>>>>> }
>>>>>
>>>>> -int smu_cmn_get_metrics_table(struct smu_context *smu,
>>>>> - void *metrics_table,
>>>>> - bool bypass_cache)
>>>>> -{
>>>>> - int ret = 0;
>>>>> -
>>>>> - mutex_lock(&smu->metrics_lock);
>>>>> - ret = smu_cmn_get_metrics_table_locked(smu,
>>>>> - metrics_table,
>>>>> - bypass_cache);
>>>>> - mutex_unlock(&smu->metrics_lock);
>>>>> -
>>>>> - return ret;
>>>>> -}
>>>>> -
>>>>> void smu_cmn_init_soft_gpu_metrics(void *table, uint8_t frev,
>>>>> uint8_t
>>>> crev)
>>>>> {
>>>>> struct metrics_table_header *header = (struct
>>>>> metrics_table_header *)table; diff --git
>>>>> a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
>>>>> index 67a25da79256..f0b4fb2a0960 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
>>>>> @@ -101,10 +101,6 @@ int smu_cmn_write_watermarks_table(struct
>>>>> smu_context *smu);
>>>>>
>>>>> int smu_cmn_write_pptable(struct smu_context *smu);
>>>>>
>>>>> -int smu_cmn_get_metrics_table_locked(struct smu_context *smu,
>>>>> - void *metrics_table,
>>>>> - bool bypass_cache);
>>>>> -
>>>>> int smu_cmn_get_metrics_table(struct smu_context *smu,
>>>>> void *metrics_table,
>>>>> bool bypass_cache);
>>>>>
More information about the amd-gfx
mailing list