[PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid
Lazar, Lijo
lijo.lazar at amd.com
Mon Mar 28 15:48:29 UTC 2022
On 3/28/2022 9:12 PM, Russell, Kent wrote:
> [AMD Official Use Only]
>
> Responses inline
>
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Monday, March 28, 2022 11:18 AM
>> To: Russell, Kent <Kent.Russell at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher at 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 at amd.com>
>>> Reviewed-by: Alex Deucher <alexander.deucher at 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)
>>>
More information about the amd-gfx
mailing list