Re: [PATCH 14/14] drm/amdgpu: introduce doorbell bo in kernel

["Christian König" <christian.koenig@amd.com>]

Am 03.02.23 um 20:08 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@amd.com>
>
> This patch does the following:
> - Removes doorbell ptr from adev.
> - Moves doorbell.num_doorbells into mman.num_doorbells.
> - Introduces a kernel bo for doorbell management in form of mman.doorbell_kernel_bo.
>    This bo holds the doorbell space now.
> - Introduces mman.doorbell_bo_size: to save total doorbell memory size.
> - Also introduces mman.doorbell_va to save CPU address of doorbell BO
>    mapping. This ptr will be used now for doorbell read/write from doorbell BAR.

Please don't call any CPU address VA, this is a term we usually use with 
GPUVM.

Rather use doorbell_cpu_addr for this.

Christian.

>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h          |  1 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  5 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 16 ++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h |  4 ----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      | 24 ++++++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h      |  6 +++++
>   6 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e1c1a360614e..dd3875ae1ad3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -834,7 +834,6 @@ struct amdgpu_device {
>   	spinlock_t audio_endpt_idx_lock;
>   	amdgpu_block_rreg_t		audio_endpt_rreg;
>   	amdgpu_block_wreg_t		audio_endpt_wreg;
> -	struct amdgpu_doorbell		doorbell;
>   
>   	/* clock/pll info */
>   	struct amdgpu_clock            clock;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 28076da2258f..aea943e337df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -109,11 +109,10 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
>   		*aperture_base = adev->gmc.doorbell_aper_base;
>   		*aperture_size = 0;
>   		*start_offset = 0;
> -	} else if (adev->gmc.doorbell_aper_size > adev->doorbell.num_doorbells *
> -						sizeof(u32)) {
> +	} else if (adev->gmc.doorbell_aper_size > adev->mman.doorbell_kernel_bo_size) {
>   		*aperture_base = adev->gmc.doorbell_aper_base;
>   		*aperture_size = adev->gmc.doorbell_aper_size;
> -		*start_offset = adev->doorbell.num_doorbells * sizeof(u32);
> +		*start_offset = adev->mman.doorbell_kernel_bo_size;
>   	} else {
>   		*aperture_base = 0;
>   		*aperture_size = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7c21ffe63ebc..9c3ce1ae66ea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -593,8 +593,8 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32 index)
>   	if (amdgpu_device_skip_hw_access(adev))
>   		return 0;
>   
> -	if (index < adev->doorbell.num_doorbells) {
> -		return readl(adev->mman.doorbell_aper_base_kaddr + index);
> +	if (index < adev->mman.num_doorbells) {
> +		return readl(adev->mman.doorbell_va + index);
>   	} else {
>   		DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
>   		return 0;
> @@ -616,8 +616,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
>   	if (amdgpu_device_skip_hw_access(adev))
>   		return;
>   
> -	if (index < adev->doorbell.num_doorbells) {
> -		writel(v, adev->mman.doorbell_aper_base_kaddr + index);
> +	if (index < adev->mman.num_doorbells) {
> +		writel(v, adev->mman.doorbell_va + index);
>   	} else {
>   		DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
>   	}
> @@ -637,8 +637,8 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev, u32 index)
>   	if (amdgpu_device_skip_hw_access(adev))
>   		return 0;
>   
> -	if (index < adev->doorbell.num_doorbells) {
> -		return atomic64_read((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index));
> +	if (index < adev->mman.num_doorbells) {
> +		return atomic64_read((atomic64_t *)(adev->mman.doorbell_va + index));
>   	} else {
>   		DRM_ERROR("reading beyond doorbell aperture: 0x%08x!\n", index);
>   		return 0;
> @@ -660,8 +660,8 @@ void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
>   	if (amdgpu_device_skip_hw_access(adev))
>   		return;
>   
> -	if (index < adev->doorbell.num_doorbells) {
> -		atomic64_set((atomic64_t *)(adev->mman.doorbell_aper_base_kaddr + index), v);
> +	if (index < adev->mman.num_doorbells) {
> +		atomic64_set((atomic64_t *)(adev->mman.doorbell_va + index), v);
>   	} else {
>   		DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 464be28da4fb..97298a98561f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -24,10 +24,6 @@
>   /*
>    * GPU doorbell structures, functions & helpers
>    */
> -struct amdgpu_doorbell {
> -	u32			num_doorbells;	/* Number of doorbells actually reserved for amdgpu. */
> -};
> -
>   /* Reserved doorbells for amdgpu (including multimedia).
>    * KFD can use all the rest in the 2M doorbell bar.
>    * For asic before vega10, doorbell is 32-bit, so the
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e38720b91958..8f0537f16cc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1742,7 +1742,7 @@ static int amdgpu_ttm_doorbell_init(struct amdgpu_device *adev)
>   	if (adev->asic_type < CHIP_BONAIRE) {
>   		adev->gmc.doorbell_aper_base = 0;
>   		adev->gmc.doorbell_aper_size = 0;
> -		adev->doorbell.num_doorbells = 0;
> +		adev->mman.num_doorbells = 0;
>   		adev->mman.doorbell_aper_base_kaddr = NULL;
>   		return 0;
>   	}
> @@ -1759,13 +1759,13 @@ static int amdgpu_ttm_doorbell_init(struct amdgpu_device *adev)
>                                                       adev->gmc.doorbell_aper_size);
>   
>   	if (adev->enable_mes) {
> -		adev->doorbell.num_doorbells =
> +		adev->mman.num_doorbells =
>   			adev->gmc.doorbell_aper_size / sizeof(u32);
>   	} else {
> -		adev->doorbell.num_doorbells =
> +		adev->mman.num_doorbells =
>   			min_t(u32, adev->gmc.doorbell_aper_size / sizeof(u32),
>   			      adev->doorbell_index.max_assignment+1);
> -		if (adev->doorbell.num_doorbells == 0)
> +		if (adev->mman.num_doorbells == 0)
>   			return -EINVAL;
>   
>   		/* For Vega, reserve and map two pages on doorbell BAR since SDMA
> @@ -1775,7 +1775,7 @@ static int amdgpu_ttm_doorbell_init(struct amdgpu_device *adev)
>   		 * the max num_doorbells should + 1 page (0x400 in dword)
>   		 */
>   		if (adev->asic_type >= CHIP_VEGA10)
> -			adev->doorbell.num_doorbells += 0x400;
> +			adev->mman.num_doorbells += 0x400;
>   	}
>   
>   	return 0;
> @@ -1901,6 +1901,18 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   	if (r)
>   		return r;
>   
> +	adev->mman.doorbell_kernel_bo_size = adev->mman.num_doorbells * sizeof(u32);
> +	r = amdgpu_bo_create_kernel_at(adev,
> +					  0,
> +					  adev->mman.doorbell_kernel_bo_size,
> +					  AMDGPU_GEM_DOMAIN_DOORBELL,
> +					  &adev->mman.doorbell_kernel_bo,
> +					  &adev->mman.doorbell_va);
> +	if (r) {
> +		DRM_ERROR("Failed to create doorbell BO\n");
> +		return r;
> +	}
> +
>   	DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>   		 (unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
>   
> @@ -1992,6 +2004,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   				      NULL, NULL);
>   	amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
>   					&adev->mman.sdma_access_ptr);
> +	amdgpu_bo_free_kernel(&adev->mman.doorbell_kernel_bo,
> +			      NULL, &adev->mman.doorbell_va);
>   	amdgpu_ttm_fw_reserve_vram_fini(adev);
>   	amdgpu_ttm_drv_reserve_vram_fini(adev);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index a492de079c2e..405294ca9834 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -98,6 +98,12 @@ struct amdgpu_mman {
>   	/* PAGE_SIZE'd BO for process memory r/w over SDMA. */
>   	struct amdgpu_bo	*sdma_access_bo;
>   	void			*sdma_access_ptr;
> +
> +	/* doorbells reserved for the kernel driver */
> +	u32			num_doorbells;	/* Number of doorbells actually reserved for amdgpu. */
> +	uint64_t		doorbell_kernel_bo_size;
> +	struct amdgpu_bo	*doorbell_kernel_bo;
> +	void			*doorbell_va;
>   };
>   
>   struct amdgpu_copy_mem {