[PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

Russell, Kent Kent.Russell at amd.com
Wed Nov 13 18:36:07 UTC 2019


Thanks Alex, I appreciate the explanation!

Kent

From: Deucher, Alexander <Alexander.Deucher at amd.com>
Sent: Wednesday, November 13, 2019 1:31 PM
To: Russell, Kent <Kent.Russell at amd.com>; Zhao, Yong <Yong.Zhao at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

CI just refers to the dGPUs (bonaire and hawaii), the CIK refers to the whole family (CI dGPUs, Indus (kaveri platform), and Kerala (kabini/mullins platform).

Alex
________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org<mailto:amd-gfx-bounces at lists.freedesktop.org>> on behalf of Russell, Kent <Kent.Russell at amd.com<mailto:Kent.Russell at amd.com>>
Sent: Wednesday, November 13, 2019 8:28 AM
To: Zhao, Yong <Yong.Zhao at amd.com<mailto:Yong.Zhao at amd.com>>; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> <amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>>
Cc: Zhao, Yong <Yong.Zhao at amd.com<mailto:Yong.Zhao at amd.com>>
Subject: RE: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

Should we use "CI" instead of "CIK" in the comments? I thought CIK was a short form for CI kickers, while CI encompasses all CI ASICs. Even though we had _cik as the suffix for most of the CI stuff. Just wondering about accuracy.

 Kent

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org<mailto:amd-gfx-bounces at lists.freedesktop.org>> On Behalf Of Yong Zhao
Sent: Tuesday, November 12, 2019 5:19 PM
To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
Cc: Zhao, Yong <Yong.Zhao at amd.com<mailto:Yong.Zhao at amd.com>>
Subject: [PATCH 2/2] drm/amdkfd: Eliminate ops_asic_specific in kernel queue

The ops_asic_specific function pointers are actually quite generic after using a simple if condition. Eliminate it by code refactoring.

Change-Id: Icb891289cca31acdbe2d2eea76a426f1738b9c08
Signed-off-by: Yong Zhao <Yong.Zhao at amd.com<mailto:Yong.Zhao at amd.com>>
---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 63 ++++++++-----------  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |  4 --  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 36 -----------  .../gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c  | 48 --------------
 4 files changed, 26 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index a750b1d110eb..59ee9053498c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -87,9 +87,17 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
         kq->pq_kernel_addr = kq->pq->cpu_ptr;
         kq->pq_gpu_addr = kq->pq->gpu_addr;

-       retval = kq->ops_asic_specific.initialize(kq, dev, type, queue_size);
-       if (!retval)
-               goto err_eop_allocate_vidmem;
+       /* For CIK family asics, kq->eop_mem is not needed */
+       if (dev->device_info->asic_family > CHIP_HAWAII) {
+               retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem);
+               if (retval != 0)
+                       goto err_eop_allocate_vidmem;
+
+               kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
+               kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
+
+               memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+       }

         retval = kfd_gtt_sa_allocate(dev, sizeof(*kq->rptr_kernel),
                                         &kq->rptr_mem);
@@ -200,7 +208,12 @@ static void uninitialize(struct kernel_queue *kq)

         kfd_gtt_sa_free(kq->dev, kq->rptr_mem);
         kfd_gtt_sa_free(kq->dev, kq->wptr_mem);
-       kq->ops_asic_specific.uninitialize(kq);
+
+       /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
+        * is able to handle NULL properly.
+        */
+       kfd_gtt_sa_free(kq->dev, kq->eop_mem);
+
         kfd_gtt_sa_free(kq->dev, kq->pq);
         kfd_release_kernel_doorbell(kq->dev,
                                         kq->queue->properties.doorbell_ptr);
@@ -280,8 +293,15 @@ static void submit_packet(struct kernel_queue *kq)
         }
         pr_debug("\n");
 #endif
-
-       kq->ops_asic_specific.submit_packet(kq);
+       if (kq->dev->device_info->doorbell_size == 8) {
+               *kq->wptr64_kernel = kq->pending_wptr64;
+               write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
+                                       kq->pending_wptr64);
+       } else {
+               *kq->wptr_kernel = kq->pending_wptr;
+               write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
+                                       kq->pending_wptr);
+       }
 }

 static void rollback_packet(struct kernel_queue *kq) @@ -310,42 +330,11 @@ struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,
         kq->ops.submit_packet = submit_packet;
         kq->ops.rollback_packet = rollback_packet;

-       switch (dev->device_info->asic_family) {
-       case CHIP_KAVERI:
-       case CHIP_HAWAII:
-       case CHIP_CARRIZO:
-       case CHIP_TONGA:
-       case CHIP_FIJI:
-       case CHIP_POLARIS10:
-       case CHIP_POLARIS11:
-       case CHIP_POLARIS12:
-       case CHIP_VEGAM:
-               kernel_queue_init_vi(&kq->ops_asic_specific);
-               break;
-
-       case CHIP_VEGA10:
-       case CHIP_VEGA12:
-       case CHIP_VEGA20:
-       case CHIP_RAVEN:
-       case CHIP_RENOIR:
-       case CHIP_ARCTURUS:
-       case CHIP_NAVI10:
-       case CHIP_NAVI12:
-       case CHIP_NAVI14:
-               kernel_queue_init_v9(&kq->ops_asic_specific);
-               break;
-       default:
-               WARN(1, "Unexpected ASIC family %u",
-                    dev->device_info->asic_family);
-               goto out_free;
-       }
-
         if (kq->ops.initialize(kq, dev, type, KFD_KERNEL_QUEUE_SIZE))
                 return kq;

         pr_err("Failed to init kernel queue\n");

-out_free:
         kfree(kq);
         return NULL;
 }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
index a9a35897d8b7..475e9499c0af 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
@@ -66,7 +66,6 @@ struct kernel_queue_ops {

 struct kernel_queue {
         struct kernel_queue_ops ops;
-       struct kernel_queue_ops ops_asic_specific;

         /* data */
         struct kfd_dev          *dev;
@@ -99,7 +98,4 @@ struct kernel_queue {
         struct list_head        list;
 };

-void kernel_queue_init_vi(struct kernel_queue_ops *ops); -void kernel_queue_init_v9(struct kernel_queue_ops *ops);
-
 #endif /* KFD_KERNEL_QUEUE_H_ */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
index 9e0eaf446bab..2de01009f1b6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
@@ -27,42 +27,6 @@
 #include "kfd_pm4_opcodes.h"
 #include "gc/gc_10_1_0_sh_mask.h"

-static bool initialize_v9(struct kernel_queue *kq, struct kfd_dev *dev,
-                       enum kfd_queue_type type, unsigned int queue_size)
-{
-       int retval;
-
-       retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem);
-       if (retval)
-               return false;
-
-       kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
-       kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
-
-       memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
-
-       return true;
-}
-
-static void uninitialize_v9(struct kernel_queue *kq) -{
-       kfd_gtt_sa_free(kq->dev, kq->eop_mem);
-}
-
-static void submit_packet_v9(struct kernel_queue *kq) -{
-       *kq->wptr64_kernel = kq->pending_wptr64;
-       write_kernel_doorbell64(kq->queue->properties.doorbell_ptr,
-                               kq->pending_wptr64);
-}
-
-void kernel_queue_init_v9(struct kernel_queue_ops *ops) -{
-       ops->initialize = initialize_v9;
-       ops->uninitialize = uninitialize_v9;
-       ops->submit_packet = submit_packet_v9;
-}
-
 static int pm_map_process_v9(struct packet_manager *pm,
                 uint32_t *buffer, struct qcm_process_device *qpd)  { diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
index 64f13f34d819..bed4d0ccb6b1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
@@ -26,54 +26,6 @@
 #include "kfd_pm4_headers_vi.h"
 #include "kfd_pm4_opcodes.h"

-static bool initialize_vi(struct kernel_queue *kq, struct kfd_dev *dev,
-                       enum kfd_queue_type type, unsigned int queue_size);
-static void uninitialize_vi(struct kernel_queue *kq); -static void submit_packet_vi(struct kernel_queue *kq);
-
-void kernel_queue_init_vi(struct kernel_queue_ops *ops) -{
-       ops->initialize = initialize_vi;
-       ops->uninitialize = uninitialize_vi;
-       ops->submit_packet = submit_packet_vi;
-}
-
-static bool initialize_vi(struct kernel_queue *kq, struct kfd_dev *dev,
-                       enum kfd_queue_type type, unsigned int queue_size)
-{
-       int retval;
-
-       /*For CIK family asics, kq->eop_mem is not needed */
-       if (dev->device_info->asic_family <= CHIP_HAWAII)
-               return true;
-
-       retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem);
-       if (retval != 0)
-               return false;
-
-       kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
-       kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
-
-       memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
-
-       return true;
-}
-
-static void uninitialize_vi(struct kernel_queue *kq) -{
-       /* For CIK family asics, kq->eop_mem is Null, kfd_gtt_sa_free()
-        * is able to handle NULL properly.
-        */
-       kfd_gtt_sa_free(kq->dev, kq->eop_mem);
-}
-
-static void submit_packet_vi(struct kernel_queue *kq) -{
-       *kq->wptr_kernel = kq->pending_wptr;
-       write_kernel_doorbell(kq->queue->properties.doorbell_ptr,
-                               kq->pending_wptr);
-}
-
 unsigned int pm_build_pm4_header(unsigned int opcode, size_t packet_size)  {
         union PM4_MES_TYPE_3_HEADER header;
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20191113/89bc0f64/attachment-0001.html>


More information about the amd-gfx mailing list