[PATCH] drm/amdkfd: Cleanup kfd_dev struct
Christian König
ckoenig.leichtzumerken at gmail.com
Thu Oct 27 06:32:39 UTC 2022
Am 26.10.22 um 21:36 schrieb Alex Deucher:
> From: Mukul Joshi <mukul.joshi at amd.com>
>
> Cleanup kfd_dev struct by removing ddev and pdev as both
> drm_device and pci_dev can be fetched from amdgpu_device.
>
> Signed-off-by: Mukul Joshi <mukul.joshi at amd.com>
> Tested-by: Amber Lin <Amber.Lin at amd.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
Acked-by: Christian König <christian.koenig at amd.com>
Should we go even a step further and make the kfd dev a member of the
adev structure so that we can upcast?
Regards,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 3 +--
> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 12 ++++-----
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 16 +++++-------
> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c | 29 +++++++++++-----------
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 4 +--
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 12 ++++-----
> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 25 +++++++++----------
> 9 files changed, 49 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 0561812aa0a4..e2b0f8049b9f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -195,7 +195,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
> }
>
> adev->kfd.init_complete = kgd2kfd_device_init(adev->kfd.dev,
> - adev_to_drm(adev), &gpu_resources);
> + &gpu_resources);
>
> amdgpu_amdkfd_total_mem_size += adev->gmc.real_vram_size;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 647220a8762d..f50e3ba4d7a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -353,7 +353,6 @@ int kgd2kfd_init(void);
> void kgd2kfd_exit(void);
> struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf);
> bool kgd2kfd_device_init(struct kfd_dev *kfd,
> - struct drm_device *ddev,
> const struct kgd2kfd_shared_resources *gpu_resources);
> void kgd2kfd_device_exit(struct kfd_dev *kfd);
> void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm);
> @@ -381,7 +380,7 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
> }
>
> static inline
> -bool kgd2kfd_device_init(struct kfd_dev *kfd, struct drm_device *ddev,
> +bool kgd2kfd_device_init(struct kfd_dev *kfd,
> const struct kgd2kfd_shared_resources *gpu_resources)
> {
> return false;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 3c771d580098..acb8bc29218b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -2115,8 +2115,8 @@ static void kfd_find_numa_node_in_srat(struct kfd_dev *kdev)
> struct acpi_table_header *table_header = NULL;
> struct acpi_subtable_header *sub_header = NULL;
> unsigned long table_end, subtable_len;
> - u32 pci_id = pci_domain_nr(kdev->pdev->bus) << 16 |
> - pci_dev_id(kdev->pdev);
> + u32 pci_id = pci_domain_nr(kdev->adev->pdev->bus) << 16 |
> + pci_dev_id(kdev->adev->pdev);
> u32 bdf;
> acpi_status status;
> struct acpi_srat_cpu_affinity *cpu;
> @@ -2191,7 +2191,7 @@ static void kfd_find_numa_node_in_srat(struct kfd_dev *kdev)
> numa_node = 0;
>
> if (numa_node != NUMA_NO_NODE)
> - set_dev_node(&kdev->pdev->dev, numa_node);
> + set_dev_node(&kdev->adev->pdev->dev, numa_node);
> }
> #endif
>
> @@ -2252,14 +2252,14 @@ static int kfd_fill_gpu_direct_io_link_to_cpu(int *avail_size,
> sub_type_hdr->proximity_domain_from = proximity_domain;
>
> #ifdef CONFIG_ACPI_NUMA
> - if (kdev->pdev->dev.numa_node == NUMA_NO_NODE)
> + if (kdev->adev->pdev->dev.numa_node == NUMA_NO_NODE)
> kfd_find_numa_node_in_srat(kdev);
> #endif
> #ifdef CONFIG_NUMA
> - if (kdev->pdev->dev.numa_node == NUMA_NO_NODE)
> + if (kdev->adev->pdev->dev.numa_node == NUMA_NO_NODE)
> sub_type_hdr->proximity_domain_to = 0;
> else
> - sub_type_hdr->proximity_domain_to = kdev->pdev->dev.numa_node;
> + sub_type_hdr->proximity_domain_to = kdev->adev->pdev->dev.numa_node;
> #else
> sub_type_hdr->proximity_domain_to = 0;
> #endif
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index be7a0b5a2dbc..909397fdb7d8 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -228,7 +228,6 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
> {
> struct kfd_dev *kfd = NULL;
> const struct kfd2kgd_calls *f2g = NULL;
> - struct pci_dev *pdev = adev->pdev;
> uint32_t gfx_target_version = 0;
>
> switch (adev->asic_type) {
> @@ -430,7 +429,6 @@ struct kfd_dev *kgd2kfd_probe(struct amdgpu_device *adev, bool vf)
>
> kfd->adev = adev;
> kfd_device_info_init(kfd, vf, gfx_target_version);
> - kfd->pdev = pdev;
> kfd->init_complete = false;
> kfd->kfd2kgd = f2g;
> atomic_set(&kfd->compute_profile, 0);
> @@ -512,12 +510,10 @@ static void kfd_smi_init(struct kfd_dev *dev)
> }
>
> bool kgd2kfd_device_init(struct kfd_dev *kfd,
> - struct drm_device *ddev,
> const struct kgd2kfd_shared_resources *gpu_resources)
> {
> unsigned int size, map_process_packet_size;
>
> - kfd->ddev = ddev;
> kfd->mec_fw_version = amdgpu_amdkfd_get_fw_version(kfd->adev,
> KGD_ENGINE_MEC1);
> kfd->mec2_fw_version = amdgpu_amdkfd_get_fw_version(kfd->adev,
> @@ -542,7 +538,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> kfd->mec_fw_version < kfd->device_info.no_atomic_fw_version)) {
> dev_info(kfd_device,
> "skipped device %x:%x, PCI rejects atomics %d<%d\n",
> - kfd->pdev->vendor, kfd->pdev->device,
> + kfd->adev->pdev->vendor, kfd->adev->pdev->device,
> kfd->mec_fw_version,
> kfd->device_info.no_atomic_fw_version);
> return false;
> @@ -651,8 +647,8 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> kfd_smi_init(kfd);
>
> kfd->init_complete = true;
> - dev_info(kfd_device, "added device %x:%x\n", kfd->pdev->vendor,
> - kfd->pdev->device);
> + dev_info(kfd_device, "added device %x:%x\n", kfd->adev->pdev->vendor,
> + kfd->adev->pdev->device);
>
> pr_debug("Starting kfd with the following scheduling policy %d\n",
> kfd->dqm->sched_policy);
> @@ -677,7 +673,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
> amdgpu_amdkfd_free_gws(kfd->adev, kfd->gws);
> dev_err(kfd_device,
> "device %x:%x NOT added due to errors\n",
> - kfd->pdev->vendor, kfd->pdev->device);
> + kfd->adev->pdev->vendor, kfd->adev->pdev->device);
> out:
> return kfd->init_complete;
> }
> @@ -790,7 +786,7 @@ int kgd2kfd_resume_iommu(struct kfd_dev *kfd)
> if (err)
> dev_err(kfd_device,
> "Failed to resume IOMMU for device %x:%x\n",
> - kfd->pdev->vendor, kfd->pdev->device);
> + kfd->adev->pdev->vendor, kfd->adev->pdev->device);
> return err;
> }
>
> @@ -802,7 +798,7 @@ static int kfd_resume(struct kfd_dev *kfd)
> if (err)
> dev_err(kfd_device,
> "Error starting queue manager for device %x:%x\n",
> - kfd->pdev->vendor, kfd->pdev->device);
> + kfd->adev->pdev->vendor, kfd->adev->pdev->device);
>
> return err;
> }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
> index fbd0afe4da42..ec1bf611624e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
> @@ -49,7 +49,7 @@ int kfd_iommu_check_device(struct kfd_dev *kfd)
> return -ENODEV;
>
> iommu_info.flags = 0;
> - err = amd_iommu_device_info(kfd->pdev, &iommu_info);
> + err = amd_iommu_device_info(kfd->adev->pdev, &iommu_info);
> if (err)
> return err;
>
> @@ -71,7 +71,7 @@ int kfd_iommu_device_init(struct kfd_dev *kfd)
> return 0;
>
> iommu_info.flags = 0;
> - err = amd_iommu_device_info(kfd->pdev, &iommu_info);
> + err = amd_iommu_device_info(kfd->adev->pdev, &iommu_info);
> if (err < 0) {
> dev_err(kfd_device,
> "error getting iommu info. is the iommu enabled?\n");
> @@ -121,7 +121,7 @@ int kfd_iommu_bind_process_to_device(struct kfd_process_device *pdd)
> return -EINVAL;
> }
>
> - err = amd_iommu_bind_pasid(dev->pdev, p->pasid, p->lead_thread);
> + err = amd_iommu_bind_pasid(dev->adev->pdev, p->pasid, p->lead_thread);
> if (!err)
> pdd->bound = PDD_BOUND;
>
> @@ -139,7 +139,8 @@ void kfd_iommu_unbind_process(struct kfd_process *p)
>
> for (i = 0; i < p->n_pdds; i++)
> if (p->pdds[i]->bound == PDD_BOUND)
> - amd_iommu_unbind_pasid(p->pdds[i]->dev->pdev, p->pasid);
> + amd_iommu_unbind_pasid(p->pdds[i]->dev->adev->pdev,
> + p->pasid);
> }
>
> /* Callback for process shutdown invoked by the IOMMU driver */
> @@ -222,7 +223,7 @@ static int kfd_bind_processes_to_device(struct kfd_dev *kfd)
> continue;
> }
>
> - err = amd_iommu_bind_pasid(kfd->pdev, p->pasid,
> + err = amd_iommu_bind_pasid(kfd->adev->pdev, p->pasid,
> p->lead_thread);
> if (err < 0) {
> pr_err("Unexpected pasid 0x%x binding failure\n",
> @@ -282,9 +283,9 @@ void kfd_iommu_suspend(struct kfd_dev *kfd)
>
> kfd_unbind_processes_from_device(kfd);
>
> - amd_iommu_set_invalidate_ctx_cb(kfd->pdev, NULL);
> - amd_iommu_set_invalid_ppr_cb(kfd->pdev, NULL);
> - amd_iommu_free_device(kfd->pdev);
> + amd_iommu_set_invalidate_ctx_cb(kfd->adev->pdev, NULL);
> + amd_iommu_set_invalid_ppr_cb(kfd->adev->pdev, NULL);
> + amd_iommu_free_device(kfd->adev->pdev);
> }
>
> /** kfd_iommu_resume - Restore IOMMU after resume
> @@ -302,20 +303,20 @@ int kfd_iommu_resume(struct kfd_dev *kfd)
>
> pasid_limit = kfd_get_pasid_limit();
>
> - err = amd_iommu_init_device(kfd->pdev, pasid_limit);
> + err = amd_iommu_init_device(kfd->adev->pdev, pasid_limit);
> if (err)
> return -ENXIO;
>
> - amd_iommu_set_invalidate_ctx_cb(kfd->pdev,
> + amd_iommu_set_invalidate_ctx_cb(kfd->adev->pdev,
> iommu_pasid_shutdown_callback);
> - amd_iommu_set_invalid_ppr_cb(kfd->pdev,
> + amd_iommu_set_invalid_ppr_cb(kfd->adev->pdev,
> iommu_invalid_ppr_cb);
>
> err = kfd_bind_processes_to_device(kfd);
> if (err) {
> - amd_iommu_set_invalidate_ctx_cb(kfd->pdev, NULL);
> - amd_iommu_set_invalid_ppr_cb(kfd->pdev, NULL);
> - amd_iommu_free_device(kfd->pdev);
> + amd_iommu_set_invalidate_ctx_cb(kfd->adev->pdev, NULL);
> + amd_iommu_set_invalid_ppr_cb(kfd->adev->pdev, NULL);
> + amd_iommu_free_device(kfd->adev->pdev);
> return err;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index bf610e3b683b..552c3ac85a13 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -254,8 +254,6 @@ struct kfd_dev {
> struct amdgpu_device *adev;
>
> struct kfd_device_info device_info;
> - struct pci_dev *pdev;
> - struct drm_device *ddev;
>
> unsigned int id; /* topology stub index */
>
> @@ -1365,7 +1363,7 @@ void kfd_dec_compute_active(struct kfd_dev *dev);
> static inline int kfd_devcgroup_check_permission(struct kfd_dev *kfd)
> {
> #if defined(CONFIG_CGROUP_DEVICE) || defined(CONFIG_CGROUP_BPF)
> - struct drm_device *ddev = kfd->ddev;
> + struct drm_device *ddev = adev_to_drm(kfd->adev);
>
> return devcgroup_check_permission(DEVCG_DEV_CHAR, DRM_MAJOR,
> ddev->render->index,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 951b63677248..a26257171ab7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1050,8 +1050,8 @@ static void kfd_process_destroy_pdds(struct kfd_process *p)
> * for auto suspend
> */
> if (pdd->runtime_inuse) {
> - pm_runtime_mark_last_busy(pdd->dev->ddev->dev);
> - pm_runtime_put_autosuspend(pdd->dev->ddev->dev);
> + pm_runtime_mark_last_busy(adev_to_drm(pdd->dev->adev)->dev);
> + pm_runtime_put_autosuspend(adev_to_drm(pdd->dev->adev)->dev);
> pdd->runtime_inuse = false;
> }
>
> @@ -1633,9 +1633,9 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
> * pdd is destroyed.
> */
> if (!pdd->runtime_inuse) {
> - err = pm_runtime_get_sync(dev->ddev->dev);
> + err = pm_runtime_get_sync(adev_to_drm(dev->adev)->dev);
> if (err < 0) {
> - pm_runtime_put_autosuspend(dev->ddev->dev);
> + pm_runtime_put_autosuspend(adev_to_drm(dev->adev)->dev);
> return ERR_PTR(err);
> }
> }
> @@ -1655,8 +1655,8 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
> out:
> /* balance runpm reference count and exit with error */
> if (!pdd->runtime_inuse) {
> - pm_runtime_mark_last_busy(dev->ddev->dev);
> - pm_runtime_put_autosuspend(dev->ddev->dev);
> + pm_runtime_mark_last_busy(adev_to_drm(dev->adev)->dev);
> + pm_runtime_put_autosuspend(adev_to_drm(dev->adev)->dev);
> }
>
> return ERR_PTR(err);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index af4140685bf3..39fbe19b978a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -259,7 +259,7 @@ void svm_range_free_dma_mappings(struct svm_range *prange)
> pr_debug("failed to find device idx %d\n", gpuidx);
> continue;
> }
> - dev = &pdd->dev->pdev->dev;
> + dev = &pdd->dev->adev->pdev->dev;
> svm_range_dma_unmap(dev, dma_addr, 0, prange->npages);
> kvfree(dma_addr);
> prange->dma_addr[gpuidx] = NULL;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 3f0a4a415907..ec8c9c10d348 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -115,7 +115,7 @@ struct kfd_dev *kfd_device_by_pci_dev(const struct pci_dev *pdev)
> down_read(&topology_lock);
>
> list_for_each_entry(top_dev, &topology_device_list, list)
> - if (top_dev->gpu && top_dev->gpu->pdev == pdev) {
> + if (top_dev->gpu && top_dev->gpu->adev->pdev == pdev) {
> device = top_dev->gpu;
> break;
> }
> @@ -1169,13 +1169,12 @@ static uint32_t kfd_generate_gpu_id(struct kfd_dev *gpu)
>
> local_mem_size = gpu->local_mem_info.local_mem_size_private +
> gpu->local_mem_info.local_mem_size_public;
> -
> - buf[0] = gpu->pdev->devfn;
> - buf[1] = gpu->pdev->subsystem_vendor |
> - (gpu->pdev->subsystem_device << 16);
> - buf[2] = pci_domain_nr(gpu->pdev->bus);
> - buf[3] = gpu->pdev->device;
> - buf[4] = gpu->pdev->bus->number;
> + buf[0] = gpu->adev->pdev->devfn;
> + buf[1] = gpu->adev->pdev->subsystem_vendor |
> + (gpu->adev->pdev->subsystem_device << 16);
> + buf[2] = pci_domain_nr(gpu->adev->pdev->bus);
> + buf[3] = gpu->adev->pdev->device;
> + buf[4] = gpu->adev->pdev->bus->number;
> buf[5] = lower_32_bits(local_mem_size);
> buf[6] = upper_32_bits(local_mem_size);
>
> @@ -1269,7 +1268,7 @@ static void kfd_set_iolink_no_atomics(struct kfd_topology_device *dev,
> if (target_gpu_dev) {
> uint32_t cap;
>
> - pcie_capability_read_dword(target_gpu_dev->gpu->pdev,
> + pcie_capability_read_dword(target_gpu_dev->gpu->adev->pdev,
> PCI_EXP_DEVCAP2, &cap);
>
> if (!(cap & (PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> @@ -1688,13 +1687,13 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
> cu_info.num_shader_arrays_per_engine;
>
> dev->node_props.gfx_target_version = gpu->device_info.gfx_target_version;
> - dev->node_props.vendor_id = gpu->pdev->vendor;
> - dev->node_props.device_id = gpu->pdev->device;
> + dev->node_props.vendor_id = gpu->adev->pdev->vendor;
> + dev->node_props.device_id = gpu->adev->pdev->device;
> dev->node_props.capability |=
> ((dev->gpu->adev->rev_id << HSA_CAP_ASIC_REVISION_SHIFT) &
> HSA_CAP_ASIC_REVISION_MASK);
> - dev->node_props.location_id = pci_dev_id(gpu->pdev);
> - dev->node_props.domain = pci_domain_nr(gpu->pdev->bus);
> + dev->node_props.location_id = pci_dev_id(gpu->adev->pdev);
> + dev->node_props.domain = pci_domain_nr(gpu->adev->pdev->bus);
> dev->node_props.max_engine_clk_fcompute =
> amdgpu_amdkfd_get_max_engine_clock_in_mhz(dev->gpu->adev);
> dev->node_props.max_engine_clk_ccompute =
More information about the amd-gfx
mailing list