Re: [PATCH 09/12] drm/msm/adreno: Add adreno family

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 07/07/2023 02:35, Konrad Dybcio wrote:
> On 6.07.2023 23:10, Rob Clark wrote:
>> From: Rob Clark <robdclark@chromium.org>
>>
>> Sometimes it is useful to know the sub-generation (or "family").  And in
>> any case, this helps us get away from infering the generation from the
>> numerical chip-id.
>>
>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>> ---
> [...]
> 
>>   		.rev   = ADRENO_REV(5, 0, 8, ANY_ID),
>> +		.family = ADRENO_5XX,
>>   		.revn = 508,
> 508 is from 530 fam
> 
>>   		.fw = {
>>   			[ADRENO_FW_PM4] = "a530_pm4.fw",
>> @@ -156,6 +168,7 @@ static const struct adreno_info gpulist[] = {
>>   		.zapfw = "a508_zap.mdt",
>>   	}, {
>>   		.rev   = ADRENO_REV(5, 0, 9, ANY_ID),
>> +		.family = ADRENO_5XX,
>>   		.revn = 509,
> 509 and 512 are from 540 fam
> 
>>   		.fw = {
>>   			[ADRENO_FW_PM4] = "a530_pm4.fw",
>> @@ -173,6 +186,7 @@ static const struct adreno_info gpulist[] = {
>>   		.zapfw = "a512_zap.mdt",
>>   	}, {
>>   		.rev   = ADRENO_REV(5, 1, 0, ANY_ID),
>> +		.family = ADRENO_5XX,
>>   		.revn = 510,
> 510 is 530ish but I think it's closer to 505 or whatever the
> 8953 gpu was called

I'd say, there were following generations here:

- a505 / a506 / a508
- a509 / a512
- a510
- a530
- a540

Indeed a50x were close to a530 in some aspects and a509/512 being closer 
to a540, but I don't think they were the same family.

> 
> [...]
> 
>> -	priv->is_a2xx = config.rev.core == 2;
>> +	priv->is_a2xx = info->family < ADRENO_3XX;
> is this variable even needed now that there are explicit family values?
> 
> Konrad
>>   	priv->has_cached_coherent =
>>   		!!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT);
>>   
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> index 2e62a7ce9f13..75ff7fb46099 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> @@ -1079,8 +1079,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>>   	u32 speedbin;
>>   	int ret;
>>   
>> +	adreno_gpu->funcs = funcs;
>> +	adreno_gpu->info = adreno_info(config->rev);
>> +	adreno_gpu->rev = *rev;
>> +
>>   	/* Only handle the core clock when GMU is not in use (or is absent). */
>> -	if (adreno_has_gmu_wrapper(adreno_gpu) || config->rev.core < 6) {
>> +	if (adreno_has_gmu_wrapper(adreno_gpu) ||
>> +	    adreno_gpu->info->family < ADRENO_6XX_GEN1) {
>>   		/*
>>   		 * This can only be done before devm_pm_opp_of_add_table(), or
>>   		 * dev_pm_opp_set_config() will WARN_ON()
>> @@ -1096,10 +1101,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>>   			devm_pm_opp_set_clkname(dev, "core");
>>   	}
>>   
>> -	adreno_gpu->funcs = funcs;
>> -	adreno_gpu->info = adreno_info(config->rev);
>> -	adreno_gpu->rev = *rev;
>> -
>>   	if (adreno_read_speedbin(dev, &speedbin) || !speedbin)
>>   		speedbin = 0xffff;
>>   	adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin);
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> index 6066cfaaea52..2fa14dcd4e40 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> @@ -29,6 +29,25 @@ enum {
>>   	ADRENO_FW_MAX,
>>   };
>>   
>> +/**
>> + * @enum adreno_family: identify generation and possibly sub-generation
>> + *
>> + * In some cases there are distinct sub-generations within a major revision
>> + * so it helps to be able to group the GPU devices by generation and if
>> + * necessary sub-generation.
>> + */
>> +enum adreno_family {
>> +	ADRENO_2XX_GEN1,  /* a20x */
>> +	ADRENO_2XX_GEN2,  /* a22x */
>> +	ADRENO_3XX,
>> +	ADRENO_4XX,
>> +	ADRENO_5XX,
>> +	ADRENO_6XX_GEN1,  /* a630 family */
>> +	ADRENO_6XX_GEN2,  /* a640 family */
>> +	ADRENO_6XX_GEN3,  /* a650 family */
>> +	ADRENO_6XX_GEN4,  /* a660 family */
>> +};
>> +
>>   #define ADRENO_QUIRK_TWO_PASS_USE_WFI		BIT(0)
>>   #define ADRENO_QUIRK_FAULT_DETECT_MASK		BIT(1)
>>   #define ADRENO_QUIRK_LMLOADKILL_DISABLE		BIT(2)
>> @@ -63,6 +82,7 @@ extern const struct adreno_reglist a660_hwcg[], a690_hwcg[];
>>   struct adreno_info {
>>   	const char *machine;
>>   	struct adreno_rev rev;
>> +	enum adreno_family family;
>>   	uint32_t revn;
>>   	const char *fw[ADRENO_FW_MAX];
>>   	uint32_t gmem;
>> @@ -188,14 +208,14 @@ static inline bool adreno_is_a2xx(const struct adreno_gpu *gpu)
>>   {
>>   	if (WARN_ON_ONCE(!gpu->info))
>>   		return false;
>> -	return (gpu->info->revn < 300);
>> +	return gpu->info->family < ADRENO_2XX_GEN2;
>>   }
>>   
>>   static inline bool adreno_is_a20x(const struct adreno_gpu *gpu)
>>   {
>>   	if (WARN_ON_ONCE(!gpu->info))
>>   		return false;
>> -	return (gpu->info->revn < 210);
>> +	return gpu->info->family == ADRENO_2XX_GEN1;
>>   }
>>   
>>   static inline bool adreno_is_a225(const struct adreno_gpu *gpu)
>> @@ -338,29 +358,31 @@ static inline int adreno_is_a690(const struct adreno_gpu *gpu)
>>   /* check for a615, a616, a618, a619 or any a630 derivatives */
>>   static inline int adreno_is_a630_family(const struct adreno_gpu *gpu)
>>   {
>> -	return adreno_is_revn(gpu, 630) ||
>> -		adreno_is_revn(gpu, 615) ||
>> -		adreno_is_revn(gpu, 616) ||
>> -		adreno_is_revn(gpu, 618) ||
>> -		adreno_is_revn(gpu, 619);
>> +	if (WARN_ON_ONCE(!gpu->info))
>> +		return false;
>> +	return gpu->info->family == ADRENO_6XX_GEN1;
>>   }
>>   
>>   static inline int adreno_is_a660_family(const struct adreno_gpu *gpu)
>>   {
>> -	return adreno_is_a660(gpu) || adreno_is_a690(gpu) || adreno_is_7c3(gpu);
>> +	if (WARN_ON_ONCE(!gpu->info))
>> +		return false;
>> +	return gpu->info->family == ADRENO_6XX_GEN4;
>>   }
>>   
>>   /* check for a650, a660, or any derivatives */
>>   static inline int adreno_is_a650_family(const struct adreno_gpu *gpu)
>>   {
>> -	return adreno_is_revn(gpu, 650) ||
>> -		adreno_is_revn(gpu, 620) ||
>> -		adreno_is_a660_family(gpu);
>> +	if (WARN_ON_ONCE(!gpu->info))
>> +		return false;
>> +	return gpu->info->family >= ADRENO_6XX_GEN3;
>>   }
>>   
>>   static inline int adreno_is_a640_family(const struct adreno_gpu *gpu)
>>   {
>> -	return adreno_is_a640(gpu) || adreno_is_a680(gpu);
>> +	if (WARN_ON_ONCE(!gpu->info))
>> +		return false;
>> +	return gpu->info->family == ADRENO_6XX_GEN2;
>>   }
>>   
>>   u64 adreno_private_address_space_size(struct msm_gpu *gpu);

-- 
With best wishes
Dmitry