Re: [Freedreno] [PATCH 1/1] drm/msm/a6xx: Add support for an interconnect path

[Rob Clark]

On Wed, Nov 28, 2018 at 1:29 PM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/freedreno