On Mon, Aug 27, 2018 at 12:04 AM Quan, Evan <evan.q...@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 <alexdeuc...@gmail.com>
> > Sent: Friday, August 24, 2018 11:48 PM
> > To: Quan, Evan <evan.q...@amd.com>
> > Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Xu, Feifei
> > <feifei...@amd.com>; Kuehling, Felix <felix.kuehl...@amd.com>; Deucher,
> > Alexander <alexander.deuc...@amd.com>; Zhu, Rex <rex....@amd.com>;
> > Zhang, Hawking <hawking.zh...@amd.com>
> > Subject: Re: [PATCH] drm/amd/powerplay: expose vega20 OD features
> >
> > On Fri, Aug 24, 2018 at 3:45 AM Evan Quan <evan.q...@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.q...@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@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
