* [PATCH 0/5] Add support for GPU DDR BW scaling @ 2020-03-31 7:55 Sharat Masetty 2020-03-31 7:55 ` [PATCH 1/5] arm64: dts: qcom: sc7180: Add interconnect bindings for GPU Sharat Masetty ` (4 more replies) 0 siblings, 5 replies; 10+ messages in thread From: Sharat Masetty @ 2020-03-31 7:55 UTC (permalink / raw) To: freedreno, devicetree Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty This series adds support for GPU DDR bandwidth scaling and is based on the bindings from Sarvana[1]. This work is based on Sibi's work for CPU side [2] which also lists all the needed dependencies to get this series working. My workspace is based on a chrome tag [3]. Although the bindings add support for both peak and average bandwidth votes, I have only added support for peak bandwidth votes. [1]: https://patchwork.kernel.org/cover/11277199/ [2]: https://patchwork.kernel.org/cover/11353185/ (this lists further dependencies) [3]: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2097039/3 Sharat Masetty (5): arm64: dts: qcom: sc7180: Add interconnect bindings for GPU arm64: dts: qcom: sc7180: Add GPU DDR BW opp table drm: msm: scale DDR BW along with GPU frequency drm: msm: a6xx: Fix off by one error when setting GPU freq dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU .../devicetree/bindings/display/msm/gpu.txt | 63 +++++++++++++++++++++- arch/arm64/boot/dts/qcom/sc7180.dtsi | 52 +++++++++++++++++- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 43 ++++++++++++--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 44 +++++++++++++-- drivers/gpu/drm/msm/msm_gpu.h | 9 ++++ 5 files changed, 197 insertions(+), 14 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] arm64: dts: qcom: sc7180: Add interconnect bindings for GPU 2020-03-31 7:55 [PATCH 0/5] Add support for GPU DDR BW scaling Sharat Masetty @ 2020-03-31 7:55 ` Sharat Masetty 2020-03-31 7:55 ` [PATCH 2/5] arm64: dts: qcom: sc7180: Add GPU DDR BW opp table Sharat Masetty ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Sharat Masetty @ 2020-03-31 7:55 UTC (permalink / raw) To: freedreno, devicetree Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty This patch adds the interconnect bindings to the GPU node. This enables the GPU->DDR path bandwidth voting. Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 1097e8b..51630dd 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1718,6 +1718,8 @@ operating-points-v2 = <&gpu_opp_table>; qcom,gmu = <&gmu>; + interconnects = <&gem_noc MASTER_GFX3D &mc_virt SLAVE_EBI1>; + gpu_opp_table: opp-table { compatible = "operating-points-v2"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] arm64: dts: qcom: sc7180: Add GPU DDR BW opp table 2020-03-31 7:55 [PATCH 0/5] Add support for GPU DDR BW scaling Sharat Masetty 2020-03-31 7:55 ` [PATCH 1/5] arm64: dts: qcom: sc7180: Add interconnect bindings for GPU Sharat Masetty @ 2020-03-31 7:55 ` Sharat Masetty 2020-03-31 7:55 ` [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency Sharat Masetty ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Sharat Masetty @ 2020-03-31 7:55 UTC (permalink / raw) To: freedreno, devicetree Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty This patch adds a new opp table listing the GPU DDR bandwidth opps. Also adds a required_opp binding to the GPUs main OPP table which holds a phandle to a bandwidth opp in the new table. This enables linking the GPU power level opp to the DDR bandwidth opp and helps with scaling DDR along with GPU frequency. Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 50 +++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 51630dd..74b023b 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1715,7 +1715,8 @@ reg-names = "kgsl_3d0_reg_memory", "cx_mem", "cx_dbgc"; interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>; iommus = <&adreno_smmu 0>; - operating-points-v2 = <&gpu_opp_table>; + operating-points-v2 = <&gpu_opp_table>, + <&gpu_ddr_bw_opp_table>; qcom,gmu = <&gmu>; interconnects = <&gem_noc MASTER_GFX3D &mc_virt SLAVE_EBI1>; @@ -1726,40 +1727,87 @@ opp-800000000 { opp-hz = /bits/ 64 <800000000>; opp-level = <RPMH_REGULATOR_LEVEL_TURBO>; + required-opps = <&gpu_ddr_bw_opp9>; }; opp-650000000 { opp-hz = /bits/ 64 <650000000>; opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>; + required-opps = <&gpu_ddr_bw_opp8>; }; opp-565000000 { opp-hz = /bits/ 64 <565000000>; opp-level = <RPMH_REGULATOR_LEVEL_NOM>; + required-opps = <&gpu_ddr_bw_opp6>; }; opp-430000000 { opp-hz = /bits/ 64 <430000000>; opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>; + required-opps = <&gpu_ddr_bw_opp6>; }; opp-355000000 { opp-hz = /bits/ 64 <355000000>; opp-level = <RPMH_REGULATOR_LEVEL_SVS>; + required-opps = <&gpu_ddr_bw_opp4>; }; opp-267000000 { opp-hz = /bits/ 64 <267000000>; opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>; + required-opps = <&gpu_ddr_bw_opp4>; }; opp-180000000 { opp-hz = /bits/ 64 <180000000>; opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>; + required-opps = <&gpu_ddr_bw_opp2>; }; }; }; + gpu_ddr_bw_opp_table: gpu-ddr-bw-opp-table { + compatible = "operating-points-v2"; + + gpu_ddr_bw_opp1: opp-300000000 { + opp-peak-kBps =/bits/ 32 <1200000>; + }; + + gpu_ddr_bw_opp2: opp-451000000 { + opp-peak-kBps =/bits/ 32 <1804000>; + }; + + gpu_ddr_bw_opp3: opp-547000000 { + opp-peak-kBps =/bits/ 32 <2188000>; + }; + + gpu_ddr_bw_opp4: opp-768000000 { + opp-peak-kBps =/bits/ 32 <3072000>; + }; + + gpu_ddr_bw_opp5: opp-1017000000 { + opp-peak-kBps =/bits/ 32 <4068000>; + }; + + gpu_ddr_bw_opp6: opp-1353000000 { + opp-peak-kBps =/bits/ 32 <5412000>; + }; + + gpu_ddr_bw_opp7: opp-1555000000 { + opp-peak-kBps =/bits/ 32 <6220000>; + }; + + gpu_ddr_bw_opp8: opp-1804000000 { + opp-peak-kBps =/bits/ 32 <7216000>; + }; + + gpu_ddr_bw_opp9: opp-2133000000 { + opp-peak-kBps =/bits/ 32 <8532000>; + }; + }; + adreno_smmu: iommu@5040000 { compatible = "qcom,sc7180-smmu-v2", "qcom,smmu-v2"; reg = <0 0x05040000 0 0x10000>; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency 2020-03-31 7:55 [PATCH 0/5] Add support for GPU DDR BW scaling Sharat Masetty 2020-03-31 7:55 ` [PATCH 1/5] arm64: dts: qcom: sc7180: Add interconnect bindings for GPU Sharat Masetty 2020-03-31 7:55 ` [PATCH 2/5] arm64: dts: qcom: sc7180: Add GPU DDR BW opp table Sharat Masetty @ 2020-03-31 7:55 ` Sharat Masetty 2020-03-31 17:26 ` Jordan Crouse 2020-03-31 7:55 ` [PATCH 4/5] drm: msm: a6xx: Fix off by one error when setting GPU freq Sharat Masetty 2020-03-31 7:55 ` [PATCH 5/5] dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU Sharat Masetty 4 siblings, 1 reply; 10+ messages in thread From: Sharat Masetty @ 2020-03-31 7:55 UTC (permalink / raw) To: freedreno, devicetree Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty This patch adds support to parse the OPP tables attached the GPU device, the main opp table and the DDR bandwidth opp table. Additionally, vote for the GPU->DDR bandwidth when setting the GPU frequency by querying the linked DDR BW opp to the GPU opp. Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 41 ++++++++++++++++++++++++++---- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 44 +++++++++++++++++++++++++++++---- drivers/gpu/drm/msm/msm_gpu.h | 9 +++++++ 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 748cd37..489d9b6 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -100,6 +100,40 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF)); } +void a6xx_gmu_set_icc_vote(struct msm_gpu *gpu, unsigned long gpu_freq) +{ + struct dev_pm_opp *gpu_opp, *ddr_opp; + struct opp_table **tables = gpu->opp_tables; + unsigned long peak_bw; + + if (!gpu->opp_tables[GPU_DDR_OPP_TABLE_INDEX]) + goto done; + + gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true); + if (IS_ERR_OR_NULL(gpu_opp)) + goto done; + + ddr_opp = dev_pm_opp_xlate_required_opp(tables[GPU_OPP_TABLE_INDEX], + tables[GPU_DDR_OPP_TABLE_INDEX], + gpu_opp); + dev_pm_opp_put(gpu_opp); + + if (IS_ERR_OR_NULL(ddr_opp)) + goto done; + + peak_bw = dev_pm_opp_get_bw(ddr_opp, NULL); + dev_pm_opp_put(ddr_opp); + + icc_set_bw(gpu->icc_path, 0, peak_bw); + return; +done: + /* + * If there is a problem, for now leave it at max so that the + * performance is nominal. + */ + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); +} + static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) { struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); @@ -128,11 +162,8 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) gmu->freq = gmu->gpu_freqs[index]; - /* - * Eventually we will want to scale the path vote with the frequency but - * for now leave it at max so that the performance is nominal. - */ - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + if (gpu->icc_path) + a6xx_gmu_set_icc_vote(gpu, gmu->freq); } void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 2d13694..bbbcc7a 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -882,7 +882,7 @@ static int adreno_get_pwrlevels(struct device *dev, { unsigned long freq = ULONG_MAX; struct dev_pm_opp *opp; - int ret; + int ret, i; gpu->fast_rate = 0; @@ -890,9 +890,29 @@ static int adreno_get_pwrlevels(struct device *dev, if (!of_find_property(dev->of_node, "operating-points-v2", NULL)) ret = adreno_get_legacy_pwrlevels(dev); else { - ret = dev_pm_opp_of_add_table(dev); - if (ret) - DRM_DEV_ERROR(dev, "Unable to set the OPP table\n"); + int count = of_count_phandle_with_args(dev->of_node, + "operating-points-v2", NULL); + + count = min(count, GPU_DDR_OPP_TABLE_INDEX + 1); + count = max(count, 1); + + for (i = 0; i < count; i++) { + ret = dev_pm_opp_of_add_table_indexed(dev, i); + if (ret) { + DRM_DEV_ERROR(dev, "Add OPP table %d: failed %d\n", + i, ret); + goto err; + } + + gpu->opp_tables[i] = + dev_pm_opp_get_opp_table_indexed(dev, i); + if (!gpu->opp_tables[i]) { + DRM_DEV_ERROR(dev, "Get OPP table failed index %d\n", + i); + ret = -EINVAL; + goto err; + } + } } if (!ret) { @@ -919,12 +939,24 @@ static int adreno_get_pwrlevels(struct device *dev, gpu->icc_path = NULL; return 0; +err: + for (; i >= 0; i--) { + if (gpu->opp_tables[i]) { + dev_pm_opp_put_opp_table(gpu->opp_tables[i]); + gpu->opp_tables[i] = NULL; + } + } + + dev_pm_opp_remove_table(dev); + return ret; } 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) { + int ret = 0; + struct adreno_platform_config *config = pdev->dev.platform_data; struct msm_gpu_config adreno_gpu_config = { 0 }; struct msm_gpu *gpu = &adreno_gpu->base; @@ -945,7 +977,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, adreno_gpu_config.nr_rings = nr_rings; - adreno_get_pwrlevels(&pdev->dev, gpu); + ret = adreno_get_pwrlevels(&pdev->dev, gpu); + if (ret) + return ret; pm_runtime_set_autosuspend_delay(&pdev->dev, adreno_gpu->info->inactive_period); diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index ab8f0f9c..5b98b48 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -66,6 +66,12 @@ struct msm_gpu_funcs { void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq); }; +/* opp table indices */ +enum { + GPU_OPP_TABLE_INDEX, + GPU_DDR_OPP_TABLE_INDEX, +}; + struct msm_gpu { const char *name; struct drm_device *dev; @@ -113,6 +119,9 @@ struct msm_gpu { struct icc_path *icc_path; + /* gpu/ddr opp tables */ + struct opp_table *opp_tables[2]; + /* Hang and Inactivity Detection: */ #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency 2020-03-31 7:55 ` [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency Sharat Masetty @ 2020-03-31 17:26 ` Jordan Crouse 2020-04-01 12:30 ` Sharat Masetty 0 siblings, 1 reply; 10+ messages in thread From: Jordan Crouse @ 2020-03-31 17:26 UTC (permalink / raw) To: Sharat Masetty Cc: freedreno, devicetree, dri-devel, linux-arm-msm, linux-kernel, mka, sibis, saravanak, viresh.kumar On Tue, Mar 31, 2020 at 01:25:51PM +0530, Sharat Masetty wrote: > This patch adds support to parse the OPP tables attached the GPU device, > the main opp table and the DDR bandwidth opp table. Additionally, vote > for the GPU->DDR bandwidth when setting the GPU frequency by querying > the linked DDR BW opp to the GPU opp. > > Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 41 ++++++++++++++++++++++++++---- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 44 +++++++++++++++++++++++++++++---- > drivers/gpu/drm/msm/msm_gpu.h | 9 +++++++ > 3 files changed, 84 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 748cd37..489d9b6 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -100,6 +100,40 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) > A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF)); > } > > +void a6xx_gmu_set_icc_vote(struct msm_gpu *gpu, unsigned long gpu_freq) > +{ > + struct dev_pm_opp *gpu_opp, *ddr_opp; > + struct opp_table **tables = gpu->opp_tables; > + unsigned long peak_bw; > + > + if (!gpu->opp_tables[GPU_DDR_OPP_TABLE_INDEX]) > + goto done; > + > + gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true); > + if (IS_ERR_OR_NULL(gpu_opp)) > + goto done; > + > + ddr_opp = dev_pm_opp_xlate_required_opp(tables[GPU_OPP_TABLE_INDEX], > + tables[GPU_DDR_OPP_TABLE_INDEX], > + gpu_opp); > + dev_pm_opp_put(gpu_opp); > + > + if (IS_ERR_OR_NULL(ddr_opp)) > + goto done; I think that the final approach is still up in the air but either way we're going to pull the bandwidth from an OPP, its just a question of which OPP. > + peak_bw = dev_pm_opp_get_bw(ddr_opp, NULL); > + dev_pm_opp_put(ddr_opp); > + > + icc_set_bw(gpu->icc_path, 0, peak_bw); > + return; > +done: > + /* > + * If there is a problem, for now leave it at max so that the > + * performance is nominal. > + */ > + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > +} > + > static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) > { > struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); > @@ -128,11 +162,8 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) > > gmu->freq = gmu->gpu_freqs[index]; > > - /* > - * Eventually we will want to scale the path vote with the frequency but > - * for now leave it at max so that the performance is nominal. > - */ > - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > + if (gpu->icc_path) > + a6xx_gmu_set_icc_vote(gpu, gmu->freq); This function is annoying because we call it from two different spots, but it feels wasteful that devfreq gives us an OPP pointer and we go out of our way to not use it only to search for it again in the set_icc_vote function. I think maybe we should pass the OPP through from msm_gpu.c. We could have a helper function to pull the initial opp in a6xx_gmu_resume to make it clean. > } > > void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > index 2d13694..bbbcc7a 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -882,7 +882,7 @@ static int adreno_get_pwrlevels(struct device *dev, > { > unsigned long freq = ULONG_MAX; > struct dev_pm_opp *opp; > - int ret; > + int ret, i; > > gpu->fast_rate = 0; > > @@ -890,9 +890,29 @@ static int adreno_get_pwrlevels(struct device *dev, > if (!of_find_property(dev->of_node, "operating-points-v2", NULL)) > ret = adreno_get_legacy_pwrlevels(dev); > else { > - ret = dev_pm_opp_of_add_table(dev); > - if (ret) > - DRM_DEV_ERROR(dev, "Unable to set the OPP table\n"); > + int count = of_count_phandle_with_args(dev->of_node, > + "operating-points-v2", NULL); > + > + count = min(count, GPU_DDR_OPP_TABLE_INDEX + 1); > + count = max(count, 1); > + > + for (i = 0; i < count; i++) { > + ret = dev_pm_opp_of_add_table_indexed(dev, i); > + if (ret) { > + DRM_DEV_ERROR(dev, "Add OPP table %d: failed %d\n", > + i, ret); > + goto err; > + } > + > + gpu->opp_tables[i] = > + dev_pm_opp_get_opp_table_indexed(dev, i); > + if (!gpu->opp_tables[i]) { > + DRM_DEV_ERROR(dev, "Get OPP table failed index %d\n", > + i); > + ret = -EINVAL; > + goto err; > + } > + } > } > > if (!ret) { > @@ -919,12 +939,24 @@ static int adreno_get_pwrlevels(struct device *dev, > gpu->icc_path = NULL; > > return 0; > +err: > + for (; i >= 0; i--) { > + if (gpu->opp_tables[i]) { > + dev_pm_opp_put_opp_table(gpu->opp_tables[i]); > + gpu->opp_tables[i] = NULL; > + } > + } > + > + dev_pm_opp_remove_table(dev); > + return ret; > } > > 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) > { > + int ret = 0; > + > struct adreno_platform_config *config = pdev->dev.platform_data; > struct msm_gpu_config adreno_gpu_config = { 0 }; > struct msm_gpu *gpu = &adreno_gpu->base; > @@ -945,7 +977,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > > adreno_gpu_config.nr_rings = nr_rings; > > - adreno_get_pwrlevels(&pdev->dev, gpu); > + ret = adreno_get_pwrlevels(&pdev->dev, gpu); > + if (ret) > + return ret; > > pm_runtime_set_autosuspend_delay(&pdev->dev, > adreno_gpu->info->inactive_period); > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index ab8f0f9c..5b98b48 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.h > +++ b/drivers/gpu/drm/msm/msm_gpu.h > @@ -66,6 +66,12 @@ struct msm_gpu_funcs { > void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq); > }; > > +/* opp table indices */ > +enum { > + GPU_OPP_TABLE_INDEX, > + GPU_DDR_OPP_TABLE_INDEX, > +}; > + > struct msm_gpu { > const char *name; > struct drm_device *dev; > @@ -113,6 +119,9 @@ struct msm_gpu { > > struct icc_path *icc_path; > > + /* gpu/ddr opp tables */ > + struct opp_table *opp_tables[2]; You don't need an array here. We're not going to have that many tables. struct opp_table *gpu_opp_table; struct opp_table *bw_opp_table; Is sufficient and we don't need an enum. > + > /* Hang and Inactivity Detection: > */ > #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ > -- > 2.7.4 > Jordan -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency 2020-03-31 17:26 ` Jordan Crouse @ 2020-04-01 12:30 ` Sharat Masetty 0 siblings, 0 replies; 10+ messages in thread From: Sharat Masetty @ 2020-04-01 12:30 UTC (permalink / raw) To: freedreno, devicetree, dri-devel, linux-arm-msm, linux-kernel, mka, sibis, saravanak, viresh.kumar On 3/31/2020 10:56 PM, Jordan Crouse wrote: > On Tue, Mar 31, 2020 at 01:25:51PM +0530, Sharat Masetty wrote: >> This patch adds support to parse the OPP tables attached the GPU device, >> the main opp table and the DDR bandwidth opp table. Additionally, vote >> for the GPU->DDR bandwidth when setting the GPU frequency by querying >> the linked DDR BW opp to the GPU opp. >> >> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 41 ++++++++++++++++++++++++++---- >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 44 +++++++++++++++++++++++++++++---- >> drivers/gpu/drm/msm/msm_gpu.h | 9 +++++++ >> 3 files changed, 84 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> index 748cd37..489d9b6 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> @@ -100,6 +100,40 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) >> A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF)); >> } >> >> +void a6xx_gmu_set_icc_vote(struct msm_gpu *gpu, unsigned long gpu_freq) >> +{ >> + struct dev_pm_opp *gpu_opp, *ddr_opp; >> + struct opp_table **tables = gpu->opp_tables; >> + unsigned long peak_bw; >> + >> + if (!gpu->opp_tables[GPU_DDR_OPP_TABLE_INDEX]) >> + goto done; >> + >> + gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true); >> + if (IS_ERR_OR_NULL(gpu_opp)) >> + goto done; >> + >> + ddr_opp = dev_pm_opp_xlate_required_opp(tables[GPU_OPP_TABLE_INDEX], >> + tables[GPU_DDR_OPP_TABLE_INDEX], >> + gpu_opp); >> + dev_pm_opp_put(gpu_opp); >> + >> + if (IS_ERR_OR_NULL(ddr_opp)) >> + goto done; > I think that the final approach is still up in the air but either way we're > going to pull the bandwidth from an OPP, its just a question of which OPP. > >> + peak_bw = dev_pm_opp_get_bw(ddr_opp, NULL); >> + dev_pm_opp_put(ddr_opp); >> + >> + icc_set_bw(gpu->icc_path, 0, peak_bw); >> + return; >> +done: >> + /* >> + * If there is a problem, for now leave it at max so that the >> + * performance is nominal. >> + */ >> + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); >> +} >> + >> static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) >> { >> struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); >> @@ -128,11 +162,8 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) >> >> gmu->freq = gmu->gpu_freqs[index]; >> >> - /* >> - * Eventually we will want to scale the path vote with the frequency but >> - * for now leave it at max so that the performance is nominal. >> - */ >> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); >> + if (gpu->icc_path) >> + a6xx_gmu_set_icc_vote(gpu, gmu->freq); > This function is annoying because we call it from two different spots, but it > feels wasteful that devfreq gives us an OPP pointer and we go out of our way to > not use it only to search for it again in the set_icc_vote function. I think > maybe we should pass the OPP through from msm_gpu.c. We could have a helper > function to pull the initial opp in a6xx_gmu_resume to make it clean. Yes Jordan, it makes sense. I did think about this too, but may be I was a bit too lazy to change the existing plumbing :) I will take care of this in the next iteration. > >> } >> >> void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> index 2d13694..bbbcc7a 100644 >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c >> @@ -882,7 +882,7 @@ static int adreno_get_pwrlevels(struct device *dev, >> { >> unsigned long freq = ULONG_MAX; >> struct dev_pm_opp *opp; >> - int ret; >> + int ret, i; >> >> gpu->fast_rate = 0; >> >> @@ -890,9 +890,29 @@ static int adreno_get_pwrlevels(struct device *dev, >> if (!of_find_property(dev->of_node, "operating-points-v2", NULL)) >> ret = adreno_get_legacy_pwrlevels(dev); >> else { >> - ret = dev_pm_opp_of_add_table(dev); >> - if (ret) >> - DRM_DEV_ERROR(dev, "Unable to set the OPP table\n"); >> + int count = of_count_phandle_with_args(dev->of_node, >> + "operating-points-v2", NULL); >> + >> + count = min(count, GPU_DDR_OPP_TABLE_INDEX + 1); >> + count = max(count, 1); >> + >> + for (i = 0; i < count; i++) { >> + ret = dev_pm_opp_of_add_table_indexed(dev, i); >> + if (ret) { >> + DRM_DEV_ERROR(dev, "Add OPP table %d: failed %d\n", >> + i, ret); >> + goto err; >> + } >> + >> + gpu->opp_tables[i] = >> + dev_pm_opp_get_opp_table_indexed(dev, i); >> + if (!gpu->opp_tables[i]) { >> + DRM_DEV_ERROR(dev, "Get OPP table failed index %d\n", >> + i); >> + ret = -EINVAL; >> + goto err; >> + } >> + } >> } >> >> if (!ret) { >> @@ -919,12 +939,24 @@ static int adreno_get_pwrlevels(struct device *dev, >> gpu->icc_path = NULL; >> >> return 0; >> +err: >> + for (; i >= 0; i--) { >> + if (gpu->opp_tables[i]) { >> + dev_pm_opp_put_opp_table(gpu->opp_tables[i]); >> + gpu->opp_tables[i] = NULL; >> + } >> + } >> + >> + dev_pm_opp_remove_table(dev); >> + return ret; >> } >> >> 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) >> { >> + int ret = 0; >> + >> struct adreno_platform_config *config = pdev->dev.platform_data; >> struct msm_gpu_config adreno_gpu_config = { 0 }; >> struct msm_gpu *gpu = &adreno_gpu->base; >> @@ -945,7 +977,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, >> >> adreno_gpu_config.nr_rings = nr_rings; >> >> - adreno_get_pwrlevels(&pdev->dev, gpu); >> + ret = adreno_get_pwrlevels(&pdev->dev, gpu); >> + if (ret) >> + return ret; >> >> pm_runtime_set_autosuspend_delay(&pdev->dev, >> adreno_gpu->info->inactive_period); >> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h >> index ab8f0f9c..5b98b48 100644 >> --- a/drivers/gpu/drm/msm/msm_gpu.h >> +++ b/drivers/gpu/drm/msm/msm_gpu.h >> @@ -66,6 +66,12 @@ struct msm_gpu_funcs { >> void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq); >> }; >> >> +/* opp table indices */ >> +enum { >> + GPU_OPP_TABLE_INDEX, >> + GPU_DDR_OPP_TABLE_INDEX, >> +}; >> + >> struct msm_gpu { >> const char *name; >> struct drm_device *dev; >> @@ -113,6 +119,9 @@ struct msm_gpu { >> >> struct icc_path *icc_path; >> >> + /* gpu/ddr opp tables */ >> + struct opp_table *opp_tables[2]; > You don't need an array here. We're not going to have that many tables. > > struct opp_table *gpu_opp_table; > struct opp_table *bw_opp_table; > > Is sufficient and we don't need an enum. > >> + >> /* Hang and Inactivity Detection: >> */ >> #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ >> -- >> 2.7.4 >> > Jordan > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] drm: msm: a6xx: Fix off by one error when setting GPU freq 2020-03-31 7:55 [PATCH 0/5] Add support for GPU DDR BW scaling Sharat Masetty ` (2 preceding siblings ...) 2020-03-31 7:55 ` [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency Sharat Masetty @ 2020-03-31 7:55 ` Sharat Masetty 2020-03-31 16:43 ` Jordan Crouse 2020-03-31 7:55 ` [PATCH 5/5] dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU Sharat Masetty 4 siblings, 1 reply; 10+ messages in thread From: Sharat Masetty @ 2020-03-31 7:55 UTC (permalink / raw) To: freedreno, devicetree Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty This patch fixes an error in the for loop, thereby allowing search on the full list of possible GPU power levels. Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 489d9b6..81b8559 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -176,7 +176,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) if (freq == gmu->freq) return; - for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++) + for (perf_index = 0; perf_index < gmu->nr_gpu_freqs; perf_index++) if (freq == gmu->gpu_freqs[perf_index]) break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/5] drm: msm: a6xx: Fix off by one error when setting GPU freq 2020-03-31 7:55 ` [PATCH 4/5] drm: msm: a6xx: Fix off by one error when setting GPU freq Sharat Masetty @ 2020-03-31 16:43 ` Jordan Crouse 0 siblings, 0 replies; 10+ messages in thread From: Jordan Crouse @ 2020-03-31 16:43 UTC (permalink / raw) To: Sharat Masetty Cc: freedreno, devicetree, dri-devel, linux-arm-msm, linux-kernel, mka, sibis, saravanak, viresh.kumar On Tue, Mar 31, 2020 at 01:25:52PM +0530, Sharat Masetty wrote: > This patch fixes an error in the for loop, thereby allowing search on > the full list of possible GPU power levels. > > Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> Oh fun. This qualifies for drm-fixes. Can you pull this out of the stack and CC stable? Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 489d9b6..81b8559 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -176,7 +176,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) > if (freq == gmu->freq) > return; > > - for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++) > + for (perf_index = 0; perf_index < gmu->nr_gpu_freqs; perf_index++) > if (freq == gmu->gpu_freqs[perf_index]) > break; > > -- > 2.7.4 > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU 2020-03-31 7:55 [PATCH 0/5] Add support for GPU DDR BW scaling Sharat Masetty ` (3 preceding siblings ...) 2020-03-31 7:55 ` [PATCH 4/5] drm: msm: a6xx: Fix off by one error when setting GPU freq Sharat Masetty @ 2020-03-31 7:55 ` Sharat Masetty 2020-04-10 17:48 ` Rob Herring 4 siblings, 1 reply; 10+ messages in thread From: Sharat Masetty @ 2020-03-31 7:55 UTC (permalink / raw) To: freedreno, devicetree Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty Update the documentation for listing the multiple optional GPU and the DDR OPP tables to help enable DDR scaling. Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> --- .../devicetree/bindings/display/msm/gpu.txt | 63 +++++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt index 70025cb..ff3ae1b 100644 --- a/Documentation/devicetree/bindings/display/msm/gpu.txt +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt @@ -21,7 +21,10 @@ Required properties: following devices should not list clocks: - qcom,adreno-630.2 - iommus: optional phandle to an adreno iommu instance -- operating-points-v2: optional phandle to the OPP operating points +- operating-points-v2: optional phandles to the OPP operating point tables + one for the GPU OPPs and the other for the GPU->DDR OPPs. Note that if + multiple OPP tables are specified, the GPU OPP table(considered primary) + should be the first in the phandle list. - interconnects: optional phandle to an interconnect provider. See ../interconnect/interconnect.txt for details. - qcom,gmu: For GMU attached devices a phandle to the GMU device that will @@ -75,7 +78,7 @@ Example a6xx (with GMU): iommus = <&adreno_smmu 0>; - operating-points-v2 = <&gpu_opp_table>; + operating-points-v2 = <&gpu_opp_table>, <&gpu_ddr_bw_opp_table>; interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>; @@ -85,5 +88,61 @@ Example a6xx (with GMU): memory-region = <&zap_shader_region>; firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn" }; + + gpu_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-430000000 { + opp-hz = /bits/ 64 <430000000>; + opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>; + required-opps = <&gpu_ddr_bw_opp6>; + }; + + opp-355000000 { + opp-hz = /bits/ 64 <355000000>; + opp-level = <RPMH_REGULATOR_LEVEL_SVS>; + required-opps = <&gpu_ddr_bw_opp4>; + }; + + opp-267000000 { + opp-hz = /bits/ 64 <267000000>; + opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>; + required-opps = <&gpu_ddr_bw_opp4>; + }; + + opp-180000000 { + opp-hz = /bits/ 64 <180000000>; + opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>; + required-opps = <&gpu_ddr_bw_opp2>; + }; + }; + + gpu_ddr_bw_opp_table: gpu-ddr-bw-opp-table { + compatible = "operating-points-v2"; + + gpu_ddr_bw_opp1: opp-300000000 { + opp-peak-kBps =/bits/ 32 <1200000>; + }; + + gpu_ddr_bw_opp2: opp-451000000 { + opp-peak-kBps =/bits/ 32 <1804000>; + }; + + gpu_ddr_bw_opp3: opp-547000000 { + opp-peak-kBps =/bits/ 32 <2188000>; + }; + + gpu_ddr_bw_opp4: opp-768000000 { + opp-peak-kBps =/bits/ 32 <3072000>; + }; + + gpu_ddr_bw_opp5: opp-1017000000 { + opp-peak-kBps =/bits/ 32 <4068000>; + }; + + gpu_ddr_bw_opp6: opp-1353000000 { + opp-peak-kBps =/bits/ 32 <5412000>; + }; + }; }; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU 2020-03-31 7:55 ` [PATCH 5/5] dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU Sharat Masetty @ 2020-04-10 17:48 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2020-04-10 17:48 UTC (permalink / raw) To: Sharat Masetty Cc: freedreno, devicetree, dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka, sibis, saravanak, viresh.kumar, Sharat Masetty On Tue, 31 Mar 2020 13:25:53 +0530, Sharat Masetty wrote: > Update the documentation for listing the multiple optional GPU and the > DDR OPP tables to help enable DDR scaling. > > Signed-off-by: Sharat Masetty <smasetty@codeaurora.org> > --- > .../devicetree/bindings/display/msm/gpu.txt | 63 +++++++++++++++++++++- > 1 file changed, 61 insertions(+), 2 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-10 17:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-31 7:55 [PATCH 0/5] Add support for GPU DDR BW scaling Sharat Masetty 2020-03-31 7:55 ` [PATCH 1/5] arm64: dts: qcom: sc7180: Add interconnect bindings for GPU Sharat Masetty 2020-03-31 7:55 ` [PATCH 2/5] arm64: dts: qcom: sc7180: Add GPU DDR BW opp table Sharat Masetty 2020-03-31 7:55 ` [PATCH 3/5] drm: msm: scale DDR BW along with GPU frequency Sharat Masetty 2020-03-31 17:26 ` Jordan Crouse 2020-04-01 12:30 ` Sharat Masetty 2020-03-31 7:55 ` [PATCH 4/5] drm: msm: a6xx: Fix off by one error when setting GPU freq Sharat Masetty 2020-03-31 16:43 ` Jordan Crouse 2020-03-31 7:55 ` [PATCH 5/5] dt-bindings: drm/msm/gpu: Document OPP phandle list for the GPU Sharat Masetty 2020-04-10 17:48 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).