* [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
@ 2021-12-20 3:08 Guchun Chen
2021-12-20 5:43 ` Kim, Jonathan
0 siblings, 1 reply; 6+ messages in thread
From: Guchun Chen @ 2021-12-20 3:08 UTC (permalink / raw)
To: amd-gfx, alexander.deucher, Graham.Sider, Felix.Kuehling, jonathan.kim
Cc: Guchun Chen
sdma queue number is not correct like on vega20, this patch
promises the setting keeps the same after code refactor.
Additionally, improve code to use switch case to list IP
version to complete kfd device_info structure filling.
This keeps consistency with the IP parse code in amdgpu_discovery.c.
v2: use dev_warn for the default switch case;
set default sdma queue per engine(8) and IH handler to v9. (Jonathan)
Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
Signed-off-by: Guchun Chen <guchun.chen@amd.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_device.c | 77 ++++++++++++++++++++++---
1 file changed, 68 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index facc28f58c1f..36406a261203 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -59,11 +59,75 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
static int kfd_resume(struct kfd_dev *kfd);
+static void kfd_device_info_set_sdma_queue_num(struct kfd_dev *kfd)
+{
+ uint32_t sdma_version = kfd->adev->ip_versions[SDMA0_HWIP][0];
+
+ switch (sdma_version) {
+ case IP_VERSION(4, 0, 0):/* VEGA10 */
+ case IP_VERSION(4, 0, 1):/* VEGA12 */
+ case IP_VERSION(4, 1, 0):/* RAVEN */
+ case IP_VERSION(4, 1, 1):/* RAVEN */
+ case IP_VERSION(4, 1, 2):/* RENIOR */
+ case IP_VERSION(5, 2, 1):/* VANGOGH */
+ case IP_VERSION(5, 2, 3):/* YELLOW_CARP */
+ kfd->device_info.num_sdma_queues_per_engine = 2;
+ break;
+ case IP_VERSION(4, 2, 0):/* VEGA20 */
+ case IP_VERSION(4, 2, 2):/* ARCTUTUS */
+ case IP_VERSION(4, 4, 0):/* ALDEBARAN */
+ case IP_VERSION(5, 0, 0):/* NAVI10 */
+ case IP_VERSION(5, 0, 1):/* CYAN_SKILLFISH */
+ case IP_VERSION(5, 0, 2):/* NAVI14 */
+ case IP_VERSION(5, 0, 5):/* NAVI12 */
+ case IP_VERSION(5, 2, 0):/* SIENNA_CICHLID */
+ case IP_VERSION(5, 2, 2):/* NAVY_FLOUDER */
+ case IP_VERSION(5, 2, 4):/* DIMGREY_CAVEFISH */
+ kfd->device_info.num_sdma_queues_per_engine = 8;
+ break;
+ default:
+ dev_warn(kfd_device,
+ "Default sdma queue per engine(8) is set due to "
+ "mismatch of sdma ip block(SDMA_HWIP:0x%x).\n",
+ sdma_version);
+ kfd->device_info.num_sdma_queues_per_engine = 8;
+ }
+}
+
+static void kfd_device_info_set_event_interrupt_class(struct kfd_dev *kfd)
+{
+ uint32_t gc_version = KFD_GC_VERSION(kfd);
+
+ switch (gc_version) {
+ case IP_VERSION(9, 0, 1): /* VEGA10 */
+ case IP_VERSION(9, 2, 1): /* VEGA12 */
+ case IP_VERSION(9, 3, 0): /* RENOIR */
+ case IP_VERSION(9, 4, 0): /* VEGA20 */
+ case IP_VERSION(9, 4, 1): /* ARCTURUS */
+ case IP_VERSION(9, 4, 2): /* ALDEBARAN */
+ case IP_VERSION(10, 3, 1): /* VANGOGH */
+ case IP_VERSION(10, 3, 3): /* YELLOW_CARP */
+ case IP_VERSION(10, 1, 3): /* CYAN_SKILLFISH */
+ case IP_VERSION(10, 1, 10): /* NAVI10 */
+ case IP_VERSION(10, 1, 2): /* NAVI12 */
+ case IP_VERSION(10, 1, 1): /* NAVI14 */
+ case IP_VERSION(10, 3, 0): /* SIENNA_CICHLID */
+ case IP_VERSION(10, 3, 2): /* NAVY_FLOUNDER */
+ case IP_VERSION(10, 3, 4): /* DIMGREY_CAVEFISH */
+ case IP_VERSION(10, 3, 5): /* BEIGE_GOBY */
+ kfd->device_info.event_interrupt_class = &event_interrupt_class_v9;
+ break;
+ default:
+ dev_warn(kfd_device, "v9 event interrupt handler is set due to "
+ "mismatch of gc ip block(GC_HWIP:0x%x).\n", gc_version);
+ kfd->device_info.event_interrupt_class = &event_interrupt_class_v9;
+ }
+}
+
static void kfd_device_info_init(struct kfd_dev *kfd,
bool vf, uint32_t gfx_target_version)
{
uint32_t gc_version = KFD_GC_VERSION(kfd);
- uint32_t sdma_version = kfd->adev->ip_versions[SDMA0_HWIP][0];
uint32_t asic_type = kfd->adev->asic_type;
kfd->device_info.max_pasid_bits = 16;
@@ -75,16 +139,11 @@ static void kfd_device_info_init(struct kfd_dev *kfd,
if (KFD_IS_SOC15(kfd)) {
kfd->device_info.doorbell_size = 8;
kfd->device_info.ih_ring_entry_size = 8 * sizeof(uint32_t);
- kfd->device_info.event_interrupt_class = &event_interrupt_class_v9;
kfd->device_info.supports_cwsr = true;
- if ((sdma_version >= IP_VERSION(4, 0, 0) &&
- sdma_version <= IP_VERSION(4, 2, 0)) ||
- sdma_version == IP_VERSION(5, 2, 1) ||
- sdma_version == IP_VERSION(5, 2, 3))
- kfd->device_info.num_sdma_queues_per_engine = 2;
- else
- kfd->device_info.num_sdma_queues_per_engine = 8;
+ kfd_device_info_set_sdma_queue_num(kfd);
+
+ kfd_device_info_set_event_interrupt_class(kfd);
/* Raven */
if (gc_version == IP_VERSION(9, 1, 0) ||
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
2021-12-20 3:08 [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2) Guchun Chen
@ 2021-12-20 5:43 ` Kim, Jonathan
2021-12-20 6:19 ` Sider, Graham
2021-12-20 6:23 ` Chen, Guchun
0 siblings, 2 replies; 6+ messages in thread
From: Kim, Jonathan @ 2021-12-20 5:43 UTC (permalink / raw)
To: Chen, Guchun, amd-gfx, Deucher, Alexander, Sider, Graham,
Kuehling, Felix
[AMD Official Use Only]
> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen@amd.com>
> Sent: December 19, 2021 10:09 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Sider, Graham
> <Graham.Sider@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> Kim, Jonathan <Jonathan.Kim@amd.com>
> Cc: Chen, Guchun <Guchun.Chen@amd.com>
> Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> init (v2)
>
> sdma queue number is not correct like on vega20, this patch promises the
I think you've also fixed Vega12 and Raven (they were being set to 8 before rather than 2). No need to mention this in your description, just double checking.
> setting keeps the same after code refactor.
> Additionally, improve code to use switch case to list IP version to complete
> kfd device_info structure filling.
> This keeps consistency with the IP parse code in amdgpu_discovery.c.
>
> v2: use dev_warn for the default switch case;
> set default sdma queue per engine(8) and IH handler to v9. (Jonathan)
>
> Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
Other than the missing checks for Raven when setting the interrupt class (see inline comments and reference kgd2kfd_probe in kfd_device.c) and one nit-pick inline, this looks good to me.
With those fixed, this patch is
Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 77
> ++++++++++++++++++++++---
> 1 file changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index facc28f58c1f..36406a261203 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -59,11 +59,75 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
>
> static int kfd_resume(struct kfd_dev *kfd);
>
> +static void kfd_device_info_set_sdma_queue_num(struct kfd_dev *kfd) {
> + uint32_t sdma_version = kfd->adev-
> >ip_versions[SDMA0_HWIP][0];
> +
> + switch (sdma_version) {
Please pull in the indentation for all cases to line up with the switch block.
> + case IP_VERSION(4, 0, 0):/* VEGA10 */
> + case IP_VERSION(4, 0, 1):/* VEGA12 */
> + case IP_VERSION(4, 1, 0):/* RAVEN */
> + case IP_VERSION(4, 1, 1):/* RAVEN */
As mentioned, you've also fixed Vega12 & Raven here I presume since afaik, they're based off Vega10?
> + case IP_VERSION(4, 1, 2):/* RENIOR */
> + case IP_VERSION(5, 2, 1):/* VANGOGH */
> + case IP_VERSION(5, 2, 3):/* YELLOW_CARP */
> + kfd->device_info.num_sdma_queues_per_engine =
> 2;
> + break;
> + case IP_VERSION(4, 2, 0):/* VEGA20 */
> + case IP_VERSION(4, 2, 2):/* ARCTUTUS */
> + case IP_VERSION(4, 4, 0):/* ALDEBARAN */
> + case IP_VERSION(5, 0, 0):/* NAVI10 */
> + case IP_VERSION(5, 0, 1):/* CYAN_SKILLFISH */
> + case IP_VERSION(5, 0, 2):/* NAVI14 */
> + case IP_VERSION(5, 0, 5):/* NAVI12 */
> + case IP_VERSION(5, 2, 0):/* SIENNA_CICHLID */
> + case IP_VERSION(5, 2, 2):/* NAVY_FLOUDER */
> + case IP_VERSION(5, 2, 4):/* DIMGREY_CAVEFISH */
> + kfd->device_info.num_sdma_queues_per_engine =
> 8;
> + break;
> + default:
> + dev_warn(kfd_device,
> + "Default sdma queue per engine(8) is set due
> to "
> + "mismatch of sdma ip
> block(SDMA_HWIP:0x%x).\n",
> + sdma_version);
> + kfd->device_info.num_sdma_queues_per_engine =
> 8;
> + }
> +}
> +
> +static void kfd_device_info_set_event_interrupt_class(struct kfd_dev
> +*kfd) {
> + uint32_t gc_version = KFD_GC_VERSION(kfd);
> +
> + switch (gc_version) {
> + case IP_VERSION(9, 0, 1): /* VEGA10 */
Missing check for case IP_VERSION(9, 1, 0): /* RAVEN */
> + case IP_VERSION(9, 2, 1): /* VEGA12 */
Missing check for case IP_VERSION(9, 2, 2): /* RAVEN */
Thanks,
Jon
> + case IP_VERSION(9, 3, 0): /* RENOIR */
> + case IP_VERSION(9, 4, 0): /* VEGA20 */
> + case IP_VERSION(9, 4, 1): /* ARCTURUS */
> + case IP_VERSION(9, 4, 2): /* ALDEBARAN */
> + case IP_VERSION(10, 3, 1): /* VANGOGH */
> + case IP_VERSION(10, 3, 3): /* YELLOW_CARP */
> + case IP_VERSION(10, 1, 3): /* CYAN_SKILLFISH */
> + case IP_VERSION(10, 1, 10): /* NAVI10 */
> + case IP_VERSION(10, 1, 2): /* NAVI12 */
> + case IP_VERSION(10, 1, 1): /* NAVI14 */
> + case IP_VERSION(10, 3, 0): /* SIENNA_CICHLID */
> + case IP_VERSION(10, 3, 2): /* NAVY_FLOUNDER */
> + case IP_VERSION(10, 3, 4): /* DIMGREY_CAVEFISH */
> + case IP_VERSION(10, 3, 5): /* BEIGE_GOBY */
> + kfd->device_info.event_interrupt_class =
> &event_interrupt_class_v9;
> + break;
> + default:
> + dev_warn(kfd_device, "v9 event interrupt handler is set due
> to "
> + "mismatch of gc ip block(GC_HWIP:0x%x).\n",
> gc_version);
> + kfd->device_info.event_interrupt_class =
> &event_interrupt_class_v9;
> + }
> +}
> +
> static void kfd_device_info_init(struct kfd_dev *kfd,
> bool vf, uint32_t gfx_target_version) {
> uint32_t gc_version = KFD_GC_VERSION(kfd);
> - uint32_t sdma_version = kfd->adev-
> >ip_versions[SDMA0_HWIP][0];
> uint32_t asic_type = kfd->adev->asic_type;
>
> kfd->device_info.max_pasid_bits = 16;
> @@ -75,16 +139,11 @@ static void kfd_device_info_init(struct kfd_dev
> *kfd,
> if (KFD_IS_SOC15(kfd)) {
> kfd->device_info.doorbell_size = 8;
> kfd->device_info.ih_ring_entry_size = 8 * sizeof(uint32_t);
> - kfd->device_info.event_interrupt_class =
> &event_interrupt_class_v9;
> kfd->device_info.supports_cwsr = true;
>
> - if ((sdma_version >= IP_VERSION(4, 0, 0) &&
> - sdma_version <= IP_VERSION(4, 2, 0)) ||
> - sdma_version == IP_VERSION(5, 2, 1) ||
> - sdma_version == IP_VERSION(5, 2, 3))
> - kfd->device_info.num_sdma_queues_per_engine =
> 2;
> - else
> - kfd->device_info.num_sdma_queues_per_engine =
> 8;
> + kfd_device_info_set_sdma_queue_num(kfd);
> +
> + kfd_device_info_set_event_interrupt_class(kfd);
>
> /* Raven */
> if (gc_version == IP_VERSION(9, 1, 0) ||
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
2021-12-20 5:43 ` Kim, Jonathan
@ 2021-12-20 6:19 ` Sider, Graham
2021-12-20 6:26 ` Chen, Guchun
2021-12-20 12:51 ` Kim, Jonathan
2021-12-20 6:23 ` Chen, Guchun
1 sibling, 2 replies; 6+ messages in thread
From: Sider, Graham @ 2021-12-20 6:19 UTC (permalink / raw)
To: Kim, Jonathan, Chen, Guchun, amd-gfx, Deucher, Alexander,
Kuehling, Felix
[Public]
> -----Original Message-----
> From: Kim, Jonathan <Jonathan.Kim@amd.com>
> Sent: Monday, December 20, 2021 12:44 AM
> To: Chen, Guchun <Guchun.Chen@amd.com>; amd-
> gfx@lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Sider, Graham
> <Graham.Sider@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> init (v2)
>
> [AMD Official Use Only]
>
> > -----Original Message-----
> > From: Chen, Guchun <Guchun.Chen@amd.com>
> > Sent: December 19, 2021 10:09 PM
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Sider, Graham
> <Graham.Sider@amd.com>;
> > Kuehling, Felix <Felix.Kuehling@amd.com>; Kim, Jonathan
> > <Jonathan.Kim@amd.com>
> > Cc: Chen, Guchun <Guchun.Chen@amd.com>
> > Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> > init (v2)
> >
> > sdma queue number is not correct like on vega20, this patch promises
> > the
>
> I think you've also fixed Vega12 and Raven (they were being set to 8 before
> rather than 2). No need to mention this in your description, just double
> checking.
>
I believe it was only Vega20 that was being set incorrectly. The condition was:
sdma_version >= IP_VERSION(4, 0, 0) &&
sdma_version <= IP_VERSION(4, 2, 0))
which encapsulates Vega12 and Raven setting sdma_queues_per_engine to 2, but also accidently encapsulates Vega20.
> > setting keeps the same after code refactor.
> > Additionally, improve code to use switch case to list IP version to
> > complete kfd device_info structure filling.
> > This keeps consistency with the IP parse code in amdgpu_discovery.c.
> >
> > v2: use dev_warn for the default switch case;
> > set default sdma queue per engine(8) and IH handler to v9.
> > (Jonathan)
> >
> > Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> > Signed-off-by: Guchun Chen <guchun.chen@amd.com>
>
> Other than the missing checks for Raven when setting the interrupt class
> (see inline comments and reference kgd2kfd_probe in kfd_device.c) and
> one nit-pick inline, this looks good to me.
>
> With those fixed, this patch is
> Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>
>
Other than Jon's comments, this patch is also
Reviewed-by: Graham Sider <Graham.Sider@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_device.c | 77
> > ++++++++++++++++++++++---
> > 1 file changed, 68 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > index facc28f58c1f..36406a261203 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -59,11 +59,75 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
> >
> > static int kfd_resume(struct kfd_dev *kfd);
> >
> > +static void kfd_device_info_set_sdma_queue_num(struct kfd_dev *kfd)
> {
> > + uint32_t sdma_version = kfd->adev-
> > >ip_versions[SDMA0_HWIP][0];
> > +
> > + switch (sdma_version) {
>
> Please pull in the indentation for all cases to line up with the switch block.
>
> > + case IP_VERSION(4, 0, 0):/* VEGA10 */
> > + case IP_VERSION(4, 0, 1):/* VEGA12 */
> > + case IP_VERSION(4, 1, 0):/* RAVEN */
> > + case IP_VERSION(4, 1, 1):/* RAVEN */
>
> As mentioned, you've also fixed Vega12 & Raven here I presume since afaik,
> they're based off Vega10?
>
> > + case IP_VERSION(4, 1, 2):/* RENIOR */
> > + case IP_VERSION(5, 2, 1):/* VANGOGH */
> > + case IP_VERSION(5, 2, 3):/* YELLOW_CARP */
> > + kfd->device_info.num_sdma_queues_per_engine =
> > 2;
> > + break;
> > + case IP_VERSION(4, 2, 0):/* VEGA20 */
> > + case IP_VERSION(4, 2, 2):/* ARCTUTUS */
> > + case IP_VERSION(4, 4, 0):/* ALDEBARAN */
> > + case IP_VERSION(5, 0, 0):/* NAVI10 */
> > + case IP_VERSION(5, 0, 1):/* CYAN_SKILLFISH */
> > + case IP_VERSION(5, 0, 2):/* NAVI14 */
> > + case IP_VERSION(5, 0, 5):/* NAVI12 */
> > + case IP_VERSION(5, 2, 0):/* SIENNA_CICHLID */
> > + case IP_VERSION(5, 2, 2):/* NAVY_FLOUDER */
> > + case IP_VERSION(5, 2, 4):/* DIMGREY_CAVEFISH */
> > + kfd->device_info.num_sdma_queues_per_engine =
> > 8;
> > + break;
> > + default:
> > + dev_warn(kfd_device,
> > + "Default sdma queue per engine(8) is set
> > + due
> > to "
> > + "mismatch of sdma ip
> > block(SDMA_HWIP:0x%x).\n",
> > + sdma_version);
> > + kfd->device_info.num_sdma_queues_per_engine =
> > 8;
> > + }
> > +}
> > +
> > +static void kfd_device_info_set_event_interrupt_class(struct kfd_dev
> > +*kfd) {
> > + uint32_t gc_version = KFD_GC_VERSION(kfd);
> > +
> > + switch (gc_version) {
> > + case IP_VERSION(9, 0, 1): /* VEGA10 */
>
> Missing check for case IP_VERSION(9, 1, 0): /* RAVEN */
>
> > + case IP_VERSION(9, 2, 1): /* VEGA12 */
>
> Missing check for case IP_VERSION(9, 2, 2): /* RAVEN */
>
> Thanks,
>
> Jon
>
> > + case IP_VERSION(9, 3, 0): /* RENOIR */
> > + case IP_VERSION(9, 4, 0): /* VEGA20 */
> > + case IP_VERSION(9, 4, 1): /* ARCTURUS */
> > + case IP_VERSION(9, 4, 2): /* ALDEBARAN */
> > + case IP_VERSION(10, 3, 1): /* VANGOGH */
> > + case IP_VERSION(10, 3, 3): /* YELLOW_CARP */
> > + case IP_VERSION(10, 1, 3): /* CYAN_SKILLFISH */
> > + case IP_VERSION(10, 1, 10): /* NAVI10 */
> > + case IP_VERSION(10, 1, 2): /* NAVI12 */
> > + case IP_VERSION(10, 1, 1): /* NAVI14 */
> > + case IP_VERSION(10, 3, 0): /* SIENNA_CICHLID */
> > + case IP_VERSION(10, 3, 2): /* NAVY_FLOUNDER */
> > + case IP_VERSION(10, 3, 4): /* DIMGREY_CAVEFISH */
> > + case IP_VERSION(10, 3, 5): /* BEIGE_GOBY */
> > + kfd->device_info.event_interrupt_class =
> > &event_interrupt_class_v9;
> > + break;
> > + default:
> > + dev_warn(kfd_device, "v9 event interrupt handler is set
> > + due
> > to "
> > + "mismatch of gc ip block(GC_HWIP:0x%x).\n",
> > gc_version);
> > + kfd->device_info.event_interrupt_class =
> > &event_interrupt_class_v9;
> > + }
> > +}
> > +
> > static void kfd_device_info_init(struct kfd_dev *kfd,
> > bool vf, uint32_t gfx_target_version) {
> > uint32_t gc_version = KFD_GC_VERSION(kfd);
> > - uint32_t sdma_version = kfd->adev-
> > >ip_versions[SDMA0_HWIP][0];
> > uint32_t asic_type = kfd->adev->asic_type;
> >
> > kfd->device_info.max_pasid_bits = 16; @@ -75,16 +139,11 @@
> > static void kfd_device_info_init(struct kfd_dev *kfd,
> > if (KFD_IS_SOC15(kfd)) {
> > kfd->device_info.doorbell_size = 8;
> > kfd->device_info.ih_ring_entry_size = 8 * sizeof(uint32_t);
> > - kfd->device_info.event_interrupt_class =
> > &event_interrupt_class_v9;
> > kfd->device_info.supports_cwsr = true;
> >
> > - if ((sdma_version >= IP_VERSION(4, 0, 0) &&
> > - sdma_version <= IP_VERSION(4, 2, 0)) ||
> > - sdma_version == IP_VERSION(5, 2, 1) ||
> > - sdma_version == IP_VERSION(5, 2, 3))
> > - kfd->device_info.num_sdma_queues_per_engine =
> > 2;
> > - else
> > - kfd->device_info.num_sdma_queues_per_engine =
> > 8;
> > + kfd_device_info_set_sdma_queue_num(kfd);
> > +
> > + kfd_device_info_set_event_interrupt_class(kfd);
> >
> > /* Raven */
> > if (gc_version == IP_VERSION(9, 1, 0) ||
> > --
> > 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
2021-12-20 5:43 ` Kim, Jonathan
2021-12-20 6:19 ` Sider, Graham
@ 2021-12-20 6:23 ` Chen, Guchun
1 sibling, 0 replies; 6+ messages in thread
From: Chen, Guchun @ 2021-12-20 6:23 UTC (permalink / raw)
To: Kim, Jonathan, amd-gfx, Deucher, Alexander, Sider, Graham,
Kuehling, Felix
[Public]
> sdma queue number is not correct like on vega20, this patch promises
> the
I think you've also fixed Vega12 and Raven (they were being set to 8 before rather than 2). No need to mention this in your description, just double checking.
No, sdma queue number on vega12 and Raven is correct, it's 2. Anyway, I have dropped it in the commit message of V3.
Regards,
Guchun
-----Original Message-----
From: Kim, Jonathan <Jonathan.Kim@amd.com>
Sent: Monday, December 20, 2021 1:44 PM
To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Sider, Graham <Graham.Sider@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>
Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
[AMD Official Use Only]
> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen@amd.com>
> Sent: December 19, 2021 10:09 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Sider, Graham <Graham.Sider@amd.com>;
> Kuehling, Felix <Felix.Kuehling@amd.com>; Kim, Jonathan
> <Jonathan.Kim@amd.com>
> Cc: Chen, Guchun <Guchun.Chen@amd.com>
> Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> init (v2)
>
> sdma queue number is not correct like on vega20, this patch promises
> the
I think you've also fixed Vega12 and Raven (they were being set to 8 before rather than 2). No need to mention this in your description, just double checking.
> setting keeps the same after code refactor.
> Additionally, improve code to use switch case to list IP version to
> complete kfd device_info structure filling.
> This keeps consistency with the IP parse code in amdgpu_discovery.c.
>
> v2: use dev_warn for the default switch case;
> set default sdma queue per engine(8) and IH handler to v9.
> (Jonathan)
>
> Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
Other than the missing checks for Raven when setting the interrupt class (see inline comments and reference kgd2kfd_probe in kfd_device.c) and one nit-pick inline, this looks good to me.
With those fixed, this patch is
Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 77
> ++++++++++++++++++++++---
> 1 file changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index facc28f58c1f..36406a261203 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -59,11 +59,75 @@ static void kfd_gtt_sa_fini(struct kfd_dev *kfd);
>
> static int kfd_resume(struct kfd_dev *kfd);
>
> +static void kfd_device_info_set_sdma_queue_num(struct kfd_dev *kfd) {
> + uint32_t sdma_version = kfd->adev-
> >ip_versions[SDMA0_HWIP][0];
> +
> + switch (sdma_version) {
Please pull in the indentation for all cases to line up with the switch block.
> + case IP_VERSION(4, 0, 0):/* VEGA10 */
> + case IP_VERSION(4, 0, 1):/* VEGA12 */
> + case IP_VERSION(4, 1, 0):/* RAVEN */
> + case IP_VERSION(4, 1, 1):/* RAVEN */
As mentioned, you've also fixed Vega12 & Raven here I presume since afaik, they're based off Vega10?
> + case IP_VERSION(4, 1, 2):/* RENIOR */
> + case IP_VERSION(5, 2, 1):/* VANGOGH */
> + case IP_VERSION(5, 2, 3):/* YELLOW_CARP */
> + kfd->device_info.num_sdma_queues_per_engine =
> 2;
> + break;
> + case IP_VERSION(4, 2, 0):/* VEGA20 */
> + case IP_VERSION(4, 2, 2):/* ARCTUTUS */
> + case IP_VERSION(4, 4, 0):/* ALDEBARAN */
> + case IP_VERSION(5, 0, 0):/* NAVI10 */
> + case IP_VERSION(5, 0, 1):/* CYAN_SKILLFISH */
> + case IP_VERSION(5, 0, 2):/* NAVI14 */
> + case IP_VERSION(5, 0, 5):/* NAVI12 */
> + case IP_VERSION(5, 2, 0):/* SIENNA_CICHLID */
> + case IP_VERSION(5, 2, 2):/* NAVY_FLOUDER */
> + case IP_VERSION(5, 2, 4):/* DIMGREY_CAVEFISH */
> + kfd->device_info.num_sdma_queues_per_engine =
> 8;
> + break;
> + default:
> + dev_warn(kfd_device,
> + "Default sdma queue per engine(8) is set
> + due
> to "
> + "mismatch of sdma ip
> block(SDMA_HWIP:0x%x).\n",
> + sdma_version);
> + kfd->device_info.num_sdma_queues_per_engine =
> 8;
> + }
> +}
> +
> +static void kfd_device_info_set_event_interrupt_class(struct kfd_dev
> +*kfd) {
> + uint32_t gc_version = KFD_GC_VERSION(kfd);
> +
> + switch (gc_version) {
> + case IP_VERSION(9, 0, 1): /* VEGA10 */
Missing check for case IP_VERSION(9, 1, 0): /* RAVEN */
> + case IP_VERSION(9, 2, 1): /* VEGA12 */
Missing check for case IP_VERSION(9, 2, 2): /* RAVEN */
Thanks,
Jon
> + case IP_VERSION(9, 3, 0): /* RENOIR */
> + case IP_VERSION(9, 4, 0): /* VEGA20 */
> + case IP_VERSION(9, 4, 1): /* ARCTURUS */
> + case IP_VERSION(9, 4, 2): /* ALDEBARAN */
> + case IP_VERSION(10, 3, 1): /* VANGOGH */
> + case IP_VERSION(10, 3, 3): /* YELLOW_CARP */
> + case IP_VERSION(10, 1, 3): /* CYAN_SKILLFISH */
> + case IP_VERSION(10, 1, 10): /* NAVI10 */
> + case IP_VERSION(10, 1, 2): /* NAVI12 */
> + case IP_VERSION(10, 1, 1): /* NAVI14 */
> + case IP_VERSION(10, 3, 0): /* SIENNA_CICHLID */
> + case IP_VERSION(10, 3, 2): /* NAVY_FLOUNDER */
> + case IP_VERSION(10, 3, 4): /* DIMGREY_CAVEFISH */
> + case IP_VERSION(10, 3, 5): /* BEIGE_GOBY */
> + kfd->device_info.event_interrupt_class =
> &event_interrupt_class_v9;
> + break;
> + default:
> + dev_warn(kfd_device, "v9 event interrupt handler is set
> + due
> to "
> + "mismatch of gc ip block(GC_HWIP:0x%x).\n",
> gc_version);
> + kfd->device_info.event_interrupt_class =
> &event_interrupt_class_v9;
> + }
> +}
> +
> static void kfd_device_info_init(struct kfd_dev *kfd,
> bool vf, uint32_t gfx_target_version) {
> uint32_t gc_version = KFD_GC_VERSION(kfd);
> - uint32_t sdma_version = kfd->adev-
> >ip_versions[SDMA0_HWIP][0];
> uint32_t asic_type = kfd->adev->asic_type;
>
> kfd->device_info.max_pasid_bits = 16; @@ -75,16 +139,11 @@
> static void kfd_device_info_init(struct kfd_dev *kfd,
> if (KFD_IS_SOC15(kfd)) {
> kfd->device_info.doorbell_size = 8;
> kfd->device_info.ih_ring_entry_size = 8 * sizeof(uint32_t);
> - kfd->device_info.event_interrupt_class =
> &event_interrupt_class_v9;
> kfd->device_info.supports_cwsr = true;
>
> - if ((sdma_version >= IP_VERSION(4, 0, 0) &&
> - sdma_version <= IP_VERSION(4, 2, 0)) ||
> - sdma_version == IP_VERSION(5, 2, 1) ||
> - sdma_version == IP_VERSION(5, 2, 3))
> - kfd->device_info.num_sdma_queues_per_engine =
> 2;
> - else
> - kfd->device_info.num_sdma_queues_per_engine =
> 8;
> + kfd_device_info_set_sdma_queue_num(kfd);
> +
> + kfd_device_info_set_event_interrupt_class(kfd);
>
> /* Raven */
> if (gc_version == IP_VERSION(9, 1, 0) ||
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
2021-12-20 6:19 ` Sider, Graham
@ 2021-12-20 6:26 ` Chen, Guchun
2021-12-20 12:51 ` Kim, Jonathan
1 sibling, 0 replies; 6+ messages in thread
From: Chen, Guchun @ 2021-12-20 6:26 UTC (permalink / raw)
To: Sider, Graham, Kim, Jonathan, amd-gfx, Deucher, Alexander,
Kuehling, Felix
[Public]
Email crossed:).
Graham, I have sent v3 to review, and will add you as another reviewed-by when pushing this patch. Thanks for the review from you and Jonathan.
Merry Xmas!
Regards,
Guchun
-----Original Message-----
From: Sider, Graham <Graham.Sider@amd.com>
Sent: Monday, December 20, 2021 2:19 PM
To: Kim, Jonathan <Jonathan.Kim@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>
Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
[Public]
> -----Original Message-----
> From: Kim, Jonathan <Jonathan.Kim@amd.com>
> Sent: Monday, December 20, 2021 12:44 AM
> To: Chen, Guchun <Guchun.Chen@amd.com>; amd-
> gfx@lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Sider, Graham <Graham.Sider@amd.com>;
> Kuehling, Felix <Felix.Kuehling@amd.com>
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> device init (v2)
>
> [AMD Official Use Only]
>
> > -----Original Message-----
> > From: Chen, Guchun <Guchun.Chen@amd.com>
> > Sent: December 19, 2021 10:09 PM
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Sider, Graham
> <Graham.Sider@amd.com>;
> > Kuehling, Felix <Felix.Kuehling@amd.com>; Kim, Jonathan
> > <Jonathan.Kim@amd.com>
> > Cc: Chen, Guchun <Guchun.Chen@amd.com>
> > Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> > init (v2)
> >
> > sdma queue number is not correct like on vega20, this patch promises
> > the
>
> I think you've also fixed Vega12 and Raven (they were being set to 8
> before rather than 2). No need to mention this in your description,
> just double checking.
>
I believe it was only Vega20 that was being set incorrectly. The condition was:
sdma_version >= IP_VERSION(4, 0, 0) &&
sdma_version <= IP_VERSION(4, 2, 0))
which encapsulates Vega12 and Raven setting sdma_queues_per_engine to 2, but also accidently encapsulates Vega20.
> > setting keeps the same after code refactor.
> > Additionally, improve code to use switch case to list IP version to
> > complete kfd device_info structure filling.
> > This keeps consistency with the IP parse code in amdgpu_discovery.c.
> >
> > v2: use dev_warn for the default switch case;
> > set default sdma queue per engine(8) and IH handler to v9.
> > (Jonathan)
> >
> > Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> > Signed-off-by: Guchun Chen <guchun.chen@amd.com>
>
> Other than the missing checks for Raven when setting the interrupt
> class (see inline comments and reference kgd2kfd_probe in
> kfd_device.c) and one nit-pick inline, this looks good to me.
>
> With those fixed, this patch is
> Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>
>
Other than Jon's comments, this patch is also
Reviewed-by: Graham Sider <Graham.Sider@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_device.c | 77
> > ++++++++++++++++++++++---
> > 1 file changed, 68 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > index facc28f58c1f..36406a261203 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -59,11 +59,75 @@ static void kfd_gtt_sa_fini(struct kfd_dev
> > *kfd);
> >
> > static int kfd_resume(struct kfd_dev *kfd);
> >
> > +static void kfd_device_info_set_sdma_queue_num(struct kfd_dev *kfd)
> {
> > + uint32_t sdma_version = kfd->adev-
> > >ip_versions[SDMA0_HWIP][0];
> > +
> > + switch (sdma_version) {
>
> Please pull in the indentation for all cases to line up with the switch block.
>
> > + case IP_VERSION(4, 0, 0):/* VEGA10 */
> > + case IP_VERSION(4, 0, 1):/* VEGA12 */
> > + case IP_VERSION(4, 1, 0):/* RAVEN */
> > + case IP_VERSION(4, 1, 1):/* RAVEN */
>
> As mentioned, you've also fixed Vega12 & Raven here I presume since
> afaik, they're based off Vega10?
>
> > + case IP_VERSION(4, 1, 2):/* RENIOR */
> > + case IP_VERSION(5, 2, 1):/* VANGOGH */
> > + case IP_VERSION(5, 2, 3):/* YELLOW_CARP */
> > + kfd->device_info.num_sdma_queues_per_engine =
> > 2;
> > + break;
> > + case IP_VERSION(4, 2, 0):/* VEGA20 */
> > + case IP_VERSION(4, 2, 2):/* ARCTUTUS */
> > + case IP_VERSION(4, 4, 0):/* ALDEBARAN */
> > + case IP_VERSION(5, 0, 0):/* NAVI10 */
> > + case IP_VERSION(5, 0, 1):/* CYAN_SKILLFISH */
> > + case IP_VERSION(5, 0, 2):/* NAVI14 */
> > + case IP_VERSION(5, 0, 5):/* NAVI12 */
> > + case IP_VERSION(5, 2, 0):/* SIENNA_CICHLID */
> > + case IP_VERSION(5, 2, 2):/* NAVY_FLOUDER */
> > + case IP_VERSION(5, 2, 4):/* DIMGREY_CAVEFISH */
> > + kfd->device_info.num_sdma_queues_per_engine =
> > 8;
> > + break;
> > + default:
> > + dev_warn(kfd_device,
> > + "Default sdma queue per engine(8) is
> > + set due
> > to "
> > + "mismatch of sdma ip
> > block(SDMA_HWIP:0x%x).\n",
> > + sdma_version);
> > + kfd->device_info.num_sdma_queues_per_engine =
> > 8;
> > + }
> > +}
> > +
> > +static void kfd_device_info_set_event_interrupt_class(struct
> > +kfd_dev
> > +*kfd) {
> > + uint32_t gc_version = KFD_GC_VERSION(kfd);
> > +
> > + switch (gc_version) {
> > + case IP_VERSION(9, 0, 1): /* VEGA10 */
>
> Missing check for case IP_VERSION(9, 1, 0): /* RAVEN */
>
> > + case IP_VERSION(9, 2, 1): /* VEGA12 */
>
> Missing check for case IP_VERSION(9, 2, 2): /* RAVEN */
>
> Thanks,
>
> Jon
>
> > + case IP_VERSION(9, 3, 0): /* RENOIR */
> > + case IP_VERSION(9, 4, 0): /* VEGA20 */
> > + case IP_VERSION(9, 4, 1): /* ARCTURUS */
> > + case IP_VERSION(9, 4, 2): /* ALDEBARAN */
> > + case IP_VERSION(10, 3, 1): /* VANGOGH */
> > + case IP_VERSION(10, 3, 3): /* YELLOW_CARP */
> > + case IP_VERSION(10, 1, 3): /* CYAN_SKILLFISH */
> > + case IP_VERSION(10, 1, 10): /* NAVI10 */
> > + case IP_VERSION(10, 1, 2): /* NAVI12 */
> > + case IP_VERSION(10, 1, 1): /* NAVI14 */
> > + case IP_VERSION(10, 3, 0): /* SIENNA_CICHLID */
> > + case IP_VERSION(10, 3, 2): /* NAVY_FLOUNDER */
> > + case IP_VERSION(10, 3, 4): /* DIMGREY_CAVEFISH */
> > + case IP_VERSION(10, 3, 5): /* BEIGE_GOBY */
> > + kfd->device_info.event_interrupt_class =
> > &event_interrupt_class_v9;
> > + break;
> > + default:
> > + dev_warn(kfd_device, "v9 event interrupt handler is
> > + set due
> > to "
> > + "mismatch of gc ip block(GC_HWIP:0x%x).\n",
> > gc_version);
> > + kfd->device_info.event_interrupt_class =
> > &event_interrupt_class_v9;
> > + }
> > +}
> > +
> > static void kfd_device_info_init(struct kfd_dev *kfd,
> > bool vf, uint32_t gfx_target_version) {
> > uint32_t gc_version = KFD_GC_VERSION(kfd);
> > - uint32_t sdma_version = kfd->adev-
> > >ip_versions[SDMA0_HWIP][0];
> > uint32_t asic_type = kfd->adev->asic_type;
> >
> > kfd->device_info.max_pasid_bits = 16; @@ -75,16 +139,11 @@
> > static void kfd_device_info_init(struct kfd_dev *kfd,
> > if (KFD_IS_SOC15(kfd)) {
> > kfd->device_info.doorbell_size = 8;
> > kfd->device_info.ih_ring_entry_size = 8 * sizeof(uint32_t);
> > - kfd->device_info.event_interrupt_class =
> > &event_interrupt_class_v9;
> > kfd->device_info.supports_cwsr = true;
> >
> > - if ((sdma_version >= IP_VERSION(4, 0, 0) &&
> > - sdma_version <= IP_VERSION(4, 2, 0)) ||
> > - sdma_version == IP_VERSION(5, 2, 1) ||
> > - sdma_version == IP_VERSION(5, 2, 3))
> > - kfd->device_info.num_sdma_queues_per_engine =
> > 2;
> > - else
> > - kfd->device_info.num_sdma_queues_per_engine =
> > 8;
> > + kfd_device_info_set_sdma_queue_num(kfd);
> > +
> > + kfd_device_info_set_event_interrupt_class(kfd);
> >
> > /* Raven */
> > if (gc_version == IP_VERSION(9, 1, 0) ||
> > --
> > 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)
2021-12-20 6:19 ` Sider, Graham
2021-12-20 6:26 ` Chen, Guchun
@ 2021-12-20 12:51 ` Kim, Jonathan
1 sibling, 0 replies; 6+ messages in thread
From: Kim, Jonathan @ 2021-12-20 12:51 UTC (permalink / raw)
To: Sider, Graham, Chen, Guchun, amd-gfx, Deucher, Alexander,
Kuehling, Felix
> -----Original Message-----
> From: Sider, Graham <Graham.Sider@amd.com>
> Sent: December 20, 2021 1:19 AM
> To: Kim, Jonathan <Jonathan.Kim@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Deucher,
> Alexander <Alexander.Deucher@amd.com>; Kuehling, Felix
> <Felix.Kuehling@amd.com>
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> device init (v2)
>
> [Public]
>
> > -----Original Message-----
> > From: Kim, Jonathan <Jonathan.Kim@amd.com>
> > Sent: Monday, December 20, 2021 12:44 AM
> > To: Chen, Guchun <Guchun.Chen@amd.com>; amd-
> > gfx@lists.freedesktop.org; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Sider, Graham
> <Graham.Sider@amd.com>;
> > Kuehling, Felix <Felix.Kuehling@amd.com>
> > Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> > device init (v2)
> >
> > [AMD Official Use Only]
> >
> > > -----Original Message-----
> > > From: Chen, Guchun <Guchun.Chen@amd.com>
> > > Sent: December 19, 2021 10:09 PM
> > > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > > <Alexander.Deucher@amd.com>; Sider, Graham
> > <Graham.Sider@amd.com>;
> > > Kuehling, Felix <Felix.Kuehling@amd.com>; Kim, Jonathan
> > > <Jonathan.Kim@amd.com>
> > > Cc: Chen, Guchun <Guchun.Chen@amd.com>
> > > Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> device
> > > init (v2)
> > >
> > > sdma queue number is not correct like on vega20, this patch promises
> > > the
> >
> > I think you've also fixed Vega12 and Raven (they were being set to 8
> > before rather than 2). No need to mention this in your description,
> > just double checking.
> >
>
> I believe it was only Vega20 that was being set incorrectly. The condition
> was:
>
> sdma_version >= IP_VERSION(4, 0, 0) &&
> sdma_version <= IP_VERSION(4, 2, 0))
>
> which encapsulates Vega12 and Raven setting sdma_queues_per_engine to
> 2, but also accidently encapsulates Vega20.
Ah right. It was a range check before.
Thanks,
Jon
>
> > > setting keeps the same after code refactor.
> > > Additionally, improve code to use switch case to list IP version to
> > > complete kfd device_info structure filling.
> > > This keeps consistency with the IP parse code in amdgpu_discovery.c.
> > >
> > > v2: use dev_warn for the default switch case;
> > > set default sdma queue per engine(8) and IH handler to v9.
> > > (Jonathan)
> > >
> > > Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> > > Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> >
> > Other than the missing checks for Raven when setting the interrupt
> > class (see inline comments and reference kgd2kfd_probe in
> > kfd_device.c) and one nit-pick inline, this looks good to me.
> >
> > With those fixed, this patch is
> > Reviewed-by: Jonathan Kim <jonathan.kim@amd.com>
> >
>
> Other than Jon's comments, this patch is also
>
> Reviewed-by: Graham Sider <Graham.Sider@amd.com>
>
> > > ---
> > > drivers/gpu/drm/amd/amdkfd/kfd_device.c | 77
> > > ++++++++++++++++++++++---
> > > 1 file changed, 68 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > > b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > > index facc28f58c1f..36406a261203 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > > @@ -59,11 +59,75 @@ static void kfd_gtt_sa_fini(struct kfd_dev
> > > *kfd);
> > >
> > > static int kfd_resume(struct kfd_dev *kfd);
> > >
> > > +static void kfd_device_info_set_sdma_queue_num(struct kfd_dev
> *kfd)
> > {
> > > + uint32_t sdma_version = kfd->adev-
> > > >ip_versions[SDMA0_HWIP][0];
> > > +
> > > + switch (sdma_version) {
> >
> > Please pull in the indentation for all cases to line up with the switch block.
> >
> > > + case IP_VERSION(4, 0, 0):/* VEGA10 */
> > > + case IP_VERSION(4, 0, 1):/* VEGA12 */
> > > + case IP_VERSION(4, 1, 0):/* RAVEN */
> > > + case IP_VERSION(4, 1, 1):/* RAVEN */
> >
> > As mentioned, you've also fixed Vega12 & Raven here I presume since
> > afaik, they're based off Vega10?
> >
> > > + case IP_VERSION(4, 1, 2):/* RENIOR */
> > > + case IP_VERSION(5, 2, 1):/* VANGOGH */
> > > + case IP_VERSION(5, 2, 3):/* YELLOW_CARP */
> > > + kfd->device_info.num_sdma_queues_per_engine =
> > > 2;
> > > + break;
> > > + case IP_VERSION(4, 2, 0):/* VEGA20 */
> > > + case IP_VERSION(4, 2, 2):/* ARCTUTUS */
> > > + case IP_VERSION(4, 4, 0):/* ALDEBARAN */
> > > + case IP_VERSION(5, 0, 0):/* NAVI10 */
> > > + case IP_VERSION(5, 0, 1):/* CYAN_SKILLFISH */
> > > + case IP_VERSION(5, 0, 2):/* NAVI14 */
> > > + case IP_VERSION(5, 0, 5):/* NAVI12 */
> > > + case IP_VERSION(5, 2, 0):/* SIENNA_CICHLID */
> > > + case IP_VERSION(5, 2, 2):/* NAVY_FLOUDER */
> > > + case IP_VERSION(5, 2, 4):/* DIMGREY_CAVEFISH */
> > > + kfd->device_info.num_sdma_queues_per_engine =
> > > 8;
> > > + break;
> > > + default:
> > > + dev_warn(kfd_device,
> > > + "Default sdma queue per engine(8) is
> > > + set due
> > > to "
> > > + "mismatch of sdma ip
> > > block(SDMA_HWIP:0x%x).\n",
> > > + sdma_version);
> > > + kfd->device_info.num_sdma_queues_per_engine =
> > > 8;
> > > + }
> > > +}
> > > +
> > > +static void kfd_device_info_set_event_interrupt_class(struct
> > > +kfd_dev
> > > +*kfd) {
> > > + uint32_t gc_version = KFD_GC_VERSION(kfd);
> > > +
> > > + switch (gc_version) {
> > > + case IP_VERSION(9, 0, 1): /* VEGA10 */
> >
> > Missing check for case IP_VERSION(9, 1, 0): /* RAVEN */
> >
> > > + case IP_VERSION(9, 2, 1): /* VEGA12 */
> >
> > Missing check for case IP_VERSION(9, 2, 2): /* RAVEN */
> >
> > Thanks,
> >
> > Jon
> >
> > > + case IP_VERSION(9, 3, 0): /* RENOIR */
> > > + case IP_VERSION(9, 4, 0): /* VEGA20 */
> > > + case IP_VERSION(9, 4, 1): /* ARCTURUS */
> > > + case IP_VERSION(9, 4, 2): /* ALDEBARAN */
> > > + case IP_VERSION(10, 3, 1): /* VANGOGH */
> > > + case IP_VERSION(10, 3, 3): /* YELLOW_CARP */
> > > + case IP_VERSION(10, 1, 3): /* CYAN_SKILLFISH */
> > > + case IP_VERSION(10, 1, 10): /* NAVI10 */
> > > + case IP_VERSION(10, 1, 2): /* NAVI12 */
> > > + case IP_VERSION(10, 1, 1): /* NAVI14 */
> > > + case IP_VERSION(10, 3, 0): /* SIENNA_CICHLID */
> > > + case IP_VERSION(10, 3, 2): /* NAVY_FLOUNDER */
> > > + case IP_VERSION(10, 3, 4): /* DIMGREY_CAVEFISH */
> > > + case IP_VERSION(10, 3, 5): /* BEIGE_GOBY */
> > > + kfd->device_info.event_interrupt_class =
> > > &event_interrupt_class_v9;
> > > + break;
> > > + default:
> > > + dev_warn(kfd_device, "v9 event interrupt handler is
> > > + set due
> > > to "
> > > + "mismatch of gc ip block(GC_HWIP:0x%x).\n",
> > > gc_version);
> > > + kfd->device_info.event_interrupt_class =
> > > &event_interrupt_class_v9;
> > > + }
> > > +}
> > > +
> > > static void kfd_device_info_init(struct kfd_dev *kfd,
> > > bool vf, uint32_t gfx_target_version) {
> > > uint32_t gc_version = KFD_GC_VERSION(kfd);
> > > - uint32_t sdma_version = kfd->adev-
> > > >ip_versions[SDMA0_HWIP][0];
> > > uint32_t asic_type = kfd->adev->asic_type;
> > >
> > > kfd->device_info.max_pasid_bits = 16; @@ -75,16 +139,11 @@
> > > static void kfd_device_info_init(struct kfd_dev *kfd,
> > > if (KFD_IS_SOC15(kfd)) {
> > > kfd->device_info.doorbell_size = 8;
> > > kfd->device_info.ih_ring_entry_size = 8 * sizeof(uint32_t);
> > > - kfd->device_info.event_interrupt_class =
> > > &event_interrupt_class_v9;
> > > kfd->device_info.supports_cwsr = true;
> > >
> > > - if ((sdma_version >= IP_VERSION(4, 0, 0) &&
> > > - sdma_version <= IP_VERSION(4, 2, 0)) ||
> > > - sdma_version == IP_VERSION(5, 2, 1) ||
> > > - sdma_version == IP_VERSION(5, 2, 3))
> > > - kfd->device_info.num_sdma_queues_per_engine =
> > > 2;
> > > - else
> > > - kfd->device_info.num_sdma_queues_per_engine =
> > > 8;
> > > + kfd_device_info_set_sdma_queue_num(kfd);
> > > +
> > > + kfd_device_info_set_event_interrupt_class(kfd);
> > >
> > > /* Raven */
> > > if (gc_version == IP_VERSION(9, 1, 0) ||
> > > --
> > > 2.17.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-20 12:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-20 3:08 [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2) Guchun Chen
2021-12-20 5:43 ` Kim, Jonathan
2021-12-20 6:19 ` Sider, Graham
2021-12-20 6:26 ` Chen, Guchun
2021-12-20 12:51 ` Kim, Jonathan
2021-12-20 6:23 ` Chen, Guchun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.