RE: [PATCH] drm/amdgpu: register pm sysfs for sriov
Hi Alex
Many thanks for your review.
Best Regards
Yintian Tao
-Original Message-
From: Alex Deucher
Sent: Thursday, June 06, 2019 10:28 AM
To: Tao, Yintian
Cc: amd-gfx list
Subject: Re: [PATCH] drm/amdgpu: register pm sysfs for sriov
On Wed, Jun 5, 2019 at 9:54 AM Yintian Tao 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
> 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, _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(>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(>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, );
> 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;
>
> +
Re: [PATCH] drm/amdgpu: register pm sysfs for sriov
On Wed, Jun 5, 2019 at 9:54 AM Yintian Tao 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
> 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, _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(>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(>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, );
> 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, );
> 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, _attr_pp_dpm_sclk);
> + if (ret) {
> + DRM_ERROR("failed to create device file pp_dpm_sclk\n");
> + return ret;
> +
RE: [PATCH] drm/amdgpu: register pm sysfs for sriov
Hi Alex
Can you help have a review？ Thanks in advance.
Best Regards
Yintian Tao
-Original Message-
From: amd-gfx On Behalf Of Yintian Tao
Sent: Wednesday, June 05, 2019 10:09 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
Cc: Tao, Yintian
Subject: [PATCH] drm/amdgpu: register pm sysfs for sriov
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
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, _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..93e5205 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(>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);
if (strncmp("low", buf, strlen("low")) == 0) { @@ -885,6 +890,9 @@
static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
int ret;
uint32_t mask = 0;
+ if (amdgpu_sriov_vf(adev))
+ return 0;
+
ret = amdgpu_read_mask(buf, count, );
if (ret)
return ret;
@@ -907,6 +915,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(>smu, PP_MCLK, buf);
else if (adev->powerplay.pp_funcs->print_clock_levels)
@@ -925,6 +937,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, );
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) {
+ int ret = 0;
+
+ if (!(amdgpu_sriov_vf(adev) && amdgim_is_hwperf(adev)))
+ return ret;
+
+ ret = device_create_file(adev->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, _attr_pp_dpm_mclk);
+ if (ret) {
+ DRM_ERROR("failed to create device file pp_dpm_mclk\n");
+
