On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> From: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
>
> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> the highest. Falling back to it when things go wrong is largely
> suboptimal, as more often than not, the top frequencies are not
> supposed to work on other bins.
Isn't it better to just return an error here instead of trying to guess
which speedbin to use?
If that's not the case, I think the commit should be expanded with
actually setting default_speedbin for the existing GPUs.
>
> Let the developer specify the intended "lowest common denominator" bin
> in struct adreno_info. If not specified, partial struct initialization
> will ensure it's set to zero, retaining previous behavior.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> [Konrad: clean up, add commit message]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 0674aca0f8a3..4cbdfabbcee5 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
> DRM_DEV_ERROR(dev,
> "missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
> speedbin);
> - supp_hw = BIT(0); /* Default */
> + supp_hw = BIT(info->default_speedbin); /* Default */
> }
>
> ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 77526892eb8c..460b399be37b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -110,6 +110,7 @@ struct adreno_info {
> * {SHRT_MAX, 0} sentinal.
> */
> struct adreno_speedbin *speedbins;
> + unsigned int default_speedbin;
> };
>
> #define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
