On Thu, Nov 12, 2020 at 09:19:04PM +0530, Akhil P Oommen wrote: > So far a530v2 gpu has support for detecting its supported opps > based on a fuse value called speed-bin. This patch makes this > support generic across gpu families. This is in preparation to > extend speed-bin support to a6x family. > > Signed-off-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx> > --- > This patch is rebased on top of msm-next-staging branch in rob's tree. > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 -------------- > drivers/gpu/drm/msm/adreno/adreno_device.c | 4 ++ > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 71 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 +++ > 4 files changed, 80 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index 8fa5c91..7d42321 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -1531,38 +1531,6 @@ static const struct adreno_gpu_funcs funcs = { > .get_timestamp = a5xx_get_timestamp, > }; > > -static void check_speed_bin(struct device *dev) > -{ > - struct nvmem_cell *cell; > - u32 val; > - > - /* > - * If the OPP table specifies a opp-supported-hw property then we have > - * to set something with dev_pm_opp_set_supported_hw() or the table > - * doesn't get populated so pick an arbitrary value that should > - * ensure the default frequencies are selected but not conflict with any > - * actual bins > - */ > - val = 0x80; > - > - cell = nvmem_cell_get(dev, "speed_bin"); > - > - if (!IS_ERR(cell)) { > - void *buf = nvmem_cell_read(cell, NULL); > - > - if (!IS_ERR(buf)) { > - u8 bin = *((u8 *) buf); > - > - val = (1 << bin); > - kfree(buf); > - } > - > - nvmem_cell_put(cell); > - } > - > - dev_pm_opp_set_supported_hw(dev, &val, 1); > -} > - > struct msm_gpu *a5xx_gpu_init(struct drm_device *dev) > { > struct msm_drm_private *priv = dev->dev_private; > @@ -1588,8 +1556,6 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev) > > a5xx_gpu->lm_leakage = 0x4E001A; > > - check_speed_bin(&pdev->dev); > - > ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 4); > if (ret) { > a5xx_destroy(&(a5xx_gpu->base.base)); > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c > index 87c8b03..e0ff16c 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > @@ -18,6 +18,8 @@ bool snapshot_debugbus = false; > MODULE_PARM_DESC(snapshot_debugbus, "Include debugbus sections in GPU devcoredump (if not fused off)"); > module_param_named(snapshot_debugbus, snapshot_debugbus, bool, 0600); > > +const u32 a530v2_speedbins[] = {0, 1, 2, 3, 4, 5, 6, 7}; > + > static const struct adreno_info gpulist[] = { > { > .rev = ADRENO_REV(2, 0, 0, 0), > @@ -163,6 +165,8 @@ static const struct adreno_info gpulist[] = { > ADRENO_QUIRK_FAULT_DETECT_MASK, > .init = a5xx_gpu_init, > .zapfw = "a530_zap.mdt", > + .speedbins = a530v2_speedbins, > + .speedbins_count = ARRAY_SIZE(a530v2_speedbins), > }, { > .rev = ADRENO_REV(5, 4, 0, 2), > .revn = 540, > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > index f21561d..cdd0c11 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -14,6 +14,7 @@ > #include <linux/pm_opp.h> > #include <linux/slab.h> > #include <linux/soc/qcom/mdt_loader.h> > +#include <linux/nvmem-consumer.h> > #include <soc/qcom/ocmem.h> > #include "adreno_gpu.h" > #include "msm_gem.h" > @@ -891,6 +892,69 @@ void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem) > adreno_ocmem->hdl); > } > > +static int adreno_set_supported_hw(struct device *dev, > + struct adreno_gpu *adreno_gpu) > +{ > + u8 speedbins_count = adreno_gpu->info->speedbins_count; > + const u32 *speedbins = adreno_gpu->info->speedbins; We don't need to make this generic and put it in the table. Just call the function from the target specific code and pass the speedbin array and size from there. > + struct nvmem_cell *cell; > + u32 bin, i; > + u32 val = 0; > + void *buf, *opp_table; > + > + cell = nvmem_cell_get(dev, "speed_bin"); > + /* > + * -ENOENT means that the platform doesn't support speedbin which is > + * fine > + */ > + if (PTR_ERR(cell) == -ENOENT) > + return 0; > + else if (IS_ERR(cell)) > + return PTR_ERR(cell); > + > + /* A speedbin table is must if the platform supports speedbin */ > + if (!speedbins) { > + DRM_DEV_ERROR(dev, "speed-bin table is missing\n"); > + return -ENOENT; > + } > + > + buf = nvmem_cell_read(cell, NULL); > + if (IS_ERR(buf)) { > + nvmem_cell_put(cell); > + return PTR_ERR(buf); > + } > + > + bin = *((u32 *) buf); > + > + for (i = 0; i < speedbins_count; i++) { > + if (bin == speedbins[i]) { > + val = (1 << i); > + break; > + } > + } > + > + kfree(buf); > + nvmem_cell_put(cell); > + > + if (!val) { > + DRM_DEV_ERROR(dev, "missing support for speed-bin: %u\n", bin); > + return -ENOENT; > + } > + > + opp_table = dev_pm_opp_set_supported_hw(dev, &val, 1); > + if (IS_ERR(opp_table)) > + return PTR_ERR(opp_table); > + > + adreno_gpu->opp_table = opp_table; > + return 0; > +} > + > +static void adreno_put_supported_hw(struct opp_table *opp_table) > +{ > + if (opp_table) > + dev_pm_opp_put_supported_hw(opp_table); > +} > + > int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > struct adreno_gpu *adreno_gpu, > const struct adreno_gpu_funcs *funcs, int nr_rings) > @@ -899,6 +963,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > struct adreno_platform_config *config = dev->platform_data; > struct msm_gpu_config adreno_gpu_config = { 0 }; > struct msm_gpu *gpu = &adreno_gpu->base; > + int ret; > > adreno_gpu->funcs = funcs; > adreno_gpu->info = adreno_info(config->rev); > @@ -910,6 +975,10 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > > adreno_gpu_config.nr_rings = nr_rings; > > + ret = adreno_set_supported_hw(dev, adreno_gpu); > + if (ret) > + return ret; This bit should be in the target specific code > + > adreno_get_pwrlevels(dev, gpu); > > pm_runtime_set_autosuspend_delay(dev, > @@ -936,4 +1005,6 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) > > icc_put(gpu->icc_path); > icc_put(gpu->ocmem_icc_path); > + > + adreno_put_supported_hw(adreno_gpu->opp_table); And this bit too, though it would be easier to just call the put function directly without having a intermediate function. Also the OPP function should be NULL aware but thats a different story. Jordan > } > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > index c3775f7..a756ad7 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > @@ -55,6 +55,7 @@ struct adreno_reglist { > }; > > extern const struct adreno_reglist a630_hwcg[], a640_hwcg[], a650_hwcg[]; > +extern const u32 a618_speedbins[]; > > struct adreno_info { > struct adreno_rev rev; > @@ -67,6 +68,8 @@ struct adreno_info { > const char *zapfw; > u32 inactive_period; > const struct adreno_reglist *hwcg; > + const u32 *speedbins; > + const u8 speedbins_count; > }; > > const struct adreno_info *adreno_info(struct adreno_rev rev); > @@ -112,6 +115,8 @@ struct adreno_gpu { > * code (a3xx_gpu.c) and stored in this common location. > */ > const unsigned int *reg_offsets; > + > + struct opp_table *opp_table; > }; > #define to_adreno_gpu(x) container_of(x, struct adreno_gpu, base) > > -- > 2.7.4 > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel