I can add a guard to be safe. When I checked previously it just returned zeroes, but that might have just been good luck. And I'll remove the mutex too. Thanks!
Kent
> -----Original Message-----
> From: Quan, Evan <Evan.Quan@xxxxxxx>
> Sent: Thursday, February 10, 2022 11:46 PM
> To: Russell, Kent <Kent.Russell@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Russell, Kent <Kent.Russell@xxxxxxx>
> Subject: RE: [PATCH] drm/amdgpu: Add unique_id support for sienna cichlid
>
> [AMD Official Use Only]
>
> If this is only available with the latest pmfw, you might need to add some version guard
> there.
> Otherwise, garbage data might be got with latest driver + old pmfw.
>
> Also, the "metrics_lock" was already dropped from latest drm-next. So, it seems you
> worked with an outdated kernel.
>
> BR
> Evan
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Kent
> > Russell
> > Sent: Thursday, February 10, 2022 11:43 PM
> > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Russell, Kent <Kent.Russell@xxxxxxx>
> > Subject: [PATCH] drm/amdgpu: Add unique_id support for sienna cichlid
> >
> > This is being added to SMU Metrics, so add the required tie-ins in the kernel.
> > Also create the corresponding unique_id sysfs file.
> >
> > Signed-off-by: Kent Russell <kent.russell@xxxxxxx>
> > ---
> > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 3 +-
> > .../pmfw_if/smu11_driver_if_sienna_cichlid.h | 12 +++++--
> > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 33
> > +++++++++++++++++++
> > 3 files changed, 45 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > index ad5da252228b..f638bcfc3faa 100644
> > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > @@ -1969,7 +1969,8 @@ static int default_attr_update(struct
> > amdgpu_device *adev, struct amdgpu_device_
> > if (asic_type != CHIP_VEGA10 &&
> > asic_type != CHIP_VEGA20 &&
> > asic_type != CHIP_ARCTURUS &&
> > - asic_type != CHIP_ALDEBARAN)
> > + asic_type != CHIP_ALDEBARAN &&
> > + asic_type != CHIP_SIENNA_CICHLID)
> > *states = ATTR_STATE_UNSUPPORTED;
> > } else if (DEVICE_ATTR_IS(pp_features)) {
> > if (adev->flags & AMD_IS_APU || asic_type < CHIP_VEGA10)
> > diff --git
> > a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_
> > cichlid.h
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_
> > cichlid.h
> > index b253be602cc2..c09dec2c4e1e 100644
> > ---
> > a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_
> > cichlid.h
> > +++
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_
> > ci
> > +++ chlid.h
> > @@ -1419,8 +1419,12 @@ typedef struct {
> > uint8_t PcieRate ;
> > uint8_t PcieWidth ;
> > uint16_t AverageGfxclkFrequencyTarget;
> > - uint16_t Padding16_2;
> >
> > + //PMFW-8711
> > + uint32_t PublicSerialNumLower32;
> > + uint32_t PublicSerialNumUpper32;
> > +
> > + uint16_t Padding16_2;
> > } SmuMetrics_t;
> >
> > typedef struct {
> > @@ -1476,8 +1480,12 @@ typedef struct {
> > uint8_t PcieRate ;
> > uint8_t PcieWidth ;
> > uint16_t AverageGfxclkFrequencyTarget;
> > - uint16_t Padding16_2;
> >
> > + //PMFW-8711
> > + uint32_t PublicSerialNumLower32;
> > + uint32_t PublicSerialNumUpper32;
> > +
> > + uint16_t Padding16_2;
> > } SmuMetrics_V2_t;
> >
> > typedef struct {
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > index 2a7da2bad96a..048014f05b35 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> > @@ -451,6 +451,38 @@ static int sienna_cichlid_setup_pptable(struct
> > smu_context *smu)
> > return ret;
> > }
> >
> > +static void sienna_cichlid_get_unique_id(struct smu_context *smu) {
> > + struct amdgpu_device *adev = smu->adev;
> > + struct smu_table_context *smu_table = &smu->smu_table;
> > + SmuMetrics_t *metrics =
> > + &(((SmuMetricsExternal_t *)(smu_table->metrics_table))-
> > >SmuMetrics);
> > + SmuMetrics_V2_t *metrics_v2 =
> > + &(((SmuMetricsExternal_t *)(smu_table->metrics_table))-
> > >SmuMetrics_V2);
> > + uint32_t upper32 = 0, lower32 = 0;
> > + int ret;
> > +
> > + mutex_lock(&smu->metrics_lock);
> > + ret = smu_cmn_get_metrics_table_locked(smu, NULL, false);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + bool use_metrics_v2 = ((smu->adev->ip_versions[MP1_HWIP][0] ==
> > IP_VERSION(11, 0, 7)) &&
> > + (smu->smc_fw_version >= 0x3A4300)) ? true : false;
> > +
> > + upper32 = use_metrics_v2 ? metrics_v2->PublicSerialNumUpper32 :
> > + metrics->PublicSerialNumUpper32;
> > + lower32 = use_metrics_v2 ? metrics_v2->PublicSerialNumLower32 :
> > + metrics->PublicSerialNumLower32;
> > +
> > +out_unlock:
> > + mutex_unlock(&smu->metrics_lock);
> > +
> > + adev->unique_id = ((uint64_t)upper32 << 32) | lower32;
> > + if (adev->serial[0] == '\0')
> > + sprintf(adev->serial, "%016llx", adev->unique_id); }
> > +
> > static int sienna_cichlid_tables_init(struct smu_context *smu) {
> > struct smu_table_context *smu_table = &smu->smu_table; @@ -
> > 4012,6 +4044,7 @@ static const struct pptable_funcs
> > sienna_cichlid_ppt_funcs = {
> > .set_mp1_state = sienna_cichlid_set_mp1_state,
> > .stb_collect_info = sienna_cichlid_stb_get_data_direct,
> > .get_ecc_info = sienna_cichlid_get_ecc_info,
> > + .get_unique_id = sienna_cichlid_get_unique_id,
> > };
> >
> > void sienna_cichlid_set_ppt_funcs(struct smu_context *smu)
> > --
> > 2.25.1
