Re: [Freedreno] [PATCH 2/2] drm/msm/a6xx: Add support for Adreno 7c Gen 3 gpu

Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx> · Mon, 26 Jul 2021 11:16:56 +0530

On 7/24/2021 11:42 PM, Rob Clark wrote:
()

On Fri, Jul 23, 2021 at 3:38 AM Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx> wrote:

This patch adds support for the gpu found in the Snapdragon 7c Gen 3
compute platform. This gpu is similar to the exisiting a660 gpu with
minor delta in the programing sequence. As the Adreno GPUs are moving
away from a numeric chipid based naming scheme to a string, it was
decided to use 0x06030500 as the gpu id of this gpu to communicate
to the userspace driver.

Signed-off-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx>
---
  drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 20 ++++++++++++++++++-
  drivers/gpu/drm/msm/adreno/a6xx_gmu.h      |  1 +
  drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h  |  2 ++
  drivers/gpu/drm/msm/adreno/a6xx_gpu.c      | 21 ++++++++++++++------
  drivers/gpu/drm/msm/adreno/a6xx_hfi.c      | 32 ++++++++++++++++++++++++++++++
  drivers/gpu/drm/msm/adreno/adreno_device.c | 12 +++++++++++
  drivers/gpu/drm/msm/adreno/adreno_gpu.h    | 11 ++++++++--
  7 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index b349692..332301f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -933,6 +933,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)

         /* Use a known rate to bring up the GMU */
         clk_set_rate(gmu->core_clk, 200000000);
+       clk_set_rate(gmu->hub_clk, 150000000);
         ret = clk_bulk_prepare_enable(gmu->nr_clocks, gmu->clocks);
         if (ret) {
                 pm_runtime_put(gmu->gxpd);
@@ -1094,6 +1095,7 @@ static void a6xx_gmu_shutdown(struct a6xx_gmu *gmu)

  int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
  {
+       struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
         struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
         struct msm_gpu *gpu = &a6xx_gpu->base.base;

@@ -1117,9 +1119,22 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
          * domain. Usually the GMU does this but only if the shutdown sequence
          * was successful
          */
-       if (!IS_ERR_OR_NULL(gmu->gxpd))
+       if (!IS_ERR_OR_NULL(gmu->gxpd)) {
+               /*
+                * Toggle the loop_en bit, across disabling the gx gdsc,
+                * with a delay of 10 XO cycles before disabling gx
+                * gdsc. This is to prevent CPR measurements from
+                * failing.
+                */
+               if (adreno_is_a660(adreno_gpu))
+                       gmu_rmw(gmu, REG_A6XX_GPU_CPR_FSM_CTL, 1, 0);
+
                 pm_runtime_put_sync(gmu->gxpd);

+               if (adreno_is_a660(adreno_gpu))
+                       gmu_rmw(gmu, REG_A6XX_GPU_CPR_FSM_CTL, 1, 1);

This kinda seems like it should be a separate patch.. but I noticed
you silently turned adreno_is_a660() into what should probably be
adreno_is_a660_family()

I'd suggest to break this out into it's own patch, so it is clear that
it effects a660 as well, and then a next patch to rename
adreno_is_a660_family()

Longer term, we might want to think about refactoring all the
if(adreno_is_xyz()) into a features table (see i915_pci.c for ideas)

I agree. This should have been a seperate patch.


+       }
+
         clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);

         pm_runtime_put_sync(gmu->dev);
@@ -1393,6 +1408,9 @@ static int a6xx_gmu_clocks_probe(struct a6xx_gmu *gmu)
         gmu->core_clk = msm_clk_bulk_get_clock(gmu->clocks,
                 gmu->nr_clocks, "gmu");

+       gmu->hub_clk = msm_clk_bulk_get_clock(gmu->clocks,
+               gmu->nr_clocks, "hub");
+
         return 0;
  }

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 71dfa600..3c74f64 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -66,6 +66,7 @@ struct a6xx_gmu {
         int nr_clocks;
         struct clk_bulk_data *clocks;
         struct clk *core_clk;
+       struct clk *hub_clk;

         /* current performance index set externally */
         int current_perf_index;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h
index 8115892..d46733f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.xml.h
@@ -479,5 +479,7 @@ static inline uint32_t A6XX_GMU_GPU_NAP_CTRL_SID(uint32_t val)

  #define REG_A6XX_RSCC_TCS3_DRV0_STATUS                         0x0000053e

+#define REG_A6XX_GPU_CPR_FSM_CTL                               0x0000c001
+

  #endif /* A6XX_GMU_XML */
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 183b9f9..c0882536 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -694,6 +694,13 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
                 uavflagprd_inv = 2;
         }

+       if (adreno_is_7c3(adreno_gpu)) {
+               lower_bit = 1;
+               amsbc = 1;
+               rgb565_predicator = 1;
+               uavflagprd_inv = 2;
+       }
+
         gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL,
                 rgb565_predicator << 11 | amsbc << 4 | lower_bit << 1);
         gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, lower_bit << 1);
@@ -950,10 +957,10 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
         /* Setting the primFifo thresholds default values,
          * and vccCacheSkipDis=1 bit (0x200) for A640 and newer
         */
-       if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
-               gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
-       else if (adreno_is_a640(adreno_gpu))
+       if (adreno_is_a640(adreno_gpu) || adreno_is_7c3(adreno_gpu))
                 gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00200200);
+       else if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
+               gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
         else
                 gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00180000);

@@ -993,8 +1000,9 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
         if (adreno_is_a660(adreno_gpu)) {
                 gpu_write(gpu, REG_A6XX_CP_CHICKEN_DBG, 0x1);
                 gpu_write(gpu, REG_A6XX_RBBM_GBIF_CLIENT_QOS_CNTL, 0x0);
-               /* Set dualQ + disable afull for A660 GPU but not for A635 */
-               gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
+               /* Set dualQ + disable afull for A660 GPU but not for 7c3 */
+               if (!adreno_is_7c3(adreno_gpu))
+                       gpu_write(gpu, REG_A6XX_UCHE_CMDQ_CONFIG, 0x66906);
         }

         /* Enable expanded apriv for targets that support it */
@@ -1780,7 +1788,8 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
          */
         info = adreno_info(config->rev);

-       if (info && (info->revn == 650 || info->revn == 660))
+       if (info && (info->revn == 650 || info->revn == 660
+                       || info->revn == ADRENO_REV_7C3))
                 adreno_gpu->base.hw_apriv = true;

         a6xx_llc_slices_init(pdev, a6xx_gpu);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
index 9194337..1451c2b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -382,6 +382,36 @@ static void a660_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
         msg->cnoc_cmds_data[1][0] =  0x60000001;
  }

+static void adreno_7c3_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
+{
+       /*
+        * Send a single "off" entry just to get things running
+        * TODO: bus scaling
+        */
+       msg->bw_level_num = 1;
+
+       msg->ddr_cmds_num = 3;
+       msg->ddr_wait_bitmask = 0x07;
+
+       msg->ddr_cmds_addrs[0] = 0x50004;
+       msg->ddr_cmds_addrs[1] = 0x50000;
+       msg->ddr_cmds_addrs[2] = 0x50088;
+
+       msg->ddr_cmds_data[0][0] =  0x40000000;
+       msg->ddr_cmds_data[0][1] =  0x40000000;
+       msg->ddr_cmds_data[0][2] =  0x40000000;
+
+       /*
+        * These are the CX (CNOC) votes - these are used by the GMU but the
+        * votes are known and fixed for the target
+        */
+       msg->cnoc_cmds_num = 1;
+       msg->cnoc_wait_bitmask = 0x01;
+
+       msg->cnoc_cmds_addrs[0] = 0x5006c;
+       msg->cnoc_cmds_data[0][0] =  0x40000000;
+       msg->cnoc_cmds_data[1][0] =  0x60000001;
+}
  static void a6xx_build_bw_table(struct a6xx_hfi_msg_bw_table *msg)
  {
         /* Send a single "off" entry since the 630 GMU doesn't do bus scaling */
@@ -432,6 +462,8 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
                 a640_build_bw_table(&msg);
         else if (adreno_is_a650(adreno_gpu))
                 a650_build_bw_table(&msg);
+       else if (adreno_is_7c3(adreno_gpu))
+               adreno_7c3_build_bw_table(&msg);
         else if (adreno_is_a660(adreno_gpu))
                 a660_build_bw_table(&msg);
         else
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 6dad801..063b847 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -300,6 +300,18 @@ static const struct adreno_info gpulist[] = {
                 .init = a6xx_gpu_init,
                 .zapfw = "a660_zap.mdt",
                 .hwcg = a660_hwcg,
+       }, {
+               .rev = ADRENO_REV(6, 3, 5, ANY_ID),
+               .revn = ADRENO_REV_7C3,

So rather than adding an ARENO_REV_7C3 define, I was thinking to just
not set .revn (leave it zero) and identify based on .rev .. that (ie.
GPU_ID being zero) would also be the signal to userspace to identify
the GPU via CHIP_ID instead

You could extract out the version comparison in adreno_info() into a
helper that can be re-used in things like adreno_is_a660_family() and
adreno_is_7c3().

BR,
-R
Sure. Will update this patch.

-Akhil.

+               .name = "Adreno 7c Gen 3",
+               .fw = {
+                       [ADRENO_FW_SQE] = "a660_sqe.fw",
+                       [ADRENO_FW_GMU] = "a660_gmu.bin",
+               },
+               .gmem = SZ_512K,
+               .inactive_period = DRM_MSM_INACTIVE_PERIOD,
+               .init = a6xx_gpu_init,
+               .hwcg = a660_hwcg,
         },
  };

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 8dbe0d1..679bc59 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -247,15 +247,22 @@ static inline int adreno_is_a650(struct adreno_gpu *gpu)
         return gpu->revn == 650;
  }

+#define ADRENO_REV_7C3 0x06030500
+static inline int adreno_is_7c3(struct adreno_gpu *gpu)
+{
+       return gpu->revn == ADRENO_REV_7C3;
+}
+
  static inline int adreno_is_a660(struct adreno_gpu *gpu)
  {
-       return gpu->revn == 660;
+       return gpu->revn == 660 || gpu->revn == ADRENO_REV_7C3;
  }

  /* check for a650, a660, or any derivatives */
  static inline int adreno_is_a650_family(struct adreno_gpu *gpu)
  {
-       return gpu->revn == 650 || gpu->revn == 620 || gpu->revn == 660;
+       return gpu->revn == 650 || gpu->revn == 620 || gpu->revn == 660
+              || gpu->revn == ADRENO_REV_7C3;
  }

  int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

_______________________________________________
Freedreno mailing list
Freedreno@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/freedreno







