* [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling
@ 2020-05-07 20:14 Nirmoy Das
2020-05-07 20:21 ` Alex Deucher
0 siblings, 1 reply; 5+ messages in thread
From: Nirmoy Das @ 2020-05-07 20:14 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, nirmoy.das, christian.koenig
Create sysfs file using attributes when possible.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 +++----
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 114 +++++++--------------
2 files changed, 48 insertions(+), 102 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bf302c799832..cc41e8f5ad14 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
return ret;
}
+static const struct attribute *amdgpu_dev_attributes[] = {
+ &dev_attr_product_name.attr,
+ &dev_attr_product_number.attr,
+ &dev_attr_serial_number.attr,
+ &dev_attr_pcie_replay_count.attr,
+ NULL
+};
+
/**
* amdgpu_device_init - initialize the driver
*
@@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
queue_delayed_work(system_wq, &adev->delayed_init_work,
msecs_to_jiffies(AMDGPU_RESUME_MS));
- r = device_create_file(adev->dev, &dev_attr_pcie_replay_count);
- if (r) {
- dev_err(adev->dev, "Could not create pcie_replay_count");
- return r;
- }
-
- r = device_create_file(adev->dev, &dev_attr_product_name);
+ r = sysfs_create_files(&adev->dev->kobj, amdgpu_dev_attributes);
if (r) {
- dev_err(adev->dev, "Could not create product_name");
- return r;
- }
-
- r = device_create_file(adev->dev, &dev_attr_product_number);
- if (r) {
- dev_err(adev->dev, "Could not create product_number");
- return r;
- }
-
- r = device_create_file(adev->dev, &dev_attr_serial_number);
- if (r) {
- dev_err(adev->dev, "Could not create serial_number");
+ dev_err(adev->dev, "Could not create amdgpu device attr\n");
return r;
}
@@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
- device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
if (adev->ucode_sysfs_en)
amdgpu_ucode_sysfs_fini(adev);
- device_remove_file(adev->dev, &dev_attr_product_name);
- device_remove_file(adev->dev, &dev_attr_product_number);
- device_remove_file(adev->dev, &dev_attr_serial_number);
+
+ sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
if (IS_ENABLED(CONFIG_PERF_EVENTS))
amdgpu_pmu_fini(adev);
if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index c762deb5abc7..f375bc341acc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -3239,6 +3239,27 @@ int amdgpu_pm_load_smu_firmware(struct amdgpu_device *adev, uint32_t *smu_versio
return 0;
}
+static const struct attribute *amdgpu_pm_attributes[] = {
+ &dev_attr_power_dpm_state.attr,
+ &dev_attr_power_dpm_force_performance_level.attr,
+ &dev_attr_pp_dpm_sclk.attr,
+ &dev_attr_pp_dpm_mclk.attr,
+
+ &dev_attr_pp_sclk_od.attr,
+ &dev_attr_pp_mclk_od.attr,
+ &dev_attr_pp_power_profile_mode.attr,
+ &dev_attr_gpu_busy_percent.attr,
+ NULL
+};
+
+static const struct attribute *amdgpu_pm_non_vf_attributes[] = {
+ &dev_attr_pp_num_states.attr,
+ &dev_attr_pp_cur_state.attr,
+ &dev_attr_pp_force_state.attr,
+ &dev_attr_pp_table.attr,
+ NULL
+};
+
int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
{
struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
@@ -3260,45 +3281,6 @@ 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) {
@@ -3312,11 +3294,20 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
dev_attr_pp_dpm_fclk.store = NULL;
}
- ret = device_create_file(adev->dev, &dev_attr_pp_dpm_mclk);
+ ret = sysfs_create_files(&adev->dev->kobj, amdgpu_pm_attributes);
if (ret) {
- DRM_ERROR("failed to create device file pp_dpm_mclk\n");
+ DRM_ERROR("failed to create pm sysfs files\n");
return ret;
}
+
+ if (!amdgpu_sriov_vf(adev)) {
+ ret = sysfs_create_files(&adev->dev->kobj, amdgpu_pm_non_vf_attributes);
+ if (ret) {
+ DRM_ERROR("failed to create pm sysfs files\n");
+ return ret;
+ }
+ }
+
if (adev->asic_type >= CHIP_VEGA10) {
ret = device_create_file(adev->dev, &dev_attr_pp_dpm_socclk);
if (ret) {
@@ -3352,23 +3343,7 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
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,
@@ -3379,13 +3354,6 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
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)) {
@@ -3437,16 +3405,11 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
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);
+ sysfs_remove_files(&adev->dev->kobj, amdgpu_pm_attributes);
+ if (!amdgpu_sriov_vf(adev))
+ sysfs_remove_files(&adev->dev->kobj, amdgpu_pm_non_vf_attributes);
- 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)
@@ -3456,15 +3419,10 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
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);
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling
2020-05-07 20:14 [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling Nirmoy Das
@ 2020-05-07 20:21 ` Alex Deucher
2020-05-07 20:46 ` Das, Nirmoy
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2020-05-07 20:21 UTC (permalink / raw)
To: Nirmoy Das; +Cc: Deucher, Alexander, Christian Koenig, amd-gfx list
On Thu, May 7, 2020 at 4:15 PM Nirmoy Das <nirmoy.das@amd.com> wrote:
>
> Create sysfs file using attributes when possible.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 +++----
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
for the amdgpu_device.c changes. For amdgpu_pm.c, I think Kevin has a
patch set out to clean this up as well that also fixes up the VF
handling. May want to check with him on the pm changes.
Alex
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 114 +++++++--------------
> 2 files changed, 48 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index bf302c799832..cc41e8f5ad14 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
> return ret;
> }
>
> +static const struct attribute *amdgpu_dev_attributes[] = {
> + &dev_attr_product_name.attr,
> + &dev_attr_product_number.attr,
> + &dev_attr_serial_number.attr,
> + &dev_attr_pcie_replay_count.attr,
> + NULL
> +};
> +
> /**
> * amdgpu_device_init - initialize the driver
> *
> @@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> queue_delayed_work(system_wq, &adev->delayed_init_work,
> msecs_to_jiffies(AMDGPU_RESUME_MS));
>
> - r = device_create_file(adev->dev, &dev_attr_pcie_replay_count);
> - if (r) {
> - dev_err(adev->dev, "Could not create pcie_replay_count");
> - return r;
> - }
> -
> - r = device_create_file(adev->dev, &dev_attr_product_name);
> + r = sysfs_create_files(&adev->dev->kobj, amdgpu_dev_attributes);
> if (r) {
> - dev_err(adev->dev, "Could not create product_name");
> - return r;
> - }
> -
> - r = device_create_file(adev->dev, &dev_attr_product_number);
> - if (r) {
> - dev_err(adev->dev, "Could not create product_number");
> - return r;
> - }
> -
> - r = device_create_file(adev->dev, &dev_attr_serial_number);
> - if (r) {
> - dev_err(adev->dev, "Could not create serial_number");
> + dev_err(adev->dev, "Could not create amdgpu device attr\n");
> return r;
> }
>
> @@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> adev->rmmio = NULL;
> amdgpu_device_doorbell_fini(adev);
>
> - device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
> if (adev->ucode_sysfs_en)
> amdgpu_ucode_sysfs_fini(adev);
> - device_remove_file(adev->dev, &dev_attr_product_name);
> - device_remove_file(adev->dev, &dev_attr_product_number);
> - device_remove_file(adev->dev, &dev_attr_serial_number);
> +
> + sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
> if (IS_ENABLED(CONFIG_PERF_EVENTS))
> amdgpu_pmu_fini(adev);
> if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index c762deb5abc7..f375bc341acc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -3239,6 +3239,27 @@ int amdgpu_pm_load_smu_firmware(struct amdgpu_device *adev, uint32_t *smu_versio
> return 0;
> }
>
> +static const struct attribute *amdgpu_pm_attributes[] = {
> + &dev_attr_power_dpm_state.attr,
> + &dev_attr_power_dpm_force_performance_level.attr,
> + &dev_attr_pp_dpm_sclk.attr,
> + &dev_attr_pp_dpm_mclk.attr,
> +
> + &dev_attr_pp_sclk_od.attr,
> + &dev_attr_pp_mclk_od.attr,
> + &dev_attr_pp_power_profile_mode.attr,
> + &dev_attr_gpu_busy_percent.attr,
> + NULL
> +};
> +
> +static const struct attribute *amdgpu_pm_non_vf_attributes[] = {
> + &dev_attr_pp_num_states.attr,
> + &dev_attr_pp_cur_state.attr,
> + &dev_attr_pp_force_state.attr,
> + &dev_attr_pp_table.attr,
> + NULL
> +};
> +
> int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> {
> struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
> @@ -3260,45 +3281,6 @@ 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) {
> @@ -3312,11 +3294,20 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> dev_attr_pp_dpm_fclk.store = NULL;
> }
>
> - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_mclk);
> + ret = sysfs_create_files(&adev->dev->kobj, amdgpu_pm_attributes);
> if (ret) {
> - DRM_ERROR("failed to create device file pp_dpm_mclk\n");
> + DRM_ERROR("failed to create pm sysfs files\n");
> return ret;
> }
> +
> + if (!amdgpu_sriov_vf(adev)) {
> + ret = sysfs_create_files(&adev->dev->kobj, amdgpu_pm_non_vf_attributes);
> + if (ret) {
> + DRM_ERROR("failed to create pm sysfs files\n");
> + return ret;
> + }
> + }
> +
> if (adev->asic_type >= CHIP_VEGA10) {
> ret = device_create_file(adev->dev, &dev_attr_pp_dpm_socclk);
> if (ret) {
> @@ -3352,23 +3343,7 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> 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,
> @@ -3379,13 +3354,6 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> 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)) {
> @@ -3437,16 +3405,11 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>
> 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);
> + sysfs_remove_files(&adev->dev->kobj, amdgpu_pm_attributes);
> + if (!amdgpu_sriov_vf(adev))
> + sysfs_remove_files(&adev->dev->kobj, amdgpu_pm_non_vf_attributes);
>
> - 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)
> @@ -3456,15 +3419,10 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
> 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);
> --
> 2.26.2
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling
2020-05-07 20:21 ` Alex Deucher
@ 2020-05-07 20:46 ` Das, Nirmoy
0 siblings, 0 replies; 5+ messages in thread
From: Das, Nirmoy @ 2020-05-07 20:46 UTC (permalink / raw)
To: Alex Deucher; +Cc: Deucher, Alexander, Christian Koenig, amd-gfx list
On 5/7/2020 9:21 PM, Alex Deucher wrote:
> On Thu, May 7, 2020 at 4:15 PM Nirmoy Das <nirmoy.das@amd.com> wrote:
>> Create sysfs file using attributes when possible.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 +++----
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> for the amdgpu_device.c changes. For amdgpu_pm.c, I think Kevin has a
> patch set out to clean this up as well that also fixes up the VF
> handling. May want to check with him on the pm changes.
Thanks Alex, I didn't know. I will check with Kevin.
Regards,
Nirmoy
>
> Alex
>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 114 +++++++--------------
>> 2 files changed, 48 insertions(+), 102 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index bf302c799832..cc41e8f5ad14 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
>> return ret;
>> }
>>
>> +static const struct attribute *amdgpu_dev_attributes[] = {
>> + &dev_attr_product_name.attr,
>> + &dev_attr_product_number.attr,
>> + &dev_attr_serial_number.attr,
>> + &dev_attr_pcie_replay_count.attr,
>> + NULL
>> +};
>> +
>> /**
>> * amdgpu_device_init - initialize the driver
>> *
>> @@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>> queue_delayed_work(system_wq, &adev->delayed_init_work,
>> msecs_to_jiffies(AMDGPU_RESUME_MS));
>>
>> - r = device_create_file(adev->dev, &dev_attr_pcie_replay_count);
>> - if (r) {
>> - dev_err(adev->dev, "Could not create pcie_replay_count");
>> - return r;
>> - }
>> -
>> - r = device_create_file(adev->dev, &dev_attr_product_name);
>> + r = sysfs_create_files(&adev->dev->kobj, amdgpu_dev_attributes);
>> if (r) {
>> - dev_err(adev->dev, "Could not create product_name");
>> - return r;
>> - }
>> -
>> - r = device_create_file(adev->dev, &dev_attr_product_number);
>> - if (r) {
>> - dev_err(adev->dev, "Could not create product_number");
>> - return r;
>> - }
>> -
>> - r = device_create_file(adev->dev, &dev_attr_serial_number);
>> - if (r) {
>> - dev_err(adev->dev, "Could not create serial_number");
>> + dev_err(adev->dev, "Could not create amdgpu device attr\n");
>> return r;
>> }
>>
>> @@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>> adev->rmmio = NULL;
>> amdgpu_device_doorbell_fini(adev);
>>
>> - device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
>> if (adev->ucode_sysfs_en)
>> amdgpu_ucode_sysfs_fini(adev);
>> - device_remove_file(adev->dev, &dev_attr_product_name);
>> - device_remove_file(adev->dev, &dev_attr_product_number);
>> - device_remove_file(adev->dev, &dev_attr_serial_number);
>> +
>> + sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
>> if (IS_ENABLED(CONFIG_PERF_EVENTS))
>> amdgpu_pmu_fini(adev);
>> if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> index c762deb5abc7..f375bc341acc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> @@ -3239,6 +3239,27 @@ int amdgpu_pm_load_smu_firmware(struct amdgpu_device *adev, uint32_t *smu_versio
>> return 0;
>> }
>>
>> +static const struct attribute *amdgpu_pm_attributes[] = {
>> + &dev_attr_power_dpm_state.attr,
>> + &dev_attr_power_dpm_force_performance_level.attr,
>> + &dev_attr_pp_dpm_sclk.attr,
>> + &dev_attr_pp_dpm_mclk.attr,
>> +
>> + &dev_attr_pp_sclk_od.attr,
>> + &dev_attr_pp_mclk_od.attr,
>> + &dev_attr_pp_power_profile_mode.attr,
>> + &dev_attr_gpu_busy_percent.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute *amdgpu_pm_non_vf_attributes[] = {
>> + &dev_attr_pp_num_states.attr,
>> + &dev_attr_pp_cur_state.attr,
>> + &dev_attr_pp_force_state.attr,
>> + &dev_attr_pp_table.attr,
>> + NULL
>> +};
>> +
>> int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>> {
>> struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
>> @@ -3260,45 +3281,6 @@ 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) {
>> @@ -3312,11 +3294,20 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>> dev_attr_pp_dpm_fclk.store = NULL;
>> }
>>
>> - ret = device_create_file(adev->dev, &dev_attr_pp_dpm_mclk);
>> + ret = sysfs_create_files(&adev->dev->kobj, amdgpu_pm_attributes);
>> if (ret) {
>> - DRM_ERROR("failed to create device file pp_dpm_mclk\n");
>> + DRM_ERROR("failed to create pm sysfs files\n");
>> return ret;
>> }
>> +
>> + if (!amdgpu_sriov_vf(adev)) {
>> + ret = sysfs_create_files(&adev->dev->kobj, amdgpu_pm_non_vf_attributes);
>> + if (ret) {
>> + DRM_ERROR("failed to create pm sysfs files\n");
>> + return ret;
>> + }
>> + }
>> +
>> if (adev->asic_type >= CHIP_VEGA10) {
>> ret = device_create_file(adev->dev, &dev_attr_pp_dpm_socclk);
>> if (ret) {
>> @@ -3352,23 +3343,7 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>> 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,
>> @@ -3379,13 +3354,6 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>> 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)) {
>> @@ -3437,16 +3405,11 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>>
>> 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);
>> + sysfs_remove_files(&adev->dev->kobj, amdgpu_pm_attributes);
>> + if (!amdgpu_sriov_vf(adev))
>> + sysfs_remove_files(&adev->dev->kobj, amdgpu_pm_non_vf_attributes);
>>
>> - 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)
>> @@ -3456,15 +3419,10 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>> 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);
>> --
>> 2.26.2
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cnirmoy.das%40amd.com%7C7347df60ad514491ffb908d7f2c4383b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637244796884676621&sdata=XjTrg30N9ifFJ79Ykl08OqhgIXGfZi6WxmZiFEgebJE%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling
2020-05-08 8:48 Nirmoy Das
@ 2020-05-08 9:17 ` Christian König
0 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2020-05-08 9:17 UTC (permalink / raw)
To: Nirmoy Das, amd-gfx; +Cc: alexander.deucher, nirmoy.das
Am 08.05.20 um 10:48 schrieb Nirmoy Das:
> Create sysfs file using attributes.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 ++++++++--------------
> 1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index bf302c799832..cc41e8f5ad14 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
> return ret;
> }
>
> +static const struct attribute *amdgpu_dev_attributes[] = {
> + &dev_attr_product_name.attr,
> + &dev_attr_product_number.attr,
> + &dev_attr_serial_number.attr,
> + &dev_attr_pcie_replay_count.attr,
> + NULL
> +};
> +
> /**
> * amdgpu_device_init - initialize the driver
> *
> @@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> queue_delayed_work(system_wq, &adev->delayed_init_work,
> msecs_to_jiffies(AMDGPU_RESUME_MS));
>
> - r = device_create_file(adev->dev, &dev_attr_pcie_replay_count);
> - if (r) {
> - dev_err(adev->dev, "Could not create pcie_replay_count");
> - return r;
> - }
> -
> - r = device_create_file(adev->dev, &dev_attr_product_name);
> + r = sysfs_create_files(&adev->dev->kobj, amdgpu_dev_attributes);
> if (r) {
> - dev_err(adev->dev, "Could not create product_name");
> - return r;
> - }
> -
> - r = device_create_file(adev->dev, &dev_attr_product_number);
> - if (r) {
> - dev_err(adev->dev, "Could not create product_number");
> - return r;
> - }
> -
> - r = device_create_file(adev->dev, &dev_attr_serial_number);
> - if (r) {
> - dev_err(adev->dev, "Could not create serial_number");
> + dev_err(adev->dev, "Could not create amdgpu device attr\n");
> return r;
> }
>
> @@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> adev->rmmio = NULL;
> amdgpu_device_doorbell_fini(adev);
>
> - device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
> if (adev->ucode_sysfs_en)
> amdgpu_ucode_sysfs_fini(adev);
> - device_remove_file(adev->dev, &dev_attr_product_name);
> - device_remove_file(adev->dev, &dev_attr_product_number);
> - device_remove_file(adev->dev, &dev_attr_serial_number);
> +
> + sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
> if (IS_ENABLED(CONFIG_PERF_EVENTS))
> amdgpu_pmu_fini(adev);
> if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling
@ 2020-05-08 8:48 Nirmoy Das
2020-05-08 9:17 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Nirmoy Das @ 2020-05-08 8:48 UTC (permalink / raw)
To: amd-gfx; +Cc: alexander.deucher, nirmoy.das, christian.koenig
Create sysfs file using attributes.
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 ++++++++--------------
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index bf302c799832..cc41e8f5ad14 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2918,6 +2918,14 @@ static int amdgpu_device_get_job_timeout_settings(struct amdgpu_device *adev)
return ret;
}
+static const struct attribute *amdgpu_dev_attributes[] = {
+ &dev_attr_product_name.attr,
+ &dev_attr_product_number.attr,
+ &dev_attr_serial_number.attr,
+ &dev_attr_pcie_replay_count.attr,
+ NULL
+};
+
/**
* amdgpu_device_init - initialize the driver
*
@@ -3267,27 +3275,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
queue_delayed_work(system_wq, &adev->delayed_init_work,
msecs_to_jiffies(AMDGPU_RESUME_MS));
- r = device_create_file(adev->dev, &dev_attr_pcie_replay_count);
- if (r) {
- dev_err(adev->dev, "Could not create pcie_replay_count");
- return r;
- }
-
- r = device_create_file(adev->dev, &dev_attr_product_name);
+ r = sysfs_create_files(&adev->dev->kobj, amdgpu_dev_attributes);
if (r) {
- dev_err(adev->dev, "Could not create product_name");
- return r;
- }
-
- r = device_create_file(adev->dev, &dev_attr_product_number);
- if (r) {
- dev_err(adev->dev, "Could not create product_number");
- return r;
- }
-
- r = device_create_file(adev->dev, &dev_attr_serial_number);
- if (r) {
- dev_err(adev->dev, "Could not create serial_number");
+ dev_err(adev->dev, "Could not create amdgpu device attr\n");
return r;
}
@@ -3370,12 +3360,10 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
adev->rmmio = NULL;
amdgpu_device_doorbell_fini(adev);
- device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
if (adev->ucode_sysfs_en)
amdgpu_ucode_sysfs_fini(adev);
- device_remove_file(adev->dev, &dev_attr_product_name);
- device_remove_file(adev->dev, &dev_attr_product_number);
- device_remove_file(adev->dev, &dev_attr_serial_number);
+
+ sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
if (IS_ENABLED(CONFIG_PERF_EVENTS))
amdgpu_pmu_fini(adev);
if (amdgpu_discovery && adev->asic_type >= CHIP_NAVI10)
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-08 9:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 20:14 [PATCH 1/1] drm/amdgpu: cleanup sysfs file handling Nirmoy Das
2020-05-07 20:21 ` Alex Deucher
2020-05-07 20:46 ` Das, Nirmoy
2020-05-08 8:48 Nirmoy Das
2020-05-08 9:17 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.