[PATCH 1/3] drm/amdkfd: device pgmap owner at the svm migrate init

[Felix Kuehling]

Am 2021-05-05 um 6:59 p.m. schrieb Alex Sierra:
> pgmap owner member at the svm migrate init could be referenced
> to either adev or hive, depending on device topology.
>
> Signed-off-by: Alex Sierra <alex.sierra at amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 6b810863f6ba..5723c6bc2afc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -426,7 +426,7 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>  	migrate.start = start;
>  	migrate.end = end;
>  	migrate.flags = MIGRATE_VMA_SELECT_SYSTEM;
> -	migrate.pgmap_owner = adev;
> +	migrate.pgmap_owner = (adev->hive) ? (void *)adev->hive : (void *)adev;

Why do you need the cast to (void *) here ...


>  
>  	size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
>  	size *= npages;
> @@ -641,7 +641,7 @@ svm_migrate_vma_to_ram(struct amdgpu_device *adev, struct svm_range *prange,
>  	migrate.start = start;
>  	migrate.end = end;
>  	migrate.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
> -	migrate.pgmap_owner = adev;
> +	migrate.pgmap_owner = (adev->hive) ? (void *)adev->hive : (void *)adev;
>  
>  	size = 2 * sizeof(*migrate.src) + sizeof(uint64_t) + sizeof(dma_addr_t);
>  	size *= npages;
> @@ -907,7 +907,7 @@ int svm_migrate_init(struct amdgpu_device *adev)
>  	pgmap->range.start = res->start;
>  	pgmap->range.end = res->end;
>  	pgmap->ops = &svm_migrate_pgmap_ops;
> -	pgmap->owner = adev;
> +	pgmap->owner = (adev->hive) ? adev->hive : adev;

... but not here?

Having to duplicate this in several places in this patch and the later
patches is error prone. Maybe it would be worth defining a macro for this:

#define SVM_ADEV_PGMAP_OWNER(adev) ((adev)->hive ? (adev)->hive : (adev))

That makes it easy to find all places that need this owner, and if we
ever change the logic for this, it's easy to change.

Regards,
  Felix


>  	pgmap->flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
>  	r = devm_memremap_pages(adev->dev, pgmap);
>  	if (IS_ERR(r)) {