Re: [PATCH 3/6] drm/amdgpu: add powerplay sclk OD support through sysfs

[Eric Huang <jinhuieric.huang@amd.com>]

Thanks for your catch, Nils.

On 16-05-14 02:27 AM, Nils Wallménius wrote:
> Hi Eric,
>
> A little nitpick below.
>
> Regards
> Nils
>
> On Fri, May 13, 2016 at 8:48 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>
>> From: Eric Huang <JinHuiEric.Huang@amd.com>
>>
>> Add a new sysfs entry pp_sclk_od to support sclk overdrive(OD)
>> overclocking,
>> the entry is read/write, the value of input/output is an integer which is
>> the
>> over percentage of the highest sclk.
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  6 +++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c            | 49
>> +++++++++++++++++++++++
>>   drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 44
>> ++++++++++++++++++++
>>   drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  2 +
>>   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h         |  2 +
>>   5 files changed, 103 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 992f00b..367dbc4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -2335,6 +2335,12 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>>   #define amdgpu_dpm_force_clock_level(adev, type, level) \
>>
>> (adev)->powerplay.pp_funcs->force_clock_level((adev)->powerplay.pp_handle,
>> type, level)
>>
>> +#define amdgpu_dpm_get_sclk_od(adev) \
>> +
>>   (adev)->powerplay.pp_funcs->get_sclk_od((adev)->powerplay.pp_handle)
>> +
>> +#define amdgpu_dpm_set_sclk_od(adev, value) \
>> +
>>   (adev)->powerplay.pp_funcs->set_sclk_od((adev)->powerplay.pp_handle, value)
>> +
>>   #define amdgpu_dpm_dispatch_task(adev, event_id, input, output)
>>        \
>>
>> (adev)->powerplay.pp_funcs->dispatch_tasks((adev)->powerplay.pp_handle,
>> (event_id), (input), (output))
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> index 589b36e..f7ecaf4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> @@ -471,6 +471,46 @@ fail:
>>          return count;
>>   }
>>
>> +static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
>> +               struct device_attribute *attr,
>> +               char *buf)
>> +{
>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = ddev->dev_private;
>> +       uint32_t value = 0;
>> +
>> +       if (adev->pp_enabled)
>> +               value = amdgpu_dpm_get_sclk_od(adev);
>> +
>> +       return snprintf(buf, PAGE_SIZE, "%d\n", value);;
>>
> Double semicolon here ^
>
>
>> +}
>> +
>> +static ssize_t amdgpu_set_pp_sclk_od(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;
>> +       int ret;
>> +       long int value;
>> +
>> +       ret = kstrtol(buf, 0, &value);
>> +
>> +       if (ret) {
>> +               count = -EINVAL;
>> +               goto fail;
>> +       }
>> +
>> +       if (adev->pp_enabled)
>> +               amdgpu_dpm_set_sclk_od(adev, (uint32_t)value);
>> +
>> +       amdgpu_dpm_dispatch_task(adev, AMD_PP_EVENT_READJUST_POWER_STATE,
>> NULL, NULL);
>> +
>> +fail:
>> +       return count;
>> +}
>> +
>>   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,
>> @@ -492,6 +532,9 @@ static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
>>   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 ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>>                                        struct device_attribute *attr,
>> @@ -1125,6 +1168,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>>                          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 = amdgpu_debugfs_pm_init(adev);
>>          if (ret) {
>> @@ -1151,6 +1199,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>>                  device_remove_file(adev->dev, &dev_attr_pp_dpm_sclk);
>>                  device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk);
>>                  device_remove_file(adev->dev, &dev_attr_pp_dpm_pcie);
>> +               device_remove_file(adev->dev, &dev_attr_pp_sclk_od);
>>          }
>>   }
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> index 8e345bf..e0f2440 100644
>> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> @@ -530,6 +530,10 @@ int pp_dpm_dispatch_tasks(void *handle, enum
>> amd_pp_event event_id, void *input,
>>          case AMD_PP_EVENT_COMPLETE_INIT:
>>                  ret = pem_handle_event(pp_handle->eventmgr, event_id,
>> &data);
>>                  break;
>> +       case AMD_PP_EVENT_READJUST_POWER_STATE:
>> +               pp_handle->hwmgr->current_ps = pp_handle->hwmgr->boot_ps;
>> +               ret = pem_handle_event(pp_handle->eventmgr, event_id,
>> &data);
>> +               break;
>>          default:
>>                  break;
>>          }
>> @@ -800,6 +804,44 @@ static int pp_dpm_print_clock_levels(void *handle,
>>          return hwmgr->hwmgr_func->print_clock_levels(hwmgr, type, buf);
>>   }
>>
>> +static int pp_dpm_get_sclk_od(void *handle)
>> +{
>> +       struct pp_hwmgr *hwmgr;
>> +
>> +       if (!handle)
>> +               return -EINVAL;
>> +
>> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
>> +
>> +       PP_CHECK_HW(hwmgr);
>> +
>> +       if (hwmgr->hwmgr_func->get_sclk_od == NULL) {
>> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
>> +               return 0;
>> +       }
>> +
>> +       return hwmgr->hwmgr_func->get_sclk_od(hwmgr);
>> +}
>> +
>> +static int pp_dpm_set_sclk_od(void *handle, uint32_t value)
>> +{
>> +       struct pp_hwmgr *hwmgr;
>> +
>> +       if (!handle)
>> +               return -EINVAL;
>> +
>> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
>> +
>> +       PP_CHECK_HW(hwmgr);
>> +
>> +       if (hwmgr->hwmgr_func->set_sclk_od == NULL) {
>> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
>> +               return 0;
>> +       }
>> +
>> +       return hwmgr->hwmgr_func->set_sclk_od(hwmgr, value);
>> +}
>> +
>>   const struct amd_powerplay_funcs pp_dpm_funcs = {
>>          .get_temperature = pp_dpm_get_temperature,
>>          .load_firmware = pp_dpm_load_fw,
>> @@ -822,6 +864,8 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
>>          .set_pp_table = pp_dpm_set_pp_table,
>>          .force_clock_level = pp_dpm_force_clock_level,
>>          .print_clock_levels = pp_dpm_print_clock_levels,
>> +       .get_sclk_od = pp_dpm_get_sclk_od,
>> +       .set_sclk_od = pp_dpm_set_sclk_od,
>>   };
>>
>>   static int amd_pp_instance_init(struct amd_pp_init *pp_init,
>> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> index 50b367d..154d406 100644
>> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> @@ -342,6 +342,8 @@ struct amd_powerplay_funcs {
>>          int (*set_pp_table)(void *handle, const char *buf, size_t size);
>>          int (*force_clock_level)(void *handle, enum pp_clock_type type,
>> uint32_t mask);
>>          int (*print_clock_levels)(void *handle, enum pp_clock_type type,
>> char *buf);
>> +       int (*get_sclk_od)(void *handle);
>> +       int (*set_sclk_od)(void *handle, uint32_t value);
>>   };
>>
>>   struct amd_powerplay {
>> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> index 28f5714..37ebfa2 100644
>> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> @@ -338,6 +338,8 @@ struct pp_hwmgr_func {
>>          int (*force_clock_level)(struct pp_hwmgr *hwmgr, enum
>> pp_clock_type type, uint32_t mask);
>>          int (*print_clock_levels)(struct pp_hwmgr *hwmgr, enum
>> pp_clock_type type, char *buf);
>>          int (*enable_per_cu_power_gating)(struct pp_hwmgr *hwmgr, bool
>> enable);
>> +       int (*get_sclk_od)(struct pp_hwmgr *hwmgr);
>> +       int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
>>   };
>>
>>   struct pp_table_func {
>> --
>> 2.5.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel