RE: [PATCH] drm/amdgpu/powerplay:avoid to show invalid DPM table info
[AMD Official Use Only - Internal Distribution Only]
On Fri, Apr 17, 2020 at 10:58:59AM +0800, Yuxian Dai wrote:
> for different ASIC support different the number of DPM levels, we
> should avoid to show the invalid level value.
> v1 -> v2:
> follow the suggestion,clarifiy the description for this change
> Signed-off-by: Yuxian Dai
> Change-Id: I579ef417ddc8acb4a6cf15c60094743a72d9b050
Reviewed-by: Huang Rui
git format-patch --subject-prefix="PATCH v2" HEAD~
It's to indicate v2 patch in the subject.
Thanks,
Ray
> got it. Thank for your help
> ---
> drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> index 281b7b6cf1a4..e4e7a352d032 100644
> --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> @@ -296,6 +296,8 @@ static int renoir_print_clk_levels(struct
> smu_context *smu,
>
> for (i = 0; i < count; i++) {
> GET_DPM_CUR_FREQ(clk_table, clk_type, i, value);
> + if (!value)
> + continue;
> size += sprintf(buf + size, "%d: %uMhz %s\n", i, value,
> cur_value == value ? "*" : "");
> if (cur_value == value)
> --
> 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-gfx&data=02%7C01%7Cra
> y.huang%40amd.com%7C1b5650c171874272ba8208d7e27b538d%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C637226891622403483&sdata=kUI7EzW5SgOyf
> DN57C1tZtFngwmXWTK5sNCAwJqmC3k%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu/powerplay:avoid to show invalid DPM table info
[AMD Official Use Only - Internal Distribution Only]
On Wed, Apr 15, 2020 at 07:20:31PM +0800, Yuxian Dai wrote:
> we should avoid to show the invalid level value when the DPM_LEVELS
> supported number changed
>
> Signed-off-by: Yuxian Dai
> Change-Id: Ib66d0cf34a866fa6f0cedd1d5fc642f59236787d
Please add comment in the commit message to explain some asic only have 2
level, and cause the invalid level as 0.
With that fixed, patch is
>ok
Reviewed-by: Huang Rui
> ---
> drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> index 281b7b6cf1a4..e4e7a352d032 100644
> --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> @@ -296,6 +296,8 @@ static int renoir_print_clk_levels(struct
> smu_context *smu,
>
> for (i = 0; i < count; i++) {
> GET_DPM_CUR_FREQ(clk_table, clk_type, i, value);
> + if (!value)
> + continue;
> size += sprintf(buf + size, "%d: %uMhz %s\n", i, value,
> cur_value == value ? "*" : "");
> if (cur_value == value)
> --
> 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-gfx&data=02%7C01%7Cra
> y.huang%40amd.com%7C4858c507c09943274af508d7e12f15a1%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C637225464660212899&sdata=eNcYS0dWUHPpQ
> QXPHD6M5%2BqRI8dj1OC2auB0ZQB9Rgs%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: unify fw_write_wait for new gfx9 asics
[AMD Official Use Only - Internal Distribution Only] -Original Message- From: Liu, Aaron Sent: Wednesday, April 8, 2020 2:46 PM To: amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Huang, Ray ; Dai, Yuxian (David) ; Liu, Aaron Subject: [PATCH] drm/amdgpu: unify fw_write_wait for new gfx9 asics Make the fw_write_wait default case true since presumably all new gfx9 asics will have updated firmware. That is using unique WAIT_REG_MEM packet with opration=1. Signed-off-by: Aaron Liu --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 2d56b06722bc..1d18447129b1 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -1214,6 +1214,8 @@ static void gfx_v9_0_check_fw_write_wait(struct amdgpu_device *adev) adev->gfx.mec_fw_write_wait = true; break; default: + adev->gfx.me_fw_write_wait = true; + adev->gfx.mec_fw_write_wait = true; break; } } [David]: - >much better solution -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu/powerplay: using the FCLK DPM table to set the MCLK
[AMD Official Use Only - Internal Distribution Only] On Wed, Apr 01, 2020 at 07:41:12PM +0800, Yuxian Dai wrote: > 1.Using the FCLK DPM table to set the MCLK for DPM states consist of > three entities: > FCLK > UCLK > MEMCLK > All these three clk change together, MEMCLK from FCLK, so use the fclk > frequency. > 2.we should show the current working clock freqency from clock table > metric > > Signed-off-by: Yuxian Dai > Reviewed-by: Alex Deucher > Reviewed-by: Huang Rui > Reviewed-by: Kevin Wang > --- Next time, if you submit the V2 patch, you can generate it as below command: git format-patch --subject-prefix="PATCH v2" And describe the changes from v1 -> v2 in the commit message. This will help everyone to understand your change. > I got it. Reviewed-by: Huang Rui > drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 6 ++ > drivers/gpu/drm/amd/powerplay/renoir_ppt.h | 2 +- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c > b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c > index 7bf52ecba01d..c6b39a7026a8 100644 > --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c > +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c > @@ -239,6 +239,7 @@ static int renoir_print_clk_levels(struct smu_context > *smu, > uint32_t cur_value = 0, value = 0, count = 0, min = 0, max = 0; > DpmClocks_t *clk_table = smu->smu_table.clocks_table; > SmuMetrics_t metrics; > + bool cur_value_match_level = false; > > if (!clk_table || clk_type >= SMU_CLK_COUNT) > return -EINVAL; > @@ -297,8 +298,13 @@ static int renoir_print_clk_levels(struct smu_context > *smu, > GET_DPM_CUR_FREQ(clk_table, clk_type, i, value); > size += sprintf(buf + size, "%d: %uMhz %s\n", i, value, > cur_value == value ? "*" : ""); > + if (cur_value == value) > + cur_value_match_level = true; > } > > + if (!cur_value_match_level) > + size += sprintf(buf + size, " %uMhz *\n", cur_value); > + > return size; > } > > diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > b/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > index 2a390ddd37dd..89cd6da118a3 100644 > --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > @@ -37,7 +37,7 @@ extern void renoir_set_ppt_funcs(struct smu_context *smu); > freq = table->SocClocks[dpm_level].Freq;\ > break; \ > case SMU_MCLK: \ > - freq = table->MemClocks[dpm_level].Freq;\ > + freq = table->FClocks[dpm_level].Freq; \ > break; \ > case SMU_DCEFCLK: \ > freq = table->DcfClocks[dpm_level].Freq;\ > -- > 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-gfx&data=02%7C01%7Cra > y.huang%40amd.com%7Cfa81baf94d2c409e4f0308d7d6319a8f%7C3dd8961fe4884e6 > 08e11a82d994e183d%7C0%7C0%7C637213381443101089&sdata=LF2pLZj%2Fq0C > wiMSfvDiofXFKuGVfgje7o4Iti%2FtoNj0%3D&reserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu/powerplay: using the FCLK DPM table to set the MCLK
[AMD Official Use Only - Internal Distribution Only]
1,Using the FCLK DPM table to set the MCLK for DPM states consist of
three entities:
FCLK
UCLK
MEMCLK
All these three clk change together, MEMCLK from FCLK, so use the fclk
frequency.
2,we should show the current working clock freqency from clock table metric
Change-Id: Ia45f3069fc7ae56db495cb5a3865e2c50c550774
Signed-off-by: Yuxian Dai mailto:yuxian@amd.com>>
---
drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8
drivers/gpu/drm/amd/powerplay/renoir_ppt.h | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
index 7bf52ecba01d..5c5d3f974532 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
@@ -239,6 +239,7 @@ static int renoir_print_clk_levels(struct smu_context *smu,
uint32_t cur_value = 0, value = 0, count = 0, min = 0, max = 0;
DpmClocks_t *clk_table = smu->smu_table.clocks_table;
SmuMetrics_t metrics;
+ bool cur_value_match_level = false;
if (!clk_table || clk_type >= SMU_CLK_COUNT)
return -EINVAL;
@@ -297,6 +298,13 @@ static int renoir_print_clk_levels(struct smu_context *smu,
GET_DPM_CUR_FREQ(clk_table, clk_type, i, value);
size += sprintf(buf + size, "%d: %uMhz %s\n", i, value,
cur_value == value ? "*" : "");
+ if(cur_value == value) {
+ cur_value_match_level = true;
+ }
+ }
+
+ if(!cur_value_match_level) {
+ size += sprintf(buf + size, " %uMhz *\n",cur_value);
}
[kevin]:
I have a little suggestion about coding style.
"Do not unnecessarily use braces where a single statement will do."
we'd better fix it.
thanks.
* Ok, I will remove it.
https://www.kernel.org/doc/html/latest/process/coding-style.html
Linux kernel coding style - The Linux Kernel
documentation
Linux kernel coding style¶. This is a short document describing the preferred
coding style for the linux kernel. Coding style is very personal, and I won't
force my views on anybody, but this is what goes for anything that I have to be
able to maintain, and I'd prefer it for most other things too. Please at least
consider the points made here.
www.kernel.org
return size;
diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.h
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.h
index 2a390ddd37dd..89cd6da118a3 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.h
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.h
@@ -37,7 +37,7 @@ extern void renoir_set_ppt_funcs(struct smu_context *smu);
freq = table->SocClocks[dpm_level].Freq;\
break; \
case SMU_MCLK: \
- freq = table->MemClocks[dpm_level].Freq;\
+ freq = table->FClocks[dpm_level].Freq; \
break; \
case SMU_DCEFCLK: \
freq = table->DcfClocks[dpm_level].Freq;\
--
2.17.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-gfx&data=02%7C01%7CKevin1.Wang%40amd.com%7Cac64217a79c44e06267008d7d602484e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637213177638342283&sdata=iEt1JzC0wcb7rQ%2F2TWBu7RfJGrChPpt%2Fc2jXPfqjruU%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu/powerplay: using the FCLK DPM table to set the MCLK for DPM states consist of three entities :FCLK, UCLK, MEMCLK all these three clk change together , MEMCLK from FCLK.
[AMD Official Use Only - Internal Distribution Only] On Tue, Mar 31, 2020 at 09:41:44AM -0400, Alex Deucher wrote: > On Tue, Mar 31, 2020 at 6:10 AM Yuxian Dai wrote: > > > > From: "yuxia...@amd.com" > > Your patch title is too long; it is basically the whole patch > description rather than just a title. Please split it up between the > title and descriptions. E.g., > > drm/amdgpu/powerplay: fix MCLK DPM handling for renoir > > Using the FCLK DPM table to set the MCLK for DPM states consist of > three entities: > FCLK > UCLK > MEMCLK > All these three clk change together, MEMCLK from FCLK, so use the fclk > frequency. > > With that fixed, patch is: > Reviewed-by: Alex Deucher > Yes, and usually, we don't leave the commit message as empty. David, you need describe the detailed info in the commit not the title. > missing a blank line, so the description confuse you . I will update it > according your suggestion. Thanks, Ray > > > > Change-Id: Ia45f3069fc7ae56db495cb5a3865e2c50c550774 > > Signed-off-by: Yuxian Dai > > --- > > drivers/gpu/drm/amd/powerplay/renoir_ppt.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > > b/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > > index 2a390ddd37dd..89cd6da118a3 100644 > > --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > > +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.h > > @@ -37,7 +37,7 @@ extern void renoir_set_ppt_funcs(struct smu_context *smu); > > freq = table->SocClocks[dpm_level].Freq;\ > > break; \ > > case SMU_MCLK: \ > > - freq = table->MemClocks[dpm_level].Freq;\ > > + freq = table->FClocks[dpm_level].Freq; \ > > break; \ > > case SMU_DCEFCLK: \ > > freq = table->DcfClocks[dpm_level].Freq;\ > > -- > > 2.17.1 > > > > ___ > > amd-gfx mailing list > > amd-gfx@lists.freedesktop.org > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fli > > sts.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01% > > 7Cray.huang%40amd.com%7C541f2e15da804d81c5b508d7d57949d2%7C3dd8961fe > > 4884e608e11a82d994e183d%7C0%7C0%7C637212590166168794&sdata=7x4pB > > WEPuVHbb8ro%2Fpy5%2BgiJhrxWbLrxGqHSro9KUV8%3D&reserved=0 > ___ > 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-gfx&data=02%7C01%7Cra > y.huang%40amd.com%7C541f2e15da804d81c5b508d7d57949d2%7C3dd8961fe4884e6 > 08e11a82d994e183d%7C0%7C0%7C637212590166168794&sdata=7x4pBWEPuVHbb > 8ro%2Fpy5%2BgiJhrxWbLrxGqHSro9KUV8%3D&reserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
As Ray point out. We should set the SMU_MSG_SetWorkloadMask with specified
value to indicate unsupported.
But the current a value with system error value: "-EINVAL"
The firmware maybe response with unexpected action to driver.
-Original Message-
From: Huang, Ray
Sent: Thursday, December 19, 2019 3:17 PM
To: Dai, Yuxian (David)
Cc: Liang, Prike ; amd-gfx@lists.freedesktop.org; Quan,
Evan
Subject: Re: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
overrun message
[AMD Official Use Only - Internal Distribution Only]
On Thu, Dec 19, 2019 at 03:04:12PM +0800, Dai, Yuxian (David) wrote:
> For we don't support the mode, so shouldn't print the error message, or
> regard as a error.
> For log message, the error is high level .maybe change from "error" to
> "warning" , it will be much better.
>
>
> -Original Message-
> From: Liang, Prike
> Sent: Thursday, December 19, 2019 2:46 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan ; Huang, Ray ;
> Dai, Yuxian (David) ; Liang, Prike
>
> Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode
> overrun message
>
> SMU12 not support WORKLOAD_DEFAULT_BIT and WORKLOAD_PPLIB_POWER_SAVING_BIT.
>
Probably smu firmware doesn't expose the feature mask to driver. Can you
confirmware with smu firmware guy whehter this feature is really disabled or
not in SMU12. If that, in my view, issue the message SMU_MSG_SetWorkloadMask
with an unsupported state, it doesn't make sense.
Just work around this with one time warnning is not a good solution.
Thanks,
Ray
> Signed-off-by: Prike Liang
> ---
> drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> index 784903a3..f9a1817 100644
> --- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
> @@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
> smu_context *smu, long *input, u
> /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
> workload_type = smu_workload_get_type(smu, smu->power_profile_mode);
> if (workload_type < 0) {
> - pr_err("Unsupported power profile mode %d on
> RENOIR\n",smu->power_profile_mode);
> + /*
> + * TODO: If some case need switch to powersave/default power
> mode
> + * then can consider enter WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for
> power saving.
> + */
> + pr_err_once("Unsupported power profile mode %d on
> +RENOIR\n",smu->power_profile_mode);
> return -EINVAL;
> }
>
> ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
> 1 << workload_type);
> if (ret) {
> - pr_err("Fail to set workload type %d\n", workload_type);
> + pr_err_once("Fail to set workload type %d\n", workload_type);
> return ret;
> }
>
> --
> 2.7.4
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun message
For we don't support the mode, so shouldn't print the error message, or regard
as a error.
For log message, the error is high level .maybe change from "error" to
"warning" , it will be much better.
-Original Message-
From: Liang, Prike
Sent: Thursday, December 19, 2019 2:46 PM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan ; Huang, Ray ; Dai, Yuxian
(David) ; Liang, Prike
Subject: [PATCH] drm/amd/powerplay: suppress nonsupport profile mode overrun
message
SMU12 not support WORKLOAD_DEFAULT_BIT and WORKLOAD_PPLIB_POWER_SAVING_BIT.
Signed-off-by: Prike Liang
---
drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
index 784903a3..f9a1817 100644
--- a/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/renoir_ppt.c
@@ -550,14 +550,18 @@ static int renoir_set_power_profile_mode(struct
smu_context *smu, long *input, u
/* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */
workload_type = smu_workload_get_type(smu, smu->power_profile_mode);
if (workload_type < 0) {
- pr_err("Unsupported power profile mode %d on
RENOIR\n",smu->power_profile_mode);
+ /*
+* TODO: If some case need switch to powersave/default power
mode
+* then can consider enter WORKLOAD_COMPUTE/WORKLOAD_CUSTOM for
power saving.
+*/
+ pr_err_once("Unsupported power profile mode %d on
RENOIR\n",smu->power_profile_mode);
return -EINVAL;
}
ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetWorkloadMask,
1 << workload_type);
if (ret) {
- pr_err("Fail to set workload type %d\n", workload_type);
+ pr_err_once("Fail to set workload type %d\n", workload_type);
return ret;
}
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
