From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 52AE5C433F5 for ; Mon, 28 Mar 2022 16:23:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C60B310E6F4; Mon, 28 Mar 2022 16:23:19 +0000 (UTC) Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam08on2044.outbound.protection.outlook.com [40.107.101.44]) by gabe.freedesktop.org (Postfix) with ESMTPS id EA62A10E6F4 for ; Mon, 28 Mar 2022 16:23:17 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Bs0kkeS0JyeHZzChIl7s5cpI7QzYsBCctOXu6DBIOyPrzes1XDujkP8d4yH4YXyVO9kLwJfIoRctfBS7Z/xKb85HlJ3Sq6Q8WdBW1j0hBL0kZdBFGwe4HWFXZPhTw1TEzbUTyMJ+YF/qLpthqqC3a+VToEiA0Iv7S8ghA1sN7k0WzFIwGLSBXiL+IFkJ20aMe/KFA5otRsY5dZhkUjgngHAMJ2lybNZrtyTVXKYYT1lMWe5cL87iWgeRsM8Tf4ABdhSnwj+ytXhuH4Gn9CfUMto/wqDZMPn8wlVdBfoQ4nZo+gloLzxtgGi+7iseVUeY/QwC334OyhdR5bNqlfHbgg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=A7RzNDI2Rf5sJS4QdFUK1gh+kEJBqnyesTuvZOVDOvM=; b=kyn1cCiOLadzHKwk2ctdmohSZBaFHGHWH0LkQyeEpf0wli8e5zqW1j/N1q+GMcY/tMwz6qNv93Xgkf+LNcJcJA4xPUteFom+k27BZ2m7enjev0/Hu+pRNQwnNtRdQDuoxjaiReQ6jhRkyKkKrJAYqau24LG8RhQy/vHxHVnVhD1xStlxmT35zO6FANFVnuM2KtT1MJG9xAomhJlOXkiOoEtHD2QEiyaiQ57HUJ4BoaAmCSWIe85MrKf2cHuIWe2eGk9ln4+XaFsCiNXR9jq4j0pHY1CX/qv2fh6WpzazcJLFmsV+Zex1rqaN1jngoMugaBY0OspTWQMR4edwMyTPVg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=A7RzNDI2Rf5sJS4QdFUK1gh+kEJBqnyesTuvZOVDOvM=; b=nWPNlpO0OwiHtOb5Msy0u3p436587Efe/Q0JabPvzkcB7xqn3GbLSRIXYceRg7JPZdZbYwY4WZ+UZOdwjAhSh04w0RFkutsdsHYzK0jetT+KgzldydsFN80zxcjT3tL2iuaKEDZmrpPAygWQVY6WZXerGekSOfCgmTtLCXTtRPc= Received: from DM5PR12MB1308.namprd12.prod.outlook.com (2603:10b6:3:76::7) by BN8PR12MB3108.namprd12.prod.outlook.com (2603:10b6:408:40::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5102.16; Mon, 28 Mar 2022 16:23:14 +0000 Received: from DM5PR12MB1308.namprd12.prod.outlook.com ([fe80::e05f:3a9a:b740:aa75]) by DM5PR12MB1308.namprd12.prod.outlook.com ([fe80::e05f:3a9a:b740:aa75%10]) with mapi id 15.20.5102.023; Mon, 28 Mar 2022 16:23:14 +0000 From: "Russell, Kent" To: "Lazar, Lijo" , "amd-gfx@lists.freedesktop.org" Subject: RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Thread-Topic: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichlid Thread-Index: AQHYQrETM9TbriXgqkWwcn6XPgnv7qzU6IsAgAAF2MCAAALEgIAAAGZggAAFBQCAAAPToA== Date: Mon, 28 Mar 2022 16:23:14 +0000 Message-ID: References: <20220328143518.1253762-1-kent.russell@amd.com> <20220328143518.1253762-2-kent.russell@amd.com> <80529a89-effc-368a-da12-14d7e44c860d@amd.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_88914ebd-7e6c-4e12-a031-a9906be2db14_Enabled=True; MSIP_Label_88914ebd-7e6c-4e12-a031-a9906be2db14_SiteId=3dd8961f-e488-4e60-8e11-a82d994e183d; MSIP_Label_88914ebd-7e6c-4e12-a031-a9906be2db14_SetDate=2022-03-28T15:49:54.0000000Z; MSIP_Label_88914ebd-7e6c-4e12-a031-a9906be2db14_Name=AMD Official Use Only-AIP 2.0; MSIP_Label_88914ebd-7e6c-4e12-a031-a9906be2db14_ContentBits=0; MSIP_Label_88914ebd-7e6c-4e12-a031-a9906be2db14_Method=Standard authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: bb4d09c9-db4c-462c-4ed2-08da10d7425d x-ms-traffictypediagnostic: BN8PR12MB3108:EE_ x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 7MqzYsvxLOwzzoEHIFnd0jFY6Y4Jjwmod6iFK/arTjBCaT+qZI58h/rtdp01xLTbQqGVyk9ActmZO0EO/xiT/4bjfO20Fy8/otPWQeim9zX/NrkmggijHRZLe82OpCK/GbtnR6z5zXT/YeNw6UHgXnza2Bgb6gJVd4XLlx4+jeFmHdqU7M3O2p02OjA7pEboLNzFpM5Jnw1mVRaiEgdcySBc0XLMC511g9BlZBBgF3KPe3htASrWWoblYJLzAnr/y7waQSKijeB7vYnYF/H/BACYCmANGqrHDXKxBLxah3j3Sl/e+Qr9nz+Pv9hh3r9D0rKBwt/8HY7uhII5vUKhBDDSzEDJyh1PU57Mi3RDSGQCoEcQ360dlCO5EN1F7IaJkewHNQx8i08jMUWp2yZBVxtaaQZuqUeH8SzrPkaOCCrdzrQMxSe7U30kul3J3yWV/qsrIKOinQmjMaNab/Pn+EMS3MbqRlUfxzM2xN8PY5lMT3/OzNqXRCgC+UaIprV3M3sF56tQpkHb0EiHb+1I5cda2olesxMPXVk4Rd6X0HtXuXiPl5+b7Pqkn9ve0E5J0Kw57DmAlejdt3NQFKsh3i89oGgltDd4blzVCzyCDxzff7nqlmedo4O6vj6OoYoYsdpqudQLQqDELkKcGJ6cSxBUDeTS/iIRGUfc57RtH12H+TexU06vIIs1Z/fTKkqJu/qrTJAzXH9FEIiwD6KMqw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM5PR12MB1308.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(4636009)(366004)(83380400001)(508600001)(110136005)(186003)(26005)(316002)(2906002)(86362001)(33656002)(55016003)(38070700005)(52536014)(122000001)(53546011)(9686003)(66556008)(64756008)(66446008)(7696005)(6506007)(5660300002)(66476007)(8936002)(38100700002)(8676002)(71200400001)(66946007)(76116006)(4326008)(559001)(579004); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?zlLGzKn2n5rdZhAeSi//Z/rv99u8PGRhtC35fdgNSWj+S5BkpZxtsA3GLFas?= =?us-ascii?Q?Or16MrAYsu48DL8KQCfdhIQw4D7lzQYIjBxUKhwpeJ3e8YG2BIgk8+FxCst6?= =?us-ascii?Q?DGiNmEkheCYcDlb/0XSCLAkv0NzvYg2TphmXgFuZi494Yi3DX5SNmKxLXvOi?= =?us-ascii?Q?R2YUkJHjl0ky4sUaTU/npbyo0tIQGgIbgjHs2kJrUL3mY9dMYFMGVOmkrt3h?= =?us-ascii?Q?EPHm2Na4QO2IcRAGrfLv3/sVTQHdCqIBAoKANpJxOD9/NBnGf2rht0aSOqpT?= =?us-ascii?Q?A0fhI/1EqhqfulFZJ6H9hnUpV5RHX/RDSRh8FscfseLkwo/JYeX3Jp1gO0/Y?= =?us-ascii?Q?UFlt8y7rF4WYbMHUEIuOT2IfHH5Cfurr+0F8910op6Wq3pwZ5hxGUe7zSVct?= =?us-ascii?Q?Z3u/wFony46sTBAMAGzh5mAD9I5O8K5YqiXJyzXNZqrkIFplFUtzSiSIcJtY?= =?us-ascii?Q?RoA5TqsoTgqv7NbyHmyYJrstGYKCwKojfNp0Qox1fQWqNUOu9B2thcli7WAI?= =?us-ascii?Q?kY5sUAlG8hEsDEt8y1alxUZPFdw4hNu18hNnYVnErY/rbm4XoDxjzqXeeoA7?= =?us-ascii?Q?dW0NV6qf5UYlVvERLrWTEEEcfQx+DFei3+PfnXz3qoVLnwY5vcv2dD5qG93g?= =?us-ascii?Q?9TdsHzXUnIj56ivNohgA7b7ItY27WUzvLp00Qxn6kz+1jtSqFjw8TR/jlj3N?= =?us-ascii?Q?b6pOupikZa4NK6OKkBBWvjXLLOV5LLPKOe/2rY3fscym3/LjLXQndxgMmcFn?= =?us-ascii?Q?EkShxiu371n/p4lVudq/9IkjQv30PVnLkgcWKOzhunKwSOG0JzAdbTLfZHl2?= =?us-ascii?Q?bJN7d2m3OkasJPRAsYJgyVT1lfDcAcHrrC2LrVmgJy0j4QeDEf/D+HN+OTqC?= =?us-ascii?Q?EQ0DP/Z15gTUF96W4wJn6qWkbQrd7L4e07kiyNo6qN9EiM+6UnfUsKYuSXvq?= =?us-ascii?Q?a0EYcc7Zpx1e2ZJ/AtWd/CZ2PAwxOcM4l8h0dhYfPXE6pxKxkocx4p0a3MhK?= =?us-ascii?Q?gu5Gp8Kh4q2iUM41yJUY/Y7KVEpMyDom31wqPh5dsoxzO7GuqhJRpLsYlhWS?= =?us-ascii?Q?MdeN2J7ieEsjZFHJ23rjaKOKyZKswetPNC8uZEjPAFKCRrBeOUhO+a5/piq+?= =?us-ascii?Q?ZIQKbJQALH7FM+Gp0Vzak/d833kBHF8fZu4PgT86m5gub7eOnvAxq9PXPMey?= =?us-ascii?Q?pesWd15XwZqDlQV23XxBJLiWQ52nam9dCIgVUl13/7OZUyL8JpHlR4vonAgQ?= =?us-ascii?Q?iWfXR/FEellqhviYYSh3CrLTVjMOIaw4aYxDvyKJsN7pCHahTCagZAvmkDyt?= =?us-ascii?Q?iH5fGrq+bFI30dksoF6PhThIXSV5JPyYJ93P8krOBLByYFLQLP99yJ1JUMa0?= =?us-ascii?Q?Xrgo8lNm7CiRBiajLwD2VQHjcEsQFOlH1zEbkNFpveYwTMRwjDPEHiX2FZIA?= =?us-ascii?Q?LD4s0IxlNDP23nYRTrw9P4/FEYFLwVdBGOFq/YPT1oWpu9T0zZwiK7fXqzcn?= =?us-ascii?Q?biPkkdS7dcJhedr+k8MbwHzbw7WyGL7CY3N/O3H01pp0OkZ9dm6NryqGMsGv?= =?us-ascii?Q?LU+CwtEK+6/8IvfK9NbCstT88DlsUNuGocN1piVHa/JC11lKsQVeFYEqILfg?= =?us-ascii?Q?FkLdwEnj9610ssCaP4eP9WpayTVbpVRbVG6pM0o4zxcFSOUIpMWN8T3R5wdG?= =?us-ascii?Q?89z1RjMIbgBndNRGdIvlRXyprVsDdzwSC5Z/Ju42kstxcsEe?= Content-Type: multipart/alternative; boundary="_000_DM5PR12MB1308E62E8701830E28ACD382851D9DM5PR12MB1308namp_" MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1308.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: bb4d09c9-db4c-462c-4ed2-08da10d7425d X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Mar 2022 16:23:14.2364 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: naMzZeOOS99WAZQ63LCSUPlugLyghR4hzMN0BrQlWr+ozANqMOCFfr3l2ormhNSZCV8POBcrfzp6EkL5vGtWOA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR12MB3108 X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Deucher, Alexander" Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" --_000_DM5PR12MB1308E62E8701830E28ACD382851D9DM5PR12MB1308namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable [AMD Official Use Only] Thanks Lijo, I'll do another revision to drop the PMFW ticket (I saw it hig= her 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 w= ould 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 Sent: Monday, March 28, 2022 12:08 PM To: Russell, Kent ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichl= id [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. Som= e additional fields added towards the end. Not sure if Sienna struct also c= arries those fields. Regardless, you may check if that affects the placemen= t of serial number fields (whether before or after those fields). Thanks, Lijo ________________________________ From: Russell, Kent > Sent: Monday, March 28, 2022 9:23:36 PM To: Lazar, Lijo >; amd-gfx@li= sts.freedesktop.org > Cc: Deucher, Alexander > Subject: RE: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cichl= id [AMD Official Use Only] > -----Original Message----- > From: Lazar, Lijo > > Sent: Monday, March 28, 2022 11:48 AM > To: Russell, Kent >; am= d-gfx@lists.freedesktop.org > Cc: Deucher, Alexander > > Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienna cic= hlid > > > > On 3/28/2022 9:12 PM, Russell, Kent wrote: > > [AMD Official Use Only] > > > > Responses inline > > > >> -----Original Message----- > >> From: Lazar, Lijo > > >> Sent: Monday, March 28, 2022 11:18 AM > >> To: Russell, Kent >;= amd-gfx@lists.freedesktop.org > >> Cc: Deucher, Alexander > > >> 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 th= e > >>> 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 > > >>> Reviewed-by: Alex Deucher > > >>> --- > >>> 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_de= vice *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 =3D ATTR_STATE_SUPPORTED; > >>> break; > >>> default: > >>> diff --git > >> a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_cich= lid.h > >> b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_cich= lid.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 wit= h > >> 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 s= mu_context > >> *smu) > >>> return sienna_cichlid_patch_pptable_quirk(smu); > >>> } > >>> > >>> +static void sienna_cichlid_get_unique_id(struct smu_context *smu) > >>> +{ > >>> + struct amdgpu_device *adev =3D smu->adev; > >>> + struct smu_table_context *smu_table =3D &smu->smu_table; > >>> + SmuMetrics_t *metrics =3D > >>> + &(((SmuMetricsExternal_t *)(smu_table->metrics_table))->S= muMetrics); > >>> + SmuMetrics_V2_t *metrics_v2 =3D > >>> + &(((SmuMetricsExternal_t *)(smu_table->metrics_table))->S= muMetrics_V2); > >>> + uint32_t upper32 =3D 0, lower32 =3D 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 n= ot > >> be enough. This function may be skipped if it's not MP1 11.0.7 or GC 1= 0.3.0? > >> > > > > Since the sysfs file is only supported on Sienna Cichlid (10.3.0), is i= t 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] !=3D 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 =3D smu_cmn_get_metrics_table(smu, NULL, false); > >>> + if (ret) > >>> + goto out_unlock; > >>> + > >>> + use_metrics_v2 =3D ((smu->adev->ip_versions[MP1_HWIP][0] =3D=3D I= P_VERSION(11, 0, > >> 7)) && > >>> + (smu->smc_fw_version >=3D 0x3A4300)) ? true : false; > >>> + > >>> + upper32 =3D use_metrics_v2 ? metrics_v2->PublicSerialNumUpper32 : > >>> + metrics->PublicSerialNumUpper32; > >>> + lower32 =3D use_metrics_v2 ? metrics_v2->PublicSerialNumLower32 : > >>> + metrics->PublicSerialNumLower32; > >>> + > >>> +out_unlock: > >>> + > >>> + adev->unique_id =3D ((uint64_t)upper32 << 32) | lower32; > >>> + if (adev->serial[0] =3D=3D '\0') > >>> + sprintf(adev->serial, "%016llx", adev->unique_id); > >>> +} > >>> + > >>> static int sienna_cichlid_tables_init(struct smu_context *smu) > >>> { > >>> struct smu_table_context *smu_table =3D &smu->smu_table; > >>> @@ -4182,6 +4217,7 @@ static const struct pptable_funcs sienna_cichli= d_ppt_funcs =3D { > >>> .get_ecc_info =3D sienna_cichlid_get_ecc_info, > >>> .get_default_config_table_settings =3D > >> sienna_cichlid_get_default_config_table_settings, > >>> .set_config_table =3D sienna_cichlid_set_config_table, > >>> + .get_unique_id =3D sienna_cichlid_get_unique_id, > >>> }; > >>> > >>> void sienna_cichlid_set_ppt_funcs(struct smu_context *smu) > >>> --_000_DM5PR12MB1308E62E8701830E28ACD382851D9DM5PR12MB1308namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

[AMD Official Use Only]


Th= anks 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 tha= t 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.

 

Ke= nt

 

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.freede= sktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: Add unique_id support for sienn= a cichlid

 

[AMD Official Use Only]

 

Yes, that check lo= oks good.

 <= /span>

A couple of other = things - 

 <= /span>

PMFW ticket as a c= omment is not needed.

I remember Satish = updating the same struct for smartshift related data. Some additional field= s added towards the end. Not sure if Sienna struct also carries those fields. Regardless, you may check if that affects the p= lacement of serial number fields (whether before or after those fields).

 <= /span>

 <= /span>

Thanks,
Lijo

From: Russell, Kent <Kent.Russ= ell@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 sienn= a 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.Rus= sell@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 <L= ijo.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 fo= r sienna cichlid
> >>
> >>
> >>
> >> On 3/28/2022 8:05 PM, Kent Russell wrote:
> >>> This is being added to SMU Metrics, so add the required t= ie-ins in the
> >>> kernel. Also create the corresponding unique_id sysfs fil= e.
> >>>
> >>> 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_cich= lid.h  | 12 +++++--
> >>>    .../amd/pm/swsmu/smu11/sienna_cichlid_p= pt.c   | 36 +++++++++++++++++++
> >>>    3 files changed, 47 insertions(+), 2 de= letions(-)
> >>>
> >>> 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(stru= ct amdgpu_device *adev,
> >> struct amdgpu_device_
> >>>         &nbs= p;    case IP_VERSION(9, 4, 0):
> >>>         &nbs= p;    case IP_VERSION(9, 4, 1):
> >>>         &nbs= p;    case IP_VERSION(9, 4, 2):
> >>> +         &n= bsp; case IP_VERSION(10, 3, 0):
> >>>         &nbs= p;            *state= s =3D ATTR_STATE_SUPPORTED;
> >>>         &nbs= p;            break;=
> >>>         &nbs= p;    default:
> >>> diff --git
> >> a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_si= enna_cichlid.h
> >> b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_si= enna_cichlid.h
> >>> index 3e4a314ef925..58f977320d06 100644
> >>> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driv= er_if_sienna_cichlid.h
> >>> +++
> b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu11_driver_if_sienna_cich= lid.h
> >>> @@ -1419,8 +1419,12 @@ typedef struct {
> >>>      uint8_t  PcieRate = ;            &n= bsp; ;
> >>>      uint8_t  PcieWidth&nbs= p;             = ;
> >>>      uint16_t AverageGfxclkFrequ= encyTarget;
> >>> -  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 = ;            &n= bsp; ;
> >>>      uint8_t  PcieWidth&nbs= p;             = ;
> >>>      uint16_t AverageGfxclkFrequ= encyTarget;
> >>> -  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 fo= r 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_ci= chlid_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_p= pt.c
> >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_p= pt.c
> >>> @@ -481,6 +481,41 @@ static int sienna_cichlid_setup_ppta= ble(struct smu_context
> >> *smu)
> >>>      return sienna_cichlid_patch= _pptable_quirk(smu);
> >>>    }
> >>>
> >>> +static void sienna_cichlid_get_unique_id(struct smu_cont= ext *smu)
> >>> +{
> >>> +   struct amdgpu_device *adev =3D smu->adev= ;
> >>> +   struct smu_table_context *smu_table =3D &am= p;smu->smu_table;
> >>> +   SmuMetrics_t *metrics =3D
> >>> +         &n= bsp; &(((SmuMetricsExternal_t *)(smu_table->metrics_table))->SmuM= etrics);
> >>> +   SmuMetrics_V2_t *metrics_v2 =3D
> >>> +         &n= bsp; &(((SmuMetricsExternal_t *)(smu_table->metrics_table))->SmuM= etrics_V2);
> >>> +   uint32_t upper32 =3D 0, lower32 =3D 0;
> >>> +   bool use_metrics_v2;
> >>> +   int ret;
> >>> +
> >>> +   /* Only supported as of version 0.58.83.0 *= /
> >>> +   if (smu->smc_fw_version < 0x3A5300) > >>> +         &n= bsp; return;
> >>> +
> >>
> >> Since this is shared with other ASICs, I guess this check its= elf 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 on= ly referenced by
> sienna_cichlid, unless I misunderstood something (and someone else use= s
> 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,<= br> > then it may go to the path which is not intended to be executed on tha= t
> 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= ] !=3D 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 =3D smu_cmn_get_metrics_table(smu, NULL= , false);
> >>> +   if (ret)
> >>> +         &n= bsp; goto out_unlock;
> >>> +
> >>> +   use_metrics_v2 =3D ((smu->adev->ip_ve= rsions[MP1_HWIP][0] =3D=3D IP_VERSION(11, 0,
> >> 7)) &&
> >>> +         &n= bsp; (smu->smc_fw_version >=3D 0x3A4300)) ? true : false;
> >>> +
> >>> +   upper32 =3D use_metrics_v2 ? metrics_v2->= ;PublicSerialNumUpper32 :
> >>> +         &n= bsp;            = ;        metrics->PublicSerialNumUppe= r32;
> >>> +   lower32 =3D use_metrics_v2 ? metrics_v2->= ;PublicSerialNumLower32 :
> >>> +         &n= bsp;            = ;        metrics->PublicSerialNumLowe= r32;
> >>> +
> >>> +out_unlock:
> >>> +
> >>> +   adev->unique_id =3D ((uint64_t)upper32 &= lt;< 32) | lower32;
> >>> +   if (adev->serial[0] =3D=3D '\0')
> >>> +         &n= bsp; sprintf(adev->serial, "%016llx", adev->unique_id);
> >>> +}
> >>> +
> >>>    static int sienna_cichlid_tables_init(s= truct smu_context *smu)
> >>>    {
> >>>      struct smu_table_context *s= mu_table =3D &smu->smu_table;
> >>> @@ -4182,6 +4217,7 @@ static const struct pptable_funcs s= ienna_cichlid_ppt_funcs =3D {
> >>>      .get_ecc_info =3D sienna_ci= chlid_get_ecc_info,
> >>>      .get_default_config_table_s= ettings =3D
> >> sienna_cichlid_get_default_config_table_settings,
> >>>      .set_config_table =3D sienn= a_cichlid_set_config_table,
> >>> +   .get_unique_id =3D sienna_cichlid_get_uniqu= e_id,
> >>>    };
> >>>
> >>>    void sienna_cichlid_set_ppt_funcs(struc= t smu_context *smu)
> >>>

--_000_DM5PR12MB1308E62E8701830E28ACD382851D9DM5PR12MB1308namp_--