RE: [PATCH 37/40] drm/amd/pm: enable Polaris watermark table setting
[AMD Official Use Only - Internal Distribution Only]
Hi Christian,
Just sent out a patch which should be able to address this issue you saw.
https://lists.freedesktop.org/archives/amd-gfx/2020-October/055431.html
BR
Evan
-Original Message-
From: Christian König
Sent: Thursday, October 29, 2020 5:46 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: Re: [PATCH 37/40] drm/amd/pm: enable Polaris watermark table setting
Hi guys,
this commit here completely breaks Vega20.
Any idea why a change for Polaris should have that effect? Can we please fix
this ASAP?
Thanks,
Christian.
Am 16.10.20 um 05:26 schrieb Evan Quan:
> Enable watermark table setting for Polaris.
>
> Change-Id: I395b74f2ce5b74e6d1c7659816ee386ba556e14c
> Signed-off-by: Evan Quan
> Acked-by: Alex Deucher
> ---
> .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 11 +++-
> .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 50 +++
> 2 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> index deb9164eb5fe..fd39dd67bfa4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> @@ -458,7 +458,16 @@ bool dm_pp_notify_wm_clock_changes(
> const struct dc_context *ctx,
> struct dm_pp_wm_sets_with_clock_ranges *wm_with_clock_ranges)
> {
> -/* TODO: to be implemented */
> +struct amdgpu_device *adev = ctx->driver_context;
> +void *pp_handle = adev->powerplay.pp_handle;
> +const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> +
> +if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
> +if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
> +(void *)wm_with_clock_ranges))
> +return true;
> +}
> +
> return false;
> }
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> index 3700352286c5..ce8f368c0a8c 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> @@ -49,6 +49,8 @@
> #include "processpptables.h"
> #include "pp_thermal.h"
> #include "smu7_baco.h"
> +#include "smu7_smumgr.h"
> +#include "polaris10_smumgr.h"
>
> #include "ivsrcid/ivsrcid_vislands30.h"
>
> @@ -5107,6 +5109,53 @@ static int smu7_get_clock_by_type_with_latency(struct
> pp_hwmgr *hwmgr,
> return 0;
> }
>
> +static int smu7_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
> + void *clock_range)
> +{
> +struct phm_ppt_v1_information *table_info =
> +(struct phm_ppt_v1_information *)hwmgr->pptable;
> +struct phm_ppt_v1_clock_voltage_dependency_table *dep_mclk_table =
> +table_info->vdd_dep_on_mclk;
> +struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table =
> +table_info->vdd_dep_on_sclk;
> +struct polaris10_smumgr *smu_data =
> +(struct polaris10_smumgr *)(hwmgr->smu_backend);
> +SMU74_Discrete_DpmTable *table = &(smu_data->smc_state_table);
> +struct dm_pp_wm_sets_with_clock_ranges *watermarks =
> +(struct dm_pp_wm_sets_with_clock_ranges *)clock_range;
> +uint32_t i, j, k;
> +bool valid_entry;
> +
> +if (!(hwmgr->chip_id >= CHIP_POLARIS10 &&
> + hwmgr->chip_id <= CHIP_VEGAM))
> +return -EINVAL;
> +
> +for (i = 0; i < dep_mclk_table->count; i++) {
> +for (j = 0; j < dep_sclk_table->count; j++) {
> +valid_entry = false;
> +for (k = 0; k < watermarks->num_wm_sets; k++) {
> +if (dep_sclk_table->entries[i].clk / 10 >=
> watermarks->wm_clk_ranges[k].wm_min_eng_clk_in_khz &&
> +dep_sclk_table->entries[i].clk / 10 <
> watermarks->wm_clk_ranges[k].wm_max_eng_clk_in_khz &&
> +dep_mclk_table->entries[i].clk / 10 >=
> watermarks->wm_clk_ranges[k].wm_min_mem_clk_in_khz &&
> +dep_mclk_table->entries[i].clk / 10 <
> watermarks->wm_clk_ranges[k].wm_max_mem_clk_in_khz) {
> +valid_entry = true;
> +table->DisplayWatermark[i][j] = watermarks->wm_clk_ranges[k].wm_set_id;
> +break;
> +}
> +}
> +PP_ASSERT_WITH_CODE(valid_entry,
> +"Clock is not in range of specified clock range for watermark from DAL!
> Using highest water mark set.",
> +table->DisplayWatermark[i][j] = watermarks->wm_clk_ranges[k - 1].wm_set_id);
> +}
> +}
> +
> +return smu7_copy_bytes_to_smc(hwmgr,
> + smu_data->smu7_data.dpm_table_start +
> offsetof(SMU74_Discrete_DpmTable, DisplayWatermark),
Re: [PATCH 37/40] drm/amd/pm: enable Polaris watermark table setting
On Thu, Oct 29, 2020 at 5:46 AM Christian König
wrote:
>
> Hi guys,
>
> this commit here completely breaks Vega20.
>
> Any idea why a change for Polaris should have that effect? Can we please
> fix this ASAP?
>
> Thanks,
> Christian.
>
> Am 16.10.20 um 05:26 schrieb Evan Quan:
> > Enable watermark table setting for Polaris.
> >
> > Change-Id: I395b74f2ce5b74e6d1c7659816ee386ba556e14c
> > Signed-off-by: Evan Quan
> > Acked-by: Alex Deucher
> > ---
> > .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 11 +++-
> > .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 50 +++
> > 2 files changed, 60 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > index deb9164eb5fe..fd39dd67bfa4 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> > @@ -458,7 +458,16 @@ bool dm_pp_notify_wm_clock_changes(
> > const struct dc_context *ctx,
> > struct dm_pp_wm_sets_with_clock_ranges *wm_with_clock_ranges)
> > {
> > - /* TODO: to be implemented */
> > + struct amdgpu_device *adev = ctx->driver_context;
> > + void *pp_handle = adev->powerplay.pp_handle;
> > + const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> > +
> > + if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
> > + if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
> > + (void *)wm_with_clock_ranges))
> > + return true;
> > + }
> > +
I suspect it's this hunk since it's common. I'd guess there is some
problem with that functionality on vega20.
Alex
> > return false;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> > index 3700352286c5..ce8f368c0a8c 100644
> > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
> > @@ -49,6 +49,8 @@
> > #include "processpptables.h"
> > #include "pp_thermal.h"
> > #include "smu7_baco.h"
> > +#include "smu7_smumgr.h"
> > +#include "polaris10_smumgr.h"
> >
> > #include "ivsrcid/ivsrcid_vislands30.h"
> >
> > @@ -5107,6 +5109,53 @@ static int
> > smu7_get_clock_by_type_with_latency(struct pp_hwmgr *hwmgr,
> > return 0;
> > }
> >
> > +static int smu7_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
> > + void *clock_range)
> > +{
> > + struct phm_ppt_v1_information *table_info =
> > + (struct phm_ppt_v1_information *)hwmgr->pptable;
> > + struct phm_ppt_v1_clock_voltage_dependency_table *dep_mclk_table =
> > + table_info->vdd_dep_on_mclk;
> > + struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table =
> > + table_info->vdd_dep_on_sclk;
> > + struct polaris10_smumgr *smu_data =
> > + (struct polaris10_smumgr *)(hwmgr->smu_backend);
> > + SMU74_Discrete_DpmTable *table = &(smu_data->smc_state_table);
> > + struct dm_pp_wm_sets_with_clock_ranges *watermarks =
> > + (struct dm_pp_wm_sets_with_clock_ranges *)clock_range;
> > + uint32_t i, j, k;
> > + bool valid_entry;
> > +
> > + if (!(hwmgr->chip_id >= CHIP_POLARIS10 &&
> > + hwmgr->chip_id <= CHIP_VEGAM))
> > + return -EINVAL;
> > +
> > + for (i = 0; i < dep_mclk_table->count; i++) {
> > + for (j = 0; j < dep_sclk_table->count; j++) {
> > + valid_entry = false;
> > + for (k = 0; k < watermarks->num_wm_sets; k++) {
> > + if (dep_sclk_table->entries[i].clk / 10 >=
> > watermarks->wm_clk_ranges[k].wm_min_eng_clk_in_khz &&
> > + dep_sclk_table->entries[i].clk / 10 <
> > watermarks->wm_clk_ranges[k].wm_max_eng_clk_in_khz &&
> > + dep_mclk_table->entries[i].clk / 10 >=
> > watermarks->wm_clk_ranges[k].wm_min_mem_clk_in_khz &&
> > + dep_mclk_table->entries[i].clk / 10 <
> > watermarks->wm_clk_ranges[k].wm_max_mem_clk_in_khz) {
> > + valid_entry = true;
> > + table->DisplayWatermark[i][j] =
> > watermarks->wm_clk_ranges[k].wm_set_id;
> > + break;
> > + }
> > + }
> > + PP_ASSERT_WITH_CODE(valid_entry,
> > + "Clock is not in range of specified
> > clock range for watermark from DAL! Using highest water mark set.",
> > + table->DisplayWatermark[i][j] =
> > watermarks->wm_clk_ranges[k - 1].wm_set_id);
> > +
Re: [PATCH 37/40] drm/amd/pm: enable Polaris watermark table setting
Hi guys,
this commit here completely breaks Vega20.
Any idea why a change for Polaris should have that effect? Can we please
fix this ASAP?
Thanks,
Christian.
Am 16.10.20 um 05:26 schrieb Evan Quan:
Enable watermark table setting for Polaris.
Change-Id: I395b74f2ce5b74e6d1c7659816ee386ba556e14c
Signed-off-by: Evan Quan
Acked-by: Alex Deucher
---
.../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 11 +++-
.../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 50 +++
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index deb9164eb5fe..fd39dd67bfa4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -458,7 +458,16 @@ bool dm_pp_notify_wm_clock_changes(
const struct dc_context *ctx,
struct dm_pp_wm_sets_with_clock_ranges *wm_with_clock_ranges)
{
- /* TODO: to be implemented */
+ struct amdgpu_device *adev = ctx->driver_context;
+ void *pp_handle = adev->powerplay.pp_handle;
+ const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
+
+ if (pp_funcs && pp_funcs->set_watermarks_for_clocks_ranges) {
+ if (!pp_funcs->set_watermarks_for_clocks_ranges(pp_handle,
+ (void *)wm_with_clock_ranges))
+ return true;
+ }
+
return false;
}
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index 3700352286c5..ce8f368c0a8c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -49,6 +49,8 @@
#include "processpptables.h"
#include "pp_thermal.h"
#include "smu7_baco.h"
+#include "smu7_smumgr.h"
+#include "polaris10_smumgr.h"
#include "ivsrcid/ivsrcid_vislands30.h"
@@ -5107,6 +5109,53 @@ static int smu7_get_clock_by_type_with_latency(struct pp_hwmgr *hwmgr,
return 0;
}
+static int smu7_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
+void *clock_range)
+{
+ struct phm_ppt_v1_information *table_info =
+ (struct phm_ppt_v1_information *)hwmgr->pptable;
+ struct phm_ppt_v1_clock_voltage_dependency_table *dep_mclk_table =
+ table_info->vdd_dep_on_mclk;
+ struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table =
+ table_info->vdd_dep_on_sclk;
+ struct polaris10_smumgr *smu_data =
+ (struct polaris10_smumgr *)(hwmgr->smu_backend);
+ SMU74_Discrete_DpmTable *table = &(smu_data->smc_state_table);
+ struct dm_pp_wm_sets_with_clock_ranges *watermarks =
+ (struct dm_pp_wm_sets_with_clock_ranges *)clock_range;
+ uint32_t i, j, k;
+ bool valid_entry;
+
+ if (!(hwmgr->chip_id >= CHIP_POLARIS10 &&
+ hwmgr->chip_id <= CHIP_VEGAM))
+ return -EINVAL;
+
+ for (i = 0; i < dep_mclk_table->count; i++) {
+ for (j = 0; j < dep_sclk_table->count; j++) {
+ valid_entry = false;
+ for (k = 0; k < watermarks->num_wm_sets; k++) {
+ if (dep_sclk_table->entries[i].clk / 10 >=
watermarks->wm_clk_ranges[k].wm_min_eng_clk_in_khz &&
+ dep_sclk_table->entries[i].clk / 10 <
watermarks->wm_clk_ranges[k].wm_max_eng_clk_in_khz &&
+ dep_mclk_table->entries[i].clk / 10 >=
watermarks->wm_clk_ranges[k].wm_min_mem_clk_in_khz &&
+ dep_mclk_table->entries[i].clk / 10 <
watermarks->wm_clk_ranges[k].wm_max_mem_clk_in_khz) {
+ valid_entry = true;
+ table->DisplayWatermark[i][j] =
watermarks->wm_clk_ranges[k].wm_set_id;
+ break;
+ }
+ }
+ PP_ASSERT_WITH_CODE(valid_entry,
+ "Clock is not in range of specified clock
range for watermark from DAL! Using highest water mark set.",
+ table->DisplayWatermark[i][j] =
watermarks->wm_clk_ranges[k - 1].wm_set_id);
+ }
+ }
+
+ return smu7_copy_bytes_to_smc(hwmgr,
+ smu_data->smu7_data.dpm_table_start +
offsetof(SMU74_Discrete_DpmTable, DisplayWatermark),
+ (uint8_t *)table->DisplayWatermark,
+ sizeof(uint8_t) * SMU74_MAX_LEVELS_MEMORY
* SMU74_MAX_LEVELS_GRAPHICS,
+ SMC_RAM_END);
+}
+
static int
