I'd recommend making this the first change in the series. Make 'drm/amdkfd: force raven as "dgpu" path' the second patch. That way it only needs to change one place.
A few more comments inline. Am 2020-08-18 um 9:09 a.m. schrieb Huang Rui: > It's better to use inline function to wrap the iommu checking. > > Signed-off-by: Huang Rui <[email protected]> > --- > drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 4 ++-- > .../gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c | 4 ++-- > drivers/gpu/drm/amd/amdkfd/kfd_iommu.c | 10 +++++----- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 8 ++++++++ > drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 6 +++--- > 7 files changed, 22 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > index 20ef754dc62e..0598a1682854 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c > @@ -1258,7 +1258,7 @@ bool kfd_dev_is_large_bar(struct kfd_dev *dev) > return true; > } > > - if (dev->device_info->needs_iommu_device && !ignore_crat) > + if (kfd_go_iommu_v2(dev->device_info)) > return false; > > amdgpu_amdkfd_get_local_mem_info(dev->kgd, &mem_info); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c > b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c > index 0c4161ac4102..04103273f52b 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c > @@ -309,7 +309,7 @@ static int dbgdev_address_watch_nodiq(struct kfd_dbgdev > *dbgdev, > for (i = 0; i < adw_info->num_watch_points; i++) { > dbgdev_address_watch_set_registers(adw_info, &addrHi, &addrLo, > &cntl, i, pdd->qpd.vmid, > - dbgdev->dev->device_info->needs_iommu_device && > !ignore_crat); > + kfd_go_iommu_v2(dbgdev->dev->device_info)); > > pr_debug("\t\t%30s\n", "* * * * * * * * * * * * * * * * * *"); > pr_debug("\t\t%20s %08x\n", "register index :", i); > @@ -399,7 +399,7 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev > *dbgdev, > for (i = 0; i < adw_info->num_watch_points; i++) { > dbgdev_address_watch_set_registers(adw_info, &addrHi, &addrLo, > &cntl, i, vmid, > - dbgdev->dev->device_info->needs_iommu_device && > !ignore_crat); > + kfd_go_iommu_v2(dbgdev->dev->device_info)); > > pr_debug("\t\t%30s\n", "* * * * * * * * * * * * * * * * * *"); > pr_debug("\t\t%20s %08x\n", "register index :", i); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c > index 48b86bdbb680..70cede08e555 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c > @@ -62,7 +62,7 @@ static int update_qpd_v9(struct device_queue_manager *dqm, > SH_MEM_ALIGNMENT_MODE_UNALIGNED << > SH_MEM_CONFIG__ALIGNMENT_MODE__SHIFT; > if (amdgpu_noretry && > - !(dqm->dev->device_info->needs_iommu_device && > !ignore_crat)) > + !(kfd_go_iommu_v2(dqm->dev->device_info))) > qpd->sh_mem_config |= > 1 << SH_MEM_CONFIG__RETRY_DISABLE__SHIFT; > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c > b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c > index 6044d36bceb6..fe7e31014289 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c > @@ -321,7 +321,7 @@ static void kfd_init_apertures_vi(struct > kfd_process_device *pdd, uint8_t id) > pdd->lds_base = MAKE_LDS_APP_BASE_VI(); > pdd->lds_limit = MAKE_LDS_APP_LIMIT(pdd->lds_base); > > - if (!(pdd->dev->device_info->needs_iommu_device && !ignore_crat)) { > + if (!kfd_go_iommu_v2(pdd->dev->device_info)) { > /* dGPUs: SVM aperture starting at 0 > * with small reserved space for kernel. > * Set them to CANONICAL addresses. > @@ -423,7 +423,7 @@ int kfd_init_apertures(struct kfd_process *process) > return -EINVAL; > } > > - if (!(dev->device_info->needs_iommu_device && > !ignore_crat)) { > + if (!kfd_go_iommu_v2(dev->device_info)) { > /* dGPUs: the reserved space for kernel > * before SVM > */ > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c > b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c > index c767b524bea4..ea05446572e1 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c > @@ -41,7 +41,7 @@ int kfd_iommu_check_device(struct kfd_dev *kfd) > struct amd_iommu_device_info iommu_info; > int err; > > - if (!(kfd->device_info->needs_iommu_device && !ignore_crat)) > + if (!kfd_go_iommu_v2(kfd->device_info)) > return -ENODEV; > > iommu_info.flags = 0; > @@ -63,7 +63,7 @@ int kfd_iommu_device_init(struct kfd_dev *kfd) > unsigned int pasid_limit; > int err; > > - if (!(kfd->device_info->needs_iommu_device && !ignore_crat)) > + if (!kfd_go_iommu_v2(kfd->device_info)) > return 0; > > iommu_info.flags = 0; > @@ -109,7 +109,7 @@ int kfd_iommu_bind_process_to_device(struct > kfd_process_device *pdd) > struct kfd_process *p = pdd->process; > int err; > > - if (!(dev->device_info->needs_iommu_device && !ignore_crat) || > pdd->bound == PDD_BOUND) > + if (!kfd_go_iommu_v2(dev->device_info) || pdd->bound == PDD_BOUND) > return 0; > > if (unlikely(pdd->bound == PDD_BOUND_SUSPENDED)) { > @@ -284,7 +284,7 @@ static void kfd_unbind_processes_from_device(struct > kfd_dev *kfd) > */ > void kfd_iommu_suspend(struct kfd_dev *kfd) > { > - if (!(kfd->device_info->needs_iommu_device && !ignore_crat)) > + if (!kfd_go_iommu_v2(kfd->device_info)) > return; > > kfd_unbind_processes_from_device(kfd); > @@ -304,7 +304,7 @@ int kfd_iommu_resume(struct kfd_dev *kfd) > unsigned int pasid_limit; > int err; > > - if (!(kfd->device_info->needs_iommu_device && !ignore_crat)) > + if (!kfd_go_iommu_v2(kfd->device_info)) > return 0; > > pasid_limit = kfd_get_pasid_limit(); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index 60243798cce2..5087b23ccdb2 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -1232,6 +1232,14 @@ static inline int > kfd_devcgroup_check_permission(struct kfd_dev *kfd) > #endif > } > > +static inline bool kfd_go_iommu_v2(const struct kfd_device_info *info) I'd prefer a different function name: kfd_device_use_iommu_v2. Then make the parameter const struct kfd_device *dev. > +{ > + if (!info) > + return false; > + > + return info->needs_iommu_device && !ignore_crat; > +} If you need a NULL check, you could still write it in a more compact way (I like inline functions to be one-liners): return !ignore_crat && dev && dev->device_info->needs_iommu_device; If you make this the first patch in the series, remove the !ignore_crat part. That would be added in the second change. Regards, Felix > + > /* Debugfs */ > #if defined(CONFIG_DEBUG_FS) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > index ad8139594b6f..e6f03867e0ed 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c > @@ -545,7 +545,7 @@ static ssize_t node_show(struct kobject *kobj, struct > attribute *attr, > * If the ASIC is APU except Kaveri, set local memory size > * to 0 to disable local memory support > */ > - if (!(dev->gpu->device_info->needs_iommu_device && !ignore_crat) > + if (!kfd_go_iommu_v2(dev->gpu->device_info) > || dev->gpu->device_info->asic_family == CHIP_KAVERI) { > amdgpu_amdkfd_get_local_mem_info(dev->gpu->kgd, > &local_mem_info); > @@ -1198,7 +1198,7 @@ static struct kfd_topology_device > *kfd_assign_gpu(struct kfd_dev *gpu) > /* Discrete GPUs need their own topology device list > * entries. Don't assign them to CPU/APU nodes. > */ > - if (!(gpu->device_info->needs_iommu_device && !ignore_crat) && > + if (!kfd_go_iommu_v2(gpu->device_info) && > dev->node_props.cpu_cores_count) > continue; > > @@ -1453,7 +1453,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu) > * Overwrite ATS capability according to needs_iommu_device to fix > * potential missing corresponding bit in CRAT of BIOS. > */ > - if (dev->gpu->device_info->needs_iommu_device && !ignore_crat) > + if (kfd_go_iommu_v2(dev->gpu->device_info)) > dev->node_props.capability |= HSA_CAP_ATS_PRESENT; > else > dev->node_props.capability &= ~HSA_CAP_ATS_PRESENT; _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
