Messages in this thread | | | Subject | Re: [Freedreno] [PATCH 2/2] drm/msm/a6xx: Add support for Adreno 7c Gen 3 gpu | From | Akhil P Oommen <> | Date | 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@codeaurora.org> 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@codeaurora.org> >> --- >> 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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/freedreno
| |