From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhu, Rex" Subject: Re: [PATCH 1/2] drm/amdgpu: add custom power policy support in sysfs Date: Wed, 10 Jan 2018 22:33:01 +0000 Message-ID: References: <1515582075-6326-1-git-send-email-Rex.Zhu@amd.com>, Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0960437124==" Return-path: In-Reply-To: Content-Language: en-US List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: Alex Deucher Cc: amd-gfx list --===============0960437124== Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CY4PR12MB1687E0FFA27CFA6F132C5731FB110CY4PR12MB1687namp_" --_000_CY4PR12MB1687E0FFA27CFA6F132C5731FB110CY4PR12MB1687namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yes, this new interface is more common, I just save the input and send to h= w backend. Smu7 and Vega can share this interface. Best Regards Rex ________________________________ From: Alex Deucher Sent: Thursday, January 11, 2018 4:18:11 AM To: Zhu, Rex Cc: amd-gfx list Subject: Re: [PATCH 1/2] drm/amdgpu: add custom power policy support in sys= fs On Wed, Jan 10, 2018 at 6:01 AM, Rex Zhu wrote: > when cat pp_power_profile_mode on Vega10 > NUM MODE_NAME BUSY_SET_POINT FPS USE_RLC_BUSY MIN_ACTIVE_LEVEL > 0 3D_FULL_SCREEN : 70 60 1 3 > 1 POWER_SAVING : 90 60 0 0 > 2 VIDEO*: 70 60 0 0 > 3 VR : 70 90 0 0 > 4 COMPUTER : 30 60 0 6 > 5 CUSTOM : 0 0 0 0 > > the result show all the profile mode we can support and custom mode. > user can echo the num(0-4) to pp_power_profile_mode to select the profile > mode or can echo "5 value value value value" to enter CUSTOM mode. > the four parameter is set_point/FPS/USER_RLC_BUSY/MIN_ACTIVE_LEVEL. > Is there any way we can unify this with profile stuff implemented for smu7? I'd like to avoid lots of haphazard interfaces to support specific use cases and asics. It becomes a maintenance nightmare. Alex > Change-Id: I72634646a9a179ccd57f175b4c0b3f45e538a03f > Signed-off-by: Rex Zhu > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h | 8 +++ > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 81 ++++++++++++++++++++= +++++- > drivers/gpu/drm/amd/include/kgd_pp_interface.h | 11 +++- > 3 files changed, 98 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h b/drivers/gpu/drm/am= d/amdgpu/amdgpu_dpm.h > index 8a8d09dd..986f1d5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h > @@ -366,6 +366,14 @@ enum amdgpu_pcie_gen { > (adev)->powerplay.pp_handle, virtual_addr_low, \ > virtual_addr_hi, mc_addr_low, mc_addr_hi, size) > > +#define amdgpu_dpm_get_power_profile_mode(adev, buf) \ > + ((adev)->powerplay.pp_funcs->get_power_profile_mode(\ > + (adev)->powerplay.pp_handle, buf)) > + > +#define amdgpu_dpm_set_power_profile_mode(adev, parameter, size) \ > + ((adev)->powerplay.pp_funcs->set_power_profile_mode(\ > + (adev)->powerplay.pp_handle, parameter, size)) > + > struct amdgpu_dpm { > struct amdgpu_ps *ps; > /* number of valid power states */ > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd= /amdgpu/amdgpu_pm.c > index e5ee7cf..662edca 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c > @@ -584,6 +584,73 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device *= dev, > return count; > } > > +static ssize_t amdgpu_get_pp_power_profile_mode(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct drm_device *ddev =3D dev_get_drvdata(dev); > + struct amdgpu_device *adev =3D ddev->dev_private; > + > + if (adev->powerplay.pp_funcs->get_power_profile_mode) > + return amdgpu_dpm_get_power_profile_mode(adev, buf); > + > + return snprintf(buf, PAGE_SIZE, "\n"); > +} > + > + > +static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t count) > +{ > + int ret =3D 0xff; > + struct drm_device *ddev =3D dev_get_drvdata(dev); > + struct amdgpu_device *adev =3D ddev->dev_private; > + uint32_t parameter_size =3D 0; > + long parameter[64]; > + char *sub_str, buf_cpy[128]; > + char *tmp_str; > + uint32_t i =3D 0; > + char tmp[2]; > + long int profile_mode =3D 0; > + const char delimiter[3] =3D {' ', '\n', '\0'}; > + > + tmp[0] =3D *(buf); > + tmp[1] =3D '\0'; > + ret =3D kstrtol(tmp, 0, &profile_mode); > + if (ret) > + goto fail; > + > + if (profile_mode =3D=3D PP_SMC_POWER_PROFILE_CUSTOM) { > + if (count < 2 || count > 127) > + return -EINVAL; > + while (isspace(*++buf)) > + i++; > + memcpy(buf_cpy, buf, count-i); > + tmp_str =3D buf_cpy; > + while (tmp_str[0]) { > + sub_str =3D strsep(&tmp_str, delimiter); > + ret =3D kstrtol(sub_str, 0, ¶meter[parameter_= size]); > + if (ret) { > + count =3D -EINVAL; > + goto fail; > + } > + pr_info("value is %ld \n", parameter[parameter_si= ze]); > + parameter_size++; > + while (isspace(*tmp_str)) > + tmp_str++; > + } > + } > + parameter[parameter_size] =3D profile_mode; > + if (adev->powerplay.pp_funcs->set_power_profile_mode) > + ret =3D amdgpu_dpm_set_power_profile_mode(adev, parameter= , parameter_size); > + > + if (!ret) > + return count; > +fail: > + return -EINVAL; > +} > + > static ssize_t amdgpu_get_pp_power_profile(struct device *dev, > char *buf, struct amd_pp_profile *query) > { > @@ -772,7 +839,9 @@ static DEVICE_ATTR(pp_gfx_power_profile, S_IRUGO | S_= IWUSR, > static DEVICE_ATTR(pp_compute_power_profile, S_IRUGO | S_IWUSR, > amdgpu_get_pp_compute_power_profile, > amdgpu_set_pp_compute_power_profile); > - > +static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR, > + amdgpu_get_pp_power_profile_mode, > + amdgpu_set_pp_power_profile_mode); > static ssize_t amdgpu_hwmon_show_temp(struct device *dev, > struct device_attribute *attr, > char *buf) > @@ -1405,6 +1474,14 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *ade= v) > return ret; > } > > + ret =3D device_create_file(adev->dev, > + &dev_attr_pp_power_profile_mode); > + if (ret) { > + DRM_ERROR("failed to create device file " > + "pp_power_profile_mode\n"); > + return ret; > + } > + > ret =3D amdgpu_debugfs_pm_init(adev); > if (ret) { > DRM_ERROR("Failed to register debugfs file for dpm!\n"); > @@ -1440,6 +1517,8 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *ade= v) > &dev_attr_pp_gfx_power_profile); > device_remove_file(adev->dev, > &dev_attr_pp_compute_power_profile); > + device_remove_file(adev->dev, > + &dev_attr_pp_power_profile_mode); > } > > void amdgpu_pm_compute_clocks(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu= /drm/amd/include/kgd_pp_interface.h > index 0f89d2a8..a823c03 100644 > --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h > +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h > @@ -140,7 +140,14 @@ struct amd_pp_init { > uint32_t feature_mask; > }; > > - > +enum PP_SMC_POWER_PROFILE { > + PP_SMC_POWER_PROFILE_FULLSCREEN3D =3D 0x0, > + PP_SMC_POWER_PROFILE_POWERSAVING =3D 0x1, > + PP_SMC_POWER_PROFILE_VIDEO =3D 0x2, > + PP_SMC_POWER_PROFILE_VR =3D 0x3, > + PP_SMC_POWER_PROFILE_COMPUTE =3D 0x4, > + PP_SMC_POWER_PROFILE_CUSTOM =3D 0x5, > +}; > > enum { > PP_GROUP_UNKNOWN =3D 0, > @@ -289,6 +296,8 @@ struct amd_pm_funcs { > struct pp_display_clock_request *clock); > int (*get_display_mode_validation_clocks)(void *handle, > struct amd_pp_simple_clock_info *clocks); > + int (*get_power_profile_mode)(void *handle, char *buf); > + int (*set_power_profile_mode)(void *handle, long *input, uint32_t= size); > }; > > #endif > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx --_000_CY4PR12MB1687E0FFA27CFA6F132C5731FB110CY4PR12MB1687namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yes, this new interface is more common, I just save the input and send to h= w backend. Smu7 and Vega can share this interface.


Best Regards
Rex
From: Alex Deucher <alex= deucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Thursday, January 11, 2018 4:18:11 AM
To: Zhu, Rex
Cc: amd-gfx list
Subject: Re: [PATCH 1/2] drm/amdgpu: add custom power policy support= in sysfs
 
On Wed, Jan 10, 2018 at 6:01 AM, Rex Zhu <Rex.Z= hu-5C7GfCeVMHo@public.gmane.org> wrote:
> when cat pp_power_profile_mode on Vega10
> NUM        MODE_NAME BUSY_SET_POINT= FPS USE_RLC_BUSY MIN_ACTIVE_LEVEL
>   0 3D_FULL_SCREEN :      &nbs= p;      70  60     &= nbsp;    1        &n= bsp;     3
>   1   POWER_SAVING :     =         90  60   &nb= sp;      0      &nbs= p;       0
>   2          VI= DEO*:           &nbs= p; 70  60          0 = ;             0=
>   3         &nb= sp;   VR :         &= nbsp;   70  90       &nbs= p;  0           = ;   0
>   4       COMPUTER : &nbs= p;           30  60&= nbsp;         0   &n= bsp;          6
>   5         CUSTOM :=             &nb= sp; 0   0          0=             &nb= sp; 0
>
> the result show all the profile mode we can support and custom mode. > user can echo the num(0-4) to pp_power_profile_mode to select the prof= ile
> mode or can echo "5 value value value value" to enter CUSTOM= mode.
> the four parameter is set_point/FPS/USER_RLC_BUSY/MIN_ACTIVE_LEVEL. >

Is there any way we can unify this with profile stuff implemented for
smu7?  I'd like to avoid lots of haphazard interfaces to support
specific use cases and asics.  It becomes a maintenance nightmare.

Alex

> Change-Id: I72634646a9a179ccd57f175b4c0b3f45e538a03f
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h    &= nbsp;   |  8 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c    &n= bsp;    | 81 +++++++++&#= 43;++++++++++++++&#= 43;-
>  drivers/gpu/drm/amd/include/kgd_pp_interface.h | 11 ++&#= 43;-
>  3 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h b/drivers/gpu/drm= /amd/amdgpu/amdgpu_dpm.h
> index 8a8d09dd..986f1d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> @@ -366,6 +366,14 @@ enum amdgpu_pcie_gen {
>            = ;             (= adev)->powerplay.pp_handle, virtual_addr_low, \
>            = ;             v= irtual_addr_hi, mc_addr_low, mc_addr_hi, size)
>
> +#define amdgpu_dpm_get_power_profile_mode(adev, buf) \
> +           = ;    ((adev)->powerplay.pp_funcs->get_power_profile_mo= de(\
> +           = ;            (adev)-= >powerplay.pp_handle, buf))
> +
> +#define amdgpu_dpm_set_power_profile_mode(adev, parameter, size) = \
> +           = ;    ((adev)->powerplay.pp_funcs->set_power_profile_mo= de(\
> +           = ;            (adev)-= >powerplay.pp_handle, parameter, size))
> +
>  struct amdgpu_dpm {
>         struct amdgpu_ps =        *ps;
>         /* number of valid pow= er states */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/= amd/amdgpu/amdgpu_pm.c
> index e5ee7cf..662edca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -584,6 +584,73 @@ static ssize_t amdgpu_set_pp_mclk_od(struct d= evice *dev,
>         return count;
>  }
>
> +static ssize_t amdgpu_get_pp_power_profile_mode(struct device *de= v,
> +           = ;    struct device_attribute *attr,
> +           = ;    char *buf)
> +{
> +       struct drm_device *ddev =3D = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = =3D ddev->dev_private;
> +
> +       if (adev->powerplay.pp_fu= ncs->get_power_profile_mode)
> +           = ;    return amdgpu_dpm_get_power_profile_mode(adev, buf); > +
> +       return snprintf(buf, PAGE_SI= ZE, "\n");
> +}
> +
> +
> +static ssize_t amdgpu_set_pp_power_profile_mode(struct device *de= v,
> +           = ;    struct device_attribute *attr,
> +           = ;    const char *buf,
> +           = ;    size_t count)
> +{
> +       int ret =3D 0xff;
> +       struct drm_device *ddev =3D = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = =3D ddev->dev_private;
> +       uint32_t parameter_size =3D = 0;
> +       long parameter[64];
> +       char *sub_str, buf_cpy[128];=
> +       char *tmp_str;
> +       uint32_t i =3D 0;
> +       char tmp[2];
> +       long int profile_mode =3D 0;=
> +       const char delimiter[3] =3D = {' ', '\n', '\0'};
> +
> +       tmp[0] =3D *(buf);
> +       tmp[1] =3D '\0';
> +       ret =3D kstrtol(tmp, 0, &= ;profile_mode);
> +       if (ret)
> +           = ;    goto fail;
> +
> +       if (profile_mode =3D=3D PP_S= MC_POWER_PROFILE_CUSTOM) {
> +           = ;    if (count < 2 || count > 127)
> +           = ;            return = -EINVAL;
> +           = ;    while (isspace(*++buf))
> +           = ;            i+&= #43;;
> +           = ;    memcpy(buf_cpy, buf, count-i);
> +           = ;    tmp_str =3D buf_cpy;
> +           = ;    while (tmp_str[0]) {
> +           = ;            sub_str= =3D strsep(&tmp_str, delimiter);
> +           = ;            ret =3D= kstrtol(sub_str, 0, &parameter[parameter_size]);
> +           = ;            if (ret= ) {
> +           = ;            &n= bsp;       count =3D -EINVAL;
> +           = ;            &n= bsp;       goto fail;
> +           = ;            }
> +           = ;            pr_info= ("value is %ld \n", parameter[parameter_size]);
> +           = ;            paramet= er_size++;
> +           = ;            while (= isspace(*tmp_str))
> +           = ;            &n= bsp;       tmp_str++;
> +           = ;    }
> +       }
> +       parameter[parameter_size] = =3D profile_mode;
> +       if (adev->powerplay.pp_fu= ncs->set_power_profile_mode)
> +           = ;    ret =3D amdgpu_dpm_set_power_profile_mode(adev, paramet= er, parameter_size);
> +
> +       if (!ret)
> +           = ;    return count;
> +fail:
> +       return -EINVAL;
> +}
> +
>  static ssize_t amdgpu_get_pp_power_profile(struct device *dev, >            = ;     char *buf, struct amd_pp_profile *query)
>  {
> @@ -772,7 +839,9 @@ static DEVICE_ATTR(pp_gfx_power_profile, S_IRU= GO | S_IWUSR,
>  static DEVICE_ATTR(pp_compute_power_profile, S_IRUGO | S_IWUSR,<= br> >            = ;     amdgpu_get_pp_compute_power_profile,
>            = ;     amdgpu_set_pp_compute_power_profile);
> -
> +static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR,
> +           = ;    amdgpu_get_pp_power_profile_mode,
> +           = ;    amdgpu_set_pp_power_profile_mode);
>  static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>            = ;            &n= bsp;            = ;  struct device_attribute *attr,
>            = ;            &n= bsp;            = ;  char *buf)
> @@ -1405,6 +1474,14 @@ int amdgpu_pm_sysfs_init(struct amdgpu_devi= ce *adev)
>            = ;     return ret;
>         }
>
> +       ret =3D device_create_file(a= dev->dev,
> +           = ;            &de= v_attr_pp_power_profile_mode);
> +       if (ret) {
> +           = ;    DRM_ERROR("failed to create device file "
> +           = ;            &n= bsp;       "pp_power_profile_mode\n"= ;);
> +           = ;    return ret;
> +       }
> +
>         ret =3D amdgpu_debugfs= _pm_init(adev);
>         if (ret) {
>            = ;     DRM_ERROR("Failed to register debugfs file f= or dpm!\n");
> @@ -1440,6 +1517,8 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_devi= ce *adev)
>            = ;             &= amp;dev_attr_pp_gfx_power_profile);
>         device_remove_file(ade= v->dev,
>            = ;             &= amp;dev_attr_pp_compute_power_profile);
> +       device_remove_file(adev->= dev,
> +           = ;            &de= v_attr_pp_power_profile_mode);
>  }
>
>  void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/= gpu/drm/amd/include/kgd_pp_interface.h
> index 0f89d2a8..a823c03 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -140,7 +140,14 @@ struct amd_pp_init {
>         uint32_t feature_mask;=
>  };
>
> -
> +enum PP_SMC_POWER_PROFILE {
> +       PP_SMC_POWER_PROFILE_FULLSCR= EEN3D =3D 0x0,
> +       PP_SMC_POWER_PROFILE_POWERSA= VING  =3D 0x1,
> +       PP_SMC_POWER_PROFILE_VIDEO&n= bsp;       =3D 0x2,
> +       PP_SMC_POWER_PROFILE_VR = ;          =3D 0x3,
> +       PP_SMC_POWER_PROFILE_COMPUTE=       =3D 0x4,
> +       PP_SMC_POWER_PROFILE_CUSTOM&= nbsp;      =3D 0x5,
> +};
>
>  enum {
>         PP_GROUP_UNKNOWN =3D 0= ,
> @@ -289,6 +296,8 @@ struct amd_pm_funcs {
>            = ;            &n= bsp;        struct pp_display_clock_requ= est *clock);
>         int (*get_display_mode= _validation_clocks)(void *handle,
>            = ;     struct amd_pp_simple_clock_info *clocks);
> +       int (*get_power_profile_mode= )(void *handle, char *buf);
> +       int (*set_power_profile_mode= )(void *handle, long *input, uint32_t size);
>  };
>
>  #endif
> --
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> htt= ps://lists.freedesktop.org/mailman/listinfo/amd-gfx
--_000_CY4PR12MB1687E0FFA27CFA6F132C5731FB110CY4PR12MB1687namp_-- --===============0960437124== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4Cg== --===============0960437124==--