Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Deucher, Alexander
The changes to the pp_dpm_files are purely informational with this patch (you 
can't actually interact with the stable pstate) so I think they should be 
dropped and exposed some other way.  Setting the profile modes via 
force_performance_level forces the clocks and disables clock and power gating.  
That is the interface you should use to interact with it.


Alex


From: amd-gfx  on behalf of Russell, 
Kent 
Sent: Monday, January 8, 2018 3:22 PM
To: Alex Deucher; Kuehling, Felix
Cc: amd-gfx list
Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

Sorry, I just re-read and saw that this will be in sclk and mclk, not just 
mclk. So will this have a direct impact on any other functionality, or will it 
just disable clock/power gating and prevent the clocks from changing?

 Kent

-Original Message-
From: Russell, Kent
Sent: Monday, January 08, 2018 2:52 PM
To: 'Alex Deucher'; Kuehling, Felix
Cc: amd-gfx list
Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

And yes, it will confuse the SMI in a couple functions, but thankfully it's 
nice enough that I can fix it with a couple lines.

So just to make sure that I understand this correctly, if a user were to echo P 
to the pp_dpm_mclk file, it would disable clockgating and powergating and keep 
the memory clocks at a stable level (likely level 0)? And this would have no 
impact on the SCLK or Power Profile or anything else, just the mclk? Just 
asking so I know how to implement it in the SMI. Thanks!

 Kent

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Monday, January 08, 2018 2:35 PM
To: Kuehling, Felix
Cc: amd-gfx list; Russell, Kent
Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling  wrote:
> [+Kent]
>
> What does stable pstate mean? What is it used for?

This is used for the profiling stuff in force_performance_level.  It disables 
clock and power gating and sets stable clock levels for doing performance 
profiling.

Alex

>
> Hi Kent,
>
> Is this going to confuse rocm_smi?
>
> Regards,
>   Felix
>
>
> On 2018-01-08 04:57 AM, Rex Zhu wrote:
>> The additional output are at the end of sclk/mclk info as cat
>> pp_dpm_mclk
>> 0: 300Mhz *
>> 1: 1650Mhz
>> P: 300Mhz
>>
>> Signed-off-by: Rex Zhu 
>>
>> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  4 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> index f68dd08..03dfba0 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->entries[i].clk / 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   now =
>> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   CZ_NUM_NBPMEMORYCLOCK-i, 
>> data->sys_info.nbp_memory_clock[i-1] / 100,
>>   (CZ_NUM_NBPMEMORYCLOCK-i ==
>> now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> index 409a56b..88c6ad8 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   data->gfx_max_freq_limit / 100,
>>   ((data->gfx_max_freq_limit / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
>> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   mclk_table->entries[i].clk / 100,
>>   ((m

RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Deucher, Alexander
No, it doesn't do anything today and wouldn't do anything with this patch 
either.  With this patch, it just prints the stable pstate clock level in the 
output if you read the files so you know what the clocks are when you enable 
the profiling modes.  That's why I think we should expose the stable pstate 
clocks via a different interface to avoid confusion and not break anything (we 
can't break existing smi tools out in the wild.  To actually enable the various 
profiling modes, you need to echo one of the profile_* options to 
force_performance_level.  When you select a profile_* option, that disables 
clock and powergating and sets the stable pstate clocks.

Alex

-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Russell, Kent
Sent: Monday, January 8, 2018 2:52 PM
To: Alex Deucher ; Kuehling, Felix 

Cc: amd-gfx list 
Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

And yes, it will confuse the SMI in a couple functions, but thankfully it's 
nice enough that I can fix it with a couple lines. 

So just to make sure that I understand this correctly, if a user were to echo P 
to the pp_dpm_mclk file, it would disable clockgating and powergating and keep 
the memory clocks at a stable level (likely level 0)? And this would have no 
impact on the SCLK or Power Profile or anything else, just the mclk? Just 
asking so I know how to implement it in the SMI. Thanks!

 Kent

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Monday, January 08, 2018 2:35 PM
To: Kuehling, Felix
Cc: amd-gfx list; Russell, Kent
Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling  wrote:
> [+Kent]   
>
> What does stable pstate mean? What is it used for?

This is used for the profiling stuff in force_performance_level.  It disables 
clock and power gating and sets stable clock levels for doing performance 
profiling.

Alex

>
> Hi Kent,
>
> Is this going to confuse rocm_smi?
>
> Regards,
>   Felix
>
>
> On 2018-01-08 04:57 AM, Rex Zhu wrote:
>> The additional output are at the end of sclk/mclk info as cat 
>> pp_dpm_mclk
>> 0: 300Mhz *
>> 1: 1650Mhz
>> P: 300Mhz
>>
>> Signed-off-by: Rex Zhu 
>>
>> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  4 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> index f68dd08..03dfba0 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->entries[i].clk / 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   now =
>> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   CZ_NUM_NBPMEMORYCLOCK-i, 
>> data->sys_info.nbp_memory_clock[i-1] / 100,
>>   (CZ_NUM_NBPMEMORYCLOCK-i ==
>> now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> index 409a56b..88c6ad8 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   data->gfx_max_freq_limit / 100,
>>   ((data->gfx_max_freq_limit / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
>> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   mclk_table->entries[i].clk / 100,
>>   ((mclk_table->entries[i].clk / 100)
>>== now) ? "*" : "");
>> + 

RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Russell, Kent
Oh perfect. pp_dpm_sclk/pp_dpm_mclk just report what the stable pstate would 
be. To enact that pstate, we would use the options available in 
force_performance_level. Thank you for the clarification!

Kent

From: Deucher, Alexander
Sent: Monday, January 08, 2018 4:19 PM
To: Russell, Kent; Alex Deucher; Kuehling, Felix
Cc: amd-gfx list
Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels


The changes to the pp_dpm_files are purely informational with this patch (you 
can't actually interact with the stable pstate) so I think they should be 
dropped and exposed some other way.  Setting the profile modes via 
force_performance_level forces the clocks and disables clock and power gating.  
That is the interface you should use to interact with it.



Alex


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Russell, Kent mailto:kent.russ...@amd.com>>
Sent: Monday, January 8, 2018 3:22 PM
To: Alex Deucher; Kuehling, Felix
Cc: amd-gfx list
Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

Sorry, I just re-read and saw that this will be in sclk and mclk, not just 
mclk. So will this have a direct impact on any other functionality, or will it 
just disable clock/power gating and prevent the clocks from changing?

 Kent

-Original Message-
From: Russell, Kent
Sent: Monday, January 08, 2018 2:52 PM
To: 'Alex Deucher'; Kuehling, Felix
Cc: amd-gfx list
Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

And yes, it will confuse the SMI in a couple functions, but thankfully it's 
nice enough that I can fix it with a couple lines.

So just to make sure that I understand this correctly, if a user were to echo P 
to the pp_dpm_mclk file, it would disable clockgating and powergating and keep 
the memory clocks at a stable level (likely level 0)? And this would have no 
impact on the SCLK or Power Profile or anything else, just the mclk? Just 
asking so I know how to implement it in the SMI. Thanks!

 Kent

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Monday, January 08, 2018 2:35 PM
To: Kuehling, Felix
Cc: amd-gfx list; Russell, Kent
Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling 
mailto:felix.kuehl...@amd.com>> wrote:
> [+Kent]
>
> What does stable pstate mean? What is it used for?

This is used for the profiling stuff in force_performance_level.  It disables 
clock and power gating and sets stable clock levels for doing performance 
profiling.

Alex

>
> Hi Kent,
>
> Is this going to confuse rocm_smi?
>
> Regards,
>   Felix
>
>
> On 2018-01-08 04:57 AM, Rex Zhu wrote:
>> The additional output are at the end of sclk/mclk info as cat
>> pp_dpm_mclk
>> 0: 300Mhz *
>> 1: 1650Mhz
>> P: 300Mhz
>>
>> Signed-off-by: Rex Zhu mailto:rex@amd.com>>
>>
>> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  4 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> index f68dd08..03dfba0 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->entries[i].clk / 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   now =
>> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   CZ_NUM_NBPMEMORYCLOCK-i, 
>> data->sys_info.nbp_memory_clock[i-1] / 100,
>>   (CZ_NUM_NBPMEMORYCLOCK-i ==
>> now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> index 409a56b..88c6ad8 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>> 

RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Russell, Kent
And yes, it will confuse the SMI in a couple functions, but thankfully it's 
nice enough that I can fix it with a couple lines. 

So just to make sure that I understand this correctly, if a user were to echo P 
to the pp_dpm_mclk file, it would disable clockgating and powergating and keep 
the memory clocks at a stable level (likely level 0)? And this would have no 
impact on the SCLK or Power Profile or anything else, just the mclk? Just 
asking so I know how to implement it in the SMI. Thanks!

 Kent

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: Monday, January 08, 2018 2:35 PM
To: Kuehling, Felix
Cc: amd-gfx list; Russell, Kent
Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling  wrote:
> [+Kent]   
>
> What does stable pstate mean? What is it used for?

This is used for the profiling stuff in force_performance_level.  It disables 
clock and power gating and sets stable clock levels for doing performance 
profiling.

Alex

>
> Hi Kent,
>
> Is this going to confuse rocm_smi?
>
> Regards,
>   Felix
>
>
> On 2018-01-08 04:57 AM, Rex Zhu wrote:
>> The additional output are at the end of sclk/mclk info as cat 
>> pp_dpm_mclk
>> 0: 300Mhz *
>> 1: 1650Mhz
>> P: 300Mhz
>>
>> Signed-off-by: Rex Zhu 
>>
>> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  4 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> index f68dd08..03dfba0 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->entries[i].clk / 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   now = 
>> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   CZ_NUM_NBPMEMORYCLOCK-i, 
>> data->sys_info.nbp_memory_clock[i-1] / 100,
>>   (CZ_NUM_NBPMEMORYCLOCK-i == 
>> now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> index 409a56b..88c6ad8 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   data->gfx_max_freq_limit / 100,
>>   ((data->gfx_max_freq_limit / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
>> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   mclk_table->entries[i].clk / 100,
>>   ((mclk_table->entries[i].clk / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index 72031bd..1bdcd86 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -4372,6 +4372,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->dpm_levels[i].value / 
>> 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   smum_send_msg_to_smc(hwmgr, 
>> PPSMC_MSG_API_GetMclkFrequency); @@ -4388,6 +4389,7 @@ stat

Re: [PATCH libdrm 3/4] amdgpu: use the high VA range if possible v2

2018-01-08 Thread Marek Olšák
Actually, 0x8000 is fine.

For the series:

Reviewed-by: Marek Olšák 

Marek
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Russell, Kent
Sorry, I just re-read and saw that this will be in sclk and mclk, not just 
mclk. So will this have a direct impact on any other functionality, or will it 
just disable clock/power gating and prevent the clocks from changing?

 Kent

-Original Message-
From: Russell, Kent 
Sent: Monday, January 08, 2018 2:52 PM
To: 'Alex Deucher'; Kuehling, Felix
Cc: amd-gfx list
Subject: RE: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

And yes, it will confuse the SMI in a couple functions, but thankfully it's 
nice enough that I can fix it with a couple lines. 

So just to make sure that I understand this correctly, if a user were to echo P 
to the pp_dpm_mclk file, it would disable clockgating and powergating and keep 
the memory clocks at a stable level (likely level 0)? And this would have no 
impact on the SCLK or Power Profile or anything else, just the mclk? Just 
asking so I know how to implement it in the SMI. Thanks!

 Kent

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Monday, January 08, 2018 2:35 PM
To: Kuehling, Felix
Cc: amd-gfx list; Russell, Kent
Subject: Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when 
print_clock_levels

On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling  wrote:
> [+Kent]   
>
> What does stable pstate mean? What is it used for?

This is used for the profiling stuff in force_performance_level.  It disables 
clock and power gating and sets stable clock levels for doing performance 
profiling.

Alex

>
> Hi Kent,
>
> Is this going to confuse rocm_smi?
>
> Regards,
>   Felix
>
>
> On 2018-01-08 04:57 AM, Rex Zhu wrote:
>> The additional output are at the end of sclk/mclk info as cat 
>> pp_dpm_mclk
>> 0: 300Mhz *
>> 1: 1650Mhz
>> P: 300Mhz
>>
>> Signed-off-by: Rex Zhu 
>>
>> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  4 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> index f68dd08..03dfba0 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->entries[i].clk / 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   now =
>> PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   CZ_NUM_NBPMEMORYCLOCK-i, 
>> data->sys_info.nbp_memory_clock[i-1] / 100,
>>   (CZ_NUM_NBPMEMORYCLOCK-i ==
>> now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> index 409a56b..88c6ad8 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   data->gfx_max_freq_limit / 100,
>>   ((data->gfx_max_freq_limit / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
>> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   mclk_table->entries[i].clk / 100,
>>   ((mclk_table->entries[i].clk / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n",
>> + hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index 72031bd..1bdcd86 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -4372,6 +4372,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>> 

Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Alex Deucher
On Mon, Jan 8, 2018 at 2:20 PM, Felix Kuehling  wrote:
> [+Kent]
>
> What does stable pstate mean? What is it used for?

This is used for the profiling stuff in force_performance_level.  It
disables clock and power gating and sets stable clock levels for doing
performance profiling.

Alex

>
> Hi Kent,
>
> Is this going to confuse rocm_smi?
>
> Regards,
>   Felix
>
>
> On 2018-01-08 04:57 AM, Rex Zhu wrote:
>> The additional output are at the end of sclk/mclk info as
>> cat pp_dpm_mclk
>> 0: 300Mhz *
>> 1: 1650Mhz
>> P: 300Mhz
>>
>> Signed-off-by: Rex Zhu 
>>
>> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  4 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> index f68dd08..03dfba0 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
>> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->entries[i].clk / 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
>> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   CZ_NUM_NBPMEMORYCLOCK-i, 
>> data->sys_info.nbp_memory_clock[i-1] / 100,
>>   (CZ_NUM_NBPMEMORYCLOCK-i == now) ? "*" 
>> : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> index 409a56b..88c6ad8 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
>> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   data->gfx_max_freq_limit / 100,
>>   ((data->gfx_max_freq_limit / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
>> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>>   mclk_table->entries[i].clk / 100,
>>   ((mclk_table->entries[i].clk / 100)
>>== now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> hwmgr->pstate_mclk/100);
>>   break;
>>   default:
>>   break;
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index 72031bd..1bdcd86 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -4372,6 +4372,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, sclk_table->dpm_levels[i].value / 
>> 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> hwmgr->pstate_sclk/100);
>>   break;
>>   case PP_MCLK:
>>   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_API_GetMclkFrequency);
>> @@ -4388,6 +4389,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr 
>> *hwmgr,
>>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>>   i, mclk_table->dpm_levels[i].value / 
>> 100,
>>   (i == now) ? "*" : "");
>> + size += sprintf(buf + size, "P: %uMhz\n", 
>> hwmgr->pstate_mclk/100);
>>   break;
>>   case PP_PCIE:
>>   pcie_speed = smu7_get_current_pcie_speed(hwmgr);
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> index cb35f4f..cab50fc 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> @@ -4565,6 +4565,7 @@ static int 

Re: [PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Felix Kuehling
[+Kent]

What does stable pstate mean? What is it used for?

Hi Kent,

Is this going to confuse rocm_smi?

Regards,
  Felix


On 2018-01-08 04:57 AM, Rex Zhu wrote:
> The additional output are at the end of sclk/mclk info as
> cat pp_dpm_mclk
> 0: 300Mhz *
> 1: 1650Mhz
> P: 300Mhz
>
> Signed-off-by: Rex Zhu 
>
> Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
>  drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>  4 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index f68dd08..03dfba0 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>   i, sclk_table->entries[i].clk / 100,
>   (i == now) ? "*" : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_sclk/100);
>   break;
>   case PP_MCLK:
>   now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
> @@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>   CZ_NUM_NBPMEMORYCLOCK-i, 
> data->sys_info.nbp_memory_clock[i-1] / 100,
>   (CZ_NUM_NBPMEMORYCLOCK-i == now) ? "*" 
> : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_mclk/100);
>   break;
>   default:
>   break;
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
> index 409a56b..88c6ad8 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
> @@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>   data->gfx_max_freq_limit / 100,
>   ((data->gfx_max_freq_limit / 100)
>== now) ? "*" : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_sclk/100);
>   break;
>   case PP_MCLK:
>   PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
> @@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
>   mclk_table->entries[i].clk / 100,
>   ((mclk_table->entries[i].clk / 100)
>== now) ? "*" : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_mclk/100);
>   break;
>   default:
>   break;
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 72031bd..1bdcd86 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -4372,6 +4372,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr 
> *hwmgr,
>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>   i, sclk_table->dpm_levels[i].value / 
> 100,
>   (i == now) ? "*" : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_sclk/100);
>   break;
>   case PP_MCLK:
>   smum_send_msg_to_smc(hwmgr, PPSMC_MSG_API_GetMclkFrequency);
> @@ -4388,6 +4389,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr 
> *hwmgr,
>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>   i, mclk_table->dpm_levels[i].value / 
> 100,
>   (i == now) ? "*" : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_mclk/100);
>   break;
>   case PP_PCIE:
>   pcie_speed = smu7_get_current_pcie_speed(hwmgr);
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index cb35f4f..cab50fc 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -4565,6 +4565,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr 
> *hwmgr,
>   size += sprintf(buf + size, "%d: %uMhz %s\n",
>   i, sclk_table->dpm_levels[i].value / 
> 100,
>   (i == now) ? "*" : "");
> + size += sprintf(buf + size, "P: %uMhz\n", 
> hwmgr->pstate_sclk

Re: [PATCH] drm/amdgpu: use %pap format string for phys_addr_t

2018-01-08 Thread Felix Kuehling
This commit is Reviewed-by: Felix Kuehling 

Thanks,
  Felix


On 2018-01-08 07:53 AM, Arnd Bergmann wrote:
> The newly added get_local_mem_info() function prints a phys_addr_t
> using 0x%llx, which is wrong on most 32-bit systems, as shown by
> this warning:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c: In function 'get_local_mem_info':
> include/linux/kern_levels.h:5:18: error: format '%llx' expects argument of 
> type 'long long unsigned int', but argument 2 has type 'resource_size_t {aka 
> unsigned int}' [-Werror=format=]
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c:297:31: note: format string is 
> defined here
>   pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 0x%llx\n",
>
> Passing the address by reference to the special %pap format string will
> produce the correct output and avoid the warning.
>
> Fixes: 30f1c0421ec5 ("drm/amdgpu: Implement get_local_mem_info")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 335e454e2ee1..1d605e1c1d66 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -294,8 +294,8 @@ void get_local_mem_info(struct kgd_dev *kgd,
>   }
>   mem_info->vram_width = adev->mc.vram_width;
>  
> - pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 
> 0x%llx\n",
> - adev->mc.aper_base, aper_limit,
> + pr_debug("Address base: %pap limit %pap public 0x%llx private 0x%llx\n",
> + &adev->mc.aper_base, &aper_limit,
>   mem_info->local_mem_size_public,
>   mem_info->local_mem_size_private);
>  

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm] amdgpu: fix not to add amdgpu.ids when building without amdgpu

2018-01-08 Thread Michel Dänzer
On 2018-01-04 07:31 AM, Seung-Woo Kim wrote:
> The amdgpu.ids is only required when building with amdgpu support.
> Fix not to add it without amdgpu.
> 
> Signed-off-by: Seung-Woo Kim 
> ---
>  data/Makefile.am |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/data/Makefile.am b/data/Makefile.am
> index eba915d..897a7f3 100644
> --- a/data/Makefile.am
> +++ b/data/Makefile.am
> @@ -20,4 +20,6 @@
>  #  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>  
>  libdrmdatadir = @libdrmdatadir@
> +if HAVE_AMDGPU
>  dist_libdrmdata_DATA = amdgpu.ids
> +endif
> 

Reviewed and pushed, thanks!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm 3/4] amdgpu: use the high VA range if possible v2

2018-01-08 Thread Marek Olšák
Can we put the 32-bit address space higher? E.g. high bits = 0xfff0 ?

Marek

On Sun, Jan 7, 2018 at 10:11 AM, Christian König
 wrote:
> Retire the low range on Vega10 this frees up everything below 
> 0x8000 for HMM.
>
> v2: keep the 32bit range working.
>
> Signed-off-by: Christian König 
> ---
>  amdgpu/amdgpu_device.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> index d7077184..a0d01727 100644
> --- a/amdgpu/amdgpu_device.c
> +++ b/amdgpu/amdgpu_device.c
> @@ -264,13 +264,23 @@ int amdgpu_device_initialize(int fd,
> goto cleanup;
> }
>
> -   start = dev->dev_info.virtual_address_offset;
> -   max = MIN2(dev->dev_info.virtual_address_max, 0x1ULL);
> +   if (dev->dev_info.high_va_offset && dev->dev_info.high_va_max) {
> +   start = dev->dev_info.high_va_offset;
> +   max = dev->dev_info.high_va_max;
> +   } else {
> +   start = dev->dev_info.virtual_address_offset;
> +   max = dev->dev_info.virtual_address_max;
> +   }
> +
> +   max = MIN2(max, (start & ~0x) + 0x1ULL);
> amdgpu_vamgr_init(&dev->vamgr_32, start, max,
>   dev->dev_info.virtual_address_alignment);
>
> -   start = MAX2(dev->dev_info.virtual_address_offset, 0x1ULL);
> -   max = MAX2(dev->dev_info.virtual_address_max, 0x1ULL);
> +   start = max;
> +   if (dev->dev_info.high_va_offset && dev->dev_info.high_va_max)
> +   max = dev->dev_info.high_va_max;
> +   else
> +   max = dev->dev_info.virtual_address_max;
> amdgpu_vamgr_init(&dev->vamgr, start, max,
>   dev->dev_info.virtual_address_alignment);
>
> --
> 2.11.0
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[bug report] drm/amdgpu: move debugfs functions to their own file

2018-01-08 Thread Dan Carpenter
[ Not really a new bug, but moving the function makes it show up as a
  new bug.  - dan ]

Hello Alex Deucher,

The patch 75758255dc0f: "drm/amdgpu: move debugfs functions to their
own file" from Dec 14, 2017, leads to the following static checker
warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:696 
amdgpu_debugfs_regs_init()
warn: possible NULL dereference of 'ent'

drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
   677  int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
   678  {
   679  struct drm_minor *minor = adev->ddev->primary;
   680  struct dentry *ent, *root = minor->debugfs_root;
   681  unsigned i, j;
   682  
   683  for (i = 0; i < ARRAY_SIZE(debugfs_regs); i++) {
   684  ent = debugfs_create_file(debugfs_regs_names[i],
   685S_IFREG | S_IRUGO, root,
   686adev, debugfs_regs[i]);
   687  if (IS_ERR(ent)) {
   688  for (j = 0; j < i; j++) {
   689  debugfs_remove(adev->debugfs_regs[i]);
   690  adev->debugfs_regs[i] = NULL;
   691  }
   692  return PTR_ERR(ent);
   693  }
   694  
   695  if (!i)
   696  i_size_write(ent->d_inode, adev->rmmio_size);
 
debugfs_create_file() returns ERR_PTR(-ENODEV) if it's not enabled in
the .config or it returns a NULL pointer.

   697  adev->debugfs_regs[i] = ent;
   698  }
   699  
   700  return 0;
   701  }

regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH libdrm] amdgpu: Don't print error message if parse_one_line returned -EAGAIN

2018-01-08 Thread Marek Olšák
Reviewed-by: Marek Olšák 

Marek

On Mon, Jan 8, 2018 at 11:24 AM, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> It means it just didn't find an entry for the GPU in the amdgpu.ids file.
>
> Fixes spurious
>
>  amdgpu_parse_asic_ids: Cannot parse ASIC IDs: Resource temporarily 
> unavailable
>
> error messages in that case.
>
> Reported-by: Marek Olšák 
> Signed-off-by: Michel Dänzer 
> ---
>  amdgpu/amdgpu_asic_id.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
> index 0c8925e5..62459c09 100644
> --- a/amdgpu/amdgpu_asic_id.c
> +++ b/amdgpu/amdgpu_asic_id.c
> @@ -155,7 +155,7 @@ void amdgpu_parse_asic_ids(struct amdgpu_device *dev)
> if (r == -EINVAL) {
> fprintf(stderr, "Invalid format: %s: line %d: %s\n",
> AMDGPU_ASIC_ID_TABLE, line_num, line);
> -   } else if (r) {
> +   } else if (r && r != -EAGAIN) {
> fprintf(stderr, "%s: Cannot parse ASIC IDs: %s\n",
> __func__, strerror(-r));
> }
> --
> 2.15.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 1/2] drm/amd/pp: Add memory clock info display on Cz/St

2018-01-08 Thread Deucher, Alexander
Reviewed-by: Alex Deucher 


From: amd-gfx  on behalf of Rex Zhu 

Sent: Monday, January 8, 2018 4:57:30 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex
Subject: [PATCH 1/2] drm/amd/pp: Add memory clock info display on Cz/St

show mclk info as in MHz on Cz/St as
0: 333Mhz *
1: 800Mhz

Change-Id: Ie5932ac81b15565edb154ec6c00b35a99ab52b73
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
index b83fe97..f68dd08 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
@@ -1584,6 +1584,7 @@ static int cz_force_clock_level(struct pp_hwmgr *hwmgr,
 static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
 enum pp_clock_type type, char *buf)
 {
+   struct cz_hwmgr *data = (struct cz_hwmgr *)(hwmgr->backend);
 struct phm_clock_voltage_dependency_table *sclk_table =
 hwmgr->dyn_state.vddc_dependency_on_sclk;
 int i, now, size = 0;
@@ -1601,6 +1602,18 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
 i, sclk_table->entries[i].clk / 100,
 (i == now) ? "*" : "");
 break;
+   case PP_MCLK:
+   now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
+   CGS_IND_REG__SMC,
+   ixTARGET_AND_CURRENT_PROFILE_INDEX),
+   TARGET_AND_CURRENT_PROFILE_INDEX,
+   CURR_MCLK_INDEX);
+
+   for (i = CZ_NUM_NBPMEMORYCLOCK; i > 0; i--)
+   size += sprintf(buf + size, "%d: %uMhz %s\n",
+   CZ_NUM_NBPMEMORYCLOCK-i, 
data->sys_info.nbp_memory_clock[i-1] / 100,
+   (CZ_NUM_NBPMEMORYCLOCK-i == now) ? "*" 
: "");
+   break;
 default:
 break;
 }
--
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: use %pap format string for phys_addr_t

2018-01-08 Thread Arnd Bergmann
The newly added get_local_mem_info() function prints a phys_addr_t
using 0x%llx, which is wrong on most 32-bit systems, as shown by
this warning:

drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c: In function 'get_local_mem_info':
include/linux/kern_levels.h:5:18: error: format '%llx' expects argument of type 
'long long unsigned int', but argument 2 has type 'resource_size_t {aka 
unsigned int}' [-Werror=format=]
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c:297:31: note: format string is 
defined here
  pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 0x%llx\n",

Passing the address by reference to the special %pap format string will
produce the correct output and avoid the warning.

Fixes: 30f1c0421ec5 ("drm/amdgpu: Implement get_local_mem_info")
Signed-off-by: Arnd Bergmann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 335e454e2ee1..1d605e1c1d66 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -294,8 +294,8 @@ void get_local_mem_info(struct kgd_dev *kgd,
}
mem_info->vram_width = adev->mc.vram_width;
 
-   pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 
0x%llx\n",
-   adev->mc.aper_base, aper_limit,
+   pr_debug("Address base: %pap limit %pap public 0x%llx private 0x%llx\n",
+   &adev->mc.aper_base, &aper_limit,
mem_info->local_mem_size_public,
mem_info->local_mem_size_private);
 
-- 
2.9.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: fix 64bit BAR detection

2018-01-08 Thread Christian König
Windows added by the BIOS are not marked as 64bit because they are
usually not changeable anyway.

This fixes large BAR support on my new Ryzen build system.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e91575311646..7114f2527edc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -626,7 +626,7 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
root = root->parent;
 
pci_bus_for_each_resource(root, res, i) {
-   if (res && res->flags & IORESOURCE_MEM_64 &&
+   if (res && res->flags & (IORESOURCE_MEM | IORESOURCE_MEM_64) &&
res->start > 0x1ull)
break;
}
-- 
2.14.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH libdrm] amdgpu: Don't print error message if parse_one_line returned -EAGAIN

2018-01-08 Thread Michel Dänzer
From: Michel Dänzer 

It means it just didn't find an entry for the GPU in the amdgpu.ids file.

Fixes spurious

 amdgpu_parse_asic_ids: Cannot parse ASIC IDs: Resource temporarily unavailable

error messages in that case.

Reported-by: Marek Olšák 
Signed-off-by: Michel Dänzer 
---
 amdgpu/amdgpu_asic_id.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
index 0c8925e5..62459c09 100644
--- a/amdgpu/amdgpu_asic_id.c
+++ b/amdgpu/amdgpu_asic_id.c
@@ -155,7 +155,7 @@ void amdgpu_parse_asic_ids(struct amdgpu_device *dev)
if (r == -EINVAL) {
fprintf(stderr, "Invalid format: %s: line %d: %s\n",
AMDGPU_ASIC_ID_TABLE, line_num, line);
-   } else if (r) {
+   } else if (r && r != -EAGAIN) {
fprintf(stderr, "%s: Cannot parse ASIC IDs: %s\n",
__func__, strerror(-r));
}
-- 
2.15.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 1/2] drm/amd/pp: Add memory clock info display on Cz/St

2018-01-08 Thread Rex Zhu
show mclk info as in MHz on Cz/St as
0: 333Mhz *
1: 800Mhz

Change-Id: Ie5932ac81b15565edb154ec6c00b35a99ab52b73
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
index b83fe97..f68dd08 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
@@ -1584,6 +1584,7 @@ static int cz_force_clock_level(struct pp_hwmgr *hwmgr,
 static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
enum pp_clock_type type, char *buf)
 {
+   struct cz_hwmgr *data = (struct cz_hwmgr *)(hwmgr->backend);
struct phm_clock_voltage_dependency_table *sclk_table =
hwmgr->dyn_state.vddc_dependency_on_sclk;
int i, now, size = 0;
@@ -1601,6 +1602,18 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
i, sclk_table->entries[i].clk / 100,
(i == now) ? "*" : "");
break;
+   case PP_MCLK:
+   now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
+   CGS_IND_REG__SMC,
+   ixTARGET_AND_CURRENT_PROFILE_INDEX),
+   TARGET_AND_CURRENT_PROFILE_INDEX,
+   CURR_MCLK_INDEX);
+
+   for (i = CZ_NUM_NBPMEMORYCLOCK; i > 0; i--)
+   size += sprintf(buf + size, "%d: %uMhz %s\n",
+   CZ_NUM_NBPMEMORYCLOCK-i, 
data->sys_info.nbp_memory_clock[i-1] / 100,
+   (CZ_NUM_NBPMEMORYCLOCK-i == now) ? "*" 
: "");
+   break;
default:
break;
}
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 2/2] drm/amd/pp: Add stable Pstate clk display when print_clock_levels

2018-01-08 Thread Rex Zhu
The additional output are at the end of sclk/mclk info as
cat pp_dpm_mclk
0: 300Mhz *
1: 1650Mhz
P: 300Mhz

Signed-off-by: Rex Zhu 

Change-Id: Idf8eeedb5d399d9ffb7de7a2fb2976c7fa7c01a8
---
 drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 2 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c | 2 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 2 ++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
index f68dd08..03dfba0 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
@@ -1601,6 +1601,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
size += sprintf(buf + size, "%d: %uMhz %s\n",
i, sclk_table->entries[i].clk / 100,
(i == now) ? "*" : "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_sclk/100);
break;
case PP_MCLK:
now = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device,
@@ -1613,6 +1614,7 @@ static int cz_print_clock_levels(struct pp_hwmgr *hwmgr,
size += sprintf(buf + size, "%d: %uMhz %s\n",
CZ_NUM_NBPMEMORYCLOCK-i, 
data->sys_info.nbp_memory_clock[i-1] / 100,
(CZ_NUM_NBPMEMORYCLOCK-i == now) ? "*" 
: "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_mclk/100);
break;
default:
break;
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
index 409a56b..88c6ad8 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.c
@@ -756,6 +756,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
data->gfx_max_freq_limit / 100,
((data->gfx_max_freq_limit / 100)
 == now) ? "*" : "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_sclk/100);
break;
case PP_MCLK:
PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr,
@@ -773,6 +774,7 @@ static int rv_print_clock_levels(struct pp_hwmgr *hwmgr,
mclk_table->entries[i].clk / 100,
((mclk_table->entries[i].clk / 100)
 == now) ? "*" : "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_mclk/100);
break;
default:
break;
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 72031bd..1bdcd86 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -4372,6 +4372,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
size += sprintf(buf + size, "%d: %uMhz %s\n",
i, sclk_table->dpm_levels[i].value / 
100,
(i == now) ? "*" : "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_sclk/100);
break;
case PP_MCLK:
smum_send_msg_to_smc(hwmgr, PPSMC_MSG_API_GetMclkFrequency);
@@ -4388,6 +4389,7 @@ static int smu7_print_clock_levels(struct pp_hwmgr *hwmgr,
size += sprintf(buf + size, "%d: %uMhz %s\n",
i, mclk_table->dpm_levels[i].value / 
100,
(i == now) ? "*" : "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_mclk/100);
break;
case PP_PCIE:
pcie_speed = smu7_get_current_pcie_speed(hwmgr);
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index cb35f4f..cab50fc 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4565,6 +4565,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
size += sprintf(buf + size, "%d: %uMhz %s\n",
i, sclk_table->dpm_levels[i].value / 
100,
(i == now) ? "*" : "");
+   size += sprintf(buf + size, "P: %uMhz\n", 
hwmgr->pstate_sclk/100);
break;
case PP_MCLK:
if (data->registry_data.mclk_dpm_key_disabled)
@@ -4583,6 +4584,7 @@ static int vega10_print_clock_levels(struct pp_hwmgr 
*hwmgr,
size += sprintf(buf + size, "%d: %uMhz %s\n",