* [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
@ 2019-01-18 20:24 Jordan Crouse
2019-01-18 20:28 ` [Freedreno] " Jordan Crouse
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jordan Crouse @ 2019-01-18 20:24 UTC (permalink / raw)
To: freedreno
Cc: linux-kernel, Kees Cook, Jonathan Marek, David Airlie,
linux-arm-msm, Sharat Masetty, dianders, dri-devel,
bjorn.andersson, Rob Clark, Mamta Shukla, Daniel Vetter,
andy.gross, Colin Ian King, Stephen Boyd, georgi.djakov,
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.
v6: use icc_set_bw() instead of icc_set()
v5: Remove hardcoded interconnect name and just use the default
v4: Don't use a port string at all to skip the need for names in the DT
v3: Use macros and change port string per Georgi Djakov
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
drivers/gpu/drm/msm/Kconfig | 1 +
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 +++
4 files changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index cf549f1..78c9e5a5 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -5,6 +5,7 @@ config DRM_MSM
depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
depends on OF && COMMON_CLK
depends on MMU
+ depends on INTERCONNECT || !INTERCONNECT
select QCOM_MDT_LOADER if ARCH_QCOM
select REGULATOR
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 5beb83d..c48fe46 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2017-2018 The Linux Foundation. All rights reserved. */
#include <linux/clk.h>
+#include <linux/interconnect.h>
#include <linux/pm_opp.h>
#include <soc/qcom/cmd-db.h>
@@ -84,6 +85,9 @@ 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);
@@ -106,6 +110,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 at max so that the performance is nominal.
+ */
+ icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
}
void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
@@ -705,6 +715,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;
@@ -720,6 +732,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_bw(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 */
@@ -760,6 +775,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;
@@ -806,6 +823,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_bw(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 2e4372e..91a98c8 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -18,6 +18,7 @@
*/
#include <linux/ascii85.h>
+#include <linux/interconnect.h>
#include <linux/kernel.h>
#include <linux/pm_opp.h>
#include <linux/slab.h>
@@ -747,6 +748,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, NULL);
+ if (IS_ERR(gpu->icc_path))
+ gpu->icc_path = NULL;
+
return 0;
}
@@ -788,10 +794,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 efb49bb..2f13592a 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -19,6 +19,7 @@
#define __MSM_GPU_H__
#include <linux/clk.h>
+#include <linux/interconnect.h>
#include <linux/regulator/consumer.h>
#include "msm_drv.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.7.4
_______________________________________________
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] 7+ messages in thread* Re: [Freedreno] [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
2019-01-18 20:24 [PATCH v6] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse
@ 2019-01-18 20:28 ` Jordan Crouse
2019-01-18 21:52 ` Doug Anderson
2019-01-18 23:04 ` Evan Green
2 siblings, 0 replies; 7+ messages in thread
From: Jordan Crouse @ 2019-01-18 20:28 UTC (permalink / raw)
To: freedreno
Cc: Kees Cook, Jonathan Marek, David Airlie, linux-arm-msm,
Sharat Masetty, linux-kernel, dri-devel, dianders, Rob Clark,
Mamta Shukla, Daniel Vetter, andy.gross, Colin Ian King,
Stephen Boyd, bjorn.andersson, georgi.djakov, linux-arm-kernel
On Fri, Jan 18, 2019 at 01:24:18PM -0700, 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.
>
> v6: use icc_set_bw() instead of icc_set()
> v5: Remove hardcoded interconnect name and just use the default
> v4: Don't use a port string at all to skip the need for names in the DT
> v3: Use macros and change port string per Georgi Djakov
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
In my excitement I forgot to mention that this depends on:
https://patchwork.kernel.org/patch/10766335/
> ---
>
> drivers/gpu/drm/msm/Kconfig | 1 +
> 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 +++
> 4 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index cf549f1..78c9e5a5 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -5,6 +5,7 @@ config DRM_MSM
> depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
> depends on OF && COMMON_CLK
> depends on MMU
> + depends on INTERCONNECT || !INTERCONNECT
> select QCOM_MDT_LOADER if ARCH_QCOM
> select REGULATOR
> select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 5beb83d..c48fe46 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2017-2018 The Linux Foundation. All rights reserved. */
>
> #include <linux/clk.h>
> +#include <linux/interconnect.h>
> #include <linux/pm_opp.h>
> #include <soc/qcom/cmd-db.h>
>
> @@ -84,6 +85,9 @@ 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);
> @@ -106,6 +110,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 at max so that the performance is nominal.
> + */
> + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> }
>
> void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
> @@ -705,6 +715,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;
>
> @@ -720,6 +732,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_bw(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 */
> @@ -760,6 +775,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;
>
> @@ -806,6 +823,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_bw(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 2e4372e..91a98c8 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -18,6 +18,7 @@
> */
>
> #include <linux/ascii85.h>
> +#include <linux/interconnect.h>
> #include <linux/kernel.h>
> #include <linux/pm_opp.h>
> #include <linux/slab.h>
> @@ -747,6 +748,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, NULL);
> + if (IS_ERR(gpu->icc_path))
> + gpu->icc_path = NULL;
> +
> return 0;
> }
>
> @@ -788,10 +794,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 efb49bb..2f13592a 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -19,6 +19,7 @@
> #define __MSM_GPU_H__
>
> #include <linux/clk.h>
> +#include <linux/interconnect.h>
> #include <linux/regulator/consumer.h>
>
> #include "msm_drv.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.7.4
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
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] 7+ messages in thread* Re: [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
2019-01-18 20:24 [PATCH v6] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse
2019-01-18 20:28 ` [Freedreno] " Jordan Crouse
@ 2019-01-18 21:52 ` Doug Anderson
2019-01-18 22:04 ` [Freedreno] " Jordan Crouse
2019-01-18 23:04 ` Evan Green
2 siblings, 1 reply; 7+ messages in thread
From: Doug Anderson @ 2019-01-18 21:52 UTC (permalink / raw)
To: Jordan Crouse, Georgi Djakov, Rob Clark
Cc: Evan Green, Kees Cook, Jonathan Marek, David Airlie,
linux-arm-msm, Sharat Masetty, LKML, dri-devel, Stephen Boyd,
Mamta Shukla, Daniel Vetter, Andy Gross, Colin Ian King,
Bjorn Andersson, freedreno, Linux ARM
Hi,
On Fri, Jan 18, 2019 at 12:24 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.
>
> v6: use icc_set_bw() instead of icc_set()
> v5: Remove hardcoded interconnect name and just use the default
> v4: Don't use a port string at all to skip the need for names in the DT
> v3: Use macros and change port string per Georgi Djakov
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>
> drivers/gpu/drm/msm/Kconfig | 1 +
> 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 +++
> 4 files changed, 33 insertions(+)
Presumably this should go in together with the bindings:
- https://patchwork.kernel.org/patch/10739233/
- https://lkml.kernel.org/r/20181220173026.3857-3-jcrouse@codeaurora.org
...since bindings and code usually go in the same tree together.
As per discussion in v3 AKA v5
<https://patchwork.kernel.org/patch/10739231/>, we're trying to figure
out where to land this. ;-)
-Doug
_______________________________________________
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] 7+ messages in thread
* Re: [Freedreno] [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
2019-01-18 21:52 ` Doug Anderson
@ 2019-01-18 22:04 ` Jordan Crouse
0 siblings, 0 replies; 7+ messages in thread
From: Jordan Crouse @ 2019-01-18 22:04 UTC (permalink / raw)
To: Doug Anderson
Cc: Kees Cook, Jonathan Marek, David Airlie, linux-arm-msm,
Sharat Masetty, LKML, Evan Green, Stephen Boyd, Rob Clark,
Mamta Shukla, dri-devel, Daniel Vetter, Andy Gross,
Colin Ian King, Bjorn Andersson, freedreno, Georgi Djakov,
Linux ARM
On Fri, Jan 18, 2019 at 01:52:20PM -0800, Doug Anderson wrote:
> Hi,
>
> On Fri, Jan 18, 2019 at 12:24 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.
> >
> > v6: use icc_set_bw() instead of icc_set()
> > v5: Remove hardcoded interconnect name and just use the default
> > v4: Don't use a port string at all to skip the need for names in the DT
> > v3: Use macros and change port string per Georgi Djakov
> >
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >
> > drivers/gpu/drm/msm/Kconfig | 1 +
> > 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 +++
> > 4 files changed, 33 insertions(+)
>
> Presumably this should go in together with the bindings:
>
> - https://patchwork.kernel.org/patch/10739233/
> - https://lkml.kernel.org/r/20181220173026.3857-3-jcrouse@codeaurora.org
>
> ...since bindings and code usually go in the same tree together.
>
>
> As per discussion in v3 AKA v5
> <https://patchwork.kernel.org/patch/10739231/>, we're trying to figure
> out where to land this. ;-)
I'm just a simple caveman driver developer. I'll let the adults figure
out where to put what. LMK if you need a rebase on a different tree.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
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] 7+ messages in thread
* Re: [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
2019-01-18 20:24 [PATCH v6] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse
2019-01-18 20:28 ` [Freedreno] " Jordan Crouse
2019-01-18 21:52 ` Doug Anderson
@ 2019-01-18 23:04 ` Evan Green
2019-01-18 23:27 ` Jordan Crouse
2 siblings, 1 reply; 7+ messages in thread
From: Evan Green @ 2019-01-18 23:04 UTC (permalink / raw)
To: Jordan Crouse
Cc: LKML, Kees Cook, Jonathan Marek, David Airlie, linux-arm-msm,
Sharat Masetty, Doug Anderson, dri-devel, Bjorn Andersson,
Rob Clark, Mamta Shukla, Daniel Vetter, Andy Gross,
Colin Ian King, Stephen Boyd, freedreno, Georgi Djakov,
linux-arm-kernel
On Fri, Jan 18, 2019 at 12:24 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.
>
> v6: use icc_set_bw() instead of icc_set()
> v5: Remove hardcoded interconnect name and just use the default
> v4: Don't use a port string at all to skip the need for names in the DT
> v3: Use macros and change port string per Georgi Djakov
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>
> drivers/gpu/drm/msm/Kconfig | 1 +
> 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 +++
> 4 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index cf549f1..78c9e5a5 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -5,6 +5,7 @@ config DRM_MSM
> depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
> depends on OF && COMMON_CLK
> depends on MMU
> + depends on INTERCONNECT || !INTERCONNECT
> select QCOM_MDT_LOADER if ARCH_QCOM
> select REGULATOR
> select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 5beb83d..c48fe46 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2017-2018 The Linux Foundation. All rights reserved. */
>
> #include <linux/clk.h>
> +#include <linux/interconnect.h>
> #include <linux/pm_opp.h>
> #include <soc/qcom/cmd-db.h>
>
> @@ -84,6 +85,9 @@ 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);
> @@ -106,6 +110,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 at max so that the performance is nominal.
> + */
> + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> }
>
> void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
> @@ -705,6 +715,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;
>
> @@ -720,6 +732,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_bw(gpu->icc_path, 0, MBps_to_icc(3072));
Does the comment mention boot because this resume call happens during
init? How come this number is different from the one in
__a6xx_gmu_set_freq? Also, you're setting an average bandwidth of 0, I
guess you don't know really what your average is, and the hardware is
fine with that setting. Is that true?
-Evan
_______________________________________________
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] 7+ messages in thread* Re: [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
2019-01-18 23:04 ` Evan Green
@ 2019-01-18 23:27 ` Jordan Crouse
2019-01-18 23:45 ` Evan Green
0 siblings, 1 reply; 7+ messages in thread
From: Jordan Crouse @ 2019-01-18 23:27 UTC (permalink / raw)
To: Evan Green
Cc: LKML, Kees Cook, Jonathan Marek, David Airlie, linux-arm-msm,
Sharat Masetty, Doug Anderson, dri-devel, Bjorn Andersson,
Rob Clark, Mamta Shukla, Daniel Vetter, Andy Gross,
Colin Ian King, Stephen Boyd, freedreno, Georgi Djakov,
linux-arm-kernel
On Fri, Jan 18, 2019 at 03:04:34PM -0800, Evan Green wrote:
> On Fri, Jan 18, 2019 at 12:24 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.
> >
> > v6: use icc_set_bw() instead of icc_set()
> > v5: Remove hardcoded interconnect name and just use the default
> > v4: Don't use a port string at all to skip the need for names in the DT
> > v3: Use macros and change port string per Georgi Djakov
> >
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >
> > drivers/gpu/drm/msm/Kconfig | 1 +
> > 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 +++
> > 4 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > index cf549f1..78c9e5a5 100644
> > --- a/drivers/gpu/drm/msm/Kconfig
> > +++ b/drivers/gpu/drm/msm/Kconfig
> > @@ -5,6 +5,7 @@ config DRM_MSM
> > depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
> > depends on OF && COMMON_CLK
> > depends on MMU
> > + depends on INTERCONNECT || !INTERCONNECT
> > select QCOM_MDT_LOADER if ARCH_QCOM
> > select REGULATOR
> > select DRM_KMS_HELPER
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index 5beb83d..c48fe46 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -2,6 +2,7 @@
> > /* Copyright (c) 2017-2018 The Linux Foundation. All rights reserved. */
> >
> > #include <linux/clk.h>
> > +#include <linux/interconnect.h>
> > #include <linux/pm_opp.h>
> > #include <soc/qcom/cmd-db.h>
> >
> > @@ -84,6 +85,9 @@ 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);
> > @@ -106,6 +110,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 at max so that the performance is nominal.
> > + */
> > + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> > }
> >
> > void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
> > @@ -705,6 +715,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;
> >
> > @@ -720,6 +732,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_bw(gpu->icc_path, 0, MBps_to_icc(3072));
>
> Does the comment mention boot because this resume call happens during
> init?
Correct. Boot refers to the GMU in this context.
> How come this number is different from the one in __a6xx_gmu_set_freq?
If you never request a bus quota from the kernel the sucker is horribly
painfully slow. I'm not sure if that is just the default reset value of
the registers or if the bootloader is doing something. In any event
the GMU doesn't always initialize if you try to bring it up with the
default bus settings so we set it to a "reasonable" quota for the
init sequence. I admit I didn't come up with these numbers on my own;
some poor anonymous soul debugged this on the downstream kernel and
I just stole the same parameters.
As for _a6xx_gmu_set_freq() eventually we'll agree on a way to communicate
the bus bandwidth in the opp settings and we'll be able to adjust the quota
based on the frequency to save power so that's why the code is organized the way
it is so that a6xx_gmu_set_freq() won't have to be massively changed later.
> Also, you're setting an average bandwidth of 0, I
> guess you don't know really what your average is, and the hardware is
> fine with that setting. Is that true?
Yes, we only set the instantaneous bandwidth.
> -Evan
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
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] 7+ messages in thread* Re: [PATCH v6] drm/msm/a6xx: Add support for an interconnect path
2019-01-18 23:27 ` Jordan Crouse
@ 2019-01-18 23:45 ` Evan Green
0 siblings, 0 replies; 7+ messages in thread
From: Evan Green @ 2019-01-18 23:45 UTC (permalink / raw)
To: Evan Green, freedreno, linux-arm-msm, Doug Anderson,
Bjorn Andersson, Andy Gross, Georgi Djakov, linux-arm-kernel,
Stephen Boyd, Kees Cook, Colin Ian King, Sharat Masetty,
dri-devel, LKML, Rob Clark, David Airlie, Jonathan Marek,
Mamta Shukla, Daniel Vetter
On Fri, Jan 18, 2019 at 3:27 PM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> On Fri, Jan 18, 2019 at 03:04:34PM -0800, Evan Green wrote:
> > On Fri, Jan 18, 2019 at 12:24 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.
> > >
> > > v6: use icc_set_bw() instead of icc_set()
> > > v5: Remove hardcoded interconnect name and just use the default
> > > v4: Don't use a port string at all to skip the need for names in the DT
> > > v3: Use macros and change port string per Georgi Djakov
> > >
> > > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > > ---
> > >
> > > drivers/gpu/drm/msm/Kconfig | 1 +
> > > 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 +++
> > > 4 files changed, 33 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > > index cf549f1..78c9e5a5 100644
> > > --- a/drivers/gpu/drm/msm/Kconfig
> > > +++ b/drivers/gpu/drm/msm/Kconfig
> > > @@ -5,6 +5,7 @@ config DRM_MSM
> > > depends on ARCH_QCOM || SOC_IMX5 || (ARM && COMPILE_TEST)
> > > depends on OF && COMMON_CLK
> > > depends on MMU
> > > + depends on INTERCONNECT || !INTERCONNECT
> > > select QCOM_MDT_LOADER if ARCH_QCOM
> > > select REGULATOR
> > > select DRM_KMS_HELPER
> > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > index 5beb83d..c48fe46 100644
> > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > > @@ -2,6 +2,7 @@
> > > /* Copyright (c) 2017-2018 The Linux Foundation. All rights reserved. */
> > >
> > > #include <linux/clk.h>
> > > +#include <linux/interconnect.h>
> > > #include <linux/pm_opp.h>
> > > #include <soc/qcom/cmd-db.h>
> > >
> > > @@ -84,6 +85,9 @@ 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);
> > > @@ -106,6 +110,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 at max so that the performance is nominal.
> > > + */
> > > + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> > > }
> > >
> > > void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
> > > @@ -705,6 +715,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;
> > >
> > > @@ -720,6 +732,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_bw(gpu->icc_path, 0, MBps_to_icc(3072));
> >
> > Does the comment mention boot because this resume call happens during
> > init?
>
> Correct. Boot refers to the GMU in this context.
>
> > How come this number is different from the one in __a6xx_gmu_set_freq?
>
> If you never request a bus quota from the kernel the sucker is horribly
> painfully slow. I'm not sure if that is just the default reset value of
> the registers or if the bootloader is doing something. In any event
> the GMU doesn't always initialize if you try to bring it up with the
> default bus settings so we set it to a "reasonable" quota for the
> init sequence. I admit I didn't come up with these numbers on my own;
> some poor anonymous soul debugged this on the downstream kernel and
> I just stole the same parameters.
>
> As for _a6xx_gmu_set_freq() eventually we'll agree on a way to communicate
> the bus bandwidth in the opp settings and we'll be able to adjust the quota
> based on the frequency to save power so that's why the code is organized the way
> it is so that a6xx_gmu_set_freq() won't have to be massively changed later.
Oh I see, so it's just a placeholder to max for now until the OPP part
gets figured out. That makes sense.
Reviewed-by: Evan Green <evgreen@chromium.org>
_______________________________________________
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] 7+ messages in thread
end of thread, other threads:[~2019-01-18 23:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-18 20:24 [PATCH v6] drm/msm/a6xx: Add support for an interconnect path Jordan Crouse
2019-01-18 20:28 ` [Freedreno] " Jordan Crouse
2019-01-18 21:52 ` Doug Anderson
2019-01-18 22:04 ` [Freedreno] " Jordan Crouse
2019-01-18 23:04 ` Evan Green
2019-01-18 23:27 ` Jordan Crouse
2019-01-18 23:45 ` Evan Green
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).