RE: [PATCH] drm/amdgpu: register pm sysfs for sriov

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux