On 3/28/2022 9:12 PM, Russell, Kent wrote:
[AMD Official Use Only]

Responses inline

-----Original Message-----
From: Lazar, Lijo <lijo.la...@amd.com>
Sent: Monday, March 28, 2022 11:18 AM
To: Russell, Kent <kent.russ...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <alexander.deuc...@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid



On 3/28/2022 8:05 PM, Kent Russell wrote:
This is being added to SMU Metrics, so add the required tie-ins in the
kernel. Also create the corresponding unique_id sysfs file.

v2: Add FW version check, remove SMU mutex
v3: Fix style warning

Signed-off-by: Kent Russell <kent.russ...@amd.com>
Reviewed-by: Alex Deucher <alexander.deuc...@amd.com>
---
   drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
   .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 12 +++++--
   .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 36 +++++++++++++++++++
   3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 4151db2678fb..4a9aabc16fbc 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1993,6 +1993,7 @@ static int default_attr_update(struct amdgpu_device *adev,
struct amdgpu_device_
             case IP_VERSION(9, 4, 0):
             case IP_VERSION(9, 4, 1):
             case IP_VERSION(9, 4, 2):
+           case IP_VERSION(10, 3, 0):
                     *states = ATTR_STATE_SUPPORTED;
                     break;
             default:
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 3e4a314ef925..58f977320d06 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_cichlid.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;
+

Is this the case for other ASICs also which share the metrics data with
Sienna?

No, only for Sienna Cichlid. The PMFW guys didn't implement it for Navy 
Flounder or Dimgrey Cavefish.


+  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 38f04836c82f..550458f6246a 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
@@ -481,6 +481,41 @@ static int sienna_cichlid_setup_pptable(struct smu_context
*smu)
     return sienna_cichlid_patch_pptable_quirk(smu);
   }

+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;
+   bool use_metrics_v2;
+   int ret;
+
+   /* Only supported as of version 0.58.83.0 */
+   if (smu->smc_fw_version < 0x3A5300)
+           return;
+

Since this is shared with other ASICs, I guess this check itself may not
be enough. This function may be skipped if it's not MP1 11.0.7 or GC 10.3.0?


Since the sysfs file is only supported on Sienna Cichlid (10.3.0), is it a 
concern since the tie-in won't exist on the other SMU11-based ASICs? And this 
function is only referenced by sienna_cichlid, unless I misunderstood something 
(and someone else uses sienna_cichlid_tables_init).


This function also gets called as part of common init sequence - smu_get_unique_id. If PMFW version of Navi Flounder/Dimgrey ASIC is greater than Sienna, then it may go to the path which is not intended to be executed on that ASIC.

Thanks,
Lijo

  Kent

Thanks,
Lijo

+   ret = smu_cmn_get_metrics_table(smu, NULL, false);
+   if (ret)
+           goto out_unlock;
+
+   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:
+
+   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;
@@ -4182,6 +4217,7 @@ static const struct pptable_funcs 
sienna_cichlid_ppt_funcs = {
     .get_ecc_info = sienna_cichlid_get_ecc_info,
     .get_default_config_table_settings =
sienna_cichlid_get_default_config_table_settings,
     .set_config_table = sienna_cichlid_set_config_table,
+   .get_unique_id = sienna_cichlid_get_unique_id,
   };

   void sienna_cichlid_set_ppt_funcs(struct smu_context *smu)

Reply via email to