On Mon, Nov 15, 2021 at 3:49 PM Kakarya, Surbhi <Surbhi.Kakarya@xxxxxxx> wrote: > > [AMD Official Use Only] > > Hi Alex, > > I am porting the patches (http://gerrit-git.amd.com/c/brahma/ec/linux/+/396997 and http://gerrit-git.amd.com/c/brahma/ec/linux/+/528745) to provide the necessary SMU utils (basic and system_status) support in this branch. > If you just want to populate the new fields in the metrics table, that is all you need to do. No need for any of the other changes. I think the last hunk of your patch is all you need. Alex > Thanks > Surbhi > > -----Original Message----- > From: Alex Deucher <alexdeucher@xxxxxxxxx> > Sent: Friday, November 12, 2021 2:41 PM > To: Kakarya, Surbhi <Surbhi.Kakarya@xxxxxxx> > Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Zhang, Bokun <Bokun.Zhang@xxxxxxx>; Chang, HaiJun <HaiJun.Chang@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx> > Subject: Re: [PATCH] drm/amd/pm: Add sysfs interface for retrieving gpu metrics(V2) > > On Fri, Nov 12, 2021 at 12:46 PM Surbhi Kakarya <surbhi.kakarya@xxxxxxx> wrote: > > > > A new interface for UMD to retrieve gpu metrics data. This patch is > > based on an existing patch If7f3523915505c0ece0a56dfd476d2b8473440d4. > > > > It's not clear what you are trying to do here. > > > Signed-off-by: Surbhi Kakarya <Surbhi.Kakarya@xxxxxxx> > > Change-Id: I701110d78a85c092f5dda167a52350cc6dda7557 > > --- > > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 6 +++++- > > drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 2 +- > > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 4 +--- > > .../gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 10 ++++++++++ > > 4 files changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > index 01cca08a774f..d60426daddae 100644 > > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > @@ -1800,8 +1800,12 @@ static ssize_t amdgpu_get_gpu_metrics(struct device *dev, > > return ret; > > } > > > > - if (adev->powerplay.pp_funcs->get_gpu_metrics) > > + down_read(&adev->reset_sem); > > + if (is_support_sw_smu(adev)) > > + size = smu_sys_get_gpu_metrics(&adev->smu, &gpu_metrics); > > + else if (adev->powerplay.pp_funcs->get_gpu_metrics) > > size = amdgpu_dpm_get_gpu_metrics(adev, &gpu_metrics); > > + up_read(&adev->reset_sem); > > > > Why are you changing this code? > adev->powerplay.pp_funcs->get_gpu_metrics already points to > smu_sys_get_gpu_metrics(). Also why do you need to add the semaphore locking? > > > if (size <= 0) > > goto out; > > diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h > > b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h > > index 3557f4e7fc30..5ffe7e3bf1aa 100644 > > --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h > > +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h > > @@ -1397,6 +1397,6 @@ int smu_set_light_sbr(struct smu_context *smu, > > bool enable); > > > > int smu_wait_for_event(struct amdgpu_device *adev, enum smu_event_type event, > > uint64_t event_arg); > > - > > +ssize_t smu_sys_get_gpu_metrics(struct smu_context *smu, void > > +**table); > > #endif > > #endif > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > index b06c59dcc1b4..ec81abe385e3 100644 > > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > > @@ -3005,9 +3005,8 @@ static int smu_get_dpm_clock_table(void *handle, > > return ret; > > } > > > > -static ssize_t smu_sys_get_gpu_metrics(void *handle, void **table) > > +ssize_t smu_sys_get_gpu_metrics(struct smu_context *smu, void > > +**table) > > { > > - struct smu_context *smu = handle; > > ssize_t size; > > > > if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) @@ -3135,7 > > +3134,6 @@ static const struct amd_pm_funcs swsmu_pm_funcs = { > > .asic_reset_mode_2 = smu_mode2_reset, > > .set_df_cstate = smu_set_df_cstate, > > .set_xgmi_pstate = smu_set_xgmi_pstate, > > - .get_gpu_metrics = smu_sys_get_gpu_metrics, > > Why are you removing this? > > > .set_watermarks_for_clock_ranges = smu_set_watermarks_for_clock_ranges, > > .display_disable_memory_clock_switch = smu_display_disable_memory_clock_switch, > > .get_max_sustainable_clocks_by_dc = smu_get_max_sustainable_clocks_by_dc, > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > > b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > > index 3b1bf270ebc6..97d18e764665 100644 > > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > > @@ -3619,6 +3619,16 @@ static ssize_t sienna_cichlid_get_gpu_metrics(struct smu_context *smu, > > gpu_metrics->energy_accumulator = > > use_metrics_v2 ? metrics_v2->EnergyAccumulator : > > metrics->EnergyAccumulator; > > > > + if (metrics->CurrGfxVoltageOffset) > > + gpu_metrics->voltage_gfx = > > + (155000 - 625 * metrics->CurrGfxVoltageOffset) / 100; > > + if (metrics->CurrMemVidOffset) > > + gpu_metrics->voltage_mem = > > + (155000 - 625 * metrics->CurrMemVidOffset) / 100; > > + if (metrics->CurrSocVoltageOffset) > > + gpu_metrics->voltage_soc = > > + (155000 - 625 * metrics->CurrSocVoltageOffset) > > + / 100; > > + > > This change seems unrelated to the other changes in this patch. > > Alex > > > > average_gfx_activity = use_metrics_v2 ? metrics_v2->AverageGfxActivity : metrics->AverageGfxActivity; > > if (average_gfx_activity <= SMU_11_0_7_GFX_BUSY_THRESHOLD) > > gpu_metrics->average_gfxclk_frequency = > > -- > > 2.25.1 > >
