All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: Use switch case for unique_id
@ 2022-03-28 17:07 Kent Russell
  2022-03-28 17:07 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
  0 siblings, 1 reply; 15+ messages in thread
From: Kent Russell @ 2022-03-28 17:07 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Kent Russell, Kevin Wang

To ease readability, use switch to set unique_id as supported for the
supported IP_VERSIONs, and set it to unsupported by default for all
other ASICs.
This makes it easier to add IP_VERSIONs later on, and makes it obvious
that it is not supported by default, instead of the current logic that
assumes that it is supported unless it is not one of the specified
IP_VERSIONs.

v2: Rebase onto previous IP_VERSION change

Signed-off-by: Kent Russell <kent.russell@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Kevin Wang <KevinYang.Wang@amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 9ce597ded31d..4151db2678fb 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1988,11 +1988,16 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
 		if (adev->flags & AMD_IS_APU)
 			*states = ATTR_STATE_UNSUPPORTED;
 	} else if (DEVICE_ATTR_IS(unique_id)) {
-		if (gc_ver != IP_VERSION(9, 0, 1) &&
-		    gc_ver != IP_VERSION(9, 4, 0) &&
-		    gc_ver != IP_VERSION(9, 4, 1) &&
-		    gc_ver != IP_VERSION(9, 4, 2))
+		switch (gc_ver) {
+		case IP_VERSION(9, 0, 1):
+		case IP_VERSION(9, 4, 0):
+		case IP_VERSION(9, 4, 1):
+		case IP_VERSION(9, 4, 2):
+			*states = ATTR_STATE_SUPPORTED;
+			break;
+		default:
 			*states = ATTR_STATE_UNSUPPORTED;
+		}
 	} else if (DEVICE_ATTR_IS(pp_features)) {
 		if (adev->flags & AMD_IS_APU || gc_ver < IP_VERSION(9, 0, 0))
 			*states = ATTR_STATE_UNSUPPORTED;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 17:07 [PATCH 1/2] drm/amdgpu: Use switch case for unique_id Kent Russell
@ 2022-03-28 17:07 ` Kent Russell
  2022-03-29  4:34   ` Lazar, Lijo
  0 siblings, 1 reply; 15+ messages in thread
From: Kent Russell @ 2022-03-28 17:07 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Kent Russell

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
v4: Add MP1 IP_VERSION check to FW version check

Signed-off-by: Kent Russell <kent.russell@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
 .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 10 ++++-
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 37 +++++++++++++++++++
 3 files changed, 46 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..037f38b0fa15 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,11 @@ typedef struct {
   uint8_t  PcieRate               ;
   uint8_t  PcieWidth              ;
   uint16_t AverageGfxclkFrequencyTarget;
-  uint16_t Padding16_2;
 
+  uint32_t PublicSerialNumLower32;
+  uint32_t PublicSerialNumUpper32;
+
+  uint16_t Padding16_2;
 } SmuMetrics_t;
 
 typedef struct {
@@ -1476,8 +1479,11 @@ typedef struct {
   uint8_t  PcieRate               ;
   uint8_t  PcieWidth              ;
   uint16_t AverageGfxclkFrequencyTarget;
-  uint16_t Padding16_2;
 
+  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 38f04836c82f..140005bf6d9e 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,42 @@ 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 and only on Sienna Cichlid */
+	if (smu->smc_fw_version < 0x3A5300 ||
+	    smu->adev->ip_versions[MP1_HWIP][0] != IP_VERSION(11, 0, 7))
+		return;
+
+	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 +4218,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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 17:07 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
@ 2022-03-29  4:34   ` Lazar, Lijo
  2022-03-29 12:10     ` Russell, Kent
  0 siblings, 1 reply; 15+ messages in thread
From: Lazar, Lijo @ 2022-03-29  4:34 UTC (permalink / raw)
  To: Kent Russell, amd-gfx; +Cc: Alex Deucher



On 3/28/2022 10:37 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
> v4: Add MP1 IP_VERSION check to FW version check
> 
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
>   .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 10 ++++-
>   .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 37 +++++++++++++++++++
>   3 files changed, 46 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..037f38b0fa15 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,11 @@ typedef struct {
>     uint8_t  PcieRate               ;
>     uint8_t  PcieWidth              ;
>     uint16_t AverageGfxclkFrequencyTarget;
> -  uint16_t Padding16_2;
>   
> +  uint32_t PublicSerialNumLower32;
> +  uint32_t PublicSerialNumUpper32;
> +
> +  uint16_t Padding16_2;
>   } SmuMetrics_t;
>   
>   typedef struct {
> @@ -1476,8 +1479,11 @@ typedef struct {
>     uint8_t  PcieRate               ;
>     uint8_t  PcieWidth              ;
>     uint16_t AverageGfxclkFrequencyTarget;
> -  uint16_t Padding16_2;
>   
> +  uint32_t PublicSerialNumLower32;
> +  uint32_t PublicSerialNumUpper32;
> +
> +  uint16_t Padding16_2;
>   } SmuMetrics_V2_t;
>   

Hi Kent,

Are you using the latest code?

   uint8_t  PcieWidth;
   uint16_t AverageGfxclkFrequencyTarget;

} SmuMetrics_V3_t;


V3 is what I see as the latest in our source and this struct is 
consistent with the latest PMFW.

Thanks,
Lijo

>   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..140005bf6d9e 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,42 @@ 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 and only on Sienna Cichlid */
> +	if (smu->smc_fw_version < 0x3A5300 ||
> +	    smu->adev->ip_versions[MP1_HWIP][0] != IP_VERSION(11, 0, 7))
> +		return;
> +
> +	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 +4218,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)
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-29  4:34   ` Lazar, Lijo
@ 2022-03-29 12:10     ` Russell, Kent
  0 siblings, 0 replies; 15+ messages in thread
From: Russell, Kent @ 2022-03-29 12:10 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only]

Ah I didn’t see that we added a Metrics v3 in there between now and when I made my first version of this patch. I'll test it out and make sure that things are still looking alright. Thanks Lijo!

 Kent

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Tuesday, March 29, 2022 12:34 AM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
>
>
>
> On 3/28/2022 10:37 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
> > v4: Add MP1 IP_VERSION check to FW version check
> >
> > Signed-off-by: Kent Russell <kent.russell@amd.com>
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
> >   .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 10 ++++-
> >   .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 37 +++++++++++++++++++
> >   3 files changed, 46 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..037f38b0fa15 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,11 @@ typedef struct {
> >     uint8_t  PcieRate               ;
> >     uint8_t  PcieWidth              ;
> >     uint16_t AverageGfxclkFrequencyTarget;
> > -  uint16_t Padding16_2;
> >
> > +  uint32_t PublicSerialNumLower32;
> > +  uint32_t PublicSerialNumUpper32;
> > +
> > +  uint16_t Padding16_2;
> >   } SmuMetrics_t;
> >
> >   typedef struct {
> > @@ -1476,8 +1479,11 @@ typedef struct {
> >     uint8_t  PcieRate               ;
> >     uint8_t  PcieWidth              ;
> >     uint16_t AverageGfxclkFrequencyTarget;
> > -  uint16_t Padding16_2;
> >
> > +  uint32_t PublicSerialNumLower32;
> > +  uint32_t PublicSerialNumUpper32;
> > +
> > +  uint16_t Padding16_2;
> >   } SmuMetrics_V2_t;
> >
>
> Hi Kent,
>
> Are you using the latest code?
>
>    uint8_t  PcieWidth;
>    uint16_t AverageGfxclkFrequencyTarget;
>
> } SmuMetrics_V3_t;
>
>
> V3 is what I see as the latest in our source and this struct is
> consistent with the latest PMFW.
>
> Thanks,
> Lijo
>
> >   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..140005bf6d9e 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,42 @@ 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 and only on Sienna Cichlid */
> > +   if (smu->smc_fw_version < 0x3A5300 ||
> > +       smu->adev->ip_versions[MP1_HWIP][0] != IP_VERSION(11, 0, 7))
> > +           return;
> > +
> > +   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 +4218,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)
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 17:01   ` Alex Deucher
@ 2022-03-28 17:06     ` Russell, Kent
  0 siblings, 0 replies; 15+ messages in thread
From: Russell, Kent @ 2022-03-28 17:06 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Deucher, Alexander, amd-gfx list

[AMD Official Use Only]

> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Monday, March 28, 2022 1:02 PM
> To: Russell, Kent <Kent.Russell@amd.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Deucher, Alexander
> <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
>
> On Mon, Mar 28, 2022 at 12:53 PM Kent Russell <kent.russell@amd.com> 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
> > v4: Add IP_VERSION check to FW version check
> >
> > Signed-off-by: Kent Russell <kent.russell@amd.com>
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >  drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
> >  .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 10 ++++-
> >  .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 37 +++++++++++++++++++
> >  3 files changed, 46 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..037f38b0fa15 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,11 @@ typedef struct {
> >    uint8_t  PcieRate               ;
> >    uint8_t  PcieWidth              ;
> >    uint16_t AverageGfxclkFrequencyTarget;
> > -  uint16_t Padding16_2;
> >
> > +  uint32_t PublicSerialNumLower32;
> > +  uint32_t PublicSerialNumUpper32;
> > +
> > +  uint16_t Padding16_2;
> >  } SmuMetrics_t;
> >
> >  typedef struct {
> > @@ -1476,8 +1479,11 @@ typedef struct {
> >    uint8_t  PcieRate               ;
> >    uint8_t  PcieWidth              ;
> >    uint16_t AverageGfxclkFrequencyTarget;
> > -  uint16_t Padding16_2;
> >
> > +  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 38f04836c82f..9f0c353f9b42 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,42 @@ 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 and only on GC 10.3.0 */
> > +       if (smu->smc_fw_version < 0x3A5300 ||
> > +           smu->adev->ip_versions[GC_HWIP][0] != IP_VERSION(10, 3, 0))
>
> Maybe replace the GC check with MP1 check for consistency?  E.g.,
> smu->adev->ip_versions[MP1_HWIP][0] != IP_VERSION(11, 0, 7)

Sounds good.

 Kent

>
> Alex
>
> > +               return;
> > +
> > +       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 +4218,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)
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 16:53 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
@ 2022-03-28 17:01   ` Alex Deucher
  2022-03-28 17:06     ` Russell, Kent
  0 siblings, 1 reply; 15+ messages in thread
From: Alex Deucher @ 2022-03-28 17:01 UTC (permalink / raw)
  To: Kent Russell; +Cc: Alex Deucher, amd-gfx list

On Mon, Mar 28, 2022 at 12:53 PM Kent Russell <kent.russell@amd.com> 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
> v4: Add IP_VERSION check to FW version check
>
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
>  .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 10 ++++-
>  .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 37 +++++++++++++++++++
>  3 files changed, 46 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..037f38b0fa15 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,11 @@ typedef struct {
>    uint8_t  PcieRate               ;
>    uint8_t  PcieWidth              ;
>    uint16_t AverageGfxclkFrequencyTarget;
> -  uint16_t Padding16_2;
>
> +  uint32_t PublicSerialNumLower32;
> +  uint32_t PublicSerialNumUpper32;
> +
> +  uint16_t Padding16_2;
>  } SmuMetrics_t;
>
>  typedef struct {
> @@ -1476,8 +1479,11 @@ typedef struct {
>    uint8_t  PcieRate               ;
>    uint8_t  PcieWidth              ;
>    uint16_t AverageGfxclkFrequencyTarget;
> -  uint16_t Padding16_2;
>
> +  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 38f04836c82f..9f0c353f9b42 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,42 @@ 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 and only on GC 10.3.0 */
> +       if (smu->smc_fw_version < 0x3A5300 ||
> +           smu->adev->ip_versions[GC_HWIP][0] != IP_VERSION(10, 3, 0))

Maybe replace the GC check with MP1 check for consistency?  E.g.,
smu->adev->ip_versions[MP1_HWIP][0] != IP_VERSION(11, 0, 7)

Alex

> +               return;
> +
> +       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 +4218,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)
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 16:53 [PATCH 1/2] drm/amdgpu: Use switch case for unique_id Kent Russell
@ 2022-03-28 16:53 ` Kent Russell
  2022-03-28 17:01   ` Alex Deucher
  0 siblings, 1 reply; 15+ messages in thread
From: Kent Russell @ 2022-03-28 16:53 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Kent Russell

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
v4: Add IP_VERSION check to FW version check

Signed-off-by: Kent Russell <kent.russell@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  1 +
 .../pmfw_if/smu11_driver_if_sienna_cichlid.h  | 10 ++++-
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 37 +++++++++++++++++++
 3 files changed, 46 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..037f38b0fa15 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,11 @@ typedef struct {
   uint8_t  PcieRate               ;
   uint8_t  PcieWidth              ;
   uint16_t AverageGfxclkFrequencyTarget;
-  uint16_t Padding16_2;
 
+  uint32_t PublicSerialNumLower32;
+  uint32_t PublicSerialNumUpper32;
+
+  uint16_t Padding16_2;
 } SmuMetrics_t;
 
 typedef struct {
@@ -1476,8 +1479,11 @@ typedef struct {
   uint8_t  PcieRate               ;
   uint8_t  PcieWidth              ;
   uint16_t AverageGfxclkFrequencyTarget;
-  uint16_t Padding16_2;
 
+  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 38f04836c82f..9f0c353f9b42 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,42 @@ 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 and only on GC 10.3.0 */
+	if (smu->smc_fw_version < 0x3A5300 ||
+	    smu->adev->ip_versions[GC_HWIP][0] != IP_VERSION(10, 3, 0))
+		return;
+
+	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 +4218,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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 16:07           ` Lazar, Lijo
@ 2022-03-28 16:23             ` Russell, Kent
  0 siblings, 0 replies; 15+ messages in thread
From: Russell, Kent @ 2022-03-28 16:23 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander

[-- Attachment #1: Type: text/plain, Size: 9528 bytes --]

[AMD Official Use Only]

Thanks Lijo, I'll do another revision to drop the PMFW ticket (I saw it higher up and thought it was standard practice). Once the PMFW gets officially released I'll test it out and make sure that it doesn't get corrupted. I would assume that it would continue to work, but it's been a while since it was first confirmed by the PMFW dev I was working with.

Kent

From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Monday, March 28, 2022 12:08 PM
To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid


[AMD Official Use Only]

Yes, that check looks good.

A couple of other things -

PMFW ticket as a comment is not needed.
I remember Satish updating the same struct for smartshift related data. Some additional fields added towards the end. Not sure if Sienna struct also carries those fields. Regardless, you may check if that affects the placement of serial number fields (whether before or after those fields).


Thanks,
Lijo
________________________________
From: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>
Sent: Monday, March 28, 2022 9:23:36 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>
Subject: RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid

[AMD Official Use Only]

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
> Sent: Monday, March 28, 2022 11:48 AM
> To: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
>
>
>
> On 3/28/2022 9:12 PM, Russell, Kent wrote:
> > [AMD Official Use Only]
> >
> > Responses inline
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
> >> Sent: Monday, March 28, 2022 11:18 AM
> >> To: Russell, Kent <Kent.Russell@amd.com<mailto:Kent.Russell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> >> Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@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.russell@amd.com<mailto:kent.russell@amd.com>>
> >>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com<mailto:alexander.deucher@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.

Would it be sufficient to just confirm the IP_VERSION here too then?
 e.g.
/* Only supported as of version 0.58.83.0 and only on Sienna Cichlid (GC 10.3.0)*/
if (smu->smc_fw_version < 0x3A5300 || adev->ip_versions[GC_HWIP][0] != IP_VERSION(10, 3, 0))
          return;

Thus the FW has to be that version, and the IP_VERSION has to be 10.3 ? Or is there a better method to use? Thanks!

 Kent

>
> 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)
> >>>

[-- Attachment #2: Type: text/html, Size: 55921 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 15:53         ` Russell, Kent
@ 2022-03-28 16:07           ` Lazar, Lijo
  2022-03-28 16:23             ` Russell, Kent
  0 siblings, 1 reply; 15+ messages in thread
From: Lazar, Lijo @ 2022-03-28 16:07 UTC (permalink / raw)
  To: Russell, Kent, amd-gfx; +Cc: Deucher, Alexander

[-- Attachment #1: Type: text/plain, Size: 8362 bytes --]

[AMD Official Use Only]

Yes, that check looks good.

A couple of other things -

PMFW ticket as a comment is not needed.
I remember Satish updating the same struct for smartshift related data. Some additional fields added towards the end. Not sure if Sienna struct also carries those fields. Regardless, you may check if that affects the placement of serial number fields (whether before or after those fields).


Thanks,
Lijo
________________________________
From: Russell, Kent <Kent.Russell@amd.com>
Sent: Monday, March 28, 2022 9:23:36 PM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid

[AMD Official Use Only]

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Monday, March 28, 2022 11:48 AM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
>
>
>
> On 3/28/2022 9:12 PM, Russell, Kent wrote:
> > [AMD Official Use Only]
> >
> > Responses inline
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> >> Sent: Monday, March 28, 2022 11:18 AM
> >> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher@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.russell@amd.com>
> >>> Reviewed-by: Alex Deucher <alexander.deucher@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.

Would it be sufficient to just confirm the IP_VERSION here too then?
 e.g.
/* Only supported as of version 0.58.83.0 and only on Sienna Cichlid (GC 10.3.0)*/
if (smu->smc_fw_version < 0x3A5300 || adev->ip_versions[GC_HWIP][0] != IP_VERSION(10, 3, 0))
          return;

Thus the FW has to be that version, and the IP_VERSION has to be 10.3 ? Or is there a better method to use? Thanks!

 Kent

>
> 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)
> >>>

[-- Attachment #2: Type: text/html, Size: 14815 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 15:48       ` Lazar, Lijo
@ 2022-03-28 15:53         ` Russell, Kent
  2022-03-28 16:07           ` Lazar, Lijo
  0 siblings, 1 reply; 15+ messages in thread
From: Russell, Kent @ 2022-03-28 15:53 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only]

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Monday, March 28, 2022 11:48 AM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
>
>
>
> On 3/28/2022 9:12 PM, Russell, Kent wrote:
> > [AMD Official Use Only]
> >
> > Responses inline
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> >> Sent: Monday, March 28, 2022 11:18 AM
> >> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> >> Cc: Deucher, Alexander <Alexander.Deucher@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.russell@amd.com>
> >>> Reviewed-by: Alex Deucher <alexander.deucher@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.

Would it be sufficient to just confirm the IP_VERSION here too then?
 e.g.
/* Only supported as of version 0.58.83.0 and only on Sienna Cichlid (GC 10.3.0)*/
if (smu->smc_fw_version < 0x3A5300 || adev->ip_versions[GC_HWIP][0] != IP_VERSION(10, 3, 0))
          return;

Thus the FW has to be that version, and the IP_VERSION has to be 10.3 ? Or is there a better method to use? Thanks!

 Kent

>
> 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)
> >>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 15:42     ` Russell, Kent
@ 2022-03-28 15:48       ` Lazar, Lijo
  2022-03-28 15:53         ` Russell, Kent
  0 siblings, 1 reply; 15+ messages in thread
From: Lazar, Lijo @ 2022-03-28 15:48 UTC (permalink / raw)
  To: Russell, Kent, amd-gfx; +Cc: Deucher, Alexander



On 3/28/2022 9:12 PM, Russell, Kent wrote:
> [AMD Official Use Only]
> 
> Responses inline
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Monday, March 28, 2022 11:18 AM
>> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@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.russell@amd.com>
>>> Reviewed-by: Alex Deucher <alexander.deucher@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)
>>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 15:17   ` Lazar, Lijo
@ 2022-03-28 15:42     ` Russell, Kent
  2022-03-28 15:48       ` Lazar, Lijo
  0 siblings, 1 reply; 15+ messages in thread
From: Russell, Kent @ 2022-03-28 15:42 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander

[AMD Official Use Only]

Responses inline

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Monday, March 28, 2022 11:18 AM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@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.russell@amd.com>
> > Reviewed-by: Alex Deucher <alexander.deucher@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).

 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)
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 14:35 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
@ 2022-03-28 15:17   ` Lazar, Lijo
  2022-03-28 15:42     ` Russell, Kent
  0 siblings, 1 reply; 15+ messages in thread
From: Lazar, Lijo @ 2022-03-28 15:17 UTC (permalink / raw)
  To: Kent Russell, amd-gfx; +Cc: Alex Deucher



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.russell@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@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?

> +  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?

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)
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 14:35 [PATCH 1/2] drm/amdgpu: Use switch case for unique_id Kent Russell
@ 2022-03-28 14:35 ` Kent Russell
  2022-03-28 15:17   ` Lazar, Lijo
  0 siblings, 1 reply; 15+ messages in thread
From: Kent Russell @ 2022-03-28 14:35 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Kent Russell

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.russell@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@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;
+
+  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;
+
+	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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
  2022-03-28 13:06 [PATCH 1/2] drm/amdgpu: Change unique_id to use IP_VERSION Kent Russell
@ 2022-03-28 13:06 ` Kent Russell
  0 siblings, 0 replies; 15+ messages in thread
From: Kent Russell @ 2022-03-28 13:06 UTC (permalink / raw)
  To: amd-gfx; +Cc: Kent Russell

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

Signed-off-by: Kent Russell <kent.russell@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   | 35 +++++++++++++++++++
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index a5216c0f5c2b..62b73ca0aa74 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1992,6 +1992,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;
 		default:
 			*states = ATTR_STATE_UNSUPPORTED;
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;
+
+  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..1016d8646e1e 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,40 @@ 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;
+	int ret;
+
+	/* Only supported as of version 0.58.83.0 */
+	if (smu->smc_fw_version < 0x3A5300)
+		return;
+
+	ret = smu_cmn_get_metrics_table(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:
+
+	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 +4216,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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-03-29 12:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-28 17:07 [PATCH 1/2] drm/amdgpu: Use switch case for unique_id Kent Russell
2022-03-28 17:07 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
2022-03-29  4:34   ` Lazar, Lijo
2022-03-29 12:10     ` Russell, Kent
  -- strict thread matches above, loose matches on Subject: below --
2022-03-28 16:53 [PATCH 1/2] drm/amdgpu: Use switch case for unique_id Kent Russell
2022-03-28 16:53 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
2022-03-28 17:01   ` Alex Deucher
2022-03-28 17:06     ` Russell, Kent
2022-03-28 14:35 [PATCH 1/2] drm/amdgpu: Use switch case for unique_id Kent Russell
2022-03-28 14:35 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell
2022-03-28 15:17   ` Lazar, Lijo
2022-03-28 15:42     ` Russell, Kent
2022-03-28 15:48       ` Lazar, Lijo
2022-03-28 15:53         ` Russell, Kent
2022-03-28 16:07           ` Lazar, Lijo
2022-03-28 16:23             ` Russell, Kent
2022-03-28 13:06 [PATCH 1/2] drm/amdgpu: Change unique_id to use IP_VERSION Kent Russell
2022-03-28 13:06 ` [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Kent Russell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.