Re: [PATCH 4/7] drm/msm/a6xx: Add support for A650 speed binning
On 13.12.2022 16:23, Doug Anderson wrote:
> Hi,
>
> On Mon, Dec 12, 2022 at 4:24 PM Konrad Dybcio
> wrote:
>>
>> Add support for matching QFPROM fuse values to get the correct speed bin
>> on A650 (SM8250) GPUs.
>>
>> Signed-off-by: Konrad Dybcio
>> ---
>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 17 +
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> index 2c1630f0c04c..f139ec57c32d 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> @@ -1887,6 +1887,20 @@ static u32 a640_get_speed_bin(u32 fuse)
>> return UINT_MAX;
>> }
>>
>> +static u32 a650_get_speed_bin(u32 fuse)
>> +{
>> + if (fuse == 0)
>> + return 0;
>> + else if (fuse == 1)
>> + return 1;
>> + else if (fuse == 2)
>> + return 2;
>> + else if (fuse == 3)
>> + return 3;
>> +
>> + return UINT_MAX;
>
> Unlike some of the other functions, you don't need any complexity. Just do:
>
> if (fuse <= 3)
> return fuse;
>
> return UINT_MAX;
I'd prefer to keep it open-coded, it's just 8150 and 8250 that have
these simple fuse values, other SoCs have random numbers (check A618/
619 above, for example).. Plus the returned values might as well be
made-up, as it's just for opp matching.
>
>
> I'd also suggest that perhaps "UINT_MAX" isn't exactly the right
> return value for when we have an unrecognized fuse. The return type
> for the function is "u32" which is a fixed size type. UINT_MAX,
> however, is a type that is automatically sized by the compiler. Though
> it's unlikely, theoretically a compiler could be configured such that
> "unsigned int" was something other than 32 bits. Ideally either the
> return type would be changed to "unsigned int" or you'd return
> 0x as the sentinel value.
That's out of the scope of this patch, as it concerns all the
speedbin-supported GPUs. The returned value feeds 1<
> -Doug
Re: [PATCH 4/7] drm/msm/a6xx: Add support for A650 speed binning
Hi,
On Mon, Dec 12, 2022 at 4:24 PM Konrad Dybcio wrote:
>
> Add support for matching QFPROM fuse values to get the correct speed bin
> on A650 (SM8250) GPUs.
>
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 2c1630f0c04c..f139ec57c32d 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1887,6 +1887,20 @@ static u32 a640_get_speed_bin(u32 fuse)
> return UINT_MAX;
> }
>
> +static u32 a650_get_speed_bin(u32 fuse)
> +{
> + if (fuse == 0)
> + return 0;
> + else if (fuse == 1)
> + return 1;
> + else if (fuse == 2)
> + return 2;
> + else if (fuse == 3)
> + return 3;
> +
> + return UINT_MAX;
Unlike some of the other functions, you don't need any complexity. Just do:
if (fuse <= 3)
return fuse;
return UINT_MAX;
I'd also suggest that perhaps "UINT_MAX" isn't exactly the right
return value for when we have an unrecognized fuse. The return type
for the function is "u32" which is a fixed size type. UINT_MAX,
however, is a type that is automatically sized by the compiler. Though
it's unlikely, theoretically a compiler could be configured such that
"unsigned int" was something other than 32 bits. Ideally either the
return type would be changed to "unsigned int" or you'd return
0x as the sentinel value.
-Doug
[PATCH 4/7] drm/msm/a6xx: Add support for A650 speed binning
Add support for matching QFPROM fuse values to get the correct speed bin
on A650 (SM8250) GPUs.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 2c1630f0c04c..f139ec57c32d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1887,6 +1887,20 @@ static u32 a640_get_speed_bin(u32 fuse)
return UINT_MAX;
}
+static u32 a650_get_speed_bin(u32 fuse)
+{
+ if (fuse == 0)
+ return 0;
+ else if (fuse == 1)
+ return 1;
+ else if (fuse == 2)
+ return 2;
+ else if (fuse == 3)
+ return 3;
+
+ return UINT_MAX;
+}
+
static u32 adreno_7c3_get_speed_bin(u32 fuse)
{
if (fuse == 0)
@@ -1915,6 +1929,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
val = a640_get_speed_bin(fuse);
+ if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
+ val = a650_get_speed_bin(fuse);
+
if (val == UINT_MAX) {
DRM_DEV_ERROR(dev,
"missing support for speed-bin: %u. Some OPPs may not
be supported by hardware\n",
--
2.39.0
