This is a nice cleanup.

With this change, kfd2kgd_calls.get_fw_version is no longer used. You 
should remove it from kgd_kfd_interface.h. Also move the enum 
kgd_engine_type to amdgpu_amdkfd.h at the same time.

With that fixed, this patch is Reviewed-by: Felix Kuehling 
<felix.kuehl...@amd.com>

On 2019-04-12 4:10 p.m., Lin, Amber wrote:
> Method of getting firmware version is the same across ASICs, so remove
> them from ASIC-specific files and create one in amdgpu_amdkfd.c. This new
> created get_fw_version simply reads fw_version from adev->gfx than parsing
> the ucode header.
>
> Signed-off-by: Amber Lin <amber....@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c        | 37 ++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h        |  2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 61 
> -----------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 61 
> -----------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 54 --------------------
>   drivers/gpu/drm/amd/amdkfd/kfd_device.c           |  4 +-
>   6 files changed, 41 insertions(+), 178 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index acf8ae0..aeead07 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -335,6 +335,43 @@ void amdgpu_amdkfd_free_gtt_mem(struct kgd_dev *kgd, 
> void *mem_obj)
>       amdgpu_bo_unref(&(bo));
>   }
>   
> +uint32_t amdgpu_amdkfd_get_fw_version(struct kgd_dev *kgd,
> +                                   enum kgd_engine_type type)
> +{
> +     struct amdgpu_device *adev = (struct amdgpu_device *)kgd;
> +
> +     switch (type) {
> +     case KGD_ENGINE_PFP:
> +             return adev->gfx.pfp_fw_version;
> +
> +     case KGD_ENGINE_ME:
> +             return adev->gfx.me_fw_version;
> +
> +     case KGD_ENGINE_CE:
> +             return adev->gfx.ce_fw_version;
> +
> +     case KGD_ENGINE_MEC1:
> +             return adev->gfx.mec_fw_version;
> +
> +     case KGD_ENGINE_MEC2:
> +             return adev->gfx.mec2_fw_version;
> +
> +     case KGD_ENGINE_RLC:
> +             return adev->gfx.rlc_fw_version;
> +
> +     case KGD_ENGINE_SDMA1:
> +             return adev->sdma.instance[0].fw_version;
> +
> +     case KGD_ENGINE_SDMA2:
> +             return adev->sdma.instance[1].fw_version;
> +
> +     default:
> +             return 0;
> +     }
> +
> +     return 0;
> +}
> +
>   void amdgpu_amdkfd_get_local_mem_info(struct kgd_dev *kgd,
>                                     struct kfd_local_mem_info *mem_info)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index e6a5037..5c8397f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -141,6 +141,8 @@ int amdgpu_amdkfd_alloc_gtt_mem(struct kgd_dev *kgd, 
> size_t size,
>                               void **mem_obj, uint64_t *gpu_addr,
>                               void **cpu_ptr, bool mqd_gfx9);
>   void amdgpu_amdkfd_free_gtt_mem(struct kgd_dev *kgd, void *mem_obj);
> +uint32_t amdgpu_amdkfd_get_fw_version(struct kgd_dev *kgd,
> +                                   enum kgd_engine_type type);
>   void amdgpu_amdkfd_get_local_mem_info(struct kgd_dev *kgd,
>                                     struct kfd_local_mem_info *mem_info);
>   uint64_t amdgpu_amdkfd_get_gpu_clock_counter(struct kgd_dev *kgd);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> index ff7fac7..fa09e11 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> @@ -22,14 +22,12 @@
>   
>   #include <linux/fdtable.h>
>   #include <linux/uaccess.h>
> -#include <linux/firmware.h>
>   #include <linux/mmu_context.h>
>   #include <drm/drmP.h>
>   #include "amdgpu.h"
>   #include "amdgpu_amdkfd.h"
>   #include "cikd.h"
>   #include "cik_sdma.h"
> -#include "amdgpu_ucode.h"
>   #include "gfx_v7_0.h"
>   #include "gca/gfx_7_2_d.h"
>   #include "gca/gfx_7_2_enum.h"
> @@ -139,7 +137,6 @@ static bool get_atc_vmid_pasid_mapping_valid(struct 
> kgd_dev *kgd, uint8_t vmid);
>   static uint16_t get_atc_vmid_pasid_mapping_pasid(struct kgd_dev *kgd,
>                                                       uint8_t vmid);
>   
> -static uint16_t get_fw_version(struct kgd_dev *kgd, enum kgd_engine_type 
> type);
>   static void set_scratch_backing_va(struct kgd_dev *kgd,
>                                       uint64_t va, uint32_t vmid);
>   static void set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t 
> vmid,
> @@ -191,7 +188,6 @@ static const struct kfd2kgd_calls kfd2kgd = {
>       .address_watch_get_offset = kgd_address_watch_get_offset,
>       .get_atc_vmid_pasid_mapping_pasid = get_atc_vmid_pasid_mapping_pasid,
>       .get_atc_vmid_pasid_mapping_valid = get_atc_vmid_pasid_mapping_valid,
> -     .get_fw_version = get_fw_version,
>       .set_scratch_backing_va = set_scratch_backing_va,
>       .get_tile_config = get_tile_config,
>       .set_vm_context_page_table_base = set_vm_context_page_table_base,
> @@ -792,63 +788,6 @@ static void set_scratch_backing_va(struct kgd_dev *kgd,
>       unlock_srbm(kgd);
>   }
>   
> -static uint16_t get_fw_version(struct kgd_dev *kgd, enum kgd_engine_type 
> type)
> -{
> -     struct amdgpu_device *adev = (struct amdgpu_device *) kgd;
> -     const union amdgpu_firmware_header *hdr;
> -
> -     switch (type) {
> -     case KGD_ENGINE_PFP:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.pfp_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_ME:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.me_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_CE:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.ce_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_MEC1:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.mec_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_MEC2:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.mec2_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_RLC:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.rlc_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_SDMA1:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->sdma.instance[0].fw->data;
> -             break;
> -
> -     case KGD_ENGINE_SDMA2:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->sdma.instance[1].fw->data;
> -             break;
> -
> -     default:
> -             return 0;
> -     }
> -
> -     if (hdr == NULL)
> -             return 0;
> -
> -     /* Only 12 bit in use*/
> -     return hdr->common.ucode_version;
> -}
> -
>   static void set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t 
> vmid,
>                       uint64_t page_table_base)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> index 56ea929..fec3a6a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> @@ -23,12 +23,10 @@
>   #include <linux/module.h>
>   #include <linux/fdtable.h>
>   #include <linux/uaccess.h>
> -#include <linux/firmware.h>
>   #include <linux/mmu_context.h>
>   #include <drm/drmP.h>
>   #include "amdgpu.h"
>   #include "amdgpu_amdkfd.h"
> -#include "amdgpu_ucode.h"
>   #include "gfx_v8_0.h"
>   #include "gca/gfx_8_0_sh_mask.h"
>   #include "gca/gfx_8_0_d.h"
> @@ -95,7 +93,6 @@ static bool get_atc_vmid_pasid_mapping_valid(struct kgd_dev 
> *kgd,
>               uint8_t vmid);
>   static uint16_t get_atc_vmid_pasid_mapping_pasid(struct kgd_dev *kgd,
>               uint8_t vmid);
> -static uint16_t get_fw_version(struct kgd_dev *kgd, enum kgd_engine_type 
> type);
>   static void set_scratch_backing_va(struct kgd_dev *kgd,
>                                       uint64_t va, uint32_t vmid);
>   static void set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t 
> vmid,
> @@ -148,7 +145,6 @@ static const struct kfd2kgd_calls kfd2kgd = {
>                       get_atc_vmid_pasid_mapping_pasid,
>       .get_atc_vmid_pasid_mapping_valid =
>                       get_atc_vmid_pasid_mapping_valid,
> -     .get_fw_version = get_fw_version,
>       .set_scratch_backing_va = set_scratch_backing_va,
>       .get_tile_config = get_tile_config,
>       .set_vm_context_page_table_base = set_vm_context_page_table_base,
> @@ -751,63 +747,6 @@ static void set_scratch_backing_va(struct kgd_dev *kgd,
>       unlock_srbm(kgd);
>   }
>   
> -static uint16_t get_fw_version(struct kgd_dev *kgd, enum kgd_engine_type 
> type)
> -{
> -     struct amdgpu_device *adev = (struct amdgpu_device *) kgd;
> -     const union amdgpu_firmware_header *hdr;
> -
> -     switch (type) {
> -     case KGD_ENGINE_PFP:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.pfp_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_ME:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.me_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_CE:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.ce_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_MEC1:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.mec_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_MEC2:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.mec2_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_RLC:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->gfx.rlc_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_SDMA1:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->sdma.instance[0].fw->data;
> -             break;
> -
> -     case KGD_ENGINE_SDMA2:
> -             hdr = (const union amdgpu_firmware_header *)
> -                                             adev->sdma.instance[1].fw->data;
> -             break;
> -
> -     default:
> -             return 0;
> -     }
> -
> -     if (hdr == NULL)
> -             return 0;
> -
> -     /* Only 12 bit in use*/
> -     return hdr->common.ucode_version;
> -}
> -
>   static void set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t 
> vmid,
>               uint64_t page_table_base)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index 5c51d491..ef3d93b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -25,12 +25,10 @@
>   #include <linux/module.h>
>   #include <linux/fdtable.h>
>   #include <linux/uaccess.h>
> -#include <linux/firmware.h>
>   #include <linux/mmu_context.h>
>   #include <drm/drmP.h>
>   #include "amdgpu.h"
>   #include "amdgpu_amdkfd.h"
> -#include "amdgpu_ucode.h"
>   #include "soc15_hw_ip.h"
>   #include "gc/gc_9_0_offset.h"
>   #include "gc/gc_9_0_sh_mask.h"
> @@ -111,7 +109,6 @@ static uint16_t get_atc_vmid_pasid_mapping_pasid(struct 
> kgd_dev *kgd,
>               uint8_t vmid);
>   static void set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t 
> vmid,
>               uint64_t page_table_base);
> -static uint16_t get_fw_version(struct kgd_dev *kgd, enum kgd_engine_type 
> type);
>   static void set_scratch_backing_va(struct kgd_dev *kgd,
>                                       uint64_t va, uint32_t vmid);
>   static int invalidate_tlbs(struct kgd_dev *kgd, uint16_t pasid);
> @@ -158,7 +155,6 @@ static const struct kfd2kgd_calls kfd2kgd = {
>                       get_atc_vmid_pasid_mapping_pasid,
>       .get_atc_vmid_pasid_mapping_valid =
>                       get_atc_vmid_pasid_mapping_valid,
> -     .get_fw_version = get_fw_version,
>       .set_scratch_backing_va = set_scratch_backing_va,
>       .get_tile_config = amdgpu_amdkfd_get_tile_config,
>       .set_vm_context_page_table_base = set_vm_context_page_table_base,
> @@ -874,56 +870,6 @@ static void set_scratch_backing_va(struct kgd_dev *kgd,
>        */
>   }
>   
> -/* FIXME: Does this need to be ASIC-specific code? */
> -static uint16_t get_fw_version(struct kgd_dev *kgd, enum kgd_engine_type 
> type)
> -{
> -     struct amdgpu_device *adev = (struct amdgpu_device *) kgd;
> -     const union amdgpu_firmware_header *hdr;
> -
> -     switch (type) {
> -     case KGD_ENGINE_PFP:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->gfx.pfp_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_ME:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->gfx.me_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_CE:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->gfx.ce_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_MEC1:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->gfx.mec_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_MEC2:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->gfx.mec2_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_RLC:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->gfx.rlc_fw->data;
> -             break;
> -
> -     case KGD_ENGINE_SDMA1:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->sdma.instance[0].fw->data;
> -             break;
> -
> -     case KGD_ENGINE_SDMA2:
> -             hdr = (const union amdgpu_firmware_header 
> *)adev->sdma.instance[1].fw->data;
> -             break;
> -
> -     default:
> -             return 0;
> -     }
> -
> -     if (hdr == NULL)
> -             return 0;
> -
> -     /* Only 12 bit in use*/
> -     return hdr->common.ucode_version;
> -}
> -
>   static void set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t 
> vmid,
>               uint64_t page_table_base)
>   {
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 2fee306..c1e4d44 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -494,9 +494,9 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>   {
>       unsigned int size;
>   
> -     kfd->mec_fw_version = kfd->kfd2kgd->get_fw_version(kfd->kgd,
> +     kfd->mec_fw_version = amdgpu_amdkfd_get_fw_version(kfd->kgd,
>                       KGD_ENGINE_MEC1);
> -     kfd->sdma_fw_version = kfd->kfd2kgd->get_fw_version(kfd->kgd,
> +     kfd->sdma_fw_version = amdgpu_amdkfd_get_fw_version(kfd->kgd,
>                       KGD_ENGINE_SDMA1);
>       kfd->shared_resources = *gpu_resources;
>   
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to