Re: [PATCH 2/2] drm/amdkfd: Add Arcturus specific set_vm_context_page_table_base()

[Felix Kuehling <felix.kuehling@xxxxxxx>]

I agree with Christian's comments on patch 1. With those fixed, the 
series is

Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>

Regards,
  Felix

On 2019-12-02 20:42, Yong Zhao wrote:
> Since Arcturus has it own function pointer, we can move Arcturus
> specific logic to there rather than leaving it entangled with
> other GFX9 chips.
>
> Change-Id: I7df7c004a0c8ac0616ded0e65144670df50f92a7
> Signed-off-by: Yong Zhao <Yong.Zhao@xxxxxxx>
> ---
>   .../drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c   | 20 ++++++++++++++++++-
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 14 +++----------
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h |  2 --
>   3 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> index b6713e0ed1b2..3c119407dc34 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> @@ -46,6 +46,8 @@
>   #include "soc15.h"
>   #include "soc15d.h"
>   #include "amdgpu_amdkfd_gfx_v9.h"
> +#include "gfxhub_v1_0.h"
> +#include "mmhub_v9_4.h"
>   
>   #define HQD_N_REGS 56
>   #define DUMP_REG(addr) do {				\
> @@ -258,6 +260,22 @@ static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
>   	return 0;
>   }
>   
> +static void kgd_set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t vmid,
> +		uint64_t page_table_base)
> +{
> +	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> +
> +	if (!amdgpu_amdkfd_is_kfd_vmid(adev, vmid)) {
> +		pr_err("trying to set page table base for wrong VMID %u\n",
> +		       vmid);
> +		return;
> +	}
> +
> +	mmhub_v9_4_setup_vm_pt_regs(adev, vmid, page_table_base);
> +
> +	gfxhub_v1_0_setup_vm_pt_regs(adev, vmid, page_table_base);
> +}
> +
>   const struct kfd2kgd_calls arcturus_kfd2kgd = {
>   	.program_sh_mem_settings = kgd_gfx_v9_program_sh_mem_settings,
>   	.set_pasid_vmid_mapping = kgd_gfx_v9_set_pasid_vmid_mapping,
> @@ -277,7 +295,7 @@ const struct kfd2kgd_calls arcturus_kfd2kgd = {
>   	.get_atc_vmid_pasid_mapping_info =
>   			kgd_gfx_v9_get_atc_vmid_pasid_mapping_info,
>   	.get_tile_config = kgd_gfx_v9_get_tile_config,
> -	.set_vm_context_page_table_base = kgd_gfx_v9_set_vm_context_page_table_base,
> +	.set_vm_context_page_table_base = kgd_set_vm_context_page_table_base,
>   	.invalidate_tlbs = kgd_gfx_v9_invalidate_tlbs,
>   	.invalidate_tlbs_vmid = kgd_gfx_v9_invalidate_tlbs_vmid,
>   	.get_hive_id = amdgpu_amdkfd_get_hive_id,
> 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 6f1a4676ddde..e7861f0ef415 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -40,7 +40,6 @@
>   #include "soc15d.h"
>   #include "mmhub_v1_0.h"
>   #include "gfxhub_v1_0.h"
> -#include "mmhub_v9_4.h"
>   
>   
>   enum hqd_dequeue_request_type {
> @@ -758,8 +757,8 @@ uint32_t kgd_gfx_v9_address_watch_get_offset(struct kgd_dev *kgd,
>   	return 0;
>   }
>   
> -void kgd_gfx_v9_set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t vmid,
> -		uint64_t page_table_base)
> +static void kgd_gfx_v9_set_vm_context_page_table_base(struct kgd_dev *kgd,
> +			uint32_t vmid, uint64_t page_table_base)
>   {
>   	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>   
> @@ -769,14 +768,7 @@ void kgd_gfx_v9_set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t vmi
>   		return;
>   	}
>   
> -	/* TODO: take advantage of per-process address space size. For
> -	 * now, all processes share the same address space size, like
> -	 * on GFX8 and older.
> -	 */
> -	if (adev->asic_type == CHIP_ARCTURUS) {
> -		mmhub_v9_4_setup_vm_pt_regs(adev, vmid, page_table_base);
> -	} else
> -		mmhub_v1_0_setup_vm_pt_regs(adev, vmid, page_table_base);
> +	mmhub_v1_0_setup_vm_pt_regs(adev, vmid, page_table_base);
>   
>   	gfxhub_v1_0_setup_vm_pt_regs(adev, vmid, page_table_base);
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> index d9e9ad22b2bd..02b1426d17d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> @@ -57,8 +57,6 @@ uint32_t kgd_gfx_v9_address_watch_get_offset(struct kgd_dev *kgd,
>   
>   bool kgd_gfx_v9_get_atc_vmid_pasid_mapping_info(struct kgd_dev *kgd,
>   					uint8_t vmid, uint16_t *p_pasid);
> -void kgd_gfx_v9_set_vm_context_page_table_base(struct kgd_dev *kgd, uint32_t vmid,
> -		uint64_t page_table_base);
>   int kgd_gfx_v9_invalidate_tlbs(struct kgd_dev *kgd, uint16_t pasid);
>   int kgd_gfx_v9_invalidate_tlbs_vmid(struct kgd_dev *kgd, uint16_t vmid);
>   int kgd_gfx_v9_get_tile_config(struct kgd_dev *kgd,
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx