[Public]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> Sent: Friday, February 18, 2022 12:05 PM
> To: Liang, Prike <Prike.Liang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Quan, Evan
> <Evan.Quan@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>
> Subject: Re: [PATCH] drm/amd/pm: validate SMU feature enable message for
> getting feature enabled mask
>
>
>
> On 2/18/2022 9:25 AM, Prike Liang wrote:
> > There's always miss the SMU feature enabled checked in the NPI phase,
> > so let validate the SMU feature enable message directly rather than
> > add more and more MP1 version check.
> >
> > Signed-off-by: Prike Liang <Prike.Liang@xxxxxxx>
> > ---
> > drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 28 ++++++-------------------
> -
> > 1 file changed, 6 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > index f24111d28290..da1ac70ed455 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> > @@ -552,10 +552,9 @@ bool smu_cmn_clk_dpm_is_enabled(struct
> smu_context *smu,
> > int smu_cmn_get_enabled_mask(struct smu_context *smu,
> > uint64_t *feature_mask)
> > {
> > - struct amdgpu_device *adev = smu->adev;
> > uint32_t *feature_mask_high;
> > uint32_t *feature_mask_low;
> > - int ret = 0;
> > + int ret = 0, index = 0;
> >
> > if (!feature_mask)
> > return -EINVAL;
> > @@ -563,12 +562,10 @@ int smu_cmn_get_enabled_mask(struct
> smu_context *smu,
> > feature_mask_low = &((uint32_t *)feature_mask)[0];
> > feature_mask_high = &((uint32_t *)feature_mask)[1];
> >
> > - switch (adev->ip_versions[MP1_HWIP][0]) {
> > - /* For Vangogh and Yellow Carp */
> > - case IP_VERSION(11, 5, 0):
> > - case IP_VERSION(13, 0, 1):
> > - case IP_VERSION(13, 0, 3):
> > - case IP_VERSION(13, 0, 8):
> > + index = smu_cmn_to_asic_specific_index(smu,
> > + CMN2ASIC_MAPPING_MSG,
> > +
> SMU_MSG_GetEnabledSmuFeatures);
> > + if (index > 0) {
> > ret = smu_cmn_send_smc_msg_with_param(smu,
> >
> SMU_MSG_GetEnabledSmuFeatures,
> > 0,
> > @@ -580,19 +577,7 @@ int smu_cmn_get_enabled_mask(struct
> smu_context *smu,
> >
> SMU_MSG_GetEnabledSmuFeatures,
> > 1,
> > feature_mask_high);
> > - break;
> > - /*
> > - * For Cyan Skillfish and Renoir, there is no interface provided by
> PMFW
> > - * to retrieve the enabled features. So, we assume all features are
> enabled.
> > - * TODO: add other APU ASICs which suffer from the same issue here
> > - */
> > - case IP_VERSION(11, 0, 8):
> > - case IP_VERSION(12, 0, 0):
> > - case IP_VERSION(12, 0, 1):
> > - memset(feature_mask, 0xff, sizeof(*feature_mask));
> > - break;
>
> This is broken now as these ASICs don't support any message. It is best to
> take out smu_cmn_get_enabled_mask altogether and move to smu_v*.c or
> *_ppt.c as this is a callback function.
>
> Thanks,
> Lijo
>
Before this solution I also consider put the smu_cmn_get_enabled_mask implementation in each *_ppt directly, but seems need some effort for implementing on each *_ppt. How about we also check the SMU_MSG_GetEnabledSmuFeaturesHigh mapping index? As to the ASCI not support neither SMU_MSG_GetEnabledSmuFeatures nor SMU_MSG_GetEnabledSmuFeaturesHigh will hard code the feature mask in this case.
> > - /* other dGPU ASICs */
> > - default:
> > + } else {
> > ret = smu_cmn_send_smc_msg(smu,
> >
> SMU_MSG_GetEnabledSmuFeaturesHigh,
> > feature_mask_high);
> > @@ -602,7 +587,6 @@ int smu_cmn_get_enabled_mask(struct
> smu_context *smu,
> > ret = smu_cmn_send_smc_msg(smu,
> >
> SMU_MSG_GetEnabledSmuFeaturesLow,
> > feature_mask_low);
> > - break;
> > }
> >
> > return ret;
> >
