RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)

2021-12-20 Thread Kim, Jonathan



> -Original Message-
> From: Sider, Graham 
> Sent: December 20, 2021 1:19 AM
> To: Kim, Jonathan ; Chen, Guchun
> ; amd-gfx@lists.freedesktop.org; Deucher,
> Alexander ; Kuehling, Felix
> 
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> device init (v2)
> 
> [Public]
> 
> > -Original Message-
> > From: Kim, Jonathan 
> > Sent: Monday, December 20, 2021 12:44 AM
> > To: Chen, Guchun ; amd-
> > g...@lists.freedesktop.org; Deucher, Alexander
> > ; Sider, Graham
> ;
> > Kuehling, Felix 
> > Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> > device init (v2)
> >
> > [AMD Official Use Only]
> >
> > > -Original Message-
> > > From: Chen, Guchun 
> > > Sent: December 19, 2021 10:09 PM
> > > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > > ; Sider, Graham
> > ;
> > > Kuehling, Felix ; Kim, Jonathan
> > > 
> > > Cc: Chen, Guchun 
> > > 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 
> >
> > 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 
> >
> 
> Other than Jon's comments, this patch is also
> 
> Reviewed-by: Graham Sider 
> 
> > > ---
> > >  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):/* AR

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)

2021-12-19 Thread Chen, Guchun
[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  
Sent: Monday, December 20, 2021 2:19 PM
To: Kim, Jonathan ; Chen, Guchun ; 
amd-gfx@lists.freedesktop.org; Deucher, Alexander ; 
Kuehling, Felix 
Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init 
(v2)

[Public]

> -Original Message-
> From: Kim, Jonathan 
> Sent: Monday, December 20, 2021 12:44 AM
> To: Chen, Guchun ; amd- 
> g...@lists.freedesktop.org; Deucher, Alexander 
> ; Sider, Graham ; 
> Kuehling, Felix 
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd 
> device init (v2)
> 
> [AMD Official Use Only]
> 
> > -Original Message-
> > From: Chen, Guchun 
> > Sent: December 19, 2021 10:09 PM
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
> > ; Sider, Graham
> ;
> > Kuehling, Felix ; Kim, Jonathan 
> > 
> > Cc: Chen, Guchun 
> > 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 
> 
> 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 
> 

Other than Jon's comments, this patch is also

Reviewed-by: Graham Sider  

> > ---
> >  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 */
> > +  

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)

2021-12-19 Thread Chen, Guchun
[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  
Sent: Monday, December 20, 2021 1:44 PM
To: Chen, Guchun ; amd-gfx@lists.freedesktop.org; Deucher, 
Alexander ; Sider, Graham ; 
Kuehling, Felix 
Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init 
(v2)

[AMD Official Use Only]

> -Original Message-
> From: Chen, Guchun 
> Sent: December 19, 2021 10:09 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
> ; Sider, Graham ; 
> Kuehling, Felix ; Kim, Jonathan 
> 
> Cc: Chen, Guchun 
> 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 

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 

> ---
>  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 */

> +   

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)

2021-12-19 Thread Sider, Graham
[Public]

> -Original Message-
> From: Kim, Jonathan 
> Sent: Monday, December 20, 2021 12:44 AM
> To: Chen, Guchun ; amd-
> g...@lists.freedesktop.org; Deucher, Alexander
> ; Sider, Graham
> ; Kuehling, Felix 
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> init (v2)
> 
> [AMD Official Use Only]
> 
> > -Original Message-
> > From: Chen, Guchun 
> > Sent: December 19, 2021 10:09 PM
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > ; Sider, Graham
> ;
> > Kuehling, Felix ; Kim, Jonathan
> > 
> > Cc: Chen, Guchun 
> > 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 
> 
> 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 
> 

Other than Jon's comments, this patch is also

Reviewed-by: Graham Sider  

> > ---
> >  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 o

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init (v2)

2021-12-19 Thread Kim, Jonathan
[AMD Official Use Only]

> -Original Message-
> From: Chen, Guchun 
> Sent: December 19, 2021 10:09 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Sider, Graham
> ; Kuehling, Felix ;
> Kim, Jonathan 
> Cc: Chen, Guchun 
> 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 

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 

> ---
>  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 

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init

2021-12-17 Thread Chen, Guchun
[Public]

Hi Graham,

My general thought is, from what I observed, IP version does not change in a 
linear variation manner, so moving to switch case may be easier for user to 
decode this. Also, I want to get the code aligned with the IP parse code in 
amdgpu_discovery.c.

Please correct me if I am wrong.

Regards,
Guchun

-Original Message-
From: Kim, Jonathan  
Sent: Friday, December 17, 2021 11:35 PM
To: Sider, Graham ; Chen, Guchun ; 
amd-gfx@lists.freedesktop.org; Deucher, Alexander ; 
Kuehling, Felix 
Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init



> -Original Message-
> From: Sider, Graham 
> Sent: December 17, 2021 10:06 AM
> To: Chen, Guchun ; amd- 
> g...@lists.freedesktop.org; Deucher, Alexander 
> ; Kuehling, Felix ; 
> Kim, Jonathan 
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd 
> device init
> 
> [Public]
> 
> > -Original Message-
> > From: Chen, Guchun 
> > Sent: Friday, December 17, 2021 9:31 AM
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
> > ; Sider, Graham
> ;
> > Kuehling, Felix ; Kim, Jonathan 
> > 
> > Cc: Chen, Guchun 
> > Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device 
> > init
> >
> > 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.
> >
> > Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> > Signed-off-by: Guchun Chen 
> > ---
> >  drivers/gpu/drm/amd/amdkfd/kfd_device.c | 74
> > ++---
> >  1 file changed, 65 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..e50bf992f298 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -59,11 +59,72 @@ 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 */
> 
> Thanks for spotting this Guchun. My previous patch should have used a "<"
> instead of a "<=" on IP_VERSION(4, 2, 0).
> 
> > +   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_err(kfd_device,
> > +   "Failed to find sdma ip
> > blocks(SDMA_HWIP:0x%x) in %s\n",
> > +sdma_version, __func__);
> > +   }
> > +}
> > +
> > +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, 

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init

2021-12-17 Thread Kim, Jonathan



> -Original Message-
> From: Sider, Graham 
> Sent: December 17, 2021 10:06 AM
> To: Chen, Guchun ; amd-
> g...@lists.freedesktop.org; Deucher, Alexander
> ; Kuehling, Felix
> ; Kim, Jonathan 
> Subject: RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd
> device init
> 
> [Public]
> 
> > -Original Message-
> > From: Chen, Guchun 
> > Sent: Friday, December 17, 2021 9:31 AM
> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> > ; Sider, Graham
> ;
> > Kuehling, Felix ; Kim, Jonathan
> > 
> > Cc: Chen, Guchun 
> > Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> > init
> >
> > 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.
> >
> > Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> > Signed-off-by: Guchun Chen 
> > ---
> >  drivers/gpu/drm/amd/amdkfd/kfd_device.c | 74
> > ++---
> >  1 file changed, 65 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..e50bf992f298 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> > @@ -59,11 +59,72 @@ 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 */
> 
> Thanks for spotting this Guchun. My previous patch should have used a "<"
> instead of a "<=" on IP_VERSION(4, 2, 0).
> 
> > +   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_err(kfd_device,
> > +   "Failed to find sdma ip
> > blocks(SDMA_HWIP:0x%x) in %s\n",
> > +sdma_version, __func__);
> > +   }
> > +}
> > +
> > +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_inf

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init

2021-12-17 Thread Kim, Jonathan
[AMD Official Use Only]

Are safeguards required for KFD interrupt initialization to fail gracefully in 
the event of a non-assignment?
Same would apply when KGD forwards interrupts to the KFD (although the KFD 
device reference might not exist at this point if the above comment is handled 
well so a check may not apply in this case).

Also should the dev_errs mention what it's failing to do rather than just 
reporting that it could not find the HW IP block?
In the case of non-assignment of sdma queues per engine, it still seems like 
the KFD could move forward but the user wouldn't know what the context of the 
dev_err was.

Thanks,

Jon

> -Original Message-
> From: Chen, Guchun 
> Sent: December 17, 2021 9:31 AM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Sider, Graham
> ; Kuehling, Felix ;
> Kim, Jonathan 
> Cc: Chen, Guchun 
> Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device
> init
>
> 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.
>
> Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> Signed-off-by: Guchun Chen 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c | 74
> ++---
>  1 file changed, 65 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..e50bf992f298 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -59,11 +59,72 @@ 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_err(kfd_device,
> + "Failed to find sdma ip
> blocks(SDMA_HWIP:0x%x) in %s\n",
> +sdma_version, __func__);
> + }
> +}
> +
> +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_err(kfd_device, "Failed to find gc ip
> blocks(GC_HWIP:0x%x) in %s\n",
> + gc_version, __func__);
> + }
> +}
> +
>  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 

RE: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init

2021-12-17 Thread Sider, Graham
[Public]

> -Original Message-
> From: Chen, Guchun 
> Sent: Friday, December 17, 2021 9:31 AM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Sider, Graham
> ; Kuehling, Felix ;
> Kim, Jonathan 
> Cc: Chen, Guchun 
> Subject: [PATCH] drm/amdkfd: correct sdma queue number in kfd device init
> 
> 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.
> 
> Fixes: a9e2c4dc6cc4("drm/amdkfd: add kfd_device_info_init function")
> Signed-off-by: Guchun Chen 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c | 74
> ++---
>  1 file changed, 65 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..e50bf992f298 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -59,11 +59,72 @@ 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 */

Thanks for spotting this Guchun. My previous patch should have used a "<" 
instead of a "<=" on IP_VERSION(4, 2, 0).

> + 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_err(kfd_device,
> + "Failed to find sdma ip
> blocks(SDMA_HWIP:0x%x) in %s\n",
> +sdma_version, __func__);
> + }
> +}
> +
> +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_err(kfd_device, "Failed to find gc ip
> blocks(GC_HWIP:0x%x) in %s\n",
> + gc_version, __func__);
> + }
> +}

I understand the appeal of moving to a switch for the SDMA queue num above 
since its setting isn't very linear w.r.t. the SDMA versioning. That said I 
don't know if I understand moving to a switch for the event interrupt class 
here. To clarify, originally when I set all SOC15 to event_interrupt_class_v9 
it was to follow what was in the device_info structs in drm-staging-next at 
that time--that said if the plan is in a following patch to change this such 
that gfx10 are set to to event_interrupt_class_v10, what's the reasoning not to 
format it along the lines of:

if (gc_version >= IP_VERSION(10, 1, 1)
kfd->device_info.event_interrupt_class = &event_interrupt_class_v10;
else
kfd->device_info.event_interrupt_class = &event_interrupt_class_v9;

(Assuming this is done