Sorry for all of the questions, Evan. I see that we store the CUSTOM profile for Vega10/Fiji, but is that a limitation of the HW not being able to store it via the SMU? It seems like a lot of extra work to store a local copy, when we can just get the copy from the GPU itself, especially if we don't need to do that on future ASICs. I would assume that the CUSTOM profile that is kept on the GPU is valid, but would it be simpler to just initialize the CUSTOM profile to the same values as the BOOTUP values when we initialize the ASIC, then submit that to the GPU, thus ensuring that the CUSTOM profile always has some valid values? Or conversely, zeroing out the CUSTOM profile on initialization, so that the check is still valid.
I have no problem doing the work, I just want to make sure that it makes the most sense and that we're not having to redo it later if the SMU standardizes things and actually allows all future ASICs to store the CUSTOM profile on the GPU. Thanks! Kent From: Quan, Evan Sent: Thursday, March 28, 2019 10:47 AM To: Russell, Kent <kent.russ...@amd.com>; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on Vega10/20 v2 OK, I see. Yep, the new settings will be stored in SMU. But as I know, the SMU comes with some default settings for CUSTOM profile. I suspect "activity_monitor.Soc_BoosterFreqType != 0" will be also true even without your first CUSTOM profile settings(with input parameters). So, I would prefer to have a local copy of previous custom settigns(as what we do on vega10 and smu7). Evan From: Russell, Kent <kent.russ...@amd.com<mailto:kent.russ...@amd.com>> Sent: Thursday, March 28, 2019 5:29 PM To: Quan, Evan <evan.q...@amd.com<mailto:evan.q...@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Subject: Re: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on Vega10/20 v2 Am I mistaken in thinking that activity monitor gets the existing CUSTOM profile from the SMU? When I tested it, the CUSTOM profile was saved and could be retrieved through this method. I specifically set the profile to something ridiculous (1 2 3 4 5 6 7 8 9 10) and these values were retained when switching to VR and back to CUSTOM, so I was under the impression that it was saved to the SMU when we submit it to the SMU. 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:36:02 PM To: Russell, Kent; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Cc: Russell, Kent Subject: RE: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on Vega10/20 v2 For vega20, activity_monitor is also a temporary structure and you should not rely on that for judging existence of custom profile. Regards Evan > -----Original Message----- > From: amd-gfx > <amd-gfx-boun...@lists.freedesktop.org<mailto:amd-gfx-boun...@lists.freedesktop.org>> > On Behalf Of > Russell, Kent > Sent: Wednesday, March 27, 2019 9:59 PM > To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > Cc: Russell, Kent <kent.russ...@amd.com<mailto:kent.russ...@amd.com>> > Subject: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on > Vega10/20 v2 > > Don't return an error if the CUSTOM profile is selected, just apply it with > the > values saved to the GPU > > v2: Remove reference to fixed bug, check that CUSTOM profile was set > > Change-Id: I114cc9783226ee9ebb146863897e951527a85e20 > Signed-off-by: Kent Russell > <kent.russ...@amd.com<mailto:kent.russ...@amd.com>> > --- > drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 14 > +++++++++++++- > drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 14 > +++++++++++++- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c > index 85a536924571..7e9e7e254a0d 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c > @@ -4910,9 +4910,20 @@ static int vega10_set_power_profile_mode(struct > pp_hwmgr *hwmgr, long *input, ui > 1 << power_profile_mode); > > if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) { > - if (size == 0 || size > 4) > + if (size != 0 && size != 4) > return -EINVAL; > > + /* If size = 0 and the CUSTOM profile has been set already > + * (check BUSY_SET_POINT since that is always >0 for a valid > + * profile) then just apply the profile. > + */ > + if (size == 0) { > + if (data->custom_profile_mode[0] != 0) > + goto out; > + else > + return -EINVAL; > + } > + > data->custom_profile_mode[0] = busy_set_point = input[0]; > data->custom_profile_mode[1] = FPS = input[1]; > data->custom_profile_mode[2] = use_rlc_busy = input[2]; > @@ -4923,6 +4934,7 @@ static int vega10_set_power_profile_mode(struct > pp_hwmgr *hwmgr, long *input, ui > use_rlc_busy << 16 | > min_active_level<<24); > } > > +out: > hwmgr->power_profile_mode = power_profile_mode; > > return 0; > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c > index 70dc641bf94d..6dc16a9e766b 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c > @@ -3843,7 +3843,7 @@ static int vega20_set_power_profile_mode(struct > pp_hwmgr *hwmgr, long *input, ui > } > > if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) { > - if (size < 10) > + if (size < 10 && size != 0) > return -EINVAL; > > result = vega20_get_activity_monitor_coeff(hwmgr, > @@ -3853,6 +3853,17 @@ static int vega20_set_power_profile_mode(struct > pp_hwmgr *hwmgr, long *input, ui > "[SetPowerProfile] Failed to get activity > monitor!", > return result); > > + /* If size = 0 and the CUSTOM profile has been set already > + * (check BoosterFreqType since that is always >0 for a valid > + * profile) then just apply the profile. > + */ > + if (size == 0) { > + if (activity_monitor.Soc_BoosterFreqType != 0) > + goto out; > + else > + return -EINVAL; > + } > + > switch (input[0]) { > case 0: /* Gfxclk */ > activity_monitor.Gfx_FPS = input[1]; @@ -3908,6 > +3919,7 @@ static int vega20_set_power_profile_mode(struct pp_hwmgr > *hwmgr, long *input, ui > return result); > } > > +out: > /* conv PP_SMC_POWER_PROFILE* to WORKLOAD_PPLIB_*_BIT */ > workload_type = > > conv_power_profile_to_pplib_workload(power_profile_mode); > -- > 2.17.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
