Re: [PATCH] drm/amd/pm: Add sysfs interface for retrieving gpu metrics(V2)

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

 



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
> >



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

  Powered by Linux