[PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 v2

[Quan, Evan]

Oops, sorry, missed that. That will be fine.
Reviewed-by: Evan Quan <evan.quan at amd.com>

Evan

From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Russell, Kent
Sent: Thursday, March 28, 2019 5:25 PM
To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 v2

Hi Evan,

I set tmp=smu7_profiling[PP_SMC_PROFILE_CUSTOM] before checking the bupdate parameters (see below). That way I don't have ugly 100-character-long checks in the if size==0 check. Then if its invalid I just return -EINVAL, or if it'svalid then we use it to writeto the SMU. If parameters are passed in, tmp is overwritten with the passed-in values and we use it as before.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122
________________________________
From: Quan, Evan
Sent: Wednesday, March 27, 2019 9:26:41 PM
To: Russell, Kent; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
Cc: Russell, Kent
Subject: RE: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 v2

I think you should use smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM].bupdate_sclk/bupdate_mclk to judge whether there is saved custom profile.
That's where previous custom profile settings store in.
'tmp' is a temporary structures and you should not rely on that.

Regards,
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org<mailto:amd-gfx-bounces at lists.freedesktop.org>> On Behalf Of
> Russell, Kent
> Sent: Wednesday, March 27, 2019 9:59 PM
> To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
> Cc: Russell, Kent <Kent.Russell at amd.com<mailto:Kent.Russell at amd.com>>
> Subject: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on
> smu7 v2
>
> Allow changing to the CUSTOM profile without requiring the parameters
> being passed in each time. Store the values in the smu7_profiling table since
> it's defined here anyways
>
> v2: Add check that CUSTOM was previously set
>
> Change-Id: I6c5e3a1487e12410a6a7670a5cf1a6599253344d
> Signed-off-by: Kent Russell <kent.russell at amd.com<mailto:kent.russell at amd.com>>
> ---
>  .../gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 32 ++++++++++++--
> -----
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 83d3d935f3ac..048757e8f494 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -77,7 +77,7 @@
>  #define PCIE_BUS_CLK                10000
>  #define TCLK                        (PCIE_BUS_CLK / 10)
>
> -static const struct profile_mode_setting smu7_profiling[7] =
> +static struct profile_mode_setting smu7_profiling[7] =
>                                        {{0, 0, 0, 0, 0, 0, 0, 0},
>                                         {1, 0, 100, 30, 1, 0, 100, 10},
>                                         {1, 10, 0, 30, 0, 0, 0, 0},
> @@ -4984,17 +4984,27 @@ static int smu7_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, uint
>        mode = input[size];
>        switch (mode) {
>        case PP_SMC_POWER_PROFILE_CUSTOM:
> -             if (size < 8)
> +             if (size < 8 && size != 0)
>                        return -EINVAL;
> -
> -             tmp.bupdate_sclk = input[0];
> -             tmp.sclk_up_hyst = input[1];
> -             tmp.sclk_down_hyst = input[2];
> -             tmp.sclk_activity = input[3];
> -             tmp.bupdate_mclk = input[4];
> -             tmp.mclk_up_hyst = input[5];
> -             tmp.mclk_down_hyst = input[6];
> -             tmp.mclk_activity = input[7];
> +             /* If only CUSTOM is passed in, use the saved values. Check
> +              * that we actually have a CUSTOM profile by ensuring that
> +              * the "use sclk" or the "use mclk" bits are set
> +              */
> +             tmp = smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM];
> +             if (size == 0) {
> +                     if (tmp.bupdate_sclk == 0 && tmp.bupdate_mclk ==
> 0)
> +                             return -EINVAL;
> +             } else {
> +                     tmp.bupdate_sclk = input[0];
> +                     tmp.sclk_up_hyst = input[1];
> +                     tmp.sclk_down_hyst = input[2];
> +                     tmp.sclk_activity = input[3];
> +                     tmp.bupdate_mclk = input[4];
> +                     tmp.mclk_up_hyst = input[5];
> +                     tmp.mclk_down_hyst = input[6];
> +                     tmp.mclk_activity = input[7];
> +                     smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM]
> = tmp;
> +             }
>                if (!smum_update_dpm_settings(hwmgr, &tmp)) {
>                        memcpy(&data->current_profile_setting, &tmp,
> sizeof(struct profile_mode_setting));
>                        hwmgr->power_profile_mode = mode;
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190328/56562184/attachment.html>