RE: [RFC][PATCH] drm/amdgpu/powerplay/smu10: Add custom profile

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

 



[AMD Official Use Only]

Driver can exchange the custom profiling settings with SMU FW using the table below:
TABLE_CUSTOM_DPM

And the related data structure is CustomDpmSettings_t.

BR
Evan
> -----Original Message-----
> From: Alex Deucher <alexdeucher@xxxxxxxxx>
> Sent: Monday, September 13, 2021 11:11 PM
> To: Daniel Gomez <daniel@xxxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>;
> Quan, Evan <Evan.Quan@xxxxxxx>; Zhu, Changfeng
> <Changfeng.Zhu@xxxxxxx>
> Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Maling list - DRI
> developers <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; Daniel Gomez
> <dagmcr@xxxxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>
> Subject: Re: [RFC][PATCH] drm/amdgpu/powerplay/smu10: Add custom
> profile
> 
> On Wed, Sep 8, 2021 at 3:23 AM Daniel Gomez <daniel@xxxxxxxx> wrote:
> >
> > On Tue, 7 Sept 2021 at 19:23, Alex Deucher <alexdeucher@xxxxxxxxx>
> wrote:
> > >
> > > On Tue, Sep 7, 2021 at 4:53 AM Daniel Gomez <daniel@xxxxxxxx> wrote:
> > > >
> > > > Add custom power profile mode support on smu10.
> > > > Update workload bit list.
> > > > ---
> > > >
> > > > Hi,
> > > >
> > > > I'm trying to add custom profile for the Raven Ridge but not sure
> > > > if I'd need a different parameter than PPSMC_MSG_SetCustomPolicy
> > > > to configure the custom values. The code seemed to support CUSTOM
> > > > for workload types but it didn't show up in the menu or accept any
> > > > user input parameter. So far, I've added that part but a bit
> > > > confusing to me what is the policy I need for setting these
> > > > parameters or if it's maybe not possible at all.
> > > >
> > > > After applying the changes I'd configure the CUSTOM mode as follows:
> > > >
> > > > echo manual >
> > > >
> /sys/class/drm/card0/device/hwmon/hwmon1/device/power_dpm_force_
> pe
> > > > rformance_level echo "6 70 90 0 0" >
> > > >
> /sys/class/drm/card0/device/hwmon/hwmon1/device/pp_power_profile_
> m
> > > > ode
> > > >
> > > > Then, using Darren Powell script for testing modes I get the
> > > > following
> > > > output:
> > > >
> > > > 05:00.0 VGA compatible controller [0300]: Advanced Micro Devices,
> > > > Inc. [AMD/ATI] Raven Ridge [Radeon Vega Series / Radeon Vega
> > > > Mobile Series] [1002:15dd] (rev 83) === pp_dpm_sclk ===
> > > > 0: 200Mhz
> > > > 1: 400Mhz *
> > > > 2: 1100Mhz
> > > > === pp_dpm_mclk ===
> > > > 0: 400Mhz
> > > > 1: 933Mhz *
> > > > 2: 1067Mhz
> > > > 3: 1200Mhz
> > > > === pp_power_profile_mode ===
> > > > NUM        MODE_NAME BUSY_SET_POINT FPS USE_RLC_BUSY
> MIN_ACTIVE_LEVEL
> > > >   0 BOOTUP_DEFAULT :             70  60          0              0
> > > >   1 3D_FULL_SCREEN :             70  60          1              3
> > > >   2   POWER_SAVING :             90  60          0              0
> > > >   3          VIDEO :             70  60          0              0
> > > >   4             VR :             70  90          0              0
> > > >   5        COMPUTE :             30  60          0              6
> > > >   6         CUSTOM*:             70  90          0              0
> > > >
> > > > As you can also see in my changes, I've also updated the workload
> > > > bit table but I'm not completely sure about that change. With the
> > > > tests I've done, using bit 5 for the WORKLOAD_PPLIB_CUSTOM_BIT
> > > > makes the gpu sclk locked around ~36%. So, maybe I'm missing a
> > > > clock limit configuraton table somewhere. Would you give me some
> > > > hints to proceed with this?
> > >
> > > I don't think APUs support customizing the workloads the same way
> > > dGPUs do.  I think they just support predefined profiles.
> > >
> > > Alex
> >
> >
> > Thanks Alex for the quick response. Would it make sense then to remove
> > the custom workload code (PP_SMC_POWER_PROFILE_CUSTOM) from the
> smu10?
> > That workload was added in this commit:
> > f6f75ebdc06c04d3cfcd100f1b10256a9cdca407 [1] and not use at all in the
> > code as it's limited to PP_SMC_POWER_PROFILE_COMPUTE index. The
> > smu10.h also includes the custom workload bit definition and that was
> > a bit confusing for me to understand if it was half-supported or not
> > possible to use at all as I understood from your comment.
> >
> > Perhaps could also be mentioned (if that's kind of standard) in the
> > documentation[2] so, the custom pp_power_profile_mode is only
> > supported in dGPUs.
> >
> > I can send the patches if it makes sense.
> 
> I guess I was thinking of another asic.  @Huang Rui, @changzhu, @Quan,
> Evan can any of you comment on what is required for custom profiles on
> APUs?
> 
> Alex
> 
> 
> >
> > [1]:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> >
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git
> %2
> >
> Fcommit%2Fdrivers%2Fgpu%2Fdrm%2Famd%2Fpm%2Fpowerplay%2Fhwmg
> r%2Fsmu10_h
> >
> wmgr.c%3Fid%3Df6f75ebdc06c04d3cfcd100f1b10256a9cdca407&amp;data=0
> 4%7C0
> >
> 1%7CEvan.Quan%40amd.com%7Cfdb9fc6f03f84cb69dc608d976c8b517%7C3d
> d8961fe
> >
> 4884e608e11a82d994e183d%7C0%7C0%7C637671426675410633%7CUnknown
> %7CTWFpb
> >
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
> 6Mn0
> > %3D%7C1000&amp;sdata=Nsmj%2BgJpv9QZj%2FKF57E9n7LJfcOc9Jg51jy0h
> eOPTRI%3
> > D&amp;reserved=0
> > [2]:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> w.
> > kernel.org%2Fdoc%2Fhtml%2Flatest%2Fgpu%2Famdgpu.html%23pp-
> power-profil
> > e-
> mode&amp;data=04%7C01%7CEvan.Quan%40amd.com%7Cfdb9fc6f03f84cb
> 69dc608
> >
> d976c8b517%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6376714
> 2667541
> >
> 0633%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2lu
> MzIiLCJBT
> >
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=8HqoOL9kiJSfdHelJI380I
> wVyhe
> > 8Zo5E9PwCa7jsoYE%3D&amp;reserved=0
> >
> > Daniel
> >
> > >
> > >
> > > >
> > > > Thanks in advance,
> > > > Daniel
> > > >
> > > >
> > > >  drivers/gpu/drm/amd/pm/inc/smu10.h            | 14 +++--
> > > >  .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c  | 57
> > > > +++++++++++++++++--
>   .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
> > > > |  1 +
> > > >  3 files changed, 61 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/pm/inc/smu10.h
> > > > b/drivers/gpu/drm/amd/pm/inc/smu10.h
> > > > index 9e837a5014c5..b96520528240 100644
> > > > --- a/drivers/gpu/drm/amd/pm/inc/smu10.h
> > > > +++ b/drivers/gpu/drm/amd/pm/inc/smu10.h
> > > > @@ -136,12 +136,14 @@
> > > >  #define FEATURE_CORE_CSTATES_MASK     (1 <<
> FEATURE_CORE_CSTATES_BIT)
> > > >
> > > >  /* Workload bits */
> > > > -#define WORKLOAD_PPLIB_FULL_SCREEN_3D_BIT 0
> > > > -#define WORKLOAD_PPLIB_VIDEO_BIT          2
> > > > -#define WORKLOAD_PPLIB_VR_BIT             3
> > > > -#define WORKLOAD_PPLIB_COMPUTE_BIT        4
> > > > -#define WORKLOAD_PPLIB_CUSTOM_BIT         5
> > > > -#define WORKLOAD_PPLIB_COUNT              6
> > > > +#define WORKLOAD_DEFAULT_BIT              0
> > > > +#define WORKLOAD_PPLIB_FULL_SCREEN_3D_BIT 1
> > > > +#define WORKLOAD_PPLIB_POWER_SAVING_BIT   2
> > > > +#define WORKLOAD_PPLIB_VIDEO_BIT          3
> > > > +#define WORKLOAD_PPLIB_VR_BIT             4
> > > > +#define WORKLOAD_PPLIB_COMPUTE_BIT        5
> > > > +#define WORKLOAD_PPLIB_CUSTOM_BIT         6
> > > > +#define WORKLOAD_PPLIB_COUNT              7
> > > >
> > > >  typedef struct {
> > > >         /* MP1_EXT_SCRATCH0 */
> > > > diff --git
> a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> > > > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> > > > index 1de3ae77e03e..fef9f9ac1c56 100644
> > > > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> > > > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c
> > > > @@ -110,6 +110,11 @@ static int smu10_initialize_dpm_defaults(struct
> pp_hwmgr *hwmgr)
> > > >         smu10_data->num_active_display = 0;
> > > >         smu10_data->deep_sleep_dcefclk = 0;
> > > >
> > > > +       smu10_data->custom_profile_mode[0] = 0;
> > > > +       smu10_data->custom_profile_mode[1] = 0;
> > > > +       smu10_data->custom_profile_mode[2] = 0;
> > > > +       smu10_data->custom_profile_mode[3] = 0;
> > > > +
> > > >         phm_cap_unset(hwmgr->platform_descriptor.platformCaps,
> > > >
> > > > PHM_PlatformCaps_SclkDeepSleep);
> > > >
> > > > @@ -544,6 +549,10 @@ static int smu10_hwmgr_backend_init(struct
> > > > pp_hwmgr *hwmgr)
> > > >
> > > >         hwmgr->backend = data;
> > > >
> > > > +       hwmgr->workload_mask = 1 << hwmgr-
> >workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
> > > > +       hwmgr->power_profile_mode =
> PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> > > > +       hwmgr->default_power_profile_mode =
> > > > + PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
> > > > +
> > > >         result = smu10_initialize_dpm_defaults(hwmgr);
> > > >         if (result != 0) {
> > > >                 pr_err("smu10_initialize_dpm_defaults failed\n");
> > > > @@ -1408,9 +1417,15 @@ static int
> conv_power_profile_to_pplib_workload(int power_profile)
> > > >         int pplib_workload = 0;
> > > >
> > > >         switch (power_profile) {
> > > > +       case PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT:
> > > > +               pplib_workload = WORKLOAD_DEFAULT_BIT;
> > > > +               break;
> > > >         case PP_SMC_POWER_PROFILE_FULLSCREEN3D:
> > > >                 pplib_workload = WORKLOAD_PPLIB_FULL_SCREEN_3D_BIT;
> > > >                 break;
> > > > +       case PP_SMC_POWER_PROFILE_POWERSAVING:
> > > > +               pplib_workload = WORKLOAD_PPLIB_POWER_SAVING_BIT;
> > > > +               break;
> > > >         case PP_SMC_POWER_PROFILE_VIDEO:
> > > >                 pplib_workload = WORKLOAD_PPLIB_VIDEO_BIT;
> > > >                 break;
> > > > @@ -1430,22 +1445,24 @@ static int
> > > > conv_power_profile_to_pplib_workload(int power_profile)
> > > >
> > > >  static int smu10_get_power_profile_mode(struct pp_hwmgr *hwmgr,
> > > > char *buf)  {
> > > > +       struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr
> > > > + *)(hwmgr->backend);
> > > >         uint32_t i, size = 0;
> > > >         static const uint8_t
> > > > -               profile_mode_setting[6][4] = {{70, 60, 0, 0,},
> > > > +               profile_mode_setting[7][4] = {{70, 60, 0, 0,},
> > > >                                                 {70, 60, 1, 3,},
> > > >                                                 {90, 60, 0, 0,},
> > > >                                                 {70, 60, 0, 0,},
> > > >                                                 {70, 90, 0, 0,},
> > > >                                                 {30, 60, 0, 6,},
> > > >                                                 };
> > > > -       static const char *profile_name[6] = {
> > > > +       static const char *profile_name[7] = {
> > > >                                         "BOOTUP_DEFAULT",
> > > >                                         "3D_FULL_SCREEN",
> > > >                                         "POWER_SAVING",
> > > >                                         "VIDEO",
> > > >                                         "VR",
> > > > -                                       "COMPUTE"};
> > > > +                                       "COMPUTE",
> > > > +                                       "CUSTOM"};
> > > >         static const char *title[6] = {"NUM",
> > > >                         "MODE_NAME",
> > > >                         "BUSY_SET_POINT", @@ -1459,11 +1476,15 @@
> > > > static int smu10_get_power_profile_mode(struct pp_hwmgr *hwmgr,
> char *buf)
> > > >         size += sysfs_emit_at(buf, size, "%s %16s %s %s %s %s\n",title[0],
> > > >                         title[1], title[2], title[3], title[4],
> > > > title[5]);
> > > >
> > > > -       for (i = 0; i <= PP_SMC_POWER_PROFILE_COMPUTE; i++)
> > > > +       for (i = 0; i < PP_SMC_POWER_PROFILE_CUSTOM; i++)
> > > >                 size += sysfs_emit_at(buf, size,
> "%3d %14s%s: %14d %3d %10d %14d\n",
> > > >                         i, profile_name[i], (i == hwmgr->power_profile_mode) ?
> "*" : " ",
> > > >                         profile_mode_setting[i][0], profile_mode_setting[i][1],
> > > >                         profile_mode_setting[i][2],
> > > > profile_mode_setting[i][3]);
> > > > +       size += sysfs_emit_at(buf, size,
> "%3d %14s%s: %14d %3d %10d %14d\n", i,
> > > > +                       profile_name[i], (i == hwmgr->power_profile_mode) ?
> "*" : " ",
> > > > +                       smu10_data->custom_profile_mode[0], smu10_data-
> >custom_profile_mode[1],
> > > > +                       smu10_data->custom_profile_mode[2],
> > > > + smu10_data->custom_profile_mode[3]);
> > > >
> > > >         return size;
> > > >  }
> > > > @@ -1480,16 +1501,42 @@ static bool smu10_is_raven1_refresh(struct
> > > > pp_hwmgr *hwmgr)
> > > >
> > > >  static int smu10_set_power_profile_mode(struct pp_hwmgr *hwmgr,
> > > > long *input, uint32_t size)  {
> > > > +       struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr
> *)(hwmgr->backend);
> > > > +       uint8_t busy_set_point, FPS, use_rlc_busy, min_active_level;
> > > > +       uint32_t power_profile_mode = input[size];
> > > >         int workload_type = 0;
> > > >         int result = 0;
> > > >
> > > > -       if (input[size] > PP_SMC_POWER_PROFILE_COMPUTE) {
> > > > +       if (input[size] > PP_SMC_POWER_PROFILE_CUSTOM) {
> > > >                 pr_err("Invalid power profile mode %ld\n", input[size]);
> > > >                 return -EINVAL;
> > > >         }
> > > >         if (hwmgr->power_profile_mode == input[size])
> > > >                 return 0;
> > > >
> > > > +       if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM)
> {
> > > > +               if (size != 0 && size != 4)
> > > > +                       return -EINVAL;
> > > > +
> > > > +               if (size == 0) {
> > > > +                       if (smu10_data->custom_profile_mode[0] != 0)
> > > > +                               goto out;
> > > > +                       else
> > > > +                               return -EINVAL;
> > > > +               }
> > > > +
> > > > +               smu10_data->custom_profile_mode[0] = busy_set_point =
> input[0];
> > > > +               smu10_data->custom_profile_mode[1] = FPS = input[1];
> > > > +               smu10_data->custom_profile_mode[2] = use_rlc_busy =
> input[2];
> > > > +               smu10_data->custom_profile_mode[3] = min_active_level =
> input[3];
> > > > +               smum_send_msg_to_smc_with_parameter(hwmgr,
> > > > +                                       PPSMC_MSG_SetCustomPolicy,
> > > > +                                       busy_set_point | FPS<<8 |
> > > > +                                       use_rlc_busy << 16 | min_active_level<<24,
> > > > +                                       NULL);
> > > > +       }
> > > > +
> > > > +out:
> > > >         /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT
> */
> > > >         workload_type =
> > > >                 conv_power_profile_to_pplib_workload(input[size]);
> > > > diff --git
> a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
> > > > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
> > > > index 808e0ecbe1f0..4c4b2b1b510a 100644
> > > > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
> > > > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.h
> > > > @@ -302,6 +302,7 @@ struct smu10_hwmgr {
> > > >         uint32_t                             num_active_display;
> > > >
> > > >         bool                                                    fine_grain_enabled;
> > > > +       uint8_t                              custom_profile_mode[4];
> > > >  };
> > > >
> > > >  struct pp_hwmgr;
> > > > --
> > > > 2.30.2
> > > >




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux