diff mbox series

[4/7] drm/msm/a6xx: Add support for A650 speed binning

Message ID 20221213002423.259039-5-konrad.dybcio@linaro.org
State New
Headers
Series [1/7] dt-bindings: nvmem: Add compatible for SM8150 |

Commit Message

Konrad Dybcio Dec. 13, 2022, 12:24 a.m. UTC 
  Add support for matching QFPROM fuse values to get the correct speed bin
on A650 (SM8250) GPUs.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Doug Anderson Dec. 13, 2022, 3:23 p.m. UTC | #1 
Hi,

On Mon, Dec 12, 2022 at 4:24 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> Add support for matching QFPROM fuse values to get the correct speed bin
> on A650 (SM8250) GPUs.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  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
0xffffffff as the sentinel value.

-Doug
Konrad Dybcio Dec. 13, 2022, 3:34 p.m. UTC | #2 
On 13.12.2022 16:23, Doug Anderson wrote:
> Hi,
> 
> On Mon, Dec 12, 2022 at 4:24 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> Add support for matching QFPROM fuse values to get the correct speed bin
>> on A650 (SM8250) GPUs.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  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
> 0xffffffff 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<<ret, which
should be capped a bit lower than UINT_MAX, anyway. But I can
look into that in a separate patchset.

Konrad
> 
> -Doug
diff mbox series

Patch

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",