RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[AMD Official Use Only]

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux