[PATCH] drm/amd/powerplay: expose vega20 OD features
Quan, Evan
Evan.Quan at amd.com
Tue Aug 28 15:51:16 UTC 2018
Hi Alex,
Sorry, i cannot get your point.
The OD feature sysfs file used by previous ASICs is "pp_od_clk_voltage". And it binds to two APIs "amdgpu_get_pp_od_clk_voltage" and "amdgpu_set_pp_od_clk_voltage".
static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
amdgpu_get_pp_od_clk_voltage,
amdgpu_set_pp_od_clk_voltage);
I was saying we cannot reuse these two APIs for vega20 since we have no way to know the exact voltage value for each dpm level(that is what these two APIs required). Thus we cannot share the same sysfs file.
Do you mean we can bind the sysfs file to other APIs? How?
Regards,
Evan
________________________________
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Monday, August 27, 2018 10:01:49 PM
To: Quan, Evan
Cc: Zhu, Rex; Deucher, Alexander; amd-gfx list; Xu, Feifei; Kuehling, Felix; Zhang, Hawking
Subject: Re: [PATCH] drm/amd/powerplay: expose vega20 OD features
On Mon, Aug 27, 2018 at 12:04 AM Quan, Evan <Evan.Quan at amd.com> wrote:
>
> Hi @Deucher, Alexander & @Zhu, Rex,
>
>
> I can remove the od setings for power limit and fan speed related since they seem redundant with other sysfs apis.
>
> But for the clock/voltage related, i can not see how to reuse old pp_od_clk_voltage APIs.
>
>
> For the old pp_od_clk_voltage APIs,
>
> 1. they output/input the voltages in their exact values. E.g. "s 1 500 820" will update sclk level 1 to be 500 MHz at 820 mV
>
> But for vega20, we did not know its original voltage value. The output/input for voltage are just offest from its original value.
>
> So, if to reuse these APIs, we will make them have two different explanations(
>
> on vega20 or later, the voltage is offset. On previous ASICs, the voltage is exact value). I think that will confuse the users.
>
>
> 2. they support voltage set for each level. But for vega20, it supports only three pairs of clock/voltage set. And per suggestion,
>
> these three pairs should be: voltage offset of minimum clock level, voltage offset for middle clock level and voltage offset
>
> for maximum clock level.
>
>
> Also, i believe the future ASICs will also take the vega20 OD ways(that is we need the offset, not the exact value for voltage).
>
>
> So, based on previous considerations, i decide to have new APIs instead of reusing existing ones.
I'm not saying to use the same API. Use the new API, but expose the
API using the same sysfs file rather than adding a new one. Same way
we handle the power profiles; E.g., the API is different between smu7
and vega10, but they both use the same sysfs file.
Alex
>
>
> Regards,
>
> Evan
>
> ________________________________
> From: Zhu, Rex
> Sent: Saturday, August 25, 2018 12:32 AM
> To: Alex Deucher; Quan, Evan
> Cc: amd-gfx list; Xu, Feifei; Kuehling, Felix; Deucher, Alexander; Zhang, Hawking
> Subject: RE: [PATCH] drm/amd/powerplay: expose vega20 OD features
>
>
>
> > -----Original Message-----
> > From: Alex Deucher <alexdeucher at gmail.com>
> > Sent: Friday, August 24, 2018 11:48 PM
> > To: Quan, Evan <Evan.Quan at amd.com>
> > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Xu, Feifei
> > <Feifei.Xu at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>; Deucher,
> > Alexander <Alexander.Deucher at amd.com>; Zhu, Rex <Rex.Zhu at amd.com>;
> > Zhang, Hawking <Hawking.Zhang at amd.com>
> > Subject: Re: [PATCH] drm/amd/powerplay: expose vega20 OD features
> >
> > On Fri, Aug 24, 2018 at 3:45 AM Evan Quan <evan.quan at amd.com> wrote:
> > >
> > > Vega20 simplifies the OD logics and it can not fit old OD interfaces.
> > > Thus we design new OD interfaces for vega20.
> >
> > Please split this into two patches, one to add the internal od8_settings API,
> > and one to wire it up to sysfs. A few more comments below.
> >
> > >
> > > Change-Id: I888faec46a81287ae24f452ce16b42c1f6d06d7d
> > > Signed-off-by: Evan Quan <evan.quan at amd.com>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h | 8 +
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 125 ++++++++++++
> > > .../gpu/drm/amd/include/kgd_pp_interface.h | 2 +
> > > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 37 ++++
> > > .../drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 191
> > +++++++++++++++++-
> > > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 2 +
> > > 6 files changed, 362 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> > > index ff24e1cc5b65..84b3e6f87abf 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> > > @@ -357,6 +357,14 @@ enum amdgpu_pcie_gen {
> > > ((adev)->powerplay.pp_funcs->odn_edit_dpm_table(\
> > > (adev)->powerplay.pp_handle, type, parameter,
> > > size))
> > >
> > > +#define amdgpu_dpm_get_od8_settings(adev, buf) \
> > > + ((adev)->powerplay.pp_funcs->get_od8_settings(\
> > > + (adev)->powerplay.pp_handle, buf))
> > > +
> > > +#define amdgpu_dpm_set_od8_settings(adev, parameter, size) \
> > > + ((adev)->powerplay.pp_funcs->set_od8_settings(\
> > > + (adev)->powerplay.pp_handle, parameter, size))
> > > +
> > > struct amdgpu_dpm {
> > > struct amdgpu_ps *ps;
> > > /* number of valid power states */ diff --git
> > > a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > index daa55fb06171..94cd7c503372 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> > > @@ -934,6 +934,121 @@ static ssize_t amdgpu_get_busy_percent(struct
> > device *dev,
> > > return snprintf(buf, PAGE_SIZE, "%d\n", value); }
> > >
> > > +/**
> > > + * DOC: pp_od8_settings
> > > + *
> > > + * The amdgpu driver provides a sysfs API for adjusting the clocks,
> > > +voltages,
> > > + * power limit, fan speed and temperature. The pp_od8_settings is
> > > +used for
> > > + * this.
> > > + *
> > > + * Reading the file will display:
> > > + *
> > > + * - a name list of the features that are able to be adjusted
> > > + *
> > > + * - the mininum and maximum allowed value for each supported
> > > + * feature labeled in format of "[mininum - maximum]"
> > > + *
> > > + * - the current value for each supported feature labeled after
> > > + * ":"
> > > + *
> > > + * To manually adjust these settings:
> > > + *
> > > + * - write a string that contains the new value for each supported
> > > + * feature. For those which do not need to be changed, just enter
> > > + * their old value
> > > + *
> > > + * - make sure the new value is within the allowed ranges between
> > > + * the minimum and maximum
> >
> > We should probably also make it a set and commit model like we do for
> > previous asics for consistency. That way you can see the changes, and then
> > apply them.
> >
> > > + *
> > > + * All the possible supported features:
> > > + *
> > > + * - Gfx clock
> > > + * The min and max frequency can be specified by GfxclkFmin
> > > + * and GfxclkFmax(both in Mhz). Intermediate levels are
> > > + * stretched/shrunk according to ratio in original table.
> > > + *
> > > + * - Gfx voltage
> > > + * With three pairs of gfx clk(in Mhz) and offset voltage(in mV)
> > > + * combinations specified, smu fw can calibrate the voltage
> > > + * curve automatically.
> > > + * - GfxclkFreq1 <-> GfxclkOffsetVolt1
> > > + * - GfxclkFreq2 <-> GfxclkOffsetVolt2
> > > + * - GfxclkFreq3 <-> GfxclkOffsetVolt3
> > > + *
> > > + * - Uclk
> > > + * The max memory clock can be specified by UclkFmax(in Mhz).
> > > + *
> > > + * - Power limit
> > > + * The power limit can be increased or descreased by
> > > + * OverDrivePct(in percent).
> >
> > We already expose the power limit via hwmon. Do we still need it here?
> >
> > > + *
> > > + * - Fan spped
> >
> > spped -> speed
> >
> > > + * The max fan speed can be set by FanMaxinumRpm and the
> >
> > FanMaxinumRpm -> FanMaximumRpm
> >
> > > + * min fan speed by FanMinimumPwm. Also zero rpm enablement
> > > + * is specified by FanZeroRpmEnable.
> > > + *
> > > + * - Temperature
> > > + * The fan target temperature can be set by FanTargetTemperature
> > > + * and max Tj temperature by MaxOpTemp.
> >
> > The fan and temp stuff might be a better fit for the hwmon interfaces:
> > https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface
> >
> > > + *
> > > + */
> > > +
> > > +static ssize_t amdgpu_get_pp_od8_settings(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + struct drm_device *ddev = dev_get_drvdata(dev);
> > > + struct amdgpu_device *adev = ddev->dev_private;
> > > +
> > > + if (adev->powerplay.pp_funcs->get_od8_settings)
> > > + return amdgpu_dpm_get_od8_settings(adev, buf);
> > > +
> > > + return snprintf(buf, PAGE_SIZE, "\n"); }
> > > +
> > > +static ssize_t amdgpu_set_pp_od8_settings(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf,
> > > + size_t count)
> > > +{
> > > + struct drm_device *ddev = dev_get_drvdata(dev);
> > > + struct amdgpu_device *adev = ddev->dev_private;
> > > + char *sub_str, *tmp_str, buf_cpy[128];
> > > + const char delimiter[3] = {' ', '\n', '\0'};
> > > + uint32_t parameter_size = 0;
> > > + long parameter[64];
> > > + int ret = 0xff;
> > > +
> > > + if (count >= sizeof(buf_cpy) / sizeof(buf_cpy[0]))
> > > + return -EINVAL;
> > > +
> > > + memcpy(buf_cpy, buf, count);
> > > + buf_cpy[count] = '\0';
> > > +
> > > + tmp_str = buf_cpy;
> > > +
> > > + while (isspace(*tmp_str)) tmp_str++;
> > > +
> > > + while (tmp_str[0]) {
> > > + sub_str = strsep(&tmp_str, delimiter);
> > > + ret = kstrtol(sub_str, 0, ¶meter[parameter_size]);
> > > + if (ret)
> > > + return -EINVAL;
> > > + parameter_size++;
> > > +
> > > + while (isspace(*tmp_str))
> > > + tmp_str++;
> > > + }
> > > +
> > > + if (adev->powerplay.pp_funcs->set_od8_settings)
> > > + ret = amdgpu_dpm_set_od8_settings(adev, parameter,
> > > + parameter_size);
> > > +
> > > + if (ret)
> > > + return -EINVAL;
> > > +
> > > + return count;
> > > +}
> > > +
> > > static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR,
> > > amdgpu_get_dpm_state, amdgpu_set_dpm_state); static
> > DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
> > > amdgpu_get_dpm_forced_performance_level,
> > > @@ -969,6 +1084,9 @@ static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO
> > | S_IWUSR,
> > > amdgpu_set_pp_od_clk_voltage); static
> > > DEVICE_ATTR(gpu_busy_percent, S_IRUGO,
> > > amdgpu_get_busy_percent, NULL);
> > > +static DEVICE_ATTR(pp_od8_settings, S_IRUGO | S_IWUSR,
> > > + amdgpu_get_pp_od8_settings,
> > > + amdgpu_set_pp_od8_settings);
> >
> > Rather than adding a new file, would it make more sense to expose this new
> > API on the same file we use for older asics? Otherwise, every time the
> > interface changes, we'll need to add a new file. We already handle asic
> > specific differences in the power profile sysfs files.
> >
> > >
> > > static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
> > > struct device_attribute *attr,
> > > @@ -1879,6 +1997,13 @@ int amdgpu_pm_sysfs_init(struct
> > amdgpu_device *adev)
> > > "gpu_busy_level\n");
> > > return ret;
> > > }
> > > + ret = device_create_file(adev->dev,
> > > + &dev_attr_pp_od8_settings);
> > > + if (ret) {
> > > + DRM_ERROR("failed to create device file "
> > > + "pp_od8_settings\n");
> > > + return ret;
> > > + }
> > > ret = amdgpu_debugfs_pm_init(adev);
> > > if (ret) {
> > > DRM_ERROR("Failed to register debugfs file for
> > > dpm!\n"); diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > > b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > > index 6a41b81c7325..e23746ba53bf 100644
> > > --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > > +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > > @@ -269,6 +269,8 @@ struct amd_pm_funcs {
> > > int (*get_display_mode_validation_clocks)(void *handle,
> > > struct amd_pp_simple_clock_info *clocks);
> > > int (*notify_smu_enable_pwe)(void *handle);
> > > + int (*get_od8_settings)(void *handle, char *buf);
> > > + int (*set_od8_settings)(void *handle, long *input, uint32_t
> > > + size);
> > > };
> > >
> > > #endif
> > > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > index da4ebff5b74d..4bcd06306698 100644
> > > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > @@ -883,6 +883,41 @@ static int pp_odn_edit_dpm_table(void *handle,
> > uint32_t type, long *input, uint3
> > > return hwmgr->hwmgr_func->odn_edit_dpm_table(hwmgr, type,
> > > input, size); }
> > >
> > > +static int pp_get_od8_settings(void *handle, char *buf) {
> > > + struct pp_hwmgr *hwmgr = handle;
> > > +
> > > + if (!hwmgr || !hwmgr->pm_en || !buf)
> > > + return -EINVAL;
> > > +
> > > + if (hwmgr->hwmgr_func->get_od8_settings == NULL) {
> > > + pr_info("%s was not implemented.\n", __func__);
> > > + return snprintf(buf, PAGE_SIZE, "\n");
> > > + }
> > > +
> > > + return hwmgr->hwmgr_func->get_od8_settings(hwmgr, buf); }
> > > +
> > > +static int pp_set_od8_settings(void *handle, long *input, uint32_t
> > > +size) {
> > > + struct pp_hwmgr *hwmgr = handle;
> > > + int ret = -EINVAL;
> > > +
> > > + if (!hwmgr || !hwmgr->pm_en)
> > > + return ret;
> > > +
> > > + if (hwmgr->hwmgr_func->set_od8_settings == NULL) {
> > > + pr_info("%s was not implemented.\n", __func__);
> > > + return ret;
> > > + }
> > > +
> > > + mutex_lock(&hwmgr->smu_lock);
> > > + ret = hwmgr->hwmgr_func->set_od8_settings(hwmgr, input, size);
> > > + mutex_unlock(&hwmgr->smu_lock);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > static int pp_dpm_switch_power_profile(void *handle,
> > > enum PP_SMC_POWER_PROFILE type, bool en) { @@ -1272,6
> > > +1307,8 @@ static const struct amd_pm_funcs pp_dpm_funcs = {
> > > .get_power_profile_mode = pp_get_power_profile_mode,
> > > .set_power_profile_mode = pp_set_power_profile_mode,
> > > .odn_edit_dpm_table = pp_odn_edit_dpm_table,
> > > + .get_od8_settings = pp_get_od8_settings,
> > > + .set_od8_settings = pp_set_od8_settings,
> > > .set_power_limit = pp_set_power_limit,
> > > .get_power_limit = pp_get_power_limit,
> > > /* export to DC */
> > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > index fb32b28afa66..ececa2f7fe5f 100644
> > > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> > > @@ -1099,7 +1099,188 @@ static int
> > vega20_od8_initialize_default_settings(
> > > return 0;
> > > }
> > >
> > > -static int vega20_od8_set_settings(
> > > +static int vega20_get_od8_settings(
> > > + struct pp_hwmgr *hwmgr,
> > > + char *buf)
> > > +{
> > > + 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;
> > > + uint32_t size = 0;
> > > + int ret = 0;
> > > +
> > > + if (!buf)
> > > + return -EINVAL;
> > > +
> > > + 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);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_FMIN].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "GfxclkFmin",
> > > + od8_settings[OD8_SETTING_GFXCLK_FMIN].min_value,
> > > + od8_settings[OD8_SETTING_GFXCLK_FMIN].max_value,
> > > + od_table.GfxclkFmin);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_FMAX].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "GfxclkFmax",
> > > + od8_settings[OD8_SETTING_GFXCLK_FMAX].min_value,
> > > + od8_settings[OD8_SETTING_GFXCLK_FMAX].max_value,
> > > + od_table.GfxclkFmax);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_FREQ1].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "GfxclkFreq1",
> > > + od8_settings[OD8_SETTING_GFXCLK_FREQ1].min_value,
> > > + od8_settings[OD8_SETTING_GFXCLK_FREQ1].max_value,
> > > + od_table.GfxclkFreq1);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "GfxclkOffsetVolt1",
> > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].min_value,
> > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE1].max_value,
> > > + od_table.GfxclkOffsetVolt1);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_FREQ2].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "GfxclkFreq2",
> > > + od8_settings[OD8_SETTING_GFXCLK_FREQ2].min_value,
> > > + od8_settings[OD8_SETTING_GFXCLK_FREQ2].max_value,
> > > + od_table.GfxclkFreq2);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "GfxclkOffsetVolt2",
> > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].min_value,
> > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE2].max_value,
> > > + od_table.GfxclkOffsetVolt2);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_FREQ3].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "GfxclkFreq3",
> > > + od8_settings[OD8_SETTING_GFXCLK_FREQ3].min_value,
> > > + od8_settings[OD8_SETTING_GFXCLK_FREQ3].max_value,
> > > + od_table.GfxclkFreq3);
> > > +
> > > + if (od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "GfxclkOffsetVolt3",
> > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].min_value,
> > > +
> > od8_settings[OD8_SETTING_GFXCLK_VOLTAGE3].max_value,
> > > + od_table.GfxclkOffsetVolt3);
> > > +
> > > + if (od8_settings[OD8_SETTING_UCLK_FMAX].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "UclkFmax",
> > > + od8_settings[OD8_SETTING_UCLK_FMAX].min_value,
> > > + od8_settings[OD8_SETTING_UCLK_FMAX].max_value,
> > > + od_table.UclkFmax);
> > > +
> > > + if (od8_settings[OD8_SETTING_POWER_PERCENTAGE].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "OverDrivePct",
> > > +
> > od8_settings[OD8_SETTING_POWER_PERCENTAGE].min_value,
> > > +
> > od8_settings[OD8_SETTING_POWER_PERCENTAGE].max_value,
> > > + od_table.OverDrivePct);
> > > +
> > > + if (od8_settings[OD8_SETTING_FAN_ACOUSTIC_LIMIT].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "FanMaximumRpm",
> > > +
> > od8_settings[OD8_SETTING_FAN_ACOUSTIC_LIMIT].min_value,
> > > +
> > od8_settings[OD8_SETTING_FAN_ACOUSTIC_LIMIT].max_value,
> > > + od_table.FanMaximumRpm);
> > > +
> > > + if (od8_settings[OD8_SETTING_FAN_MIN_SPEED].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "FanMinimumPwm",
> > > +
> > od8_settings[OD8_SETTING_FAN_MIN_SPEED].min_value,
> > > +
> > od8_settings[OD8_SETTING_FAN_MIN_SPEED].max_value,
> > > + od_table.FanMinimumPwm);
> > > +
> > > + if (od8_settings[OD8_SETTING_FAN_TARGET_TEMP].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "FanTargetTemperautre",
> > > +
> > od8_settings[OD8_SETTING_FAN_TARGET_TEMP].min_value,
> > > +
> > od8_settings[OD8_SETTING_FAN_TARGET_TEMP].max_value,
> > > + od_table.FanTargetTemperature);
> > > +
> > > + if (od8_settings[OD8_SETTING_OPERATING_TEMP_MAX].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n", "MaxOpTemp",
> > > +
> > od8_settings[OD8_SETTING_OPERATING_TEMP_MAX].min_value,
> > > +
> > od8_settings[OD8_SETTING_OPERATING_TEMP_MAX].max_value,
> > > + od_table.MaxOpTemp);
> > > +
> > > + if (od8_settings[OD8_SETTING_FAN_ZERO_RPM_CONTROL].feature_id)
> > > + size += sprintf(buf + size, "%22s[%d - %d]: %u\n",
> > "FanZeroRpmEnable",
> > > +
> > od8_settings[OD8_SETTING_FAN_ZERO_RPM_CONTROL].min_value,
> > > +
> > od8_settings[OD8_SETTING_FAN_ZERO_RPM_CONTROL].max_value,
> > > + od_table.FanZeroRpmEnable);
> > > +
> > > + return size;
> > > +}
> > > +
> > > +static int vega20_set_od8_settings(
> > > + struct pp_hwmgr *hwmgr,
> > > + 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;
> > > + uint16_t *buf = (uint16_t *)(&od_table);
> > > + uint32_t i, j = 0, k;
> > > + int ret = 0;
> > > +
> > > + /* retrieve current overdrive settings */
> > > + 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);
> > > +
> > > + for (i = 0;
> > > + i < (sizeof(OverDriveTable_t) / sizeof(uint16_t)) - 1;
> > > + i++) {
> > > + /* overdrive table supports no dram timing setting */
> > > + k = (i == OD8_SETTING_AC_TIMING) ?
> > > + OD8_SETTING_FAN_ZERO_RPM_CONTROL : i;
> > > + /*
> > > + * Not all OD settings are supported and exported
> > > + * to user.
> > > + * So, updated only those which are supported and
> > > + * exported to user.
> > > + */
> > > + if (od8_settings[k].feature_id) {
> > > + if (input[j] < od8_settings[k].min_value ||
> > > + input[j] > od8_settings[k].max_value)
> > > + return -EINVAL;
> > > +
> > > + buf[i] = input[j];
> > > + if (j++ >= size - 1)
> > > + break;
> > > + }
> > > + }
> > > +
> > > + /* enable the new overdrive settings */
> > > + 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);
> > > +
> > > + /* update the current_value of overdrive settings */
> > > + for (i = 0;
> > > + i < (sizeof(OverDriveTable_t) / sizeof(uint16_t)) - 1;
> > > + i++) {
> > > + k = (i == OD8_SETTING_AC_TIMING) ?
> > > + OD8_SETTING_FAN_ZERO_RPM_CONTROL : i;
> > > + if (od8_settings[k].feature_id)
> > > + od8_settings[k].current_value = buf[i];
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int vega20_od8_set_single_setting(
> > > struct pp_hwmgr *hwmgr,
> > > uint32_t index,
> > > uint32_t value)
> > > @@ -1198,7 +1379,7 @@ static int vega20_set_sclk_od(
> > > do_div(od_sclk, 100);
> > > od_sclk +=
> > > golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
> > >
> > > - ret = vega20_od8_set_settings(hwmgr, OD8_SETTING_GFXCLK_FMAX,
> > od_sclk);
> > > + ret = vega20_od8_set_single_setting(hwmgr,
> > > + OD8_SETTING_GFXCLK_FMAX, od_sclk);
> > > PP_ASSERT_WITH_CODE(!ret,
> > > "[SetSclkOD] failed to set od gfxclk!",
> > > return ret);
> > > @@ -1245,7 +1426,7 @@ static int vega20_set_mclk_od(
> > > do_div(od_mclk, 100);
> > > od_mclk +=
> > > golden_mclk_table->dpm_levels[golden_mclk_table->count - 1].value;
> > >
> > > - ret = vega20_od8_set_settings(hwmgr, OD8_SETTING_UCLK_FMAX,
> > od_mclk);
> > > + ret = vega20_od8_set_single_setting(hwmgr,
> > > + OD8_SETTING_UCLK_FMAX, od_mclk);
> > > PP_ASSERT_WITH_CODE(!ret,
> > > "[SetMclkOD] failed to set od memclk!",
> > > return ret);
> > > @@ -2966,6 +3147,10 @@ static const struct pp_hwmgr_func
> > vega20_hwmgr_funcs = {
> > > vega20_get_power_profile_mode,
> > > .set_power_profile_mode =
> > > vega20_set_power_profile_mode,
> > > + .get_od8_settings =
> > > + vega20_get_od8_settings,
> > > + .set_od8_settings =
> > > + vega20_set_od8_settings,
> > > /* od related */
> > > .set_power_limit =
> > > vega20_set_power_limit, diff --git
> > > a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > > b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > > index a6d92128b19c..285af1728e2e 100644
> > > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > > @@ -328,6 +328,8 @@ struct pp_hwmgr_func {
> > > int (*set_power_limit)(struct pp_hwmgr *hwmgr, uint32_t n);
> > > int (*powergate_mmhub)(struct pp_hwmgr *hwmgr);
> > > int (*smus_notify_pwe)(struct pp_hwmgr *hwmgr);
> > > + int (*get_od8_settings)(struct pp_hwmgr *hwmgr, char *buf);
> > > + int (*set_od8_settings)(struct pp_hwmgr *hwmgr, long *input,
> > > + uint32_t size);
>
> Those new added interfaces for OD parameters get/set seems have no difference with the old interface.
>
> int (*print_clock_levels)(struct pp_hwmgr *hwmgr, enum pp_clock_type type, char *buf);
> int (*odn_edit_dpm_table)(struct pp_hwmgr *hwmgr, enum PP_OD_DPM_TABLE_COMMAND type, long *input, uint32_t size);
>
> Regards
> Rex
> > > };
> > >
> > > struct pp_table_func {
> > > --
> > > 2.18.0
> > >
> > > _______________________________________________
> > > amd-gfx mailing list
> > > amd-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180828/b348531c/attachment-0001.html>
More information about the amd-gfx
mailing list