* [PATCH 0/1] drm/msm/a6xx: Add interconnect support @ 2018-11-28 18:29 Jordan Crouse 2018-11-28 18:29 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse 0 siblings, 1 reply; 6+ messages in thread From: Jordan Crouse @ 2018-11-28 18:29 UTC (permalink / raw) To: linux-arm-kernel I heard at LPC that there was some confusion that the OPP bindings [1] were needed to land interconnect support. They aren't. There is a two step process for the SDM845 GPU. First we need a vote (any vote) because the default bus settings are way too bad for any reasonable performance. The next step after that would be to scale the bus request based on the frequency in order to save power. For that we *do* need a solution but that can be calmly discussed separately. This patch accommodates the first step. It cranks the bus to max for GPU activity and turns it off when the GPU goes to sleep. Hopefully this can alleviate the confusion and help interconnect land that much quicker. After that, we can calmly re-introduce the OPP discussion. This patch depends on interconnect support [2]. [1] https://patchwork.kernel.org/patch/10577303/ [2] https://patchwork.kernel.org/patch/10701287/ Jordan Crouse (1): drm/msm/a6xx: Add support for an interconnect path drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gpu.h | 3 +++ 3 files changed, 32 insertions(+) -- 2.18.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path 2018-11-28 18:29 [PATCH 0/1] drm/msm/a6xx: Add interconnect support Jordan Crouse @ 2018-11-28 18:29 ` Jordan Crouse 2018-11-28 19:09 ` [Freedreno] " Rob Clark 0 siblings, 1 reply; 6+ messages in thread From: Jordan Crouse @ 2018-11-28 18:29 UTC (permalink / raw) To: linux-arm-kernel Try to get the interconnect path for the GPU and vote for the maximum bandwidth to support all frequencies. This is needed for performance. Later we will want to scale the bandwidth based on the frequency to also optimize for power but that will require some device tree infrastructure that does not yet exist. Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gpu.h | 3 +++ 3 files changed, 32 insertions(+) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 546599a7ab05..b96ae2ab04bb 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -3,6 +3,7 @@ #include <linux/clk.h> #include <linux/pm_opp.h> +#include <linux/interconnect.h> #include <soc/qcom/cmd-db.h> #include "a6xx_gpu.h" @@ -63,6 +64,9 @@ static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) { + struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; int ret; gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); @@ -85,6 +89,12 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); gmu->freq = gmu->gpu_freqs[index]; + + /* + * Eventually we will want to scale the path vote with the frequency but + * for now leave it t at max so that the performance is nominal. + */ + icc_set(gpu->icc_path, 0, 7216000000); } void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) @@ -680,6 +690,8 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu) int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; struct a6xx_gmu *gmu = &a6xx_gpu->gmu; int status, ret; @@ -695,6 +707,9 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) if (ret) goto out; + /* Set the bus quota to a reasonable value for boot */ + icc_set(gpu->icc_path, 0, 3072000000); + a6xx_gmu_irq_enable(gmu); /* Check to see if we are doing a cold or warm boot */ @@ -735,6 +750,8 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; struct a6xx_gmu *gmu = &a6xx_gpu->gmu; u32 val; @@ -781,6 +798,9 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) /* Tell RPMh to power off the GPU */ a6xx_rpmh_stop(gmu); + /* Remove the bus vote */ + icc_set(gpu->icc_path, 0, 0); + clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks); pm_runtime_put_sync(gmu->dev); diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 93d70f4a2154..9bab491912cf 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -21,6 +21,7 @@ #include <linux/kernel.h> #include <linux/pm_opp.h> #include <linux/slab.h> +#include <linux/interconnect.h> #include "adreno_gpu.h" #include "msm_gem.h" #include "msm_mmu.h" @@ -695,6 +696,11 @@ static int adreno_get_pwrlevels(struct device *dev, DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate); + /* Check for an interconnect path for the bus */ + gpu->icc_path = of_icc_get(dev, "port0"); + if (IS_ERR(gpu->icc_path)) + gpu->icc_path = NULL; + return 0; } @@ -733,10 +739,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) { + struct msm_gpu *gpu = &adreno_gpu->base; unsigned int i; for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++) release_firmware(adreno_gpu->fw[i]); + icc_put(gpu->icc_path); + msm_gpu_cleanup(&adreno_gpu->base); } diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index f82bac086666..7b04d1665555 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -20,6 +20,7 @@ #include <linux/clk.h> #include <linux/regulator/consumer.h> +#include <linux/interconnect.h> #include "msm_drv.h" #include "msm_fence.h" @@ -119,6 +120,8 @@ struct msm_gpu { struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk; uint32_t fast_rate; + struct icc_path *icc_path; + /* Hang and Inactivity Detection: */ #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ -- 2.18.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Freedreno] [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path 2018-11-28 18:29 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse @ 2018-11-28 19:09 ` Rob Clark 0 siblings, 0 replies; 6+ messages in thread From: Rob Clark @ 2018-11-28 19:09 UTC (permalink / raw) To: linux-arm-kernel On Wed, Nov 28, 2018 at 1:29 PM Jordan Crouse <jcrouse@codeaurora.org> wrote: > > Try to get the interconnect path for the GPU and vote for the maximum > bandwidth to support all frequencies. This is needed for performance. > Later we will want to scale the bandwidth based on the frequency to > also optimize for power but that will require some device tree > infrastructure that does not yet exist. > > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ > drivers/gpu/drm/msm/msm_gpu.h | 3 +++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 546599a7ab05..b96ae2ab04bb 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -3,6 +3,7 @@ > > #include <linux/clk.h> > #include <linux/pm_opp.h> > +#include <linux/interconnect.h> > #include <soc/qcom/cmd-db.h> > > #include "a6xx_gpu.h" > @@ -63,6 +64,9 @@ static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) > > static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) > { > + struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > + struct msm_gpu *gpu = &adreno_gpu->base; > int ret; > > gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); > @@ -85,6 +89,12 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) > dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); > > gmu->freq = gmu->gpu_freqs[index]; > + > + /* > + * Eventually we will want to scale the path vote with the frequency but > + * for now leave it t at max so that the performance is nominal. > + */ > + icc_set(gpu->icc_path, 0, 7216000000); Should we get these hard-coded rates from 'struct adreno_info'? I guess they are not constant across all the variants of a given generation? I assume at least the port label would be the same for all? BR, -R > } > > void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) > @@ -680,6 +690,8 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu) > > int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > { > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > + struct msm_gpu *gpu = &adreno_gpu->base; > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > int status, ret; > > @@ -695,6 +707,9 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > if (ret) > goto out; > > + /* Set the bus quota to a reasonable value for boot */ > + icc_set(gpu->icc_path, 0, 3072000000); > + > a6xx_gmu_irq_enable(gmu); > > /* Check to see if we are doing a cold or warm boot */ > @@ -735,6 +750,8 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) > > int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) > { > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > + struct msm_gpu *gpu = &adreno_gpu->base; > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > u32 val; > > @@ -781,6 +798,9 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) > /* Tell RPMh to power off the GPU */ > a6xx_rpmh_stop(gmu); > > + /* Remove the bus vote */ > + icc_set(gpu->icc_path, 0, 0); > + > clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks); > > pm_runtime_put_sync(gmu->dev); > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > index 93d70f4a2154..9bab491912cf 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -21,6 +21,7 @@ > #include <linux/kernel.h> > #include <linux/pm_opp.h> > #include <linux/slab.h> > +#include <linux/interconnect.h> > #include "adreno_gpu.h" > #include "msm_gem.h" > #include "msm_mmu.h" > @@ -695,6 +696,11 @@ static int adreno_get_pwrlevels(struct device *dev, > > DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate); > > + /* Check for an interconnect path for the bus */ > + gpu->icc_path = of_icc_get(dev, "port0"); > + if (IS_ERR(gpu->icc_path)) > + gpu->icc_path = NULL; > + > return 0; > } > > @@ -733,10 +739,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > > void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) > { > + struct msm_gpu *gpu = &adreno_gpu->base; > unsigned int i; > > for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++) > release_firmware(adreno_gpu->fw[i]); > > + icc_put(gpu->icc_path); > + > msm_gpu_cleanup(&adreno_gpu->base); > } > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index f82bac086666..7b04d1665555 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.h > +++ b/drivers/gpu/drm/msm/msm_gpu.h > @@ -20,6 +20,7 @@ > > #include <linux/clk.h> > #include <linux/regulator/consumer.h> > +#include <linux/interconnect.h> > > #include "msm_drv.h" > #include "msm_fence.h" > @@ -119,6 +120,8 @@ struct msm_gpu { > struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk; > uint32_t fast_rate; > > + struct icc_path *icc_path; > + > /* Hang and Inactivity Detection: > */ > #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ > -- > 2.18.0 > > _______________________________________________ > Freedreno mailing list > Freedreno at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 0/1] drm/msm/a6xx: Add interconnect support @ 2018-11-29 17:21 Jordan Crouse 2018-11-29 17:21 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse 0 siblings, 1 reply; 6+ messages in thread From: Jordan Crouse @ 2018-11-29 17:21 UTC (permalink / raw) To: georgi.djakov Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, linux-arm-kernel This is an updated version of [1] adding interconnect support without OPP bindings to get maximum performance from the GPU. Big delta here is that I stupidly confused Bps and KBps and passed a value that overflowed the API. Correct bandwidth values are now passed. [1] https://lists.freedesktop.org/archives/freedreno/2018-November/004347.html v2: Make sure to pass values in correct units Jordan Crouse (1): drm/msm/a6xx: Add support for an interconnect path drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gpu.h | 3 +++ 3 files changed, 32 insertions(+) -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path 2018-11-29 17:21 [PATCH v2 0/1] drm/msm/a6xx: Add interconnect support Jordan Crouse @ 2018-11-29 17:21 ` Jordan Crouse 0 siblings, 0 replies; 6+ messages in thread From: Jordan Crouse @ 2018-11-29 17:21 UTC (permalink / raw) To: georgi.djakov Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, linux-arm-kernel Try to get the interconnect path for the GPU and vote for the maximum bandwidth to support all frequencies. This is needed for performance. Later we will want to scale the bandwidth based on the frequency to also optimize for power but that will require some device tree infrastructure that does not yet exist. v2: Absolute bandwidth values should be in KBps Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gpu.h | 3 +++ 3 files changed, 32 insertions(+) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 546599a7ab05..b96ae2ab04bb 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -3,6 +3,7 @@ #include <linux/clk.h> #include <linux/pm_opp.h> +#include <linux/interconnect.h> #include <soc/qcom/cmd-db.h> #include "a6xx_gpu.h" @@ -63,6 +64,9 @@ static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) { + struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; int ret; gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); @@ -85,6 +89,12 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); gmu->freq = gmu->gpu_freqs[index]; + + /* + * Eventually we will want to scale the path vote with the frequency but + * for now leave it t at max so that the performance is nominal. + */ + icc_set(gpu->icc_path, 0, 7216000000); } void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) @@ -680,6 +690,8 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu) int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; struct a6xx_gmu *gmu = &a6xx_gpu->gmu; int status, ret; @@ -695,6 +707,9 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) if (ret) goto out; + /* Set the bus quota to a reasonable value for boot */ + icc_set(gpu->icc_path, 0, 3072000000); + a6xx_gmu_irq_enable(gmu); /* Check to see if we are doing a cold or warm boot */ @@ -735,6 +750,8 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; struct a6xx_gmu *gmu = &a6xx_gpu->gmu; u32 val; @@ -781,6 +798,9 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) /* Tell RPMh to power off the GPU */ a6xx_rpmh_stop(gmu); + /* Remove the bus vote */ + icc_set(gpu->icc_path, 0, 0); + clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks); pm_runtime_put_sync(gmu->dev); diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 93d70f4a2154..9bab491912cf 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -21,6 +21,7 @@ #include <linux/kernel.h> #include <linux/pm_opp.h> #include <linux/slab.h> +#include <linux/interconnect.h> #include "adreno_gpu.h" #include "msm_gem.h" #include "msm_mmu.h" @@ -695,6 +696,11 @@ static int adreno_get_pwrlevels(struct device *dev, DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate); + /* Check for an interconnect path for the bus */ + gpu->icc_path = of_icc_get(dev, "port0"); + if (IS_ERR(gpu->icc_path)) + gpu->icc_path = NULL; + return 0; } @@ -733,10 +739,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) { + struct msm_gpu *gpu = &adreno_gpu->base; unsigned int i; for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++) release_firmware(adreno_gpu->fw[i]); + icc_put(gpu->icc_path); + msm_gpu_cleanup(&adreno_gpu->base); } diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index f82bac086666..7b04d1665555 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -20,6 +20,7 @@ #include <linux/clk.h> #include <linux/regulator/consumer.h> +#include <linux/interconnect.h> #include "msm_drv.h" #include "msm_fence.h" @@ -119,6 +120,8 @@ struct msm_gpu { struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk; uint32_t fast_rate; + struct icc_path *icc_path; + /* Hang and Inactivity Detection: */ #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 0/1] drm/msm/a6xx: Add interconnect support @ 2018-11-29 17:26 Jordan Crouse 2018-11-29 17:26 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse 0 siblings, 1 reply; 6+ messages in thread From: Jordan Crouse @ 2018-11-29 17:26 UTC (permalink / raw) To: georgi.djakov Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, linux-arm-kernel (grr - resending because I didn't actually send the correct patch. My fingers were going faster than my brain). This is an updated version of [1] adding interconnect support without OPP bindings to get maximum performance from the GPU. Big delta here is that I stupidly confused Bps and KBps and passed a value that overflowed the API. Correct bandwidth values are now passed. [1] https://lists.freedesktop.org/archives/freedreno/2018-November/004347.html v3: Actually send correct patch with correct units v2: Make sure to pass values in correct units Jordan Crouse (1): drm/msm/a6xx: Add support for an interconnect path drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gpu.h | 3 +++ 3 files changed, 32 insertions(+) -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path 2018-11-29 17:26 [PATCH v3 0/1] drm/msm/a6xx: Add interconnect support Jordan Crouse @ 2018-11-29 17:26 ` Jordan Crouse 2018-12-05 16:51 ` Georgi Djakov 0 siblings, 1 reply; 6+ messages in thread From: Jordan Crouse @ 2018-11-29 17:26 UTC (permalink / raw) To: georgi.djakov Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, linux-arm-kernel Try to get the interconnect path for the GPU and vote for the maximum bandwidth to support all frequencies. This is needed for performance. Later we will want to scale the bandwidth based on the frequency to also optimize for power but that will require some device tree infrastructure that does not yet exist. v3: Absolute bandwidth values should be specified in KBps Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org> --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ drivers/gpu/drm/msm/msm_gpu.h | 3 +++ 3 files changed, 32 insertions(+) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 546599a7ab05..fe0f5b10fd9c 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -3,6 +3,7 @@ #include <linux/clk.h> #include <linux/pm_opp.h> +#include <linux/interconnect.h> #include <soc/qcom/cmd-db.h> #include "a6xx_gpu.h" @@ -63,6 +64,9 @@ static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) { + struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; int ret; gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); @@ -85,6 +89,12 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); gmu->freq = gmu->gpu_freqs[index]; + + /* + * Eventually we will want to scale the path vote with the frequency but + * for now leave it t at max so that the performance is nominal. + */ + icc_set(gpu->icc_path, 0, 7216000); } void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) @@ -680,6 +690,8 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu) int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; struct a6xx_gmu *gmu = &a6xx_gpu->gmu; int status, ret; @@ -695,6 +707,9 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) if (ret) goto out; + /* Set the bus quota to a reasonable value for boot */ + icc_set(gpu->icc_path, 0, 3072000); + a6xx_gmu_irq_enable(gmu); /* Check to see if we are doing a cold or warm boot */ @@ -735,6 +750,8 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) { + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; + struct msm_gpu *gpu = &adreno_gpu->base; struct a6xx_gmu *gmu = &a6xx_gpu->gmu; u32 val; @@ -781,6 +798,9 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) /* Tell RPMh to power off the GPU */ a6xx_rpmh_stop(gmu); + /* Remove the bus vote */ + icc_set(gpu->icc_path, 0, 0); + clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks); pm_runtime_put_sync(gmu->dev); diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 93d70f4a2154..9bab491912cf 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -21,6 +21,7 @@ #include <linux/kernel.h> #include <linux/pm_opp.h> #include <linux/slab.h> +#include <linux/interconnect.h> #include "adreno_gpu.h" #include "msm_gem.h" #include "msm_mmu.h" @@ -695,6 +696,11 @@ static int adreno_get_pwrlevels(struct device *dev, DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate); + /* Check for an interconnect path for the bus */ + gpu->icc_path = of_icc_get(dev, "port0"); + if (IS_ERR(gpu->icc_path)) + gpu->icc_path = NULL; + return 0; } @@ -733,10 +739,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) { + struct msm_gpu *gpu = &adreno_gpu->base; unsigned int i; for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++) release_firmware(adreno_gpu->fw[i]); + icc_put(gpu->icc_path); + msm_gpu_cleanup(&adreno_gpu->base); } diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index f82bac086666..7b04d1665555 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -20,6 +20,7 @@ #include <linux/clk.h> #include <linux/regulator/consumer.h> +#include <linux/interconnect.h> #include "msm_drv.h" #include "msm_fence.h" @@ -119,6 +120,8 @@ struct msm_gpu { struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk; uint32_t fast_rate; + struct icc_path *icc_path; + /* Hang and Inactivity Detection: */ #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path 2018-11-29 17:26 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse @ 2018-12-05 16:51 ` Georgi Djakov 0 siblings, 0 replies; 6+ messages in thread From: Georgi Djakov @ 2018-12-05 16:51 UTC (permalink / raw) To: Jordan Crouse Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, linux-arm-kernel Hi Jordan, Thanks for the patch! On 11/29/18 19:26, Jordan Crouse wrote: > Try to get the interconnect path for the GPU and vote for the maximum > bandwidth to support all frequencies. This is needed for performance. > Later we will want to scale the bandwidth based on the frequency to > also optimize for power but that will require some device tree > infrastructure that does not yet exist. > > v3: Absolute bandwidth values should be specified in KBps btw. now i have also included macros in the header, that can be used to specify the bandwidth units. So now you can now use kBps_to_icc or MBps_to_icc etc. If we decide at some point that we change the units we use internally, we will not have update all the users. > > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 20 ++++++++++++++++++++ > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 9 +++++++++ > drivers/gpu/drm/msm/msm_gpu.h | 3 +++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 546599a7ab05..fe0f5b10fd9c 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -3,6 +3,7 @@ > > #include <linux/clk.h> > #include <linux/pm_opp.h> > +#include <linux/interconnect.h> > #include <soc/qcom/cmd-db.h> Alphabetic order maybe? > #include "a6xx_gpu.h" > @@ -63,6 +64,9 @@ static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) > > static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) > { > + struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu); > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > + struct msm_gpu *gpu = &adreno_gpu->base; > int ret; > > gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0); > @@ -85,6 +89,12 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) > dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); > > gmu->freq = gmu->gpu_freqs[index]; > + > + /* > + * Eventually we will want to scale the path vote with the frequency but > + * for now leave it t at max so that the performance is nominal. An extra t above. > + */ > + icc_set(gpu->icc_path, 0, 7216000); > } > > void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq) > @@ -680,6 +690,8 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu) > > int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > { > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > + struct msm_gpu *gpu = &adreno_gpu->base; > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > int status, ret; > > @@ -695,6 +707,9 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > if (ret) > goto out; > > + /* Set the bus quota to a reasonable value for boot */ > + icc_set(gpu->icc_path, 0, 3072000); Here you can do: icc_set(gpu->icc_path, 0, MBps_to_icc(3072)); > + > a6xx_gmu_irq_enable(gmu); > > /* Check to see if we are doing a cold or warm boot */ > @@ -735,6 +750,8 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) > > int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) > { > + struct adreno_gpu *adreno_gpu = &a6xx_gpu->base; > + struct msm_gpu *gpu = &adreno_gpu->base; > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > u32 val; > > @@ -781,6 +798,9 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) > /* Tell RPMh to power off the GPU */ > a6xx_rpmh_stop(gmu); > > + /* Remove the bus vote */ > + icc_set(gpu->icc_path, 0, 0); > + > clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks); > > pm_runtime_put_sync(gmu->dev); > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > index 93d70f4a2154..9bab491912cf 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -21,6 +21,7 @@ > #include <linux/kernel.h> > #include <linux/pm_opp.h> > #include <linux/slab.h> > +#include <linux/interconnect.h> Alphabetic order? > #include "adreno_gpu.h" > #include "msm_gem.h" > #include "msm_mmu.h" > @@ -695,6 +696,11 @@ static int adreno_get_pwrlevels(struct device *dev, > > DBG("fast_rate=%u, slow_rate=27000000", gpu->fast_rate); > > + /* Check for an interconnect path for the bus */ > + gpu->icc_path = of_icc_get(dev, "port0"); I was wondering if port0 is appropriate name here. I assume this is the port to DDR. Maybe we could name it gfx-mem or gpu-mem. Are there any other interconnects that need to be scaled on the a6xx? > + if (IS_ERR(gpu->icc_path)) > + gpu->icc_path = NULL; > + > return 0; > } > > @@ -733,10 +739,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > > void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu) > { > + struct msm_gpu *gpu = &adreno_gpu->base; > unsigned int i; > > for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++) > release_firmware(adreno_gpu->fw[i]); > > + icc_put(gpu->icc_path); > + > msm_gpu_cleanup(&adreno_gpu->base); > } > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index f82bac086666..7b04d1665555 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.h > +++ b/drivers/gpu/drm/msm/msm_gpu.h > @@ -20,6 +20,7 @@ > > #include <linux/clk.h> > #include <linux/regulator/consumer.h> > +#include <linux/interconnect.h> Alphabetic order? > > #include "msm_drv.h" > #include "msm_fence.h" > @@ -119,6 +120,8 @@ struct msm_gpu { > struct clk *ebi1_clk, *core_clk, *rbbmtimer_clk; > uint32_t fast_rate; > > + struct icc_path *icc_path; > + > /* Hang and Inactivity Detection: > */ > #define DRM_MSM_INACTIVE_PERIOD 66 /* in ms (roughly four frames) */ Can this driver be a module or is always built-in? If it is built-in and the interconnect support is a module we will have a problem. Maybe add something like this in the Kconfig: depends on INTERCONNECT || !INTERCONNECT Thanks, Georgi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-05 16:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-28 18:29 [PATCH 0/1] drm/msm/a6xx: Add interconnect support Jordan Crouse 2018-11-28 18:29 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse 2018-11-28 19:09 ` [Freedreno] " Rob Clark -- strict thread matches above, loose matches on Subject: below -- 2018-11-29 17:21 [PATCH v2 0/1] drm/msm/a6xx: Add interconnect support Jordan Crouse 2018-11-29 17:21 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse 2018-11-29 17:26 [PATCH v3 0/1] drm/msm/a6xx: Add interconnect support Jordan Crouse 2018-11-29 17:26 ` [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse 2018-12-05 16:51 ` Georgi Djakov
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).