Re: [PATCH 07/13] drm/amdgpu: store doorbell info in gmc structure

[Christian König <christian.koenig@xxxxxxx>]

Am 03.02.23 um 20:08 schrieb Shashank Sharma:
> From: Alex Deucher <alexander.deucher@xxxxxxx>
>
> This patch moves doorbell info into adev->gmc structure, to align
> with vram info. This will prepare structures for proper ttm management
> of the doorbell BAR.

Mhm, this is most likely not a good idea either.

The doorbell isn't managed by the GMC in any way. That are two 
completely different things in hw if I'm not completely mistaken.

Christian.

> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  8 ++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_bar_mgr.c  |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 14 +++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h |  3 ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h      |  7 +++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c      |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c        |  2 +-
>   drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c       |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c       |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c       |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c       |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c       |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c       |  4 ++--
>   13 files changed, 34 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 58689b2a2d1c..28076da2258f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -106,13 +106,13 @@ static void amdgpu_doorbell_get_kfd_info(struct amdgpu_device *adev,
>   		 * not initialized as AMDGPU manages the whole
>   		 * doorbell space.
>   		 */
> -		*aperture_base = adev->doorbell.base;
> +		*aperture_base = adev->gmc.doorbell_aper_base;
>   		*aperture_size = 0;
>   		*start_offset = 0;
> -	} else if (adev->doorbell.size > adev->doorbell.num_doorbells *
> +	} else if (adev->gmc.doorbell_aper_size > adev->doorbell.num_doorbells *
>   						sizeof(u32)) {
> -		*aperture_base = adev->doorbell.base;
> -		*aperture_size = adev->doorbell.size;
> +		*aperture_base = adev->gmc.doorbell_aper_base;
> +		*aperture_size = adev->gmc.doorbell_aper_size;
>   		*start_offset = adev->doorbell.num_doorbells * sizeof(u32);
>   	} else {
>   		*aperture_base = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bar_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bar_mgr.c
> index 3257da5c3a66..0656e5bb4f05 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bar_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bar_mgr.c
> @@ -209,7 +209,7 @@ static ssize_t amdgpu_mem_info_doorbell_total_show(struct device *dev,
>   	struct drm_device *ddev = dev_get_drvdata(dev);
>   	struct amdgpu_device *adev = drm_to_adev(ddev);
>   
> -	return sysfs_emit(buf, "%llu\n", adev->doorbell.size);
> +	return sysfs_emit(buf, "%llu\n", adev->gmc.doorbell_aper_size);
>   }
>   
>   /**
> @@ -897,7 +897,7 @@ int amdgpu_bar_mgr_init(struct amdgpu_device *adev, u32 domain)
>   		size = adev->gmc.real_vram_size;
>   	} else if (domain == AMDGPU_PL_DOORBELL) {
>   		mgr = &adev->mman.doorbell_mgr;
> -		size = adev->doorbell.size;
> +		size = adev->gmc.doorbell_aper_size;
>   	} else {
>   		return -EINVAL;
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 45588b7919fe..16580d9580d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1035,8 +1035,8 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>   
>   	/* No doorbell on SI hardware generation */
>   	if (adev->asic_type < CHIP_BONAIRE) {
> -		adev->doorbell.base = 0;
> -		adev->doorbell.size = 0;
> +		adev->gmc.doorbell_aper_base = 0;
> +		adev->gmc.doorbell_aper_size = 0;
>   		adev->doorbell.num_doorbells = 0;
>   		adev->doorbell.ptr = NULL;
>   		return 0;
> @@ -1048,15 +1048,15 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>   	amdgpu_asic_init_doorbell_index(adev);
>   
>   	/* doorbell bar mapping */
> -	adev->doorbell.base = pci_resource_start(adev->pdev, 2);
> -	adev->doorbell.size = pci_resource_len(adev->pdev, 2);
> +	adev->gmc.doorbell_aper_base = pci_resource_start(adev->pdev, 2);
> +	adev->gmc.doorbell_aper_size = pci_resource_len(adev->pdev, 2);
>   
>   	if (adev->enable_mes) {
>   		adev->doorbell.num_doorbells =
> -			adev->doorbell.size / sizeof(u32);
> +			adev->gmc.doorbell_aper_size / sizeof(u32);
>   	} else {
>   		adev->doorbell.num_doorbells =
> -			min_t(u32, adev->doorbell.size / sizeof(u32),
> +			min_t(u32, adev->gmc.doorbell_aper_size / sizeof(u32),
>   			      adev->doorbell_index.max_assignment+1);
>   		if (adev->doorbell.num_doorbells == 0)
>   			return -EINVAL;
> @@ -1071,7 +1071,7 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
>   			adev->doorbell.num_doorbells += 0x400;
>   	}
>   
> -	adev->doorbell.ptr = ioremap(adev->doorbell.base,
> +	adev->doorbell.ptr = ioremap(adev->gmc.doorbell_aper_base,
>   				     adev->doorbell.num_doorbells *
>   				     sizeof(u32));
>   	if (adev->doorbell.ptr == NULL)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> index 7199b6b0be81..c6324970eb79 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> @@ -25,9 +25,6 @@
>    * GPU doorbell structures, functions & helpers
>    */
>   struct amdgpu_doorbell {
> -	/* doorbell mmio */
> -	resource_size_t		base;
> -	resource_size_t		size;
>   	u32 __iomem		*ptr;
>   	u32			num_doorbells;	/* Number of doorbells actually reserved for amdgpu. */
>   };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index bb7076ecbf01..2a6636a7f27b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -173,6 +173,13 @@ struct amdgpu_gmc {
>   	 * about vram size near mc fb location */
>   	u64			mc_vram_size;
>   	u64			visible_vram_size;
> +	/* DOORBELL's physical address in MMIO space (for CPU to
> +	 * map DOORBELL). This is different compared to the agp/
> +	 * gart/vram_start/end field as the later is from
> +	 * GPU's view and aper_base is from CPU's view.
> +	 */
> +	resource_size_t		doorbell_aper_size;
> +	resource_size_t		doorbell_aper_base;
>   	/* AGP aperture start and end in MC address space
>   	 * Driver find a hole in the MC address space
>   	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 0c546245793b..1e09c3267c8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -126,7 +126,7 @@ static int amdgpu_mes_doorbell_init(struct amdgpu_device *adev)
>   		roundup(doorbell_start_offset,
>   			amdgpu_mes_doorbell_process_slice(adev));
>   
> -	doorbell_aperture_size = adev->doorbell.size;
> +	doorbell_aperture_size = adev->gmc.doorbell_aper_size;
>   	doorbell_aperture_size =
>   			rounddown(doorbell_aperture_size,
>   				  amdgpu_mes_doorbell_process_slice(adev));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index f202b45c413c..ebc17884df1e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3526,7 +3526,7 @@ static int gfx_v9_0_kiq_init_register(struct amdgpu_ring *ring)
>   		 */
>   		if (check_if_enlarge_doorbell_range(adev))
>   			WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
> -					(adev->doorbell.size - 4));
> +					(adev->gmc.doorbell_aper_size - 4));
>   		else
>   			WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
>   					(adev->doorbell_index.userqueue_end * 2) << 2);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> index aa761ff3a5fa..5969c159d7a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
> @@ -173,9 +173,9 @@ static void nbio_v2_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>   				    DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>   
>   		WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> -			     lower_32_bits(adev->doorbell.base));
> +			     lower_32_bits(adev->gmc.doorbell_aper_base));
>   		WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> -			     upper_32_bits(adev->doorbell.base));
> +			     upper_32_bits(adev->gmc.doorbell_aper_base));
>   	}
>   
>   	WREG32_SOC15(NBIO, 0, mmBIF_BX_PF_DOORBELL_SELFRING_GPA_APER_CNTL,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> index 15eb3658d70e..5ff12887ffab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
> @@ -169,9 +169,9 @@ static void nbio_v4_3_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>   				    DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>   
>   		WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> -			     lower_32_bits(adev->doorbell.base));
> +			     lower_32_bits(adev->gmc.doorbell_aper_base));
>   		WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> -			     upper_32_bits(adev->doorbell.base));
> +			     upper_32_bits(adev->gmc.doorbell_aper_base));
>   	}
>   
>   	WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> index 37615a77287b..1465e74e9122 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
> @@ -121,9 +121,9 @@ static void nbio_v6_1_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>   		      REG_SET_FIELD(tmp, BIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>   
>   		WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> -			     lower_32_bits(adev->doorbell.base));
> +			     lower_32_bits(adev->gmc.doorbell_aper_base));
>   		WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> -			     upper_32_bits(adev->doorbell.base));
> +			     upper_32_bits(adev->gmc.doorbell_aper_base));
>   	}
>   
>   	WREG32_SOC15(NBIO, 0, mmBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL, tmp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> index 31776b12e4c4..33bc23564870 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_2.c
> @@ -175,10 +175,10 @@ static void nbio_v7_2_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>   
>   		WREG32_SOC15(NBIO, 0,
>   			regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> -			lower_32_bits(adev->doorbell.base));
> +			lower_32_bits(adev->gmc.doorbell_aper_base));
>   		WREG32_SOC15(NBIO, 0,
>   			regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> -			upper_32_bits(adev->doorbell.base));
> +			upper_32_bits(adev->gmc.doorbell_aper_base));
>   	}
>   
>   	WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> index 19455a725939..4ce9d78aee1a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
> @@ -223,9 +223,9 @@ static void nbio_v7_4_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>   		      REG_SET_FIELD(tmp, DOORBELL_SELFRING_GPA_APER_CNTL, DOORBELL_SELFRING_GPA_APER_SIZE, 0);
>   
>   		WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_LOW,
> -			     lower_32_bits(adev->doorbell.base));
> +			     lower_32_bits(adev->gmc.doorbell_aper_base));
>   		WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> -			     upper_32_bits(adev->doorbell.base));
> +			     upper_32_bits(adev->gmc.doorbell_aper_base));
>   	}
>   
>   	WREG32_SOC15(NBIO, 0, mmDOORBELL_SELFRING_GPA_APER_CNTL, tmp);
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> index def89379b51a..1f1fa8cc4790 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_7.c
> @@ -132,10 +132,10 @@ static void nbio_v7_7_enable_doorbell_selfring_aperture(struct amdgpu_device *ad
>   
>   		WREG32_SOC15(NBIO, 0,
>   			regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_LOW,
> -			lower_32_bits(adev->doorbell.base));
> +			lower_32_bits(adev->gmc.doorbell_aper_base));
>   		WREG32_SOC15(NBIO, 0,
>   			regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_BASE_HIGH,
> -			upper_32_bits(adev->doorbell.base));
> +			upper_32_bits(adev->gmc.doorbell_aper_base));
>   	}
>   
>   	WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_DOORBELL_SELFRING_GPA_APER_CNTL,