[PATCH] drm/amd/powerplay: added vega20 overdrive support
Alex Deucher
alexdeucher at gmail.com
Thu Aug 30 15:24:51 UTC 2018
On Thu, Aug 30, 2018 at 1:56 AM Quan, Evan <Evan.Quan at amd.com> wrote:
>
> Hi Alex,
>
> OK, I got your concern. Then how about the followings?
>
> If user want to change the FMin and FMax, they just need to pass in the new clock values.
> E.g, "s 0 350 10" will update the Fmin to 350Mhz and increase its voltage by 10mV.
> "s 7 1000 -10" will update the Fmax to 1000Mhz and decrease voltage by 10mV.
Is there ever a case where you would want different values for
Fmin/Fmax and Freq1/Freq3? I'm not entirely sure how the voltage
curve and the min/max are related. If so, we should separate them and
use something like my earlier proposal where we separate the voltage
curve from the clk settings. Also it might be more consistent to make
the whole thing offset based rather than a mix of absolute values and
offsets. E.g., "s 0 50 10" would increase the fmin by 50 Mhz and
increase its voltage by 10 mV. "s 7 100 -10" would increase fmax by
100 Mhz and decrease voltage by 10mV. OD_RANGE would give the the
absolute clk and voltage ranges for each setting. If we went this
way, we'd need to also print the default settings as well, otherwise
you don't have a reference point. Maybe something like:
OD_SCLK:
0: 0Mhz 0mV (100Mhz 900mV)
4: 0Mhz 0mV (200Mhz 1000mV)
7: 0Mhz 0mV (300Mhz 1100mV)
OD_MCLK:
3: 0Mhz 0mV (500Mhz 1100mV)
...
So in this case, the first two items would be the offset you want to
apply and the last two items are the current absolute settings.
Alternatively would could use absolute values for everything and
convert to offsets for voltage in the driver. E.g.,
OD_SCLK:
0: 100Mhz 900mV
4: 200Mhz 1000mV
7: 300Mhz 1100mV
OD_MCLK:
3: 500Mhz 1100mV
...
Then it would work pretty much the same as the previous interface.
Alex
>
> Same for MCLK except the voltage offset passed in takes no effect.
>
> OD_SCLK:
> 0: 0Mhz 0mV
> 4: 0Mhz 0mV
> 7: 0Mhz 0mV
>
> OD_MCLK:
> 3: 0Mhz 0mV
>
> OD_RANGE:
> SCLK[0]: 0Mhz 0Mhz
> VDDGFX[0]: 0mV 0mV
> SCLK[4]: 0Mhz 0Mhz
> VDDGFX[4]: 0mV 0mV
> SCLK[7]: 0Mhz 0Mhz
> VDDGFX[7]: 0mV 0mV
>
> MCLK[3]: 0Mhz 0Mhz
> VDDMEM[3]: 0mV 0mV
>
>
> Regard,
> Evan
>
> > -----Original Message-----
> > From: Alex Deucher <alexdeucher at gmail.com>
> > Sent: 2018年8月30日 13:06
> > To: Quan, Evan <Evan.Quan at amd.com>
> > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Deucher, Alexander
> > <Alexander.Deucher at amd.com>; Zhu, Rex <Rex.Zhu at amd.com>
> > Subject: Re: [PATCH] drm/amd/powerplay: added vega20 overdrive support
> >
> > On Wed, Aug 29, 2018 at 11:54 PM Quan, Evan <Evan.Quan at amd.com>
> > wrote:
> > >
> > > Hi Alex,
> > >
> > >
> > > >What about the min/max sclk? Do the curve values take that into
> > > >account? What about max mclk?
> > >
> > > Voltage curve does not take these into consideration. And the max sclk and
> > mclk can be set through existing sysfs interface pp_sclk_od and pp_mclk_od.
> > For min sclk, as i know there is no interface to get it set.
> > >
> >
> > I'd prefer to allow adjusting min and max clocks via this interface for
> > consistency with vega10 and smu7. I'd like to deprecate the pp_sclk_od and
> > pp_mclk_od interfaces because the are percentage based and don't support
> > underclocking.
> >
> > Alex
> >
> > >
> > > > Are negative offsets supported?
> > > Sure.
> > >
> > >
> > > Regards,
> > >
> > > Evan
> > >
> > >
> > > ________________________________
> > > From: Alex Deucher <alexdeucher at gmail.com>
> > > Sent: Thursday, August 30, 2018 12:25:35 AM
> > > To: Quan, Evan
> > > Cc: amd-gfx list; Deucher, Alexander; Zhu, Rex
> > > Subject: Re: [PATCH] drm/amd/powerplay: added vega20 overdrive
> > support
> > >
> > > On Wed, Aug 29, 2018 at 5:13 AM Evan Quan <evan.quan at amd.com>
> > wrote:
> > > >
> > > > Vega20 supports only sclk voltage overdrive. And user can only tell
> > > > three groups of <frequency, voltage_offset>. SMU firmware will
> > > > recalculate the frequency/voltage curve. Other intermediate levles
> > > > will be stretched/shrunk accordingly.
> > > >
> > >
> > > What about the min/max sclk? Do the curve values take that into
> > > account? What about max mclk?
> > >
> > > > Change-Id: I403cb38a95863db664cf06d030ac42a19bff6b33
> > > > Signed-off-by: Evan Quan <evan.quan at amd.com>
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 26 +++
> > > > .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 165
> > +++++++++++++++++-
> > > > .../drm/amd/powerplay/hwmgr/vega20_hwmgr.h | 2 +
> > > > 3 files changed, 192 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > > index e2577518b9c6..6e0c8f583521 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > > @@ -474,6 +474,8 @@ static ssize_t amdgpu_set_pp_table(struct device
> > *dev,
> > > > * in each power level within a power state. The pp_od_clk_voltage is
> > used for
> > > > * this.
> > > > *
> > > > + * < For Vega10 and previous ASICs >
> > > > + *
> > > > * Reading the file will display:
> > > > *
> > > > * - a list of engine clock levels and voltages labeled OD_SCLK @@
> > > > -491,6 +493,30 @@ static ssize_t amdgpu_set_pp_table(struct device
> > *dev,
> > > > * "c" (commit) to the file to commit your changes. If you want to reset
> > to the
> > > > * default power levels, write "r" (reset) to the file to reset them.
> > > > *
> > > > + *
> > > > + * < For Vega20 >
> > > > + *
> > > > + * Reading the file will display:
> > > > + *
> > > > + * - three groups of engine clock and voltage offset labeled
> > > > + OD_SCLK
> > > > + *
> > > > + * - a list of valid ranges for above three groups of sclk and voltage offset
> > > > + * labeled OD_RANGE
> > > > + *
> > > > + * To manually adjust these settings:
> > > > + *
> > > > + * - First select manual using power_dpm_force_performance_level
> > > > + *
> > > > + * - Enter a new value for each group by writing a string that contains
> > > > + * "s group_index clock voltage_offset" to the file. E.g., "s 0 500 20"
> > > > + * will update group1 sclk to be 500 MHz with voltage increased 20mV
> > > > + *
> > >
> > > Are negative offsets supported?
> > >
> > > Alex
> > >
> > > > + * - When you have edited all of the states as needed, write "c" (commit)
> > > > + * to the file to commit your changes
> > > > + *
> > > > + * - If you want to reset to the default power levels, write "r" (reset)
> > > > + * to the file to reset them
> > > > + *
> > > > */
> > > >
> > > > static ssize_t amdgpu_set_pp_od_clk_voltage(struct device *dev,
> > > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > > index ececa2f7fe5f..9f6e070a76e0 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > > @@ -1096,6 +1096,13 @@ static int
> > vega20_od8_initialize_default_settings(
> > > > }
> > > > }
> > > >
> > > > + ret = vega20_copy_table_from_smc(hwmgr,
> > > > + (uint8_t *)(&data->od_table),
> > > > + TABLE_OVERDRIVE);
> > > > + PP_ASSERT_WITH_CODE(!ret,
> > > > + "Failed to export over drive table!",
> > > > + return ret);
> > > > +
> > > > return 0;
> > > > }
> > > >
> > > > @@ -2506,11 +2513,112 @@ static int
> > vega20_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
> > > > return 0;
> > > > }
> > > >
> > > > +static int vega20_odn_edit_dpm_table(struct pp_hwmgr *hwmgr,
> > > > + enum PP_OD_DPM_TABLE_COMMAND type,
> > > > + long *input, uint32_t size)
> > > > +{
> > > > + struct vega20_hwmgr *data =
> > > > + (struct vega20_hwmgr *)(hwmgr->backend);
> > > > + struct vega20_od8_single_setting *od8_settings =
> > > > + data->od8_settings.od8_settings_array;
> > > > + OverDriveTable_t od_table;
> > > > + int32_t input_index, input_clk, input_vol, i;
> > > > + int ret = 0;
> > > > +
> > > > + PP_ASSERT_WITH_CODE(input, "NULL user input for clock and
> > voltage",
> > > > + return -EINVAL);
> > > > +
> > > > + switch (type) {
> > > > + case PP_OD_EDIT_SCLK_VDDC_TABLE:
> > > > + if (!(od8_settings[OD8_SETTING_GFXCLK_FREQ1].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ2].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ3].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].feature_id
> > &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].feature_id
> > &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].feature_id))
> > {
> > > > + pr_info("Sclk voltage overdrive not supported\n");
> > > > + return 0;
> > > > + }
> > > > +
> > > > + for (i = 0; i < size; i += 3) {
> > > > + if (i + 3 > size) {
> > > > + pr_info("invalid clock voltage input\n");
> > > > + return 0;
> > > > + }
> > > > +
> > > > + input_index = input[i];
> > > > + input_clk = input[i + 1];
> > > > + input_vol = input[i + 2];
> > > > + if (input_index > 2 ||
> > > > + input_clk < od8_settings[OD8_SETTING_GFXCLK_FREQ1 +
> > input_index * 2].min_value ||
> > > > + input_clk > od8_settings[OD8_SETTING_GFXCLK_FREQ1 +
> > input_index * 2].max_value ||
> > > > + input_vol <
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1 + input_index *
> > 2].min_value ||
> > > > + input_vol >
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1 + input_index *
> > 2].max_value) {
> > > > + pr_info("input argument is not within allowed range\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + switch (input_index) {
> > > > + case 0:
> > > > + data->od_table.GfxclkFreq1 = input_clk;
> > > > + data->od_table.GfxclkOffsetVolt1 = input_vol;
> > > > + break;
> > > > + case 1:
> > > > + data->od_table.GfxclkFreq2 = input_clk;
> > > > + data->od_table.GfxclkOffsetVolt2 = input_vol;
> > > > + break;
> > > > + case 2:
> > > > + data->od_table.GfxclkFreq3 = input_clk;
> > > > + data->od_table.GfxclkOffsetVolt3 = input_vol;
> > > > + break;
> > > > + }
> > > > + }
> > > > + break;
> > > > +
> > > > + case PP_OD_EDIT_MCLK_VDDC_TABLE:
> > > > + pr_info("Mclk voltage overdrive not supported!\n");
> > > > + break;
> > > > +
> > > > + case PP_OD_RESTORE_DEFAULT_TABLE:
> > > > + ret = vega20_copy_table_from_smc(hwmgr,
> > > > + (uint8_t *)(&od_table),
> > > > + TABLE_OVERDRIVE);
> > > > + PP_ASSERT_WITH_CODE(!ret,
> > > > + "Failed to export over drive table!",
> > > > + return ret);
> > > > +
> > > > + memcpy(&data->od_table, &od_table, sizeof(od_table));
> > > > + break;
> > > > +
> > > > + case PP_OD_COMMIT_DPM_TABLE:
> > > > + memcpy(&od_table, &data->od_table,
> > > > + sizeof(od_table));
> > > > +
> > > > + ret = vega20_copy_table_to_smc(hwmgr,
> > > > + (uint8_t *)(&od_table),
> > > > + TABLE_OVERDRIVE);
> > > > + PP_ASSERT_WITH_CODE(!ret,
> > > > + "Failed to import over drive table!",
> > > > + return ret);
> > > > +
> > > > + break;
> > > > +
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > static int vega20_print_clock_levels(struct pp_hwmgr *hwmgr,
> > > > enum pp_clock_type type, char *buf) {
> > > > - int i, now, size = 0;
> > > > + struct vega20_hwmgr *data =
> > > > + (struct vega20_hwmgr *)(hwmgr->backend);
> > > > + struct vega20_od8_single_setting *od8_settings =
> > > > + data->od8_settings.od8_settings_array;
> > > > + OverDriveTable_t *od_table = &data->od_table;
> > > > struct pp_clock_levels_with_latency clocks;
> > > > + int i, now, size = 0;
> > > > int ret = 0;
> > > >
> > > > switch (type) {
> > > > @@ -2551,6 +2659,59 @@ static int vega20_print_clock_levels(struct
> > pp_hwmgr *hwmgr,
> > > > case PP_PCIE:
> > > > break;
> > > >
> > > > + case OD_SCLK:
> > > > + if (od8_settings[OD8_SETTING_GFXCLK_FREQ1].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ2].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ3].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].feature_id
> > &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].feature_id
> > &&
> > > > +
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].feature_id) {
> > > > +
> > > > + size = sprintf(buf, "%s:\n", "OD_SCLK");
> > > > + size += sprintf(buf + size, "0: %10uMhz %10dmV\n",
> > > > + od_table->GfxclkFreq1,
> > > > + od_table->GfxclkOffsetVolt1);
> > > > + size += sprintf(buf + size, "1: %10uMhz %10dmV\n",
> > > > + od_table->GfxclkFreq2,
> > > > + od_table->GfxclkOffsetVolt2);
> > > > + size += sprintf(buf + size, "2: %10uMhz %10dmV\n",
> > > > + od_table->GfxclkFreq3,
> > > > + od_table->GfxclkOffsetVolt3);
> > > > + }
> > > > + break;
> > > > +
> > > > + case OD_MCLK:
> > > > + break;
> > > > +
> > > > + case OD_RANGE:
> > > > + if (od8_settings[OD8_SETTING_GFXCLK_FREQ1].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ2].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ3].feature_id &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].feature_id
> > &&
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].feature_id
> > &&
> > > > +
> > > > + od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].feature_id) {
> > > > +
> > > > + size = sprintf(buf, "%s:\n", "OD_RANGE");
> > > > + size += sprintf(buf + size, "SCLK[0]: %7uMhz %10uMhz\n",
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ1].min_value,
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_FREQ1].max_value);
> > > > + size += sprintf(buf + size, "VDDC[0]: %7dmV %11dmV\n",
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].min_value,
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].max_value);
> > > > + size += sprintf(buf + size, "SCLK[1]: %7uMhz %10uMhz\n",
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ2].min_value,
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_FREQ2].max_value);
> > > > + size += sprintf(buf + size, "VDDC[1]: %7dmV %11dmV\n",
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].min_value,
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].max_value);
> > > > + size += sprintf(buf + size, "SCLK[2]: %7uMhz %10uMhz\n",
> > > > + od8_settings[OD8_SETTING_GFXCLK_FREQ3].min_value,
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_FREQ3].max_value);
> > > > + size += sprintf(buf + size, "VDDC[2]: %7dmV %11dmV\n",
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].min_value,
> > > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].max_value);
> > > > + }
> > > > + break;
> > > > default:
> > > > break;
> > > > }
> > > > @@ -3162,6 +3323,8 @@ static const struct pp_hwmgr_func
> > vega20_hwmgr_funcs = {
> > > > vega20_get_mclk_od,
> > > > .set_mclk_od =
> > > > vega20_set_mclk_od,
> > > > + .odn_edit_dpm_table =
> > > > + vega20_odn_edit_dpm_table,
> > > > /* for sysfs to retrive/set gfxclk/memclk */
> > > > .force_clock_level =
> > > > vega20_force_clock_level, diff --git
> > > > a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> > > > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> > > > index 72e4f2a55641..8bb90e22efb6 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> > > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.h
> > > > @@ -513,6 +513,8 @@ struct vega20_hwmgr {
> > > > /* ---- Gfxoff ---- */
> > > > bool gfxoff_allowed;
> > > > uint32_t counter_gfxoff;
> > > > +
> > > > + OverDriveTable_t od_table;
> > > > };
> > > >
> > > > #define VEGA20_DPM2_NEAR_TDP_DEC 10
> > > > --
> > > > 2.18.0
> > > >
> > > > _______________________________________________
> > > > amd-gfx mailing list
> > > > amd-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list