Re: [PATCHv2 1/2] drm/amdkfd: Populate cache info for GFX 9.4.3

[Felix Kuehling <felix.kuehling@amd.com>]

On 2023-10-27 15:04, Mukul Joshi wrote:
> GFX 9.4.3 uses a new version of the GC info table which
> contains the cache info. This patch adds a new function
> to populate the cache info from IP discovery for GFX 9.4.3.
>
> Signed-off-by: Mukul Joshi <mukul.joshi@amd.com>
> ---
> v1->v2:
> - Separate out the original patch into 2 patches.
>
>   drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 66 ++++++++++++++++++++++++++-
>   1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 0e792a8496d6..cd8e459201f1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -1404,6 +1404,66 @@ static int kfd_fill_gpu_cache_info_from_gfx_config(struct kfd_dev *kdev,
>   	return i;
>   }
>   
> +static int kfd_fill_gpu_cache_info_from_gfx_config_v2(struct kfd_dev *kdev,
> +						   struct kfd_gpu_cache_info *pcache_info)
> +{
> +	struct amdgpu_device *adev = kdev->adev;
> +	int i = 0;
> +
> +	/* TCP L1 Cache per CU */
> +	if (adev->gfx.config.gc_tcp_size_per_cu) {
> +		pcache_info[i].cache_size = adev->gfx.config.gc_tcp_size_per_cu;
> +		pcache_info[i].cache_level = 1;
> +		pcache_info[i].flags = (CRAT_CACHE_FLAGS_ENABLED |
> +					CRAT_CACHE_FLAGS_DATA_CACHE |
> +					CRAT_CACHE_FLAGS_SIMD_CACHE);
> +		pcache_info[i].num_cu_shared = 1;
> +		i++;
> +	}
> +	/* Scalar L1 Instruction Cache per SQC */
> +	if (adev->gfx.config.gc_l1_instruction_cache_size_per_sqc) {
> +		pcache_info[i].cache_size =
> +			adev->gfx.config.gc_l1_instruction_cache_size_per_sqc;
> +		pcache_info[i].cache_level = 1;
> +		pcache_info[i].flags = (CRAT_CACHE_FLAGS_ENABLED |
> +					CRAT_CACHE_FLAGS_INST_CACHE |
> +					CRAT_CACHE_FLAGS_SIMD_CACHE);
> +		pcache_info[i].num_cu_shared = adev->gfx.config.gc_num_cu_per_sqc;
> +		i++;
> +	}
> +	/* Scalar L1 Data Cache per SQC */
> +	if (adev->gfx.config.gc_l1_data_cache_size_per_sqc) {
> +		pcache_info[i].cache_size = adev->gfx.config.gc_l1_data_cache_size_per_sqc;
> +		pcache_info[i].cache_level = 1;
> +		pcache_info[i].flags = (CRAT_CACHE_FLAGS_ENABLED |
> +					CRAT_CACHE_FLAGS_DATA_CACHE |
> +					CRAT_CACHE_FLAGS_SIMD_CACHE);
> +		pcache_info[i].num_cu_shared = adev->gfx.config.gc_num_cu_per_sqc;
> +		i++;
> +	}
> +	/* L2 Data Cache per GPU (Total Tex Cache) */
> +	if (adev->gfx.config.gc_tcc_size) {
> +		pcache_info[i].cache_size = adev->gfx.config.gc_tcc_size;
> +		pcache_info[i].cache_level = 2;
> +		pcache_info[i].flags = (CRAT_CACHE_FLAGS_ENABLED |
> +					CRAT_CACHE_FLAGS_DATA_CACHE |
> +					CRAT_CACHE_FLAGS_SIMD_CACHE);
> +		pcache_info[i].num_cu_shared = adev->gfx.config.max_cu_per_sh;
> +		i++;
> +	}
> +	/* L3 Data Cache per GPU */
> +	if (adev->gmc.mall_size) {
> +		pcache_info[i].cache_size = adev->gmc.mall_size / 1024;

Is this /1024 a unit conversion? What are the units for L1/L2 caches 
compared to L3 caches?

When we report the sizes in the topology, they should be in the same 
units for all cache levels, I believe. Given that L3 is likely the 
largest, I'm a bit suspicious of this conversion.

Other than that, the series is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> +		pcache_info[i].cache_level = 3;
> +		pcache_info[i].flags = (CRAT_CACHE_FLAGS_ENABLED |
> +					CRAT_CACHE_FLAGS_DATA_CACHE |
> +					CRAT_CACHE_FLAGS_SIMD_CACHE);
> +		pcache_info[i].num_cu_shared = adev->gfx.config.max_cu_per_sh;
> +		i++;
> +	}
> +	return i;
> +}
> +
>   int kfd_get_gpu_cache_info(struct kfd_node *kdev, struct kfd_gpu_cache_info **pcache_info)
>   {
>   	int num_of_cache_types = 0;
> @@ -1461,10 +1521,14 @@ int kfd_get_gpu_cache_info(struct kfd_node *kdev, struct kfd_gpu_cache_info **pc
>   			num_of_cache_types = ARRAY_SIZE(vega20_cache_info);
>   			break;
>   		case IP_VERSION(9, 4, 2):
> -		case IP_VERSION(9, 4, 3):
>   			*pcache_info = aldebaran_cache_info;
>   			num_of_cache_types = ARRAY_SIZE(aldebaran_cache_info);
>   			break;
> +		case IP_VERSION(9, 4, 3):
> +			num_of_cache_types =
> +				kfd_fill_gpu_cache_info_from_gfx_config_v2(kdev->kfd,
> +									*pcache_info);
> +			break;
>   		case IP_VERSION(9, 1, 0):
>   		case IP_VERSION(9, 2, 2):
>   			*pcache_info = raven_cache_info;