Hi Alex
Many thanks for your review.
Best Regards
Yintian Tao
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Thursday, June 06, 2019 10:28 AM
To: Tao, Yintian <Yintian.Tao@xxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: register pm sysfs for sriov
On Wed, Jun 5, 2019 at 9:54 AM Yintian Tao <yttao@xxxxxxx> wrote:
>
> we need register pm sysfs for virt in order to support dpm level
> modification because smu ip block will not be added under SRIOV
>
> Signed-off-by: Yintian Tao <yttao@xxxxxxx>
> Change-Id: Ib0e13934c0c33da00f9d2add6be25a373c6fb957
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 61 ++++++++++++++++++++++++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h | 2 +
> 3 files changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index d00fd5d..9b9d387 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2695,6 +2695,9 @@ int amdgpu_device_init(struct amdgpu_device
> *adev,
>
> amdgpu_fbdev_init(adev);
>
> + if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
> + amdgpu_virt_pm_sysfs_init(adev);
> +
> r = amdgpu_pm_sysfs_init(adev);
> if (r)
> DRM_ERROR("registering pm debugfs failed (%d).\n", r);
> @@ -2816,6 +2819,9 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> iounmap(adev->rmmio);
> adev->rmmio = NULL;
> amdgpu_device_doorbell_fini(adev);
> + if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev))
> + amdgpu_virt_pm_sysfs_fini(adev);
> +
> amdgpu_debugfs_regs_cleanup(adev);
> device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
> amdgpu_ucode_sysfs_fini(adev); diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index a73e190..b6f16d45 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -269,8 +269,11 @@ static ssize_t amdgpu_get_dpm_forced_performance_level(struct device *dev,
> struct amdgpu_device *adev = ddev->dev_private;
> enum amd_dpm_forced_level level = 0xff;
>
> - if ((adev->flags & AMD_IS_PX) &&
> - (ddev->switch_power_state != DRM_SWITCH_POWER_ON))
> + if (amdgpu_sriov_vf(adev))
> + return 0;
> +
> + if ((adev->flags & AMD_IS_PX) &&
> + (ddev->switch_power_state != DRM_SWITCH_POWER_ON))
> return snprintf(buf, PAGE_SIZE, "off\n");
>
> if (is_support_sw_smu(adev))
> @@ -308,9 +311,11 @@ static ssize_t amdgpu_set_dpm_forced_performance_level(struct device *dev,
> (ddev->switch_power_state != DRM_SWITCH_POWER_ON))
> return -EINVAL;
>
> - if (is_support_sw_smu(adev))
> + if (!amdgpu_sriov_vf(adev) && is_support_sw_smu(adev))
> current_level = smu_get_performance_level(&adev->smu);
> - else if (adev->powerplay.pp_funcs->get_performance_level)
> + else if (!amdgpu_sriov_vf(adev) &&
> + adev->powerplay.pp_funcs &&
> + adev->powerplay.pp_funcs->get_performance_level)
> current_level =
> amdgpu_dpm_get_performance_level(adev);
Wrap the entire existing block in if (!amdgpu_sriov_vf(adev) rather than adding the check to each case.
>
> if (strncmp("low", buf, strlen("low")) == 0) { @@ -907,6
> +912,10 @@ static ssize_t amdgpu_get_pp_dpm_mclk(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = ddev->dev_private;
>
> + if (amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev) &&
> + adev->virt.ops->get_pp_clk)
> + return adev->virt.ops->get_pp_clk(adev,PP_MCLK,buf);
> +
> if (is_support_sw_smu(adev))
> return smu_print_clk_levels(&adev->smu, PP_MCLK, buf);
> else if (adev->powerplay.pp_funcs->print_clock_levels)
> @@ -925,6 +934,9 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
> int ret;
> uint32_t mask = 0;
>
> + if (amdgpu_sriov_vf(adev))
> + return 0;
> +
> ret = amdgpu_read_mask(buf, count, &mask);
> if (ret)
> return ret;
> @@ -965,6 +977,9 @@ static ssize_t amdgpu_set_pp_dpm_socclk(struct device *dev,
> int ret;
> uint32_t mask = 0;
>
> + if (amdgpu_sriov_vf(adev))
> + return 0;
> +
> ret = amdgpu_read_mask(buf, count, &mask);
> if (ret)
> return ret;
> @@ -2698,6 +2713,44 @@ void amdgpu_pm_print_power_states(struct
> amdgpu_device *adev)
>
> }
>
> +int amdgpu_virt_pm_sysfs_init(struct amdgpu_device *adev)
Please rename this to amdgpu_pm_virt_sysfs_init to match the rest of the naming conventions in the file.
> +{
> + int ret = 0;
> +
> + if (!(amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev)))
> + 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;
> + }
> +
> + 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;
> + }
> +
> + 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;
> + }
> +
> + return ret;
> +}
> +
> +void amdgpu_virt_pm_sysfs_fini(struct amdgpu_device *adev)
Same comment here. amdgpu_pm_virt_sysfs_fini.
With the above comments fixed, the patch is:
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> +{
> + if (!(amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev)))
> + return;
> +
> + device_remove_file(adev->dev, &dev_attr_power_dpm_force_performance_level);
> + device_remove_file(adev->dev, &dev_attr_pp_dpm_sclk);
> + device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk); }
> +
> int amdgpu_pm_sysfs_init(struct amdgpu_device *adev) {
> struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> index f21a771..6bb7bfa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h
> @@ -32,7 +32,9 @@ struct cg_flag_name
>
> void amdgpu_pm_acpi_event_handler(struct amdgpu_device *adev); int
> amdgpu_pm_sysfs_init(struct amdgpu_device *adev);
> +int amdgpu_virt_pm_sysfs_init(struct amdgpu_device *adev);
> void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev);
> +void amdgpu_virt_pm_sysfs_fini(struct amdgpu_device *adev);
> void amdgpu_pm_print_power_states(struct amdgpu_device *adev); void
> amdgpu_pm_compute_clocks(struct amdgpu_device *adev); void
> amdgpu_dpm_thermal_work_handler(struct work_struct *work);
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
