Yup.  I misread your comment thinking there was another datastructure you'd rather I put the callback in.


Cheers,

Tom


________________________________
From: Deucher, Alexander
Sent: Monday, September 19, 2016 11:55
To: StDenis, Tom; Alex Deucher
Cc: amd-gfx list
Subject: RE: [PATCH 1/2] drm/amd/powerplay: Add read_sensor() callback to hwmgr (v3)


No need.  The additional cleanups can come later.



Alex



From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf Of StDenis, Tom
Sent: Monday, September 19, 2016 11:55 AM
To: Alex Deucher
Cc: amd-gfx list
Subject: Re: [PATCH 1/2] drm/amd/powerplay: Add read_sensor() callback to hwmgr (v3)



Hi Alex,



Would you prefer I re-write #1 to avoid churn in the tree?



Cheers,

Tom



________________________________

From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Sent: Monday, September 19, 2016 11:53
To: Tom St Denis
Cc: amd-gfx list; StDenis, Tom
Subject: Re: [PATCH 1/2] drm/amd/powerplay: Add read_sensor() callback to hwmgr (v3)



On Mon, Sep 19, 2016 at 9:10 AM, Tom St Denis <tstdenis82-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:tstdenis82-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
> Provides standardized interface to read various sensors.
> The API is extensible (by adding to the end of the
> amd_pp_sensors enumeration list.
>
> Support has been added to Carrizo/smu7
>
> (v2) Squashed the two sensor patches into one.
> (v3) Updated to apply to smu7_hwmgr instead
>
> Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org<mailto:tom.stdenis@amd.com>>
> ---
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 20 +++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c    | 96 +++++++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 36 +++++++++
>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 12 +++
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h         |  1 +
>  5 files changed, 165 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index b1d19409bf86..ee0368381e82 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -894,6 +894,25 @@ static int pp_dpm_set_mclk_od(void *handle, uint32_t value)
>         return hwmgr->hwmgr_func->set_mclk_od(hwmgr, value);
>  }
>
> +static int pp_dpm_read_sensor(void *handle, int idx, int32_t *value)
> +{
> +       struct pp_hwmgr *hwmgr;
> +
> +       if (!handle)
> +               return -EINVAL;
> +
> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
> +
> +       PP_CHECK_HW(hwmgr);
> +
> +       if (hwmgr->hwmgr_func->read_sensor == NULL) {
> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
> +               return 0;
> +       }
> +
> +       return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value);
> +}
> +
>  const struct amd_powerplay_funcs pp_dpm_funcs = {
>         .get_temperature = pp_dpm_get_temperature,
>         .load_firmware = pp_dpm_load_fw,
> @@ -920,6 +939,7 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
>         .set_sclk_od = pp_dpm_set_sclk_od,
>         .get_mclk_od = pp_dpm_get_mclk_od,
>         .set_mclk_od = pp_dpm_set_mclk_od,
> +       .read_sensor = pp_dpm_read_sensor,

As a future patch it would be nice to hook up this sensor interface to
the existing amdgpu_pm_info code and make that asic indpendent.

Series is:
Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org<mailto:alexander.deucher-5C7GfCeVMHo@public.gmane.org>>

Alex


>  };
>
>  static int amd_pp_instance_init(struct amd_pp_init *pp_init,
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index 5ecef1732e20..9f3c5a8a903c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -1857,6 +1857,101 @@ static int cz_get_max_high_clocks(struct pp_hwmgr *hwmgr, struct amd_pp_simple_c
>         return 0;
>  }
>
> +static int cz_read_sensor(struct pp_hwmgr *hwmgr, int idx, int32_t *value)
> +{
> +       struct cz_hwmgr *cz_hwmgr = (struct cz_hwmgr *)(hwmgr->backend);
> +
> +       struct phm_clock_voltage_dependency_table *table =
> +                               hwmgr->dyn_state.vddc_dependency_on_sclk;
> +
> +       struct phm_vce_clock_voltage_dependency_table *vce_table =
> +               hwmgr->dyn_state.vce_clock_voltage_dependency_table;
> +
> +       struct phm_uvd_clock_voltage_dependency_table *uvd_table =
> +               hwmgr->dyn_state.uvd_clock_voltage_dependency_table;
> +
> +       uint32_t sclk_index = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixTARGET_AND_CURRENT_PROFILE_INDEX),
> +                                       TARGET_AND_CURRENT_PROFILE_INDEX, CURR_SCLK_INDEX);
> +       uint32_t uvd_index = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixTARGET_AND_CURRENT_PROFILE_INDEX_2),
> +                                       TARGET_AND_CURRENT_PROFILE_INDEX_2, CURR_UVD_INDEX);
> +       uint32_t vce_index = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixTARGET_AND_CURRENT_PROFILE_INDEX_2),
> +                                       TARGET_AND_CURRENT_PROFILE_INDEX_2, CURR_VCE_INDEX);
> +
> +       uint32_t sclk, vclk, dclk, ecclk, tmp, activity_percent;
> +       uint16_t vddnb, vddgfx;
> +       int result;
> +
> +       switch (idx) {
> +       case AMDGPU_PP_SENSOR_GFX_SCLK:
> +               if (sclk_index < NUM_SCLK_LEVELS) {
> +                       sclk = table->entries[sclk_index].clk;
> +                       *value = sclk;
> +                       return 0;
> +               }
> +               return -EINVAL;
> +       case AMDGPU_PP_SENSOR_VDDNB:
> +               tmp = (cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixSMUSVI_NB_CURRENTVID) &
> +                       CURRENT_NB_VID_MASK) >> CURRENT_NB_VID__SHIFT;
> +               vddnb = cz_convert_8Bit_index_to_voltage(hwmgr, tmp);
> +               *value = vddnb;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_VDDGFX:
> +               tmp = (cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixSMUSVI_GFX_CURRENTVID) &
> +                       CURRENT_GFX_VID_MASK) >> CURRENT_GFX_VID__SHIFT;
> +               vddgfx = cz_convert_8Bit_index_to_voltage(hwmgr, (u16)tmp);
> +               *value = vddgfx;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_UVD_VCLK:
> +               if (!cz_hwmgr->uvd_power_gated) {
> +                       if (uvd_index >= CZ_MAX_HARDWARE_POWERLEVELS) {
> +                               return -EINVAL;
> +                       } else {
> +                               vclk = uvd_table->entries[uvd_index].vclk;
> +                               *value = vclk;
> +                               return 0;
> +                       }
> +               }
> +               *value = 0;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_UVD_DCLK:
> +               if (!cz_hwmgr->uvd_power_gated) {
> +                       if (uvd_index >= CZ_MAX_HARDWARE_POWERLEVELS) {
> +                               return -EINVAL;
> +                       } else {
> +                               dclk = uvd_table->entries[uvd_index].dclk;
> +                               *value = dclk;
> +                               return 0;
> +                       }
> +               }
> +               *value = 0;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_VCE_ECCLK:
> +               if (!cz_hwmgr->vce_power_gated) {
> +                       if (vce_index >= CZ_MAX_HARDWARE_POWERLEVELS) {
> +                               return -EINVAL;
> +                       } else {
> +                               ecclk = vce_table->entries[vce_index].ecclk;
> +                               *value = ecclk;
> +                               return 0;
> +                       }
> +               }
> +               *value = 0;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_GPU_LOAD:
> +               result = smum_send_msg_to_smc(hwmgr->smumgr, PPSMC_MSG_GetAverageGraphicsActivity);
> +               if (0 == result) {
> +                       activity_percent = cgs_read_register(hwmgr->device, mmSMU_MP1_SRBM2P_ARG_0);
> +                       activity_percent = activity_percent > 100 ? 100 : activity_percent;
> +               } else {
> +                       activity_percent = 50;
> +               }
> +               *value = activity_percent;
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
>  static const struct pp_hwmgr_func cz_hwmgr_funcs = {
>         .backend_init = cz_hwmgr_backend_init,
>         .backend_fini = cz_hwmgr_backend_fini,
> @@ -1882,6 +1977,7 @@ static const struct pp_hwmgr_func cz_hwmgr_funcs = {
>         .get_current_shallow_sleep_clocks = cz_get_current_shallow_sleep_clocks,
>         .get_clock_by_type = cz_get_clock_by_type,
>         .get_max_high_clocks = cz_get_max_high_clocks,
> +       .read_sensor = cz_read_sensor,
>  };
>
>  int cz_hwmgr_init(struct pp_hwmgr *hwmgr)
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index f67e1e260b30..07a7d046d6f6 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -3144,6 +3144,41 @@ smu7_print_current_perforce_level(struct pp_hwmgr *hwmgr, struct seq_file *m)
>         seq_printf(m, "vce    %sabled\n", data->vce_power_gated ? "dis" : "en");
>  }
>
> +static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int idx, int32_t *value)
> +{
> +       uint32_t sclk, mclk, activity_percent;
> +       uint32_t offset;
> +       struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
> +
> +       switch (idx) {
> +       case AMDGPU_PP_SENSOR_GFX_SCLK:
> +               smum_send_msg_to_smc(hwmgr->smumgr, PPSMC_MSG_API_GetSclkFrequency);
> +               sclk = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0);
> +               *value = sclk;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_GFX_MCLK:
> +               smum_send_msg_to_smc(hwmgr->smumgr, PPSMC_MSG_API_GetMclkFrequency);
> +               mclk = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0);
> +               *value = mclk;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_GPU_LOAD:
> +               offset = data->soft_regs_start + smum_get_offsetof(hwmgr->smumgr,
> +                                                               SMU_SoftRegisters,
> +                                                               AverageGraphicsActivity);
> +
> +               activity_percent = cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, offset);
> +               activity_percent += 0x80;
> +               activity_percent >>= 8;
> +               *value = activity_percent > 100 ? 100 : activity_percent;
> +               return 0;
> +       case AMDGPU_PP_SENSOR_GPU_TEMP:
> +               *value = smu7_thermal_get_temperature(hwmgr);
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
>  static int smu7_find_dpm_states_clocks_in_dpm_table(struct pp_hwmgr *hwmgr, const void *input)
>  {
>         const struct phm_set_power_state_input *states =
> @@ -4315,6 +4350,7 @@ static struct pp_hwmgr_func smu7_hwmgr_funcs = {
>         .get_mclk_od = smu7_get_mclk_od,
>         .set_mclk_od = smu7_set_mclk_od,
>         .get_clock_by_type = smu7_get_clock_by_type,
> +       .read_sensor = smu7_read_sensor,
>  };
>
>  uint8_t smu7_get_sleep_divider_id_from_clock(uint32_t clock,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> index f941acf563a9..dfa0f38a5e76 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> @@ -29,6 +29,17 @@
>  #include "amd_shared.h"
>  #include "cgs_common.h"
>
> +enum amd_pp_sensors {
> +       AMDGPU_PP_SENSOR_GFX_SCLK = 0,
> +       AMDGPU_PP_SENSOR_VDDNB,
> +       AMDGPU_PP_SENSOR_VDDGFX,
> +       AMDGPU_PP_SENSOR_UVD_VCLK,
> +       AMDGPU_PP_SENSOR_UVD_DCLK,
> +       AMDGPU_PP_SENSOR_VCE_ECCLK,
> +       AMDGPU_PP_SENSOR_GPU_LOAD,
> +       AMDGPU_PP_SENSOR_GFX_MCLK,
> +       AMDGPU_PP_SENSOR_GPU_TEMP,
> +};
>
>  enum amd_pp_event {
>         AMD_PP_EVENT_INITIALIZE = 0,
> @@ -347,6 +358,7 @@ struct amd_powerplay_funcs {
>         int (*set_sclk_od)(void *handle, uint32_t value);
>         int (*get_mclk_od)(void *handle);
>         int (*set_mclk_od)(void *handle, uint32_t value);
> +       int (*read_sensor)(void *handle, int idx, int32_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 c9628b4db2c3..fcd45452380d 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -359,6 +359,7 @@ struct pp_hwmgr_func {
>         int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
>         int (*get_mclk_od)(struct pp_hwmgr *hwmgr);
>         int (*set_mclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
> +       int (*read_sensor)(struct pp_hwmgr *hwmgr, int idx, int32_t *value);
>  };
>
>  struct pp_table_func {
> --
> 2.10.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

amd-gfx Info Page - lists.freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx>

lists.freedesktop.org

To see the collection of prior postings to the list, visit the amd-gfx Archives. Using amd-gfx: To post a message to all the list members, send email ...