<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [AMD Official Use Only - Internal Distribution Only] <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="divRplyFwdMsg" dir="ltr">From: Zhang, Hawking <Hawking.Zhang@amd.com> Sent: Wednesday, May 6, 2020 5:26 PM To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com> Subject: RE: [PATCH 2/3] drm/amdgpu: optimize amdgpu device attribute code <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">[AMD Official Use Only - Internal Distribution Only] Hi Kelvin, Thanks for the series that remove the duplicated one_vf mode check in all the amdgpu_dpm functions. Can we split the patch into two? One for amdgpu device sysfs attr code refine, with the new dev_attr structures, the other for retiring all the unnecessary one_vf mode support. <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">thanks your comment. [kevin]: Q1, agree, i will split it into two patch.</div> <div class="PlainText"> +enum amdgpu_device_attr_states { + ATTR_STATE_UNSUPPORT = 0, + ATTR_STATE_SUPPORT, + ATTR_STATE_DEAD, + ATTR_STATE_ALIVE, +}; + The attr_states seems unnecessary to me. You need a flag to mark whether a particular attribute is supported by specific ASIC or not, right? Then just a bool variable should be good enough for this purpose, Like attr->supported. I' d like to understand the use case for DEAD and ALIVE. Accordingly, you can simplify the logic that only remove the supported ones.</div> <div class="PlainText"> </div> <div class="PlainText">[kevin]: Q2, the origin idea, it is used to store sysfs file state, but for this case, we can try to drop DEAD & ALIVE state, </div> <div class="PlainText">because the origin code logic will exit directly when create file fail. If we have to introduce more complicated flags to indicate different status, I'd prefer to go directly to initialize one_vf mode attr sets and bare-metal attr sets directly.</div> <div class="PlainText">[kevin]: Q3, i'd like to keep this patch code, in fact, not all sysfs devices need to be created on bare-metal mode.</div> <div class="PlainText">the driver must check it at runtime. eg: is_sw_smu_support(), if (asic_chip == XXX), etc..</div> <div class="PlainText"> In addition, the function naming like default_attr_perform also confusing me. Would it be the function that used to update attr status? +static int default_attr_perform(struct amdgpu_device *adev, struct amdgpu_device_attr *attr, + uint32_t mask)</div> <div class="PlainText"> </div> <div class="PlainText">[kevin]: Q4, yes, the function is used to update attr status according to asic information at runtime.</div> <div class="PlainText">maybe rename to "attr_update" is better.</div> <div class="PlainText"> </div> <div class="PlainText">Best Regards,</div> <div class="PlainText">Kevin</div> <div class="PlainText"> </div> <div class="PlainText">Regards, Hawking -----Original Message----- From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com> Sent: Wednesday, May 6, 2020 14:23 To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Wang, Kevin(Yang) <Kevin1.Wang@amd.com> Subject: [PATCH 2/3] drm/amdgpu: optimize amdgpu device attribute code unified amdgpu device attribute node functions: 1. add some helper functions to create amdgpu device attribute node. 2. create device node according to device attr flags on different VF mode. 3. rename some functions name to adapt a new interface. 4. remove unneccessary virt mode check in inernal functions (xxx_show, xxx_store). Signed-off-by: Kevin Wang <kevin1.wang@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 577 ++++++++++--------------- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h | 48 ++ 2 files changed, 271 insertions(+), 354 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index c762deb5abc7..367ac79418b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -154,18 +154,15 @@ int amdgpu_dpm_read_sensor(struct amdgpu_device *adev, enum amd_pp_sensors senso * */ -static ssize_t amdgpu_get_dpm_state(struct device *dev, - struct device_attribute *attr, - char *buf) +static ssize_t amdgpu_get_power_dpm_state(struct device *dev, + struct device_attribute *attr, + char *buf) { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; enum amd_pm_state_type pm; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -189,19 +186,16 @@ static ssize_t amdgpu_get_dpm_state(struct device *dev, (pm == POWER_STATE_TYPE_BALANCED) ? "balanced" : "performance"); } -static ssize_t amdgpu_set_dpm_state(struct device *dev, - struct device_attribute *attr, - const char *buf, - size_t count) +static ssize_t amdgpu_set_power_dpm_state(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; enum amd_pm_state_type state; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - if (strncmp("battery", buf, strlen("battery")) == 0) state = POWER_STATE_TYPE_BATTERY; else if (strncmp("balanced", buf, strlen("balanced")) == 0) @@ -294,18 +288,15 @@ static ssize_t amdgpu_set_dpm_state(struct device *dev, * */ -static ssize_t amdgpu_get_dpm_forced_performance_level(struct device *dev, - struct device_attribute *attr, - char *buf) +static ssize_t amdgpu_get_power_dpm_force_performance_level(struct device *dev, + struct device_attribute *attr, + char *buf) { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; enum amd_dpm_forced_level level = 0xff; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -332,10 +323,10 @@ static ssize_t amdgpu_get_dpm_forced_performance_level(struct device *dev, "unknown"); } -static ssize_t amdgpu_set_dpm_forced_performance_level(struct device *dev, - struct device_attribute *attr, - const char *buf, - size_t count) +static ssize_t amdgpu_set_power_dpm_force_performance_level(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; @@ -343,9 +334,6 @@ static ssize_t amdgpu_set_dpm_forced_performance_level(struct device *dev, enum amd_dpm_forced_level current_level = 0xff; int ret = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - if (strncmp("low", buf, strlen("low")) == 0) { level = AMD_DPM_FORCED_LEVEL_LOW; } else if (strncmp("high", buf, strlen("high")) == 0) { @@ -475,9 +463,6 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev, enum amd_pm_state_type pm = 0; int i = 0, ret = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -514,9 +499,6 @@ static ssize_t amdgpu_get_pp_force_state(struct device *dev, struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - if (adev->pp_force_state_enabled) return amdgpu_get_pp_cur_state(dev, attr, buf); else @@ -534,9 +516,6 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev, unsigned long idx; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - if (strlen(buf) == 1) adev->pp_force_state_enabled = false; else if (is_support_sw_smu(adev)) @@ -592,9 +571,6 @@ static ssize_t amdgpu_get_pp_table(struct device *dev, char *table = NULL; int size, ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -634,9 +610,6 @@ static ssize_t amdgpu_set_pp_table(struct device *dev, struct amdgpu_device *adev = ddev->dev_private; int ret = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -873,10 +846,10 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev, * the corresponding bit from original ppfeature masks and input the * new ppfeature masks. */ -static ssize_t amdgpu_set_pp_feature_status(struct device *dev, - struct device_attribute *attr, - const char *buf, - size_t count) +static ssize_t amdgpu_set_pp_features(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; @@ -917,9 +890,9 @@ static ssize_t amdgpu_set_pp_feature_status(struct device *dev, return count; } -static ssize_t amdgpu_get_pp_feature_status(struct device *dev, - struct device_attribute *attr, - char *buf) +static ssize_t amdgpu_get_pp_features(struct device *dev, + struct device_attribute *attr, + char *buf) { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; @@ -985,9 +958,6 @@ static ssize_t amdgpu_get_pp_dpm_sclk(struct device *dev, ssize_t size; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1051,9 +1021,6 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev, int ret; uint32_t mask = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - ret = amdgpu_read_mask(buf, count, &mask); if (ret) return ret; @@ -1085,9 +1052,6 @@ static ssize_t amdgpu_get_pp_dpm_mclk(struct device *dev, ssize_t size; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1115,9 +1079,6 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev, uint32_t mask = 0; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - ret = amdgpu_read_mask(buf, count, &mask); if (ret) return ret; @@ -1149,9 +1110,6 @@ static ssize_t amdgpu_get_pp_dpm_socclk(struct device *dev, ssize_t size; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1179,9 +1137,6 @@ static ssize_t amdgpu_set_pp_dpm_socclk(struct device *dev, int ret; uint32_t mask = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - ret = amdgpu_read_mask(buf, count, &mask); if (ret) return ret; @@ -1215,9 +1170,6 @@ static ssize_t amdgpu_get_pp_dpm_fclk(struct device *dev, ssize_t size; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1245,9 +1197,6 @@ static ssize_t amdgpu_set_pp_dpm_fclk(struct device *dev, int ret; uint32_t mask = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - ret = amdgpu_read_mask(buf, count, &mask); if (ret) return ret; @@ -1347,9 +1296,6 @@ static ssize_t amdgpu_get_pp_dpm_pcie(struct device *dev, ssize_t size; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1377,9 +1323,6 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev, int ret; uint32_t mask = 0; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - ret = amdgpu_read_mask(buf, count, &mask); if (ret) return ret; @@ -1571,9 +1514,6 @@ static ssize_t amdgpu_get_pp_power_profile_mode(struct device *dev, ssize_t size; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1615,9 +1555,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev, if (ret) return -EINVAL; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return -EINVAL; - if (profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) { if (count < 2 || count > 127) return -EINVAL; @@ -1663,17 +1600,14 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev, * The SMU firmware computes a percentage of load based on the * aggregate activity level in the IP cores. */ -static ssize_t amdgpu_get_busy_percent(struct device *dev, - struct device_attribute *attr, - char *buf) +static ssize_t amdgpu_get_gpu_busy_percent(struct device *dev, + struct device_attribute *attr, + char *buf) { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; int r, value, size = sizeof(value); - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - r = pm_runtime_get_sync(ddev->dev); if (r < 0) return r; @@ -1699,17 +1633,14 @@ static ssize_t amdgpu_get_busy_percent(struct device *dev, * The SMU firmware computes a percentage of load based on the * aggregate activity level in the IP cores. */ -static ssize_t amdgpu_get_memory_busy_percent(struct device *dev, - struct device_attribute *attr, - char *buf) +static ssize_t amdgpu_get_mem_busy_percent(struct device *dev, + struct device_attribute *attr, + char *buf) { struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; int r, value, size = sizeof(value); - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - r = pm_runtime_get_sync(ddev->dev); if (r < 0) return r; @@ -1748,9 +1679,6 @@ static ssize_t amdgpu_get_pcie_bw(struct device *dev, uint64_t count0, count1; int ret; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - ret = pm_runtime_get_sync(ddev->dev); if (ret < 0) return ret; @@ -1781,66 +1709,186 @@ static ssize_t amdgpu_get_unique_id(struct device *dev, struct drm_device *ddev = dev_get_drvdata(dev); struct amdgpu_device *adev = ddev->dev_private; - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) - return 0; - if (adev->unique_id) return snprintf(buf, PAGE_SIZE, "%016llx\n", adev->unique_id); return 0; } -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, - amdgpu_set_dpm_forced_performance_level); -static DEVICE_ATTR(pp_num_states, S_IRUGO, amdgpu_get_pp_num_states, NULL); -static DEVICE_ATTR(pp_cur_state, S_IRUGO, amdgpu_get_pp_cur_state, NULL); -static DEVICE_ATTR(pp_force_state, S_IRUGO | S_IWUSR, - amdgpu_get_pp_force_state, - amdgpu_set_pp_force_state); -static DEVICE_ATTR(pp_table, S_IRUGO | S_IWUSR, - amdgpu_get_pp_table, - amdgpu_set_pp_table); -static DEVICE_ATTR(pp_dpm_sclk, S_IRUGO | S_IWUSR, - amdgpu_get_pp_dpm_sclk, - amdgpu_set_pp_dpm_sclk); -static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR, - amdgpu_get_pp_dpm_mclk, - amdgpu_set_pp_dpm_mclk); -static DEVICE_ATTR(pp_dpm_socclk, S_IRUGO | S_IWUSR, - amdgpu_get_pp_dpm_socclk, - amdgpu_set_pp_dpm_socclk); -static DEVICE_ATTR(pp_dpm_fclk, S_IRUGO | S_IWUSR, - amdgpu_get_pp_dpm_fclk, - amdgpu_set_pp_dpm_fclk); -static DEVICE_ATTR(pp_dpm_dcefclk, S_IRUGO | S_IWUSR, - amdgpu_get_pp_dpm_dcefclk, - amdgpu_set_pp_dpm_dcefclk); -static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR, - amdgpu_get_pp_dpm_pcie, - amdgpu_set_pp_dpm_pcie); -static DEVICE_ATTR(pp_sclk_od, S_IRUGO | S_IWUSR, - amdgpu_get_pp_sclk_od, - amdgpu_set_pp_sclk_od); -static DEVICE_ATTR(pp_mclk_od, S_IRUGO | S_IWUSR, - amdgpu_get_pp_mclk_od, - amdgpu_set_pp_mclk_od); -static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR, - amdgpu_get_pp_power_profile_mode, - amdgpu_set_pp_power_profile_mode); -static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR, - amdgpu_get_pp_od_clk_voltage, - amdgpu_set_pp_od_clk_voltage); -static DEVICE_ATTR(gpu_busy_percent, S_IRUGO, - amdgpu_get_busy_percent, NULL); -static DEVICE_ATTR(mem_busy_percent, S_IRUGO, - amdgpu_get_memory_busy_percent, NULL); -static DEVICE_ATTR(pcie_bw, S_IRUGO, amdgpu_get_pcie_bw, NULL); -static DEVICE_ATTR(pp_features, S_IRUGO | S_IWUSR, - amdgpu_get_pp_feature_status, - amdgpu_set_pp_feature_status); -static DEVICE_ATTR(unique_id, S_IRUGO, amdgpu_get_unique_id, NULL); +static struct amdgpu_device_attr amdgpu_device_attrs[] = { + AMDGPU_DEVICE_ATTR_RW(power_dpm_state, ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF), + AMDGPU_DEVICE_ATTR_RW(power_dpm_force_performance_level, ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF), + AMDGPU_DEVICE_ATTR_RO(pp_num_states, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RO(pp_cur_state, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_force_state, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_table, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_dpm_sclk, ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF), + AMDGPU_DEVICE_ATTR_RW(pp_dpm_mclk, ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF), + AMDGPU_DEVICE_ATTR_RW(pp_dpm_socclk, ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF), + AMDGPU_DEVICE_ATTR_RW(pp_dpm_fclk, ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF), + AMDGPU_DEVICE_ATTR_RW(pp_dpm_dcefclk, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_dpm_pcie, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_sclk_od, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_mclk_od, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RO(mem_busy_percent, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RO(pcie_bw, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RW(pp_features, ATTR_FLAG_BASIC), + AMDGPU_DEVICE_ATTR_RO(unique_id, ATTR_FLAG_BASIC), +}; + +static int default_attr_perform(struct amdgpu_device *adev, struct amdgpu_device_attr *attr, + uint32_t mask) +{ + struct device_attribute *dev_attr = &attr->dev_attr; + const char *attr_name = dev_attr->attr.name; + struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; + enum amd_asic_type asic_type = adev->asic_type; + + if (!(attr->flags & mask)) { + attr->states = ATTR_STATE_UNSUPPORT; + return 0; + } + +#define DEVICE_ATTR_IS(_name) (!strcmp(attr_name, #_name)) + + if (DEVICE_ATTR_IS(pp_dpm_socclk)) { + if (asic_type <= CHIP_VEGA10) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(pp_dpm_dcefclk)) { + if (asic_type <= CHIP_VEGA10 || asic_type == CHIP_ARCTURUS) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(pp_dpm_fclk)) { + if (asic_type < CHIP_VEGA20) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(pp_dpm_pcie)) { + if (asic_type == CHIP_ARCTURUS) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) { + attr->states = ATTR_STATE_UNSUPPORT; + if ((is_support_sw_smu(adev) && adev->smu.od_enabled) || + (!is_support_sw_smu(adev) && hwmgr->od_enabled)) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(mem_busy_percent)) { + if (adev->flags & AMD_IS_APU || asic_type == CHIP_VEGA10) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(pcie_bw)) { + /* PCIe Perf counters won't work on APU nodes */ + if (adev->flags & AMD_IS_APU) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(unique_id)) { + if (!adev->unique_id) + attr->states = ATTR_STATE_UNSUPPORT; + } else if (DEVICE_ATTR_IS(pp_features)) { + if (adev->flags & AMD_IS_APU || asic_type <= CHIP_VEGA10) + attr->states = ATTR_STATE_UNSUPPORT; + } + + if (asic_type == CHIP_ARCTURUS) { + /* Arcturus does not support standalone mclk/socclk/fclk level setting */ + if (DEVICE_ATTR_IS(pp_dpm_mclk) || + DEVICE_ATTR_IS(pp_dpm_socclk) || + DEVICE_ATTR_IS(pp_dpm_fclk)) { + dev_attr->attr.mode &= ~S_IWUGO; + dev_attr->store = NULL; + } + } + +#undef DEVICE_ATTR_IS + + return 0; +} + + +static int amdgpu_device_attr_create(struct amdgpu_device *adev, + struct amdgpu_device_attr *attr, + uint32_t mask) +{ + int ret = 0; + struct device_attribute *dev_attr = &attr->dev_attr; + const char *name = dev_attr->attr.name; + int (*attr_perform)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr, + uint32_t mask) = default_attr_perform; + + BUG_ON(!attr); + + if (attr->states == ATTR_STATE_UNSUPPORT || + attr->states == ATTR_STATE_ALIVE) + return 0; + + if (attr->perform) { + attr_perform = attr->perform; + } + + ret = attr_perform(adev, attr, mask); + if (ret) { + dev_err(adev->dev, "failed to perform device file %s, ret = %d\n", + name, ret); + return ret; + } + + /* the attr->states maybe changed after call attr->perform function */ + if (attr->states == ATTR_STATE_UNSUPPORT) + return 0; + + ret = device_create_file(adev->dev, dev_attr); + if (ret) { + dev_err(adev->dev, "failed to create device file %s, ret = %d\n", + name, ret); + } + + attr->states = ATTR_STATE_ALIVE; + + return ret; +} + +static void amdgpu_device_attr_remove(struct amdgpu_device *adev, +struct amdgpu_device_attr *attr) { + struct device_attribute *dev_attr = &attr->dev_attr; + + if (attr->states != ATTR_STATE_ALIVE) + return; + + device_remove_file(adev->dev, dev_attr); + + attr->states = ATTR_STATE_DEAD; +} + +static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, + struct amdgpu_device_attr *attrs, + uint32_t counts, + uint32_t mask) +{ + int ret = 0; + uint32_t i = 0; + + for (i = 0; i < counts; i++) { + ret = amdgpu_device_attr_create(adev, &attrs[i], mask); + if (ret) + goto failed; + } + + return 0; + +failed: + for (; i > 0; i--) { + amdgpu_device_attr_remove(adev, &attrs[i]); + } + + return ret; +} + +static void amdgpu_device_attr_remove_groups(struct amdgpu_device *adev, + struct amdgpu_device_attr *attrs, + uint32_t counts) +{ + uint32_t i = 0; + + for (i = 0; i < counts; i++) + amdgpu_device_attr_remove(adev, &attrs[i]); } static ssize_t amdgpu_hwmon_show_temp(struct device *dev, struct device_attribute *attr, @@ -2790,7 +2838,7 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj, umode_t effective_mode = attr->mode; /* under multi-vf mode, the hwmon attributes are all not supported */ - if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)) + if (amdgpu_virt_get_sriov_vf_mode(adev) == SRIOV_VF_MODE_MULTI_VF) return 0; /* there is no fan under pp one vf mode */ @@ -3241,8 +3289,8 @@ int amdgpu_pm_load_smu_firmware(struct amdgpu_device *adev, uint32_t *smu_versio int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) { - struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; int ret; + uint32_t mask = 0; if (adev->pm.sysfs_initialized) return 0; @@ -3260,168 +3308,25 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) return ret; } - ret = device_create_file(adev->dev, &dev_attr_power_dpm_state); - if (ret) { - DRM_ERROR("failed to create device file for dpm state\n"); - return ret; - } - ret = device_create_file(adev->dev, &dev_attr_power_dpm_force_performance_level); - if (ret) { - DRM_ERROR("failed to create device file for dpm state\n"); - return ret; - } - - if (!amdgpu_sriov_vf(adev)) { - ret = device_create_file(adev->dev, &dev_attr_pp_num_states); - if (ret) { - DRM_ERROR("failed to create device file pp_num_states\n"); - return ret; - } - ret = device_create_file(adev->dev, &dev_attr_pp_cur_state); - if (ret) { - DRM_ERROR("failed to create device file pp_cur_state\n"); - return ret; - } - ret = device_create_file(adev->dev, &dev_attr_pp_force_state); - if (ret) { - DRM_ERROR("failed to create device file pp_force_state\n"); - return ret; - } - ret = device_create_file(adev->dev, &dev_attr_pp_table); - if (ret) { - DRM_ERROR("failed to create device file pp_table\n"); - return ret; - } - } - - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_sclk); - if (ret) { - DRM_ERROR("failed to create device file pp_dpm_sclk\n"); - return ret; - } - - /* Arcturus does not support standalone mclk/socclk/fclk level setting */ - if (adev->asic_type == CHIP_ARCTURUS) { - dev_attr_pp_dpm_mclk.attr.mode &= ~S_IWUGO; - dev_attr_pp_dpm_mclk.store = NULL; - - dev_attr_pp_dpm_socclk.attr.mode &= ~S_IWUGO; - dev_attr_pp_dpm_socclk.store = NULL; - - dev_attr_pp_dpm_fclk.attr.mode &= ~S_IWUGO; - dev_attr_pp_dpm_fclk.store = NULL; - } - - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_mclk); - if (ret) { - DRM_ERROR("failed to create device file pp_dpm_mclk\n"); - return ret; - } - if (adev->asic_type >= CHIP_VEGA10) { - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_socclk); - if (ret) { - DRM_ERROR("failed to create device file pp_dpm_socclk\n"); - return ret; - } - if (adev->asic_type != CHIP_ARCTURUS) { - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_dcefclk); - if (ret) { - DRM_ERROR("failed to create device file pp_dpm_dcefclk\n"); - return ret; - } - } - } - if (adev->asic_type >= CHIP_VEGA20) { - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_fclk); - if (ret) { - DRM_ERROR("failed to create device file pp_dpm_fclk\n"); - return ret; - } - } - - /* the reset are not needed for SRIOV one vf mode */ - if (amdgpu_sriov_vf(adev)) { - adev->pm.sysfs_initialized = true; - return ret; + switch (amdgpu_virt_get_sriov_vf_mode(adev)) { + case SRIOV_VF_MODE_ONE_VF: + mask = ATTR_FLAG_ONEVF; + break; + case SRIOV_VF_MODE_MULTI_VF: + mask = 0; + break; + case SRIOV_VF_MODE_BARE_METAL: + default: + mask = ATTR_FLAG_MASK_ALL; + break; } - if (adev->asic_type != CHIP_ARCTURUS) { - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_pcie); - if (ret) { - DRM_ERROR("failed to create device file pp_dpm_pcie\n"); - return ret; - } - } - ret = device_create_file(adev->dev, &dev_attr_pp_sclk_od); - if (ret) { - DRM_ERROR("failed to create device file pp_sclk_od\n"); - return ret; - } - ret = device_create_file(adev->dev, &dev_attr_pp_mclk_od); - if (ret) { - DRM_ERROR("failed to create device file pp_mclk_od\n"); - return ret; - } - ret = device_create_file(adev->dev, - &dev_attr_pp_power_profile_mode); - if (ret) { - DRM_ERROR("failed to create device file " - "pp_power_profile_mode\n"); - return ret; - } - if ((is_support_sw_smu(adev) && adev->smu.od_enabled) || - (!is_support_sw_smu(adev) && hwmgr->od_enabled)) { - ret = device_create_file(adev->dev, - &dev_attr_pp_od_clk_voltage); - if (ret) { - DRM_ERROR("failed to create device file " - "pp_od_clk_voltage\n"); - return ret; - } - } - ret = device_create_file(adev->dev, - &dev_attr_gpu_busy_percent); - if (ret) { - DRM_ERROR("failed to create device file " - "gpu_busy_level\n"); - return ret; - } - /* APU does not have its own dedicated memory */ - if (!(adev->flags & AMD_IS_APU) && - (adev->asic_type != CHIP_VEGA10)) { - ret = device_create_file(adev->dev, - &dev_attr_mem_busy_percent); - if (ret) { - DRM_ERROR("failed to create device file " - "mem_busy_percent\n"); - return ret; - } - } - /* PCIe Perf counters won't work on APU nodes */ - if (!(adev->flags & AMD_IS_APU)) { - ret = device_create_file(adev->dev, &dev_attr_pcie_bw); - if (ret) { - DRM_ERROR("failed to create device file pcie_bw\n"); - return ret; - } - } - if (adev->unique_id) - ret = device_create_file(adev->dev, &dev_attr_unique_id); - if (ret) { - DRM_ERROR("failed to create device file unique_id\n"); + ret = amdgpu_device_attr_create_groups(adev, + amdgpu_device_attrs, + ARRAY_SIZE(amdgpu_device_attrs), + mask); + if (ret) return ret; - } - - if ((adev->asic_type >= CHIP_VEGA10) && - !(adev->flags & AMD_IS_APU)) { - ret = device_create_file(adev->dev, - &dev_attr_pp_features); - if (ret) { - DRM_ERROR("failed to create device file " - "pp_features\n"); - return ret; - } - } adev->pm.sysfs_initialized = true; @@ -3430,51 +3335,15 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev) { - struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; - if (adev->pm.dpm_enabled == 0) return; if (adev->pm.int_hwmon_dev) hwmon_device_unregister(adev->pm.int_hwmon_dev); - device_remove_file(adev->dev, &dev_attr_power_dpm_state); - device_remove_file(adev->dev, &dev_attr_power_dpm_force_performance_level); - - device_remove_file(adev->dev, &dev_attr_pp_num_states); - device_remove_file(adev->dev, &dev_attr_pp_cur_state); - device_remove_file(adev->dev, &dev_attr_pp_force_state); - device_remove_file(adev->dev, &dev_attr_pp_table); - - device_remove_file(adev->dev, &dev_attr_pp_dpm_sclk); - device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk); - if (adev->asic_type >= CHIP_VEGA10) { - device_remove_file(adev->dev, &dev_attr_pp_dpm_socclk); - if (adev->asic_type != CHIP_ARCTURUS) - device_remove_file(adev->dev, &dev_attr_pp_dpm_dcefclk); - } - if (adev->asic_type != CHIP_ARCTURUS) - device_remove_file(adev->dev, &dev_attr_pp_dpm_pcie); - if (adev->asic_type >= CHIP_VEGA20) - device_remove_file(adev->dev, &dev_attr_pp_dpm_fclk); - device_remove_file(adev->dev, &dev_attr_pp_sclk_od); - device_remove_file(adev->dev, &dev_attr_pp_mclk_od); - device_remove_file(adev->dev, - &dev_attr_pp_power_profile_mode); - if ((is_support_sw_smu(adev) && adev->smu.od_enabled) || - (!is_support_sw_smu(adev) && hwmgr->od_enabled)) - device_remove_file(adev->dev, - &dev_attr_pp_od_clk_voltage); - device_remove_file(adev->dev, &dev_attr_gpu_busy_percent); - if (!(adev->flags & AMD_IS_APU) && - (adev->asic_type != CHIP_VEGA10)) - device_remove_file(adev->dev, &dev_attr_mem_busy_percent); - if (!(adev->flags & AMD_IS_APU)) - device_remove_file(adev->dev, &dev_attr_pcie_bw); - if (adev->unique_id) - device_remove_file(adev->dev, &dev_attr_unique_id); - if ((adev->asic_type >= CHIP_VEGA10) && - !(adev->flags & AMD_IS_APU)) - device_remove_file(adev->dev, &dev_attr_pp_features); + + amdgpu_device_attr_remove_groups(adev, + amdgpu_device_attrs, + ARRAY_SIZE(amdgpu_device_attrs)); } void amdgpu_pm_compute_clocks(struct amdgpu_device *adev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h index 5db0ef86e84c..5ca5f3f9e8c0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h @@ -30,6 +30,54 @@ struct cg_flag_name const char *name; }; +enum amdgpu_device_attr_flags { + ATTR_FLAG_BASIC = (1 << 0), + ATTR_FLAG_ONEVF = (1 << 16), +}; + +#define ATTR_FLAG_TYPE_MASK (0x0000ffff) +#define ATTR_FLAG_MODE_MASK (0xffff0000) +#define ATTR_FLAG_MASK_ALL (0xffffffff) + +enum amdgpu_device_attr_states { + ATTR_STATE_UNSUPPORT = 0, + ATTR_STATE_SUPPORT, + ATTR_STATE_DEAD, + ATTR_STATE_ALIVE, +}; + +struct amdgpu_device_attr { + struct device_attribute dev_attr; + enum amdgpu_device_attr_flags flags; + enum amdgpu_device_attr_states states; + int (*perform)(struct amdgpu_device *adev, + struct amdgpu_device_attr* attr, + uint32_t mask); +}; + +#define to_amdgpu_device_attr(_dev_attr) \ + container_of(_dev_attr, struct amdgpu_device_attr, dev_attr) + +#define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...) \ + { .dev_attr = __ATTR(_name, _mode, _show, _store), \ + .flags = _flags, \ + .states = ATTR_STATE_SUPPORT, \ + ##__VA_ARGS__, } + +#define AMDGPU_DEVICE_ATTR(_name, _mode, _flags, ...) \ + __AMDGPU_DEVICE_ATTR(_name, _mode, \ + amdgpu_get_##_name, amdgpu_set_##_name, \ + _flags, ##__VA_ARGS__) + +#define AMDGPU_DEVICE_ATTR_RW(_name, _flags, ...) \ + AMDGPU_DEVICE_ATTR(_name, S_IRUGO | S_IWUSR, \ + _flags, ##__VA_ARGS__) + +#define AMDGPU_DEVICE_ATTR_RO(_name, _flags, ...) \ + __AMDGPU_DEVICE_ATTR(_name, S_IRUGO, \ + amdgpu_get_##_name, NULL, \ + _flags, ##__VA_ARGS__) + void amdgpu_pm_acpi_event_handler(struct amdgpu_device *adev); int amdgpu_pm_sysfs_init(struct amdgpu_device *adev); int amdgpu_pm_virt_sysfs_init(struct amdgpu_device *adev); -- 2.17.1 </div> </div> </div> </body> </html>