| Message ID | 20221213002423.259039-5-konrad.dybcio@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SM8[12]50 GPU speedbin | expand |
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
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 --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",
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(+)