drm/msm/a6xx: Add interconnect support
Subsystems (4)
v3 cover drm/msm/a6xx: Add interconnect support
(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(+)
Signature · 6 lines
2.18.0 _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
PATCH v3 1/1 drm/msm/a6xx: Add support for an interconnect path
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
--- 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(+)
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +20 -0
@@ -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);
drivers/gpu/drm/msm/adreno/adreno_gpu.c +9 -0
@@ -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); }
drivers/gpu/drm/msm/msm_gpu.h +3 -0
@@ -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) */
Signature · 6 lines
2.18.0 _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path
Hi Jordan,
Thanks for the patch!
Quoted 7 lines · Jordan Crouse
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.
Quoted 13 lines
Signed-off-by Jordan Crouse <[email protected]>--- 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(+)drivers/gpu/drm/msm/adreno/a6xx_gmu.c +0
-0
drivers/gpu/drm/msm/adreno/a6xx_gmu.c line 7
Quoted 5 lines
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +1
-0@@ -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?
Quoted 12 lines
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +3
-0#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 );
drivers/gpu/drm/msm/adreno/a6xx_gmu.c static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index) · line 95
Quoted 7 lines
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +4
-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.
Quoted 15 lines
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +4
-0+*/ +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;
drivers/gpu/drm/msm/adreno/a6xx_gmu.c int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) · line 711
Quoted 5 lines
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +2
-0@@ -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));
Quoted 28 lines
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +6
-0+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);drivers/gpu/drm/msm/adreno/adreno_gpu.c +0
-0
drivers/gpu/drm/msm/adreno/adreno_gpu.c line 24
Quoted 4 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +1
-0@@ -21,6 +21,7 @@#include <linux/kernel.h> #include <linux/pm_opp.h> #include <linux/slab.h> +#include <linux/interconnect.h>
Alphabetic order?
drivers/gpu/drm/msm/adreno/adreno_gpu.c static int adreno_get_pwrlevels(struct device *dev, · line 700
Quoted 9 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +2
-0#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?
Quoted 25 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +6
-0+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); }drivers/gpu/drm/msm/msm_gpu.h +0
-0
drivers/gpu/drm/msm/msm_gpu.h line 23
Quoted 4 lines
drivers/gpu/drm/msm/msm_gpu.h +1
-0@@ -20,6 +20,7 @@#include <linux/clk.h> #include <linux/regulator/consumer.h> +#include <linux/interconnect.h>
Alphabetic order?
drivers/gpu/drm/msm/msm_gpu.h struct msm_gpu { · line 127
Quoted 12 lines
drivers/gpu/drm/msm/msm_gpu.h +2
-0#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
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno