AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name
@ 2021-11-19 19:52 Graham Sider
  2021-11-19 19:52 ` [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function Graham Sider
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Graham Sider @ 2021-11-19 19:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alexander.Deucher, Felix.Kuehling, Graham Sider

device_info->asic_name and amdgpu_asic_name[adev->asic_type] both
provide asic name strings, with the only difference being casing.
Propose to remove asic_name from device_info and replace sysfs entry
with amdgpu_asic_name[].

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 29 -----------------------
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h     |  1 -
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c |  2 +-
 3 files changed, 1 insertion(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index e1294fba0c26..e11fc4e20c32 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -55,7 +55,6 @@ extern const struct kfd2kgd_calls gfx_v10_3_kfd2kgd;
 
 #ifdef KFD_SUPPORT_IOMMU_V2
 static const struct kfd_device_info kaveri_device_info = {
-	.asic_name = "kaveri",
 	.gfx_target_version = 70000,
 	.max_pasid_bits = 16,
 	/* max num of queues for KV.TODO should be a dynamic value */
@@ -72,7 +71,6 @@ static const struct kfd_device_info kaveri_device_info = {
 };
 
 static const struct kfd_device_info carrizo_device_info = {
-	.asic_name = "carrizo",
 	.gfx_target_version = 80001,
 	.max_pasid_bits = 16,
 	/* max num of queues for CZ.TODO should be a dynamic value */
@@ -89,7 +87,6 @@ static const struct kfd_device_info carrizo_device_info = {
 };
 
 static const struct kfd_device_info raven_device_info = {
-	.asic_name = "raven",
 	.gfx_target_version = 90002,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -107,7 +104,6 @@ static const struct kfd_device_info raven_device_info = {
 
 #ifdef CONFIG_DRM_AMDGPU_CIK
 static const struct kfd_device_info hawaii_device_info = {
-	.asic_name = "hawaii",
 	.gfx_target_version = 70001,
 	.max_pasid_bits = 16,
 	/* max num of queues for KV.TODO should be a dynamic value */
@@ -125,7 +121,6 @@ static const struct kfd_device_info hawaii_device_info = {
 #endif
 
 static const struct kfd_device_info tonga_device_info = {
-	.asic_name = "tonga",
 	.gfx_target_version = 80002,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -141,7 +136,6 @@ static const struct kfd_device_info tonga_device_info = {
 };
 
 static const struct kfd_device_info fiji_device_info = {
-	.asic_name = "fiji",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -157,7 +151,6 @@ static const struct kfd_device_info fiji_device_info = {
 };
 
 static const struct kfd_device_info fiji_vf_device_info = {
-	.asic_name = "fiji",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -174,7 +167,6 @@ static const struct kfd_device_info fiji_vf_device_info = {
 
 
 static const struct kfd_device_info polaris10_device_info = {
-	.asic_name = "polaris10",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -190,7 +182,6 @@ static const struct kfd_device_info polaris10_device_info = {
 };
 
 static const struct kfd_device_info polaris10_vf_device_info = {
-	.asic_name = "polaris10",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -206,7 +197,6 @@ static const struct kfd_device_info polaris10_vf_device_info = {
 };
 
 static const struct kfd_device_info polaris11_device_info = {
-	.asic_name = "polaris11",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -222,7 +212,6 @@ static const struct kfd_device_info polaris11_device_info = {
 };
 
 static const struct kfd_device_info polaris12_device_info = {
-	.asic_name = "polaris12",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -238,7 +227,6 @@ static const struct kfd_device_info polaris12_device_info = {
 };
 
 static const struct kfd_device_info vegam_device_info = {
-	.asic_name = "vegam",
 	.gfx_target_version = 80003,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -254,7 +242,6 @@ static const struct kfd_device_info vegam_device_info = {
 };
 
 static const struct kfd_device_info vega10_device_info = {
-	.asic_name = "vega10",
 	.gfx_target_version = 90000,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -270,7 +257,6 @@ static const struct kfd_device_info vega10_device_info = {
 };
 
 static const struct kfd_device_info vega10_vf_device_info = {
-	.asic_name = "vega10",
 	.gfx_target_version = 90000,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -286,7 +272,6 @@ static const struct kfd_device_info vega10_vf_device_info = {
 };
 
 static const struct kfd_device_info vega12_device_info = {
-	.asic_name = "vega12",
 	.gfx_target_version = 90004,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -302,7 +287,6 @@ static const struct kfd_device_info vega12_device_info = {
 };
 
 static const struct kfd_device_info vega20_device_info = {
-	.asic_name = "vega20",
 	.gfx_target_version = 90006,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd	= 24,
@@ -318,7 +302,6 @@ static const struct kfd_device_info vega20_device_info = {
 };
 
 static const struct kfd_device_info arcturus_device_info = {
-	.asic_name = "arcturus",
 	.gfx_target_version = 90008,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd	= 24,
@@ -334,7 +317,6 @@ static const struct kfd_device_info arcturus_device_info = {
 };
 
 static const struct kfd_device_info aldebaran_device_info = {
-	.asic_name = "aldebaran",
 	.gfx_target_version = 90010,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd	= 24,
@@ -350,7 +332,6 @@ static const struct kfd_device_info aldebaran_device_info = {
 };
 
 static const struct kfd_device_info renoir_device_info = {
-	.asic_name = "renoir",
 	.gfx_target_version = 90012,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -366,7 +347,6 @@ static const struct kfd_device_info renoir_device_info = {
 };
 
 static const struct kfd_device_info navi10_device_info = {
-	.asic_name = "navi10",
 	.gfx_target_version = 100100,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -383,7 +363,6 @@ static const struct kfd_device_info navi10_device_info = {
 };
 
 static const struct kfd_device_info navi12_device_info = {
-	.asic_name = "navi12",
 	.gfx_target_version = 100101,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -400,7 +379,6 @@ static const struct kfd_device_info navi12_device_info = {
 };
 
 static const struct kfd_device_info navi14_device_info = {
-	.asic_name = "navi14",
 	.gfx_target_version = 100102,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -417,7 +395,6 @@ static const struct kfd_device_info navi14_device_info = {
 };
 
 static const struct kfd_device_info sienna_cichlid_device_info = {
-	.asic_name = "sienna_cichlid",
 	.gfx_target_version = 100300,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -434,7 +411,6 @@ static const struct kfd_device_info sienna_cichlid_device_info = {
 };
 
 static const struct kfd_device_info navy_flounder_device_info = {
-	.asic_name = "navy_flounder",
 	.gfx_target_version = 100301,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -451,7 +427,6 @@ static const struct kfd_device_info navy_flounder_device_info = {
 };
 
 static const struct kfd_device_info vangogh_device_info = {
-	.asic_name = "vangogh",
 	.gfx_target_version = 100303,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -468,7 +443,6 @@ static const struct kfd_device_info vangogh_device_info = {
 };
 
 static const struct kfd_device_info dimgrey_cavefish_device_info = {
-	.asic_name = "dimgrey_cavefish",
 	.gfx_target_version = 100302,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -485,7 +459,6 @@ static const struct kfd_device_info dimgrey_cavefish_device_info = {
 };
 
 static const struct kfd_device_info beige_goby_device_info = {
-	.asic_name = "beige_goby",
 	.gfx_target_version = 100304,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -502,7 +475,6 @@ static const struct kfd_device_info beige_goby_device_info = {
 };
 
 static const struct kfd_device_info yellow_carp_device_info = {
-	.asic_name = "yellow_carp",
 	.gfx_target_version = 100305,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
@@ -519,7 +491,6 @@ static const struct kfd_device_info yellow_carp_device_info = {
 };
 
 static const struct kfd_device_info cyan_skillfish_device_info = {
-	.asic_name = "cyan_skillfish",
 	.gfx_target_version = 100103,
 	.max_pasid_bits = 16,
 	.max_no_of_hqd  = 24,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 1054fedd7b3c..3e11febee7c6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -195,7 +195,6 @@ struct kfd_event_interrupt_class {
 };
 
 struct kfd_device_info {
-	const char *asic_name;
 	uint32_t gfx_target_version;
 	const struct kfd_event_interrupt_class *event_interrupt_class;
 	unsigned int max_pasid_bits;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 2d44b26b6657..d2bb35f1c58f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1370,7 +1370,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 
 	amdgpu_amdkfd_get_cu_info(dev->gpu->adev, &cu_info);
 
-	strncpy(dev->node_props.name, gpu->device_info->asic_name,
+	strncpy(dev->node_props.name, amdgpu_asic_name[gpu->adev->asic_type],
 			KFD_TOPOLOGY_PUBLIC_NAME_SIZE);
 
 	dev->node_props.simd_arrays_per_engine =
-- 
2.25.1


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

* [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function
  2021-11-19 19:52 [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Graham Sider
@ 2021-11-19 19:52 ` Graham Sider
  2021-11-19 21:19   ` Felix Kuehling
  2021-11-19 19:52 ` [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation Graham Sider
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Graham Sider @ 2021-11-19 19:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alexander.Deucher, Felix.Kuehling, Graham Sider

Initializes device_info structs given either asic_type (enum) if GFX
version is less than GFX9, or GC IP version if greater. Also takes in vf
and the target compiler gfx version.

Inclusion/exclusion to certain conditions for certain GC IP versions may
be necessary on npi bringup on a case-by-case basis, but for the most
part should be minimal (e.g. adding one || asic_version == IP_VERSION(X ,X, X) case).

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 61 +++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index e11fc4e20c32..676cb9c3166c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -511,6 +511,67 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
 
 static int kfd_resume(struct kfd_dev *kfd);
 
+static void kfd_device_info_init(struct kfd_dev *kfd,
+				 struct kfd_device_info *device_info,
+				 bool vf, uint32_t gfx_target_version)
+{
+	uint32_t gc_version = KFD_GC_VERSION(kfd);
+	uint32_t asic_type = kfd->adev->asic_type;
+
+	device_info->max_pasid_bits = 16;
+	device_info->max_no_of_hqd = 24;
+	device_info->num_of_watch_points = 4;
+	device_info->mqd_size_aligned = MQD_SIZE_ALIGNED;
+	device_info->gfx_target_version = gfx_target_version;
+
+	if (KFD_IS_SOC15(kfd)) {
+		device_info->doorbell_size = 8;
+		device_info->ih_ring_entry_size = 8 * sizeof(uint32_t);
+		device_info->event_interrupt_class = &event_interrupt_class_v9;
+		device_info->supports_cwsr = true;
+
+		if ((gc_version >= IP_VERSION(9, 0, 1)  &&
+		     gc_version <= IP_VERSION(9, 3, 0)) ||
+		     gc_version == IP_VERSION(10, 3, 1) ||
+		     gc_version == IP_VERSION(10, 3, 3))
+			device_info->num_sdma_queues_per_engine = 2;
+		else
+			device_info->num_sdma_queues_per_engine = 8;
+
+		/* Navi2x+, Navi1x+ */
+		if (gc_version >= IP_VERSION(10, 3, 0))
+			device_info->no_atomic_fw_version = 145;
+		else if (gc_version >= IP_VERSION(10, 1, 1))
+			device_info->no_atomic_fw_version = 92;
+
+		/* Raven */
+		if (gc_version == IP_VERSION(9, 1, 0) ||
+		    gc_version == IP_VERSION(9, 2, 2))
+			device_info->needs_iommu_device = true;
+
+		/* Navi1x+ */
+		if (gc_version >= IP_VERSION(10, 1, 1))
+			device_info->needs_pci_atomics = true;
+	} else {
+		device_info->doorbell_size = 4;
+		device_info->ih_ring_entry_size = 4 * sizeof(uint32_t);
+		device_info->event_interrupt_class = &event_interrupt_class_cik;
+		device_info->num_sdma_queues_per_engine = 2;
+
+		if (asic_type != CHIP_KAVERI &&
+		    asic_type != CHIP_HAWAII &&
+		    asic_type != CHIP_TONGA)
+			device_info->supports_cwsr = true;
+
+		if (asic_type == CHIP_KAVERI ||
+		    asic_type == CHIP_CARRIZO)
+			device_info->needs_iommu_device = true;
+
+		if (asic_type != CHIP_HAWAII && !vf)
+			device_info->needs_pci_atomics = true;
+	}
+}
+
 struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
 {
 	struct kfd_dev *kfd;
-- 
2.25.1


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

* [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation
  2021-11-19 19:52 [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Graham Sider
  2021-11-19 19:52 ` [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function Graham Sider
@ 2021-11-19 19:52 ` Graham Sider
  2021-11-19 21:30   ` Felix Kuehling
  2021-11-19 19:52 ` [PATCH v2 4/4] drm/amdkfd: remove hardcoded device_info structs Graham Sider
  2021-11-19 21:11 ` [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Felix Kuehling
  3 siblings, 1 reply; 11+ messages in thread
From: Graham Sider @ 2021-11-19 19:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alexander.Deucher, Felix.Kuehling, Graham Sider

Change unsupported asic condition to only probe f2g, move device_info
initialization post-switch and map to heap.

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 183 ++++++++++--------------
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h   |   2 +-
 2 files changed, 79 insertions(+), 106 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 676cb9c3166c..7ddea653b3d9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -574,191 +574,151 @@ static void kfd_device_info_init(struct kfd_dev *kfd,
 
 struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
 {
-	struct kfd_dev *kfd;
-	const struct kfd_device_info *device_info;
-	const struct kfd2kgd_calls *f2g;
+	struct kfd_dev *kfd = NULL;
+	struct kfd_device_info *device_info = NULL;
+	const struct kfd2kgd_calls *f2g = NULL;
 	struct pci_dev *pdev = adev->pdev;
+	uint32_t gfx_target_version = 0;
 
 	switch (adev->asic_type) {
 #ifdef KFD_SUPPORT_IOMMU_V2
 #ifdef CONFIG_DRM_AMDGPU_CIK
 	case CHIP_KAVERI:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &kaveri_device_info;
-		f2g = &gfx_v7_kfd2kgd;
+		gfx_target_version = 70000;
+		if (!vf)
+			f2g = &gfx_v7_kfd2kgd;
 		break;
 #endif
 	case CHIP_CARRIZO:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &carrizo_device_info;
-		f2g = &gfx_v8_kfd2kgd;
+		gfx_target_version = 80001;
+		if (!vf)
+			f2g = &gfx_v8_kfd2kgd;
 		break;
 #endif
 #ifdef CONFIG_DRM_AMDGPU_CIK
 	case CHIP_HAWAII:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &hawaii_device_info;
-		f2g = &gfx_v7_kfd2kgd;
+		gfx_target_version = 70001;
+		if (!vf)
+			f2g = &gfx_v7_kfd2kgd;
 		break;
 #endif
 	case CHIP_TONGA:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &tonga_device_info;
-		f2g = &gfx_v8_kfd2kgd;
+		gfx_target_version = 80002;
+		if (!vf)
+			f2g = &gfx_v8_kfd2kgd;
 		break;
 	case CHIP_FIJI:
-		if (vf)
-			device_info = &fiji_vf_device_info;
-		else
-			device_info = &fiji_device_info;
+		gfx_target_version = 80003;
 		f2g = &gfx_v8_kfd2kgd;
 		break;
 	case CHIP_POLARIS10:
-		if (vf)
-			device_info = &polaris10_vf_device_info;
-		else
-			device_info = &polaris10_device_info;
+		gfx_target_version = 80003;
 		f2g = &gfx_v8_kfd2kgd;
 		break;
 	case CHIP_POLARIS11:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &polaris11_device_info;
-		f2g = &gfx_v8_kfd2kgd;
+		gfx_target_version = 80003;
+		if (!vf)
+			f2g = &gfx_v8_kfd2kgd;
 		break;
 	case CHIP_POLARIS12:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &polaris12_device_info;
-		f2g = &gfx_v8_kfd2kgd;
+		gfx_target_version = 80003;
+		if (!vf)
+			f2g = &gfx_v8_kfd2kgd;
 		break;
 	case CHIP_VEGAM:
-		if (vf)
-			device_info = NULL;
-		else
-			device_info = &vegam_device_info;
-		f2g = &gfx_v8_kfd2kgd;
+		gfx_target_version = 80003;
+		if (!vf)
+			f2g = &gfx_v8_kfd2kgd;
 		break;
 	default:
 		switch (adev->ip_versions[GC_HWIP][0]) {
 		case IP_VERSION(9, 0, 1):
-			if (vf)
-				device_info = &vega10_vf_device_info;
-			else
-				device_info = &vega10_device_info;
+			gfx_target_version = 90000;
 			f2g = &gfx_v9_kfd2kgd;
 			break;
 #ifdef KFD_SUPPORT_IOMMU_V2
 		case IP_VERSION(9, 1, 0):
 		case IP_VERSION(9, 2, 2):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &raven_device_info;
-			f2g = &gfx_v9_kfd2kgd;
+			gfx_target_version = 90002;
+			if (!vf)
+				f2g = &gfx_v9_kfd2kgd;
 			break;
 #endif
 		case IP_VERSION(9, 2, 1):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &vega12_device_info;
-			f2g = &gfx_v9_kfd2kgd;
+			gfx_target_version = 90004;
+			if (!vf)
+				f2g = &gfx_v9_kfd2kgd;
 			break;
 		case IP_VERSION(9, 3, 0):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &renoir_device_info;
-			f2g = &gfx_v9_kfd2kgd;
+			gfx_target_version = 90012;
+			if (!vf)
+				f2g = &gfx_v9_kfd2kgd;
 			break;
 		case IP_VERSION(9, 4, 0):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &vega20_device_info;
-			f2g = &gfx_v9_kfd2kgd;
+			gfx_target_version = 90006;
+			if (!vf)
+				f2g = &gfx_v9_kfd2kgd;
 			break;
 		case IP_VERSION(9, 4, 1):
-			device_info = &arcturus_device_info;
+			gfx_target_version = 90008;
 			f2g = &arcturus_kfd2kgd;
 			break;
 		case IP_VERSION(9, 4, 2):
-			device_info = &aldebaran_device_info;
+			gfx_target_version = 90010;
 			f2g = &aldebaran_kfd2kgd;
 			break;
 		case IP_VERSION(10, 1, 10):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &navi10_device_info;
-			f2g = &gfx_v10_kfd2kgd;
+			gfx_target_version = 100100;
+			if (!vf)
+				f2g = &gfx_v10_kfd2kgd;
 			break;
 		case IP_VERSION(10, 1, 2):
-			device_info = &navi12_device_info;
+			gfx_target_version = 100101;
 			f2g = &gfx_v10_kfd2kgd;
 			break;
 		case IP_VERSION(10, 1, 1):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &navi14_device_info;
-			f2g = &gfx_v10_kfd2kgd;
+			gfx_target_version = 100102;
+			if (!vf)
+				f2g = &gfx_v10_kfd2kgd;
 			break;
 		case IP_VERSION(10, 1, 3):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &cyan_skillfish_device_info;
-			f2g = &gfx_v10_kfd2kgd;
+			gfx_target_version = 100103;
+			if (!vf)
+				f2g = &gfx_v10_kfd2kgd;
 			break;
 		case IP_VERSION(10, 3, 0):
-			device_info = &sienna_cichlid_device_info;
+			gfx_target_version = 100300;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
 		case IP_VERSION(10, 3, 2):
-			device_info = &navy_flounder_device_info;
+			gfx_target_version = 100301;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
 		case IP_VERSION(10, 3, 1):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &vangogh_device_info;
-			f2g = &gfx_v10_3_kfd2kgd;
+			gfx_target_version = 100303;
+			if (!vf)
+				f2g = &gfx_v10_3_kfd2kgd;
 			break;
 		case IP_VERSION(10, 3, 4):
-			device_info = &dimgrey_cavefish_device_info;
+			gfx_target_version = 100302;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
 		case IP_VERSION(10, 3, 5):
-			device_info = &beige_goby_device_info;
+			gfx_target_version = 100304;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
 		case IP_VERSION(10, 3, 3):
-			if (vf)
-				device_info = NULL;
-			else
-				device_info = &yellow_carp_device_info;
-			f2g = &gfx_v10_3_kfd2kgd;
+			gfx_target_version = 100305;
+			if (!vf)
+				f2g = &gfx_v10_3_kfd2kgd;
 			break;
 		default:
-			return NULL;
+			break;
 		}
 		break;
 	}
 
-	if (!device_info || !f2g) {
+	if (!f2g) {
 		if (adev->ip_versions[GC_HWIP][0])
 			dev_err(kfd_device, "GC IP %06x %s not supported in kfd\n",
 				adev->ip_versions[GC_HWIP][0], vf ? "VF" : "");
@@ -773,7 +733,14 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
 		return NULL;
 
 	kfd->adev = adev;
+
+	device_info = kzalloc(sizeof(*device_info), GFP_KERNEL);
+	if (!device_info)
+		return NULL;
+
+	kfd_device_info_init(kfd, device_info, vf, gfx_target_version);
 	kfd->device_info = device_info;
+
 	kfd->pdev = pdev;
 	kfd->init_complete = false;
 	kfd->kfd2kgd = f2g;
@@ -1039,7 +1006,13 @@ void kgd2kfd_device_exit(struct kfd_dev *kfd)
 			amdgpu_amdkfd_free_gws(kfd->adev, kfd->gws);
 	}
 
-	kfree(kfd);
+	if (kfd->device_info)
+		kfree(kfd->device_info);
+	kfd->device_info = NULL;
+
+	if (kfd)
+		kfree(kfd);
+	kfd = NULL;
 }
 
 int kgd2kfd_pre_reset(struct kfd_dev *kfd)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 3e11febee7c6..1f11e8271f2e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -232,7 +232,7 @@ struct kfd_vmid_info {
 struct kfd_dev {
 	struct amdgpu_device *adev;
 
-	const struct kfd_device_info *device_info;
+	struct kfd_device_info *device_info;
 	struct pci_dev *pdev;
 	struct drm_device *ddev;
 
-- 
2.25.1


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

* [PATCH v2 4/4] drm/amdkfd: remove hardcoded device_info structs
  2021-11-19 19:52 [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Graham Sider
  2021-11-19 19:52 ` [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function Graham Sider
  2021-11-19 19:52 ` [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation Graham Sider
@ 2021-11-19 19:52 ` Graham Sider
  2021-11-19 21:30   ` Felix Kuehling
  2021-11-19 21:11 ` [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Felix Kuehling
  3 siblings, 1 reply; 11+ messages in thread
From: Graham Sider @ 2021-11-19 19:52 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alexander.Deucher, Felix.Kuehling, Graham Sider

With device_info initialization being handled in kfd_device_info_init,
these structs may be removed. Also add comments to help matching IP
versions to asic names.

Signed-off-by: Graham Sider <Graham.Sider@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 469 +-----------------------
 1 file changed, 17 insertions(+), 452 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 7ddea653b3d9..e36487a85c93 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -53,458 +53,6 @@ extern const struct kfd2kgd_calls aldebaran_kfd2kgd;
 extern const struct kfd2kgd_calls gfx_v10_kfd2kgd;
 extern const struct kfd2kgd_calls gfx_v10_3_kfd2kgd;
 
-#ifdef KFD_SUPPORT_IOMMU_V2
-static const struct kfd_device_info kaveri_device_info = {
-	.gfx_target_version = 70000,
-	.max_pasid_bits = 16,
-	/* max num of queues for KV.TODO should be a dynamic value */
-	.max_no_of_hqd	= 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = false,
-	.needs_iommu_device = true,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info carrizo_device_info = {
-	.gfx_target_version = 80001,
-	.max_pasid_bits = 16,
-	/* max num of queues for CZ.TODO should be a dynamic value */
-	.max_no_of_hqd	= 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = true,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info raven_device_info = {
-	.gfx_target_version = 90002,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = true,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-#endif
-
-#ifdef CONFIG_DRM_AMDGPU_CIK
-static const struct kfd_device_info hawaii_device_info = {
-	.gfx_target_version = 70001,
-	.max_pasid_bits = 16,
-	/* max num of queues for KV.TODO should be a dynamic value */
-	.max_no_of_hqd	= 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = false,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-#endif
-
-static const struct kfd_device_info tonga_device_info = {
-	.gfx_target_version = 80002,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = false,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info fiji_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info fiji_vf_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-
-static const struct kfd_device_info polaris10_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info polaris10_vf_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info polaris11_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info polaris12_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info vegam_device_info = {
-	.gfx_target_version = 80003,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 4,
-	.ih_ring_entry_size = 4 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_cik,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info vega10_device_info = {
-	.gfx_target_version = 90000,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info vega10_vf_device_info = {
-	.gfx_target_version = 90000,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info vega12_device_info = {
-	.gfx_target_version = 90004,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info vega20_device_info = {
-	.gfx_target_version = 90006,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd	= 24,
-	.doorbell_size	= 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info arcturus_device_info = {
-	.gfx_target_version = 90008,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd	= 24,
-	.doorbell_size	= 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info aldebaran_device_info = {
-	.gfx_target_version = 90010,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd	= 24,
-	.doorbell_size	= 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info renoir_device_info = {
-	.gfx_target_version = 90012,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.supports_cwsr = true,
-	.needs_iommu_device = false,
-	.needs_pci_atomics = false,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info navi10_device_info = {
-	.gfx_target_version = 100100,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 145,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info navi12_device_info = {
-	.gfx_target_version = 100101,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 145,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info navi14_device_info = {
-	.gfx_target_version = 100102,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 145,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info sienna_cichlid_device_info = {
-	.gfx_target_version = 100300,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 92,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info navy_flounder_device_info = {
-	.gfx_target_version = 100301,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 92,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info vangogh_device_info = {
-	.gfx_target_version = 100303,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 92,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info dimgrey_cavefish_device_info = {
-	.gfx_target_version = 100302,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 92,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info beige_goby_device_info = {
-	.gfx_target_version = 100304,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 92,
-	.num_sdma_queues_per_engine = 8,
-};
-
-static const struct kfd_device_info yellow_carp_device_info = {
-	.gfx_target_version = 100305,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.no_atomic_fw_version = 92,
-	.num_sdma_queues_per_engine = 2,
-};
-
-static const struct kfd_device_info cyan_skillfish_device_info = {
-	.gfx_target_version = 100103,
-	.max_pasid_bits = 16,
-	.max_no_of_hqd  = 24,
-	.doorbell_size  = 8,
-	.ih_ring_entry_size = 8 * sizeof(uint32_t),
-	.event_interrupt_class = &event_interrupt_class_v9,
-	.num_of_watch_points = 4,
-	.mqd_size_aligned = MQD_SIZE_ALIGNED,
-	.needs_iommu_device = false,
-	.supports_cwsr = true,
-	.needs_pci_atomics = true,
-	.num_sdma_queues_per_engine = 8,
-};
-
 static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
 				unsigned int chunk_size);
 static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
@@ -632,11 +180,13 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
 		break;
 	default:
 		switch (adev->ip_versions[GC_HWIP][0]) {
+		/* Vega 10 */
 		case IP_VERSION(9, 0, 1):
 			gfx_target_version = 90000;
 			f2g = &gfx_v9_kfd2kgd;
 			break;
 #ifdef KFD_SUPPORT_IOMMU_V2
+		/* Raven */
 		case IP_VERSION(9, 1, 0):
 		case IP_VERSION(9, 2, 2):
 			gfx_target_version = 90002;
@@ -644,69 +194,84 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
 				f2g = &gfx_v9_kfd2kgd;
 			break;
 #endif
+		/* Vega12 */
 		case IP_VERSION(9, 2, 1):
 			gfx_target_version = 90004;
 			if (!vf)
 				f2g = &gfx_v9_kfd2kgd;
 			break;
+		/* Renoir */
 		case IP_VERSION(9, 3, 0):
 			gfx_target_version = 90012;
 			if (!vf)
 				f2g = &gfx_v9_kfd2kgd;
 			break;
+		/* Vega20 */
 		case IP_VERSION(9, 4, 0):
 			gfx_target_version = 90006;
 			if (!vf)
 				f2g = &gfx_v9_kfd2kgd;
 			break;
+		/* Arcturus */
 		case IP_VERSION(9, 4, 1):
 			gfx_target_version = 90008;
 			f2g = &arcturus_kfd2kgd;
 			break;
+		/* Aldebaran */
 		case IP_VERSION(9, 4, 2):
 			gfx_target_version = 90010;
 			f2g = &aldebaran_kfd2kgd;
 			break;
+		/* Navi10 */
 		case IP_VERSION(10, 1, 10):
 			gfx_target_version = 100100;
 			if (!vf)
 				f2g = &gfx_v10_kfd2kgd;
 			break;
+		/* Navi12 */
 		case IP_VERSION(10, 1, 2):
 			gfx_target_version = 100101;
 			f2g = &gfx_v10_kfd2kgd;
 			break;
+		/* Navi14 */
 		case IP_VERSION(10, 1, 1):
 			gfx_target_version = 100102;
 			if (!vf)
 				f2g = &gfx_v10_kfd2kgd;
 			break;
+		/* Cyan Skillfish */
 		case IP_VERSION(10, 1, 3):
 			gfx_target_version = 100103;
 			if (!vf)
 				f2g = &gfx_v10_kfd2kgd;
 			break;
+		/* Sienna Cichlid */
 		case IP_VERSION(10, 3, 0):
 			gfx_target_version = 100300;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
+		/* Navy Flounder */
 		case IP_VERSION(10, 3, 2):
 			gfx_target_version = 100301;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
+		/* Van Gogh */
 		case IP_VERSION(10, 3, 1):
 			gfx_target_version = 100303;
 			if (!vf)
 				f2g = &gfx_v10_3_kfd2kgd;
 			break;
+		/* Dimgrey Cavefish */
 		case IP_VERSION(10, 3, 4):
 			gfx_target_version = 100302;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
+		/* Beige Goby */
 		case IP_VERSION(10, 3, 5):
 			gfx_target_version = 100304;
 			f2g = &gfx_v10_3_kfd2kgd;
 			break;
+		/* Yellow Carp */
 		case IP_VERSION(10, 3, 3):
 			gfx_target_version = 100305;
 			if (!vf)
-- 
2.25.1


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

* Re: [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name
  2021-11-19 19:52 [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Graham Sider
                   ` (2 preceding siblings ...)
  2021-11-19 19:52 ` [PATCH v2 4/4] drm/amdkfd: remove hardcoded device_info structs Graham Sider
@ 2021-11-19 21:11 ` Felix Kuehling
  3 siblings, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-11-19 21:11 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Alexander.Deucher

On 2021-11-19 2:52 p.m., Graham Sider wrote:
> device_info->asic_name and amdgpu_asic_name[adev->asic_type] both
> provide asic name strings, with the only difference being casing.

That could still break compatibility since this name gets exposed to 
user mode. You could convert the name in dev->node_props to lower case 
to fix that.

Regards,
   Felix


> Propose to remove asic_name from device_info and replace sysfs entry
> with amdgpu_asic_name[].
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 29 -----------------------
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h     |  1 -
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c |  2 +-
>   3 files changed, 1 insertion(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index e1294fba0c26..e11fc4e20c32 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -55,7 +55,6 @@ extern const struct kfd2kgd_calls gfx_v10_3_kfd2kgd;
>   
>   #ifdef KFD_SUPPORT_IOMMU_V2
>   static const struct kfd_device_info kaveri_device_info = {
> -	.asic_name = "kaveri",
>   	.gfx_target_version = 70000,
>   	.max_pasid_bits = 16,
>   	/* max num of queues for KV.TODO should be a dynamic value */
> @@ -72,7 +71,6 @@ static const struct kfd_device_info kaveri_device_info = {
>   };
>   
>   static const struct kfd_device_info carrizo_device_info = {
> -	.asic_name = "carrizo",
>   	.gfx_target_version = 80001,
>   	.max_pasid_bits = 16,
>   	/* max num of queues for CZ.TODO should be a dynamic value */
> @@ -89,7 +87,6 @@ static const struct kfd_device_info carrizo_device_info = {
>   };
>   
>   static const struct kfd_device_info raven_device_info = {
> -	.asic_name = "raven",
>   	.gfx_target_version = 90002,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -107,7 +104,6 @@ static const struct kfd_device_info raven_device_info = {
>   
>   #ifdef CONFIG_DRM_AMDGPU_CIK
>   static const struct kfd_device_info hawaii_device_info = {
> -	.asic_name = "hawaii",
>   	.gfx_target_version = 70001,
>   	.max_pasid_bits = 16,
>   	/* max num of queues for KV.TODO should be a dynamic value */
> @@ -125,7 +121,6 @@ static const struct kfd_device_info hawaii_device_info = {
>   #endif
>   
>   static const struct kfd_device_info tonga_device_info = {
> -	.asic_name = "tonga",
>   	.gfx_target_version = 80002,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -141,7 +136,6 @@ static const struct kfd_device_info tonga_device_info = {
>   };
>   
>   static const struct kfd_device_info fiji_device_info = {
> -	.asic_name = "fiji",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -157,7 +151,6 @@ static const struct kfd_device_info fiji_device_info = {
>   };
>   
>   static const struct kfd_device_info fiji_vf_device_info = {
> -	.asic_name = "fiji",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -174,7 +167,6 @@ static const struct kfd_device_info fiji_vf_device_info = {
>   
>   
>   static const struct kfd_device_info polaris10_device_info = {
> -	.asic_name = "polaris10",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -190,7 +182,6 @@ static const struct kfd_device_info polaris10_device_info = {
>   };
>   
>   static const struct kfd_device_info polaris10_vf_device_info = {
> -	.asic_name = "polaris10",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -206,7 +197,6 @@ static const struct kfd_device_info polaris10_vf_device_info = {
>   };
>   
>   static const struct kfd_device_info polaris11_device_info = {
> -	.asic_name = "polaris11",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -222,7 +212,6 @@ static const struct kfd_device_info polaris11_device_info = {
>   };
>   
>   static const struct kfd_device_info polaris12_device_info = {
> -	.asic_name = "polaris12",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -238,7 +227,6 @@ static const struct kfd_device_info polaris12_device_info = {
>   };
>   
>   static const struct kfd_device_info vegam_device_info = {
> -	.asic_name = "vegam",
>   	.gfx_target_version = 80003,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -254,7 +242,6 @@ static const struct kfd_device_info vegam_device_info = {
>   };
>   
>   static const struct kfd_device_info vega10_device_info = {
> -	.asic_name = "vega10",
>   	.gfx_target_version = 90000,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -270,7 +257,6 @@ static const struct kfd_device_info vega10_device_info = {
>   };
>   
>   static const struct kfd_device_info vega10_vf_device_info = {
> -	.asic_name = "vega10",
>   	.gfx_target_version = 90000,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -286,7 +272,6 @@ static const struct kfd_device_info vega10_vf_device_info = {
>   };
>   
>   static const struct kfd_device_info vega12_device_info = {
> -	.asic_name = "vega12",
>   	.gfx_target_version = 90004,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -302,7 +287,6 @@ static const struct kfd_device_info vega12_device_info = {
>   };
>   
>   static const struct kfd_device_info vega20_device_info = {
> -	.asic_name = "vega20",
>   	.gfx_target_version = 90006,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd	= 24,
> @@ -318,7 +302,6 @@ static const struct kfd_device_info vega20_device_info = {
>   };
>   
>   static const struct kfd_device_info arcturus_device_info = {
> -	.asic_name = "arcturus",
>   	.gfx_target_version = 90008,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd	= 24,
> @@ -334,7 +317,6 @@ static const struct kfd_device_info arcturus_device_info = {
>   };
>   
>   static const struct kfd_device_info aldebaran_device_info = {
> -	.asic_name = "aldebaran",
>   	.gfx_target_version = 90010,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd	= 24,
> @@ -350,7 +332,6 @@ static const struct kfd_device_info aldebaran_device_info = {
>   };
>   
>   static const struct kfd_device_info renoir_device_info = {
> -	.asic_name = "renoir",
>   	.gfx_target_version = 90012,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -366,7 +347,6 @@ static const struct kfd_device_info renoir_device_info = {
>   };
>   
>   static const struct kfd_device_info navi10_device_info = {
> -	.asic_name = "navi10",
>   	.gfx_target_version = 100100,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -383,7 +363,6 @@ static const struct kfd_device_info navi10_device_info = {
>   };
>   
>   static const struct kfd_device_info navi12_device_info = {
> -	.asic_name = "navi12",
>   	.gfx_target_version = 100101,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -400,7 +379,6 @@ static const struct kfd_device_info navi12_device_info = {
>   };
>   
>   static const struct kfd_device_info navi14_device_info = {
> -	.asic_name = "navi14",
>   	.gfx_target_version = 100102,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -417,7 +395,6 @@ static const struct kfd_device_info navi14_device_info = {
>   };
>   
>   static const struct kfd_device_info sienna_cichlid_device_info = {
> -	.asic_name = "sienna_cichlid",
>   	.gfx_target_version = 100300,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -434,7 +411,6 @@ static const struct kfd_device_info sienna_cichlid_device_info = {
>   };
>   
>   static const struct kfd_device_info navy_flounder_device_info = {
> -	.asic_name = "navy_flounder",
>   	.gfx_target_version = 100301,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -451,7 +427,6 @@ static const struct kfd_device_info navy_flounder_device_info = {
>   };
>   
>   static const struct kfd_device_info vangogh_device_info = {
> -	.asic_name = "vangogh",
>   	.gfx_target_version = 100303,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -468,7 +443,6 @@ static const struct kfd_device_info vangogh_device_info = {
>   };
>   
>   static const struct kfd_device_info dimgrey_cavefish_device_info = {
> -	.asic_name = "dimgrey_cavefish",
>   	.gfx_target_version = 100302,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -485,7 +459,6 @@ static const struct kfd_device_info dimgrey_cavefish_device_info = {
>   };
>   
>   static const struct kfd_device_info beige_goby_device_info = {
> -	.asic_name = "beige_goby",
>   	.gfx_target_version = 100304,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -502,7 +475,6 @@ static const struct kfd_device_info beige_goby_device_info = {
>   };
>   
>   static const struct kfd_device_info yellow_carp_device_info = {
> -	.asic_name = "yellow_carp",
>   	.gfx_target_version = 100305,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> @@ -519,7 +491,6 @@ static const struct kfd_device_info yellow_carp_device_info = {
>   };
>   
>   static const struct kfd_device_info cyan_skillfish_device_info = {
> -	.asic_name = "cyan_skillfish",
>   	.gfx_target_version = 100103,
>   	.max_pasid_bits = 16,
>   	.max_no_of_hqd  = 24,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 1054fedd7b3c..3e11febee7c6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -195,7 +195,6 @@ struct kfd_event_interrupt_class {
>   };
>   
>   struct kfd_device_info {
> -	const char *asic_name;
>   	uint32_t gfx_target_version;
>   	const struct kfd_event_interrupt_class *event_interrupt_class;
>   	unsigned int max_pasid_bits;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 2d44b26b6657..d2bb35f1c58f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1370,7 +1370,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
>   
>   	amdgpu_amdkfd_get_cu_info(dev->gpu->adev, &cu_info);
>   
> -	strncpy(dev->node_props.name, gpu->device_info->asic_name,
> +	strncpy(dev->node_props.name, amdgpu_asic_name[gpu->adev->asic_type],
>   			KFD_TOPOLOGY_PUBLIC_NAME_SIZE);
>   
>   	dev->node_props.simd_arrays_per_engine =

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

* Re: [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function
  2021-11-19 19:52 ` [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function Graham Sider
@ 2021-11-19 21:19   ` Felix Kuehling
  2021-11-22 15:25     ` Sider, Graham
  0 siblings, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-11-19 21:19 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Alexander.Deucher

On 2021-11-19 2:52 p.m., Graham Sider wrote:
> Initializes device_info structs given either asic_type (enum) if GFX
> version is less than GFX9, or GC IP version if greater. Also takes in vf
> and the target compiler gfx version.
>
> Inclusion/exclusion to certain conditions for certain GC IP versions may
> be necessary on npi bringup on a case-by-case basis, but for the most
> part should be minimal (e.g. adding one || asic_version == IP_VERSION(X ,X, X) case).
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 61 +++++++++++++++++++++++++
>   1 file changed, 61 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index e11fc4e20c32..676cb9c3166c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -511,6 +511,67 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
>   
>   static int kfd_resume(struct kfd_dev *kfd);
>   
> +static void kfd_device_info_init(struct kfd_dev *kfd,
> +				 struct kfd_device_info *device_info,
> +				 bool vf, uint32_t gfx_target_version)

This will give you a compile warning about an unused static function. 
Maybe squash this with the commit that actually starts using this function.


> +{
> +	uint32_t gc_version = KFD_GC_VERSION(kfd);
> +	uint32_t asic_type = kfd->adev->asic_type;
> +
> +	device_info->max_pasid_bits = 16;
> +	device_info->max_no_of_hqd = 24;
> +	device_info->num_of_watch_points = 4;
> +	device_info->mqd_size_aligned = MQD_SIZE_ALIGNED;
> +	device_info->gfx_target_version = gfx_target_version;
> +
> +	if (KFD_IS_SOC15(kfd)) {
> +		device_info->doorbell_size = 8;
> +		device_info->ih_ring_entry_size = 8 * sizeof(uint32_t);
> +		device_info->event_interrupt_class = &event_interrupt_class_v9;
> +		device_info->supports_cwsr = true;
> +
> +		if ((gc_version >= IP_VERSION(9, 0, 1)  &&
> +		     gc_version <= IP_VERSION(9, 3, 0)) ||
> +		     gc_version == IP_VERSION(10, 3, 1) ||
> +		     gc_version == IP_VERSION(10, 3, 3))
> +			device_info->num_sdma_queues_per_engine = 2;
> +		else
> +			device_info->num_sdma_queues_per_engine = 8;

I feel this should be based on the SDMA IP version, not the GC IP version.


> +
> +		/* Navi2x+, Navi1x+ */
> +		if (gc_version >= IP_VERSION(10, 3, 0))

There needs to be a maximum check here. This case should not 
automatically apply to future ASICs e.g. GFX11.


> +			device_info->no_atomic_fw_version = 145;
> +		else if (gc_version >= IP_VERSION(10, 1, 1))
> +			device_info->no_atomic_fw_version = 92;
> +
> +		/* Raven */
> +		if (gc_version == IP_VERSION(9, 1, 0) ||
> +		    gc_version == IP_VERSION(9, 2, 2))
> +			device_info->needs_iommu_device = true;
> +
> +		/* Navi1x+ */
> +		if (gc_version >= IP_VERSION(10, 1, 1))

There needs to be a maximum check here. On future ASICs (maybe GFX11) I 
would not expect atomics to be required.

Regards,
   Felix


> +			device_info->needs_pci_atomics = true;
> +	} else {
> +		device_info->doorbell_size = 4;
> +		device_info->ih_ring_entry_size = 4 * sizeof(uint32_t);
> +		device_info->event_interrupt_class = &event_interrupt_class_cik;
> +		device_info->num_sdma_queues_per_engine = 2;
> +
> +		if (asic_type != CHIP_KAVERI &&
> +		    asic_type != CHIP_HAWAII &&
> +		    asic_type != CHIP_TONGA)
> +			device_info->supports_cwsr = true;
> +
> +		if (asic_type == CHIP_KAVERI ||
> +		    asic_type == CHIP_CARRIZO)
> +			device_info->needs_iommu_device = true;
> +
> +		if (asic_type != CHIP_HAWAII && !vf)
> +			device_info->needs_pci_atomics = true;
> +	}
> +}
> +
>   struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>   {
>   	struct kfd_dev *kfd;

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

* Re: [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation
  2021-11-19 19:52 ` [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation Graham Sider
@ 2021-11-19 21:30   ` Felix Kuehling
  2021-11-22 15:32     ` Sider, Graham
  0 siblings, 1 reply; 11+ messages in thread
From: Felix Kuehling @ 2021-11-19 21:30 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Alexander.Deucher

On 2021-11-19 2:52 p.m., Graham Sider wrote:
> Change unsupported asic condition to only probe f2g, move device_info
> initialization post-switch and map to heap.
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 183 ++++++++++--------------
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h   |   2 +-
>   2 files changed, 79 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 676cb9c3166c..7ddea653b3d9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -574,191 +574,151 @@ static void kfd_device_info_init(struct kfd_dev *kfd,
>   
>   struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>   {
> -	struct kfd_dev *kfd;
> -	const struct kfd_device_info *device_info;
> -	const struct kfd2kgd_calls *f2g;
> +	struct kfd_dev *kfd = NULL;
> +	struct kfd_device_info *device_info = NULL;
> +	const struct kfd2kgd_calls *f2g = NULL;
>   	struct pci_dev *pdev = adev->pdev;
> +	uint32_t gfx_target_version = 0;
>   
>   	switch (adev->asic_type) {
>   #ifdef KFD_SUPPORT_IOMMU_V2
>   #ifdef CONFIG_DRM_AMDGPU_CIK
>   	case CHIP_KAVERI:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &kaveri_device_info;
> -		f2g = &gfx_v7_kfd2kgd;
> +		gfx_target_version = 70000;
> +		if (!vf)
> +			f2g = &gfx_v7_kfd2kgd;
>   		break;
>   #endif
>   	case CHIP_CARRIZO:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &carrizo_device_info;
> -		f2g = &gfx_v8_kfd2kgd;
> +		gfx_target_version = 80001;
> +		if (!vf)
> +			f2g = &gfx_v8_kfd2kgd;
>   		break;
>   #endif
>   #ifdef CONFIG_DRM_AMDGPU_CIK
>   	case CHIP_HAWAII:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &hawaii_device_info;
> -		f2g = &gfx_v7_kfd2kgd;
> +		gfx_target_version = 70001;
> +		if (!vf)
> +			f2g = &gfx_v7_kfd2kgd;
>   		break;
>   #endif
>   	case CHIP_TONGA:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &tonga_device_info;
> -		f2g = &gfx_v8_kfd2kgd;
> +		gfx_target_version = 80002;
> +		if (!vf)
> +			f2g = &gfx_v8_kfd2kgd;
>   		break;
>   	case CHIP_FIJI:
> -		if (vf)
> -			device_info = &fiji_vf_device_info;
> -		else
> -			device_info = &fiji_device_info;
> +		gfx_target_version = 80003;
>   		f2g = &gfx_v8_kfd2kgd;
>   		break;
>   	case CHIP_POLARIS10:
> -		if (vf)
> -			device_info = &polaris10_vf_device_info;
> -		else
> -			device_info = &polaris10_device_info;
> +		gfx_target_version = 80003;
>   		f2g = &gfx_v8_kfd2kgd;
>   		break;
>   	case CHIP_POLARIS11:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &polaris11_device_info;
> -		f2g = &gfx_v8_kfd2kgd;
> +		gfx_target_version = 80003;
> +		if (!vf)
> +			f2g = &gfx_v8_kfd2kgd;
>   		break;
>   	case CHIP_POLARIS12:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &polaris12_device_info;
> -		f2g = &gfx_v8_kfd2kgd;
> +		gfx_target_version = 80003;
> +		if (!vf)
> +			f2g = &gfx_v8_kfd2kgd;
>   		break;
>   	case CHIP_VEGAM:
> -		if (vf)
> -			device_info = NULL;
> -		else
> -			device_info = &vegam_device_info;
> -		f2g = &gfx_v8_kfd2kgd;
> +		gfx_target_version = 80003;
> +		if (!vf)
> +			f2g = &gfx_v8_kfd2kgd;
>   		break;
>   	default:
>   		switch (adev->ip_versions[GC_HWIP][0]) {
>   		case IP_VERSION(9, 0, 1):
> -			if (vf)
> -				device_info = &vega10_vf_device_info;
> -			else
> -				device_info = &vega10_device_info;
> +			gfx_target_version = 90000;
>   			f2g = &gfx_v9_kfd2kgd;
>   			break;
>   #ifdef KFD_SUPPORT_IOMMU_V2
>   		case IP_VERSION(9, 1, 0):
>   		case IP_VERSION(9, 2, 2):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &raven_device_info;
> -			f2g = &gfx_v9_kfd2kgd;
> +			gfx_target_version = 90002;
> +			if (!vf)
> +				f2g = &gfx_v9_kfd2kgd;
>   			break;
>   #endif
>   		case IP_VERSION(9, 2, 1):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &vega12_device_info;
> -			f2g = &gfx_v9_kfd2kgd;
> +			gfx_target_version = 90004;
> +			if (!vf)
> +				f2g = &gfx_v9_kfd2kgd;
>   			break;
>   		case IP_VERSION(9, 3, 0):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &renoir_device_info;
> -			f2g = &gfx_v9_kfd2kgd;
> +			gfx_target_version = 90012;
> +			if (!vf)
> +				f2g = &gfx_v9_kfd2kgd;
>   			break;
>   		case IP_VERSION(9, 4, 0):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &vega20_device_info;
> -			f2g = &gfx_v9_kfd2kgd;
> +			gfx_target_version = 90006;
> +			if (!vf)
> +				f2g = &gfx_v9_kfd2kgd;
>   			break;
>   		case IP_VERSION(9, 4, 1):
> -			device_info = &arcturus_device_info;
> +			gfx_target_version = 90008;
>   			f2g = &arcturus_kfd2kgd;
>   			break;
>   		case IP_VERSION(9, 4, 2):
> -			device_info = &aldebaran_device_info;
> +			gfx_target_version = 90010;
>   			f2g = &aldebaran_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 1, 10):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &navi10_device_info;
> -			f2g = &gfx_v10_kfd2kgd;
> +			gfx_target_version = 100100;
> +			if (!vf)
> +				f2g = &gfx_v10_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 1, 2):
> -			device_info = &navi12_device_info;
> +			gfx_target_version = 100101;
>   			f2g = &gfx_v10_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 1, 1):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &navi14_device_info;
> -			f2g = &gfx_v10_kfd2kgd;
> +			gfx_target_version = 100102;
> +			if (!vf)
> +				f2g = &gfx_v10_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 1, 3):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &cyan_skillfish_device_info;
> -			f2g = &gfx_v10_kfd2kgd;
> +			gfx_target_version = 100103;
> +			if (!vf)
> +				f2g = &gfx_v10_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 3, 0):
> -			device_info = &sienna_cichlid_device_info;
> +			gfx_target_version = 100300;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 3, 2):
> -			device_info = &navy_flounder_device_info;
> +			gfx_target_version = 100301;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 3, 1):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &vangogh_device_info;
> -			f2g = &gfx_v10_3_kfd2kgd;
> +			gfx_target_version = 100303;
> +			if (!vf)
> +				f2g = &gfx_v10_3_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 3, 4):
> -			device_info = &dimgrey_cavefish_device_info;
> +			gfx_target_version = 100302;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 3, 5):
> -			device_info = &beige_goby_device_info;
> +			gfx_target_version = 100304;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
>   		case IP_VERSION(10, 3, 3):
> -			if (vf)
> -				device_info = NULL;
> -			else
> -				device_info = &yellow_carp_device_info;
> -			f2g = &gfx_v10_3_kfd2kgd;
> +			gfx_target_version = 100305;
> +			if (!vf)
> +				f2g = &gfx_v10_3_kfd2kgd;
>   			break;
>   		default:
> -			return NULL;
> +			break;
>   		}
>   		break;
>   	}
>   
> -	if (!device_info || !f2g) {
> +	if (!f2g) {
>   		if (adev->ip_versions[GC_HWIP][0])
>   			dev_err(kfd_device, "GC IP %06x %s not supported in kfd\n",
>   				adev->ip_versions[GC_HWIP][0], vf ? "VF" : "");
> @@ -773,7 +733,14 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>   		return NULL;
>   
>   	kfd->adev = adev;
> +
> +	device_info = kzalloc(sizeof(*device_info), GFP_KERNEL);

Just thinking out loud, no need to change this: Maybe device_info 
doesn't need to be dynamically allocated. It could just be a member of 
struct kfd_dev. Except that it would result in a bunch of cosmetic 
changes s/device_info->/device_info./g.


> +	if (!device_info)
> +		return NULL;
> +
> +	kfd_device_info_init(kfd, device_info, vf, gfx_target_version);
>   	kfd->device_info = device_info;
> +
>   	kfd->pdev = pdev;
>   	kfd->init_complete = false;
>   	kfd->kfd2kgd = f2g;
> @@ -1039,7 +1006,13 @@ void kgd2kfd_device_exit(struct kfd_dev *kfd)
>   			amdgpu_amdkfd_free_gws(kfd->adev, kfd->gws);
>   	}
>   
> -	kfree(kfd);
> +	if (kfd->device_info)
> +		kfree(kfd->device_info);

NULL-checks are unnecessary before kfree.


> +	kfd->device_info = NULL;

This is unnecessary because you're about to free kfd anyway.


> +
> +	if (kfd)
> +		kfree(kfd);

Same as above.

Regards,
   Felix


> +	kfd = NULL;
>   }
>   
>   int kgd2kfd_pre_reset(struct kfd_dev *kfd)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 3e11febee7c6..1f11e8271f2e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -232,7 +232,7 @@ struct kfd_vmid_info {
>   struct kfd_dev {
>   	struct amdgpu_device *adev;
>   
> -	const struct kfd_device_info *device_info;
> +	struct kfd_device_info *device_info;
>   	struct pci_dev *pdev;
>   	struct drm_device *ddev;
>   

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

* Re: [PATCH v2 4/4] drm/amdkfd: remove hardcoded device_info structs
  2021-11-19 19:52 ` [PATCH v2 4/4] drm/amdkfd: remove hardcoded device_info structs Graham Sider
@ 2021-11-19 21:30   ` Felix Kuehling
  0 siblings, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-11-19 21:30 UTC (permalink / raw)
  To: Graham Sider, amd-gfx; +Cc: Alexander.Deucher

On 2021-11-19 2:52 p.m., Graham Sider wrote:
> With device_info initialization being handled in kfd_device_info_init,
> these structs may be removed. Also add comments to help matching IP
> versions to asic names.
>
> Signed-off-by: Graham Sider <Graham.Sider@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 469 +-----------------------
>   1 file changed, 17 insertions(+), 452 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 7ddea653b3d9..e36487a85c93 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -53,458 +53,6 @@ extern const struct kfd2kgd_calls aldebaran_kfd2kgd;
>   extern const struct kfd2kgd_calls gfx_v10_kfd2kgd;
>   extern const struct kfd2kgd_calls gfx_v10_3_kfd2kgd;
>   
> -#ifdef KFD_SUPPORT_IOMMU_V2
> -static const struct kfd_device_info kaveri_device_info = {
> -	.gfx_target_version = 70000,
> -	.max_pasid_bits = 16,
> -	/* max num of queues for KV.TODO should be a dynamic value */
> -	.max_no_of_hqd	= 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = false,
> -	.needs_iommu_device = true,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info carrizo_device_info = {
> -	.gfx_target_version = 80001,
> -	.max_pasid_bits = 16,
> -	/* max num of queues for CZ.TODO should be a dynamic value */
> -	.max_no_of_hqd	= 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = true,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info raven_device_info = {
> -	.gfx_target_version = 90002,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = true,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -#endif
> -
> -#ifdef CONFIG_DRM_AMDGPU_CIK
> -static const struct kfd_device_info hawaii_device_info = {
> -	.gfx_target_version = 70001,
> -	.max_pasid_bits = 16,
> -	/* max num of queues for KV.TODO should be a dynamic value */
> -	.max_no_of_hqd	= 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = false,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -#endif
> -
> -static const struct kfd_device_info tonga_device_info = {
> -	.gfx_target_version = 80002,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = false,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info fiji_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info fiji_vf_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -
> -static const struct kfd_device_info polaris10_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info polaris10_vf_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info polaris11_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info polaris12_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info vegam_device_info = {
> -	.gfx_target_version = 80003,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 4,
> -	.ih_ring_entry_size = 4 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_cik,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info vega10_device_info = {
> -	.gfx_target_version = 90000,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info vega10_vf_device_info = {
> -	.gfx_target_version = 90000,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info vega12_device_info = {
> -	.gfx_target_version = 90004,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info vega20_device_info = {
> -	.gfx_target_version = 90006,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd	= 24,
> -	.doorbell_size	= 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info arcturus_device_info = {
> -	.gfx_target_version = 90008,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd	= 24,
> -	.doorbell_size	= 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info aldebaran_device_info = {
> -	.gfx_target_version = 90010,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd	= 24,
> -	.doorbell_size	= 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info renoir_device_info = {
> -	.gfx_target_version = 90012,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.supports_cwsr = true,
> -	.needs_iommu_device = false,
> -	.needs_pci_atomics = false,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info navi10_device_info = {
> -	.gfx_target_version = 100100,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 145,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info navi12_device_info = {
> -	.gfx_target_version = 100101,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 145,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info navi14_device_info = {
> -	.gfx_target_version = 100102,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 145,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info sienna_cichlid_device_info = {
> -	.gfx_target_version = 100300,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 92,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info navy_flounder_device_info = {
> -	.gfx_target_version = 100301,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 92,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info vangogh_device_info = {
> -	.gfx_target_version = 100303,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 92,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info dimgrey_cavefish_device_info = {
> -	.gfx_target_version = 100302,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 92,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info beige_goby_device_info = {
> -	.gfx_target_version = 100304,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 92,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
> -static const struct kfd_device_info yellow_carp_device_info = {
> -	.gfx_target_version = 100305,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.no_atomic_fw_version = 92,
> -	.num_sdma_queues_per_engine = 2,
> -};
> -
> -static const struct kfd_device_info cyan_skillfish_device_info = {
> -	.gfx_target_version = 100103,
> -	.max_pasid_bits = 16,
> -	.max_no_of_hqd  = 24,
> -	.doorbell_size  = 8,
> -	.ih_ring_entry_size = 8 * sizeof(uint32_t),
> -	.event_interrupt_class = &event_interrupt_class_v9,
> -	.num_of_watch_points = 4,
> -	.mqd_size_aligned = MQD_SIZE_ALIGNED,
> -	.needs_iommu_device = false,
> -	.supports_cwsr = true,
> -	.needs_pci_atomics = true,
> -	.num_sdma_queues_per_engine = 8,
> -};
> -
>   static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
>   				unsigned int chunk_size);
>   static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
> @@ -632,11 +180,13 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>   		break;
>   	default:
>   		switch (adev->ip_versions[GC_HWIP][0]) {
> +		/* Vega 10 */
>   		case IP_VERSION(9, 0, 1):
>   			gfx_target_version = 90000;
>   			f2g = &gfx_v9_kfd2kgd;
>   			break;
>   #ifdef KFD_SUPPORT_IOMMU_V2
> +		/* Raven */
>   		case IP_VERSION(9, 1, 0):
>   		case IP_VERSION(9, 2, 2):
>   			gfx_target_version = 90002;
> @@ -644,69 +194,84 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>   				f2g = &gfx_v9_kfd2kgd;
>   			break;
>   #endif
> +		/* Vega12 */
>   		case IP_VERSION(9, 2, 1):
>   			gfx_target_version = 90004;
>   			if (!vf)
>   				f2g = &gfx_v9_kfd2kgd;
>   			break;
> +		/* Renoir */
>   		case IP_VERSION(9, 3, 0):
>   			gfx_target_version = 90012;
>   			if (!vf)
>   				f2g = &gfx_v9_kfd2kgd;
>   			break;
> +		/* Vega20 */
>   		case IP_VERSION(9, 4, 0):
>   			gfx_target_version = 90006;
>   			if (!vf)
>   				f2g = &gfx_v9_kfd2kgd;
>   			break;
> +		/* Arcturus */
>   		case IP_VERSION(9, 4, 1):
>   			gfx_target_version = 90008;
>   			f2g = &arcturus_kfd2kgd;
>   			break;
> +		/* Aldebaran */
>   		case IP_VERSION(9, 4, 2):
>   			gfx_target_version = 90010;
>   			f2g = &aldebaran_kfd2kgd;
>   			break;
> +		/* Navi10 */
>   		case IP_VERSION(10, 1, 10):
>   			gfx_target_version = 100100;
>   			if (!vf)
>   				f2g = &gfx_v10_kfd2kgd;
>   			break;
> +		/* Navi12 */
>   		case IP_VERSION(10, 1, 2):
>   			gfx_target_version = 100101;
>   			f2g = &gfx_v10_kfd2kgd;
>   			break;
> +		/* Navi14 */
>   		case IP_VERSION(10, 1, 1):
>   			gfx_target_version = 100102;
>   			if (!vf)
>   				f2g = &gfx_v10_kfd2kgd;
>   			break;
> +		/* Cyan Skillfish */
>   		case IP_VERSION(10, 1, 3):
>   			gfx_target_version = 100103;
>   			if (!vf)
>   				f2g = &gfx_v10_kfd2kgd;
>   			break;
> +		/* Sienna Cichlid */
>   		case IP_VERSION(10, 3, 0):
>   			gfx_target_version = 100300;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
> +		/* Navy Flounder */
>   		case IP_VERSION(10, 3, 2):
>   			gfx_target_version = 100301;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
> +		/* Van Gogh */
>   		case IP_VERSION(10, 3, 1):
>   			gfx_target_version = 100303;
>   			if (!vf)
>   				f2g = &gfx_v10_3_kfd2kgd;
>   			break;
> +		/* Dimgrey Cavefish */
>   		case IP_VERSION(10, 3, 4):
>   			gfx_target_version = 100302;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
> +		/* Beige Goby */
>   		case IP_VERSION(10, 3, 5):
>   			gfx_target_version = 100304;
>   			f2g = &gfx_v10_3_kfd2kgd;
>   			break;
> +		/* Yellow Carp */
>   		case IP_VERSION(10, 3, 3):
>   			gfx_target_version = 100305;
>   			if (!vf)

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

* RE: [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function
  2021-11-19 21:19   ` Felix Kuehling
@ 2021-11-22 15:25     ` Sider, Graham
  2021-11-22 17:37       ` Felix Kuehling
  0 siblings, 1 reply; 11+ messages in thread
From: Sider, Graham @ 2021-11-22 15:25 UTC (permalink / raw)
  To: Kuehling, Felix, amd-gfx@lists.freedesktop.org; +Cc: Deucher, Alexander

[AMD Official Use Only]

> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling@amd.com>
> Sent: Friday, November 19, 2021 4:20 PM
> To: Sider, Graham <Graham.Sider@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function
> 
> On 2021-11-19 2:52 p.m., Graham Sider wrote:
> > Initializes device_info structs given either asic_type (enum) if GFX
> > version is less than GFX9, or GC IP version if greater. Also takes in
> > vf and the target compiler gfx version.
> >
> > Inclusion/exclusion to certain conditions for certain GC IP versions
> > may be necessary on npi bringup on a case-by-case basis, but for the
> > most part should be minimal (e.g. adding one || asic_version ==
> IP_VERSION(X ,X, X) case).
> >
> > Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 61
> +++++++++++++++++++++++++
> >   1 file changed, 61 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > index e11fc4e20c32..676cb9c3166c 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -511,6 +511,67 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
> >
> >   static int kfd_resume(struct kfd_dev *kfd);
> >
> > +static void kfd_device_info_init(struct kfd_dev *kfd,
> > +				 struct kfd_device_info *device_info,
> > +				 bool vf, uint32_t gfx_target_version)
> 
> This will give you a compile warning about an unused static function.
> Maybe squash this with the commit that actually starts using this function.
> 

Sounds good.

> 
> > +{
> > +	uint32_t gc_version = KFD_GC_VERSION(kfd);
> > +	uint32_t asic_type = kfd->adev->asic_type;
> > +
> > +	device_info->max_pasid_bits = 16;
> > +	device_info->max_no_of_hqd = 24;
> > +	device_info->num_of_watch_points = 4;
> > +	device_info->mqd_size_aligned = MQD_SIZE_ALIGNED;
> > +	device_info->gfx_target_version = gfx_target_version;
> > +
> > +	if (KFD_IS_SOC15(kfd)) {
> > +		device_info->doorbell_size = 8;
> > +		device_info->ih_ring_entry_size = 8 * sizeof(uint32_t);
> > +		device_info->event_interrupt_class =
> &event_interrupt_class_v9;
> > +		device_info->supports_cwsr = true;
> > +
> > +		if ((gc_version >= IP_VERSION(9, 0, 1)  &&
> > +		     gc_version <= IP_VERSION(9, 3, 0)) ||
> > +		     gc_version == IP_VERSION(10, 3, 1) ||
> > +		     gc_version == IP_VERSION(10, 3, 3))
> > +			device_info->num_sdma_queues_per_engine = 2;
> > +		else
> > +			device_info->num_sdma_queues_per_engine = 8;
> 
> I feel this should be based on the SDMA IP version, not the GC IP version.
> 

Can the SDMA queues/engine be determined by the SDMA IP versions? I would have thought those were instead done on a chip-by-chip basis. E.g. in amdgpu_discovery.c this is how the number of SDMA instances is defined.

> 
> > +
> > +		/* Navi2x+, Navi1x+ */
> > +		if (gc_version >= IP_VERSION(10, 3, 0))
> 
> There needs to be a maximum check here. This case should not automatically
> apply to future ASICs e.g. GFX11.
> 

Just a thought on this: assuming on future asics this field is going to continue to be populated, might it be better to just continue adding cases here as they arise? Adding a check for e.g. < GFX11, would require eventually bumping that check to < GFX12 alongside another check for >= GFX11. So at the end of the day, if a >= check is going to be needed anyway, is a maximum check necessary? Of course this wouldn't apply to below regarding the needs_pci_atomics bool, since as you mention on future asics it can be kept as defaulted to false.

> 
> > +			device_info->no_atomic_fw_version = 145;
> > +		else if (gc_version >= IP_VERSION(10, 1, 1))
> > +			device_info->no_atomic_fw_version = 92;
> > +
> > +		/* Raven */
> > +		if (gc_version == IP_VERSION(9, 1, 0) ||
> > +		    gc_version == IP_VERSION(9, 2, 2))
> > +			device_info->needs_iommu_device = true;
> > +
> > +		/* Navi1x+ */
> > +		if (gc_version >= IP_VERSION(10, 1, 1))
> 
> There needs to be a maximum check here. On future ASICs (maybe GFX11) I
> would not expect atomics to be required.
> 

See above, agreed here.

> Regards,
>    Felix
> 

Best,
Graham

> 
> > +			device_info->needs_pci_atomics = true;
> > +	} else {
> > +		device_info->doorbell_size = 4;
> > +		device_info->ih_ring_entry_size = 4 * sizeof(uint32_t);
> > +		device_info->event_interrupt_class =
> &event_interrupt_class_cik;
> > +		device_info->num_sdma_queues_per_engine = 2;
> > +
> > +		if (asic_type != CHIP_KAVERI &&
> > +		    asic_type != CHIP_HAWAII &&
> > +		    asic_type != CHIP_TONGA)
> > +			device_info->supports_cwsr = true;
> > +
> > +		if (asic_type == CHIP_KAVERI ||
> > +		    asic_type == CHIP_CARRIZO)
> > +			device_info->needs_iommu_device = true;
> > +
> > +		if (asic_type != CHIP_HAWAII && !vf)
> > +			device_info->needs_pci_atomics = true;
> > +	}
> > +}
> > +
> >   struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
> >   {
> >   	struct kfd_dev *kfd;

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

* RE: [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation
  2021-11-19 21:30   ` Felix Kuehling
@ 2021-11-22 15:32     ` Sider, Graham
  0 siblings, 0 replies; 11+ messages in thread
From: Sider, Graham @ 2021-11-22 15:32 UTC (permalink / raw)
  To: Kuehling, Felix, amd-gfx@lists.freedesktop.org; +Cc: Deucher, Alexander

[AMD Official Use Only]

> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling@amd.com>
> Sent: Friday, November 19, 2021 4:30 PM
> To: Sider, Graham <Graham.Sider@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info
> creation
> 
> On 2021-11-19 2:52 p.m., Graham Sider wrote:
> > Change unsupported asic condition to only probe f2g, move device_info
> > initialization post-switch and map to heap.
> >
> > Signed-off-by: Graham Sider <Graham.Sider@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 183 ++++++++++------------
> --
> >   drivers/gpu/drm/amd/amdkfd/kfd_priv.h   |   2 +-
> >   2 files changed, 79 insertions(+), 106 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > index 676cb9c3166c..7ddea653b3d9 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -574,191 +574,151 @@ static void kfd_device_info_init(struct
> > kfd_dev *kfd,
> >
> >   struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
> >   {
> > -	struct kfd_dev *kfd;
> > -	const struct kfd_device_info *device_info;
> > -	const struct kfd2kgd_calls *f2g;
> > +	struct kfd_dev *kfd = NULL;
> > +	struct kfd_device_info *device_info = NULL;
> > +	const struct kfd2kgd_calls *f2g = NULL;
> >   	struct pci_dev *pdev = adev->pdev;
> > +	uint32_t gfx_target_version = 0;
> >
> >   	switch (adev->asic_type) {
> >   #ifdef KFD_SUPPORT_IOMMU_V2
> >   #ifdef CONFIG_DRM_AMDGPU_CIK
> >   	case CHIP_KAVERI:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &kaveri_device_info;
> > -		f2g = &gfx_v7_kfd2kgd;
> > +		gfx_target_version = 70000;
> > +		if (!vf)
> > +			f2g = &gfx_v7_kfd2kgd;
> >   		break;
> >   #endif
> >   	case CHIP_CARRIZO:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &carrizo_device_info;
> > -		f2g = &gfx_v8_kfd2kgd;
> > +		gfx_target_version = 80001;
> > +		if (!vf)
> > +			f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   #endif
> >   #ifdef CONFIG_DRM_AMDGPU_CIK
> >   	case CHIP_HAWAII:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &hawaii_device_info;
> > -		f2g = &gfx_v7_kfd2kgd;
> > +		gfx_target_version = 70001;
> > +		if (!vf)
> > +			f2g = &gfx_v7_kfd2kgd;
> >   		break;
> >   #endif
> >   	case CHIP_TONGA:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &tonga_device_info;
> > -		f2g = &gfx_v8_kfd2kgd;
> > +		gfx_target_version = 80002;
> > +		if (!vf)
> > +			f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   	case CHIP_FIJI:
> > -		if (vf)
> > -			device_info = &fiji_vf_device_info;
> > -		else
> > -			device_info = &fiji_device_info;
> > +		gfx_target_version = 80003;
> >   		f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   	case CHIP_POLARIS10:
> > -		if (vf)
> > -			device_info = &polaris10_vf_device_info;
> > -		else
> > -			device_info = &polaris10_device_info;
> > +		gfx_target_version = 80003;
> >   		f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   	case CHIP_POLARIS11:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &polaris11_device_info;
> > -		f2g = &gfx_v8_kfd2kgd;
> > +		gfx_target_version = 80003;
> > +		if (!vf)
> > +			f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   	case CHIP_POLARIS12:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &polaris12_device_info;
> > -		f2g = &gfx_v8_kfd2kgd;
> > +		gfx_target_version = 80003;
> > +		if (!vf)
> > +			f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   	case CHIP_VEGAM:
> > -		if (vf)
> > -			device_info = NULL;
> > -		else
> > -			device_info = &vegam_device_info;
> > -		f2g = &gfx_v8_kfd2kgd;
> > +		gfx_target_version = 80003;
> > +		if (!vf)
> > +			f2g = &gfx_v8_kfd2kgd;
> >   		break;
> >   	default:
> >   		switch (adev->ip_versions[GC_HWIP][0]) {
> >   		case IP_VERSION(9, 0, 1):
> > -			if (vf)
> > -				device_info = &vega10_vf_device_info;
> > -			else
> > -				device_info = &vega10_device_info;
> > +			gfx_target_version = 90000;
> >   			f2g = &gfx_v9_kfd2kgd;
> >   			break;
> >   #ifdef KFD_SUPPORT_IOMMU_V2
> >   		case IP_VERSION(9, 1, 0):
> >   		case IP_VERSION(9, 2, 2):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &raven_device_info;
> > -			f2g = &gfx_v9_kfd2kgd;
> > +			gfx_target_version = 90002;
> > +			if (!vf)
> > +				f2g = &gfx_v9_kfd2kgd;
> >   			break;
> >   #endif
> >   		case IP_VERSION(9, 2, 1):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &vega12_device_info;
> > -			f2g = &gfx_v9_kfd2kgd;
> > +			gfx_target_version = 90004;
> > +			if (!vf)
> > +				f2g = &gfx_v9_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(9, 3, 0):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &renoir_device_info;
> > -			f2g = &gfx_v9_kfd2kgd;
> > +			gfx_target_version = 90012;
> > +			if (!vf)
> > +				f2g = &gfx_v9_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(9, 4, 0):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &vega20_device_info;
> > -			f2g = &gfx_v9_kfd2kgd;
> > +			gfx_target_version = 90006;
> > +			if (!vf)
> > +				f2g = &gfx_v9_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(9, 4, 1):
> > -			device_info = &arcturus_device_info;
> > +			gfx_target_version = 90008;
> >   			f2g = &arcturus_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(9, 4, 2):
> > -			device_info = &aldebaran_device_info;
> > +			gfx_target_version = 90010;
> >   			f2g = &aldebaran_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 1, 10):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &navi10_device_info;
> > -			f2g = &gfx_v10_kfd2kgd;
> > +			gfx_target_version = 100100;
> > +			if (!vf)
> > +				f2g = &gfx_v10_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 1, 2):
> > -			device_info = &navi12_device_info;
> > +			gfx_target_version = 100101;
> >   			f2g = &gfx_v10_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 1, 1):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &navi14_device_info;
> > -			f2g = &gfx_v10_kfd2kgd;
> > +			gfx_target_version = 100102;
> > +			if (!vf)
> > +				f2g = &gfx_v10_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 1, 3):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &cyan_skillfish_device_info;
> > -			f2g = &gfx_v10_kfd2kgd;
> > +			gfx_target_version = 100103;
> > +			if (!vf)
> > +				f2g = &gfx_v10_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 3, 0):
> > -			device_info = &sienna_cichlid_device_info;
> > +			gfx_target_version = 100300;
> >   			f2g = &gfx_v10_3_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 3, 2):
> > -			device_info = &navy_flounder_device_info;
> > +			gfx_target_version = 100301;
> >   			f2g = &gfx_v10_3_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 3, 1):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &vangogh_device_info;
> > -			f2g = &gfx_v10_3_kfd2kgd;
> > +			gfx_target_version = 100303;
> > +			if (!vf)
> > +				f2g = &gfx_v10_3_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 3, 4):
> > -			device_info = &dimgrey_cavefish_device_info;
> > +			gfx_target_version = 100302;
> >   			f2g = &gfx_v10_3_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 3, 5):
> > -			device_info = &beige_goby_device_info;
> > +			gfx_target_version = 100304;
> >   			f2g = &gfx_v10_3_kfd2kgd;
> >   			break;
> >   		case IP_VERSION(10, 3, 3):
> > -			if (vf)
> > -				device_info = NULL;
> > -			else
> > -				device_info = &yellow_carp_device_info;
> > -			f2g = &gfx_v10_3_kfd2kgd;
> > +			gfx_target_version = 100305;
> > +			if (!vf)
> > +				f2g = &gfx_v10_3_kfd2kgd;
> >   			break;
> >   		default:
> > -			return NULL;
> > +			break;
> >   		}
> >   		break;
> >   	}
> >
> > -	if (!device_info || !f2g) {
> > +	if (!f2g) {
> >   		if (adev->ip_versions[GC_HWIP][0])
> >   			dev_err(kfd_device, "GC IP %06x %s not supported
> in kfd\n",
> >   				adev->ip_versions[GC_HWIP][0], vf ? "VF" :
> ""); @@ -773,7
> > +733,14 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev,
> bool vf)
> >   		return NULL;
> >
> >   	kfd->adev = adev;
> > +
> > +	device_info = kzalloc(sizeof(*device_info), GFP_KERNEL);
> 
> Just thinking out loud, no need to change this: Maybe device_info doesn't
> need to be dynamically allocated. It could just be a member of struct
> kfd_dev. Except that it would result in a bunch of cosmetic changes
> s/device_info->/device_info./g.
> 

Either-or is fine by me, happy to make the changes if that would be preferred. Could also add this as a follow-up patch.

> 
> > +	if (!device_info)
> > +		return NULL;
> > +
> > +	kfd_device_info_init(kfd, device_info, vf, gfx_target_version);
> >   	kfd->device_info = device_info;
> > +
> >   	kfd->pdev = pdev;
> >   	kfd->init_complete = false;
> >   	kfd->kfd2kgd = f2g;
> > @@ -1039,7 +1006,13 @@ void kgd2kfd_device_exit(struct kfd_dev *kfd)
> >   			amdgpu_amdkfd_free_gws(kfd->adev, kfd->gws);
> >   	}
> >
> > -	kfree(kfd);
> > +	if (kfd->device_info)
> > +		kfree(kfd->device_info);
> 
> NULL-checks are unnecessary before kfree.
> 
> 
> > +	kfd->device_info = NULL;
> 
> This is unnecessary because you're about to free kfd anyway.
> 
> 
> > +
> > +	if (kfd)
> > +		kfree(kfd);
> 
> Same as above.
> 

All noted--thanks!

Best,
Graham

> Regards,
>    Felix
> 
> 
> > +	kfd = NULL;
> >   }
> >
> >   int kgd2kfd_pre_reset(struct kfd_dev *kfd) diff --git
> > a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > index 3e11febee7c6..1f11e8271f2e 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> > @@ -232,7 +232,7 @@ struct kfd_vmid_info {
> >   struct kfd_dev {
> >   	struct amdgpu_device *adev;
> >
> > -	const struct kfd_device_info *device_info;
> > +	struct kfd_device_info *device_info;
> >   	struct pci_dev *pdev;
> >   	struct drm_device *ddev;
> >

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

* Re: [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function
  2021-11-22 15:25     ` Sider, Graham
@ 2021-11-22 17:37       ` Felix Kuehling
  0 siblings, 0 replies; 11+ messages in thread
From: Felix Kuehling @ 2021-11-22 17:37 UTC (permalink / raw)
  To: Sider, Graham, amd-gfx@lists.freedesktop.org; +Cc: Deucher, Alexander

Am 2021-11-22 um 10:25 a.m. schrieb Sider, Graham:
> [AMD Official Use Only]
>
>> -----Original Message-----
>> From: Kuehling, Felix <Felix.Kuehling@amd.com>
>> Sent: Friday, November 19, 2021 4:20 PM
>> To: Sider, Graham <Graham.Sider@amd.com>; amd-
>> gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
>> Subject: Re: [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function
>>
>> On 2021-11-19 2:52 p.m., Graham Sider wrote:
>>> Initializes device_info structs given either asic_type (enum) if GFX
>>> version is less than GFX9, or GC IP version if greater. Also takes in
>>> vf and the target compiler gfx version.
>>>
>>> Inclusion/exclusion to certain conditions for certain GC IP versions
>>> may be necessary on npi bringup on a case-by-case basis, but for the
>>> most part should be minimal (e.g. adding one || asic_version ==
>> IP_VERSION(X ,X, X) case).
>>> Signed-off-by: Graham Sider <Graham.Sider@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 61
>> +++++++++++++++++++++++++
>>>   1 file changed, 61 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> index e11fc4e20c32..676cb9c3166c 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> @@ -511,6 +511,67 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
>>>
>>>   static int kfd_resume(struct kfd_dev *kfd);
>>>
>>> +static void kfd_device_info_init(struct kfd_dev *kfd,
>>> +				 struct kfd_device_info *device_info,
>>> +				 bool vf, uint32_t gfx_target_version)
>> This will give you a compile warning about an unused static function.
>> Maybe squash this with the commit that actually starts using this function.
>>
> Sounds good.
>
>>> +{
>>> +	uint32_t gc_version = KFD_GC_VERSION(kfd);
>>> +	uint32_t asic_type = kfd->adev->asic_type;
>>> +
>>> +	device_info->max_pasid_bits = 16;
>>> +	device_info->max_no_of_hqd = 24;
>>> +	device_info->num_of_watch_points = 4;
>>> +	device_info->mqd_size_aligned = MQD_SIZE_ALIGNED;
>>> +	device_info->gfx_target_version = gfx_target_version;
>>> +
>>> +	if (KFD_IS_SOC15(kfd)) {
>>> +		device_info->doorbell_size = 8;
>>> +		device_info->ih_ring_entry_size = 8 * sizeof(uint32_t);
>>> +		device_info->event_interrupt_class =
>> &event_interrupt_class_v9;
>>> +		device_info->supports_cwsr = true;
>>> +
>>> +		if ((gc_version >= IP_VERSION(9, 0, 1)  &&
>>> +		     gc_version <= IP_VERSION(9, 3, 0)) ||
>>> +		     gc_version == IP_VERSION(10, 3, 1) ||
>>> +		     gc_version == IP_VERSION(10, 3, 3))
>>> +			device_info->num_sdma_queues_per_engine = 2;
>>> +		else
>>> +			device_info->num_sdma_queues_per_engine = 8;
>> I feel this should be based on the SDMA IP version, not the GC IP version.
>>
> Can the SDMA queues/engine be determined by the SDMA IP versions? I would have thought those were instead done on a chip-by-chip basis. E.g. in amdgpu_discovery.c this is how the number of SDMA instances is defined.
>
>>> +
>>> +		/* Navi2x+, Navi1x+ */
>>> +		if (gc_version >= IP_VERSION(10, 3, 0))
>> There needs to be a maximum check here. This case should not automatically
>> apply to future ASICs e.g. GFX11.
>>
> Just a thought on this: assuming on future asics this field is going to continue to be populated, might it be better to just continue adding cases here as they arise? Adding a check for e.g. < GFX11, would require eventually bumping that check to < GFX12 alongside another check for >= GFX11. So at the end of the day, if a >= check is going to be needed anyway, is a maximum check necessary? Of course this wouldn't apply to below regarding the needs_pci_atomics bool, since as you mention on future asics it can be kept as defaulted to false.

The reason we had a firmware version check here is, because on these
ASICs older firmware depended on PCIe atomics, and at some version it
stopped depending on them.

On future ASICs I would expect all firmware versions to work without
PCIe atomics. So device_info->needs_pci_atomics would be set to "false"
for newer ASICs by default and you would not need a firmware version
check for them.

If we do need more firmware version checks for future generations, the
firmware versions will be different from current generations.  So you
would need to add new if-cases for those anyway. Firmware version 145
will be meaningless or plain wrong on GFX11 for instance.

Regards,
  Felix


>
>>> +			device_info->no_atomic_fw_version = 145;
>>> +		else if (gc_version >= IP_VERSION(10, 1, 1))
>>> +			device_info->no_atomic_fw_version = 92;
>>> +
>>> +		/* Raven */
>>> +		if (gc_version == IP_VERSION(9, 1, 0) ||
>>> +		    gc_version == IP_VERSION(9, 2, 2))
>>> +			device_info->needs_iommu_device = true;
>>> +
>>> +		/* Navi1x+ */
>>> +		if (gc_version >= IP_VERSION(10, 1, 1))
>> There needs to be a maximum check here. On future ASICs (maybe GFX11) I
>> would not expect atomics to be required.
>>
> See above, agreed here.
>
>> Regards,
>>    Felix
>>
> Best,
> Graham
>
>>> +			device_info->needs_pci_atomics = true;
>>> +	} else {
>>> +		device_info->doorbell_size = 4;
>>> +		device_info->ih_ring_entry_size = 4 * sizeof(uint32_t);
>>> +		device_info->event_interrupt_class =
>> &event_interrupt_class_cik;
>>> +		device_info->num_sdma_queues_per_engine = 2;
>>> +
>>> +		if (asic_type != CHIP_KAVERI &&
>>> +		    asic_type != CHIP_HAWAII &&
>>> +		    asic_type != CHIP_TONGA)
>>> +			device_info->supports_cwsr = true;
>>> +
>>> +		if (asic_type == CHIP_KAVERI ||
>>> +		    asic_type == CHIP_CARRIZO)
>>> +			device_info->needs_iommu_device = true;
>>> +
>>> +		if (asic_type != CHIP_HAWAII && !vf)
>>> +			device_info->needs_pci_atomics = true;
>>> +	}
>>> +}
>>> +
>>>   struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>>>   {
>>>   	struct kfd_dev *kfd;

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

end of thread, other threads:[~2021-11-22 17:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-19 19:52 [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Graham Sider
2021-11-19 19:52 ` [PATCH v2 2/4] drm/amdkfd: add kfd_device_info_init function Graham Sider
2021-11-19 21:19   ` Felix Kuehling
2021-11-22 15:25     ` Sider, Graham
2021-11-22 17:37       ` Felix Kuehling
2021-11-19 19:52 ` [PATCH v2 3/4] drm/amdkfd: move to dynamic device_info creation Graham Sider
2021-11-19 21:30   ` Felix Kuehling
2021-11-22 15:32     ` Sider, Graham
2021-11-19 19:52 ` [PATCH v2 4/4] drm/amdkfd: remove hardcoded device_info structs Graham Sider
2021-11-19 21:30   ` Felix Kuehling
2021-11-19 21:11 ` [PATCH v2 1/4] drm/amdkfd: replace asic_name with amdgpu_asic_name Felix Kuehling

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox