On 23.02.2023 15:43, Dmitry Baryshkov wrote: > On Thu, 23 Feb 2023 at 14:06, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote: >> >> Some (particularly SMD_RPM, a.k.a non-RPMh) SoCs implement A6XX GPUs >> but don't implement the associated GMUs. This is due to the fact that >> the GMU directly pokes at RPMh. Sadly, this means we have to take care >> of enabling & scaling power rails, clocks and bandwidth ourselves. >> >> Reuse existing Adreno-common code and modify the deeply-GMU-infused >> A6XX code to facilitate these GPUs. This involves if-ing out lots >> of GMU callbacks and introducing a new type of GMU - GMU wrapper (it's >> the actual name that Qualcomm uses in their downstream kernels). >> >> This is essentially a register region which is convenient to model >> as a device. We'll use it for managing the GDSCs. The register >> layout matches the actual GMU_CX/GX regions on the "real GMU" devices >> and lets us reuse quite a bit of gmu_read/write/rmw calls. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> >> --- >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 53 +++++- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 244 +++++++++++++++++++++++++--- >> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + >> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +- >> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 6 + >> 5 files changed, 282 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> index 90e636dcdd5b..b2c56561cde6 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > [skipped] > >> struct msm_gpu *a6xx_gpu_init(struct drm_device *dev) >> { >> struct msm_drm_private *priv = dev->dev_private; >> @@ -2063,18 +2235,36 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev) >> >> adreno_gpu->registers = NULL; >> >> + /* Check if there is a GMU phandle and set it up */ >> + node = of_parse_phandle(pdev->dev.of_node, "qcom,gmu", 0); >> + /* FIXME: How do we gracefully handle this? */ >> + BUG_ON(!node); > > I thought that we should fix it, but then I noticed that this code was > just moved from the part below. I suppose we could return einval instead of calling BUG_ON, but this would belong to a separate patch (possibly outside this series). > >> + >> + adreno_gpu->gmu_is_wrapper = of_device_is_compatible(node, "qcom,adreno-gmu-wrapper"); >> + >> /* >> * We need to know the platform type before calling into adreno_gpu_init >> * so that the hw_apriv flag can be correctly set. Snoop into the info >> * and grab the revision number >> */ >> info = adreno_info(config->rev); >> - >> - if (info && (info->revn == 650 || info->revn == 660 || >> - adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), info->rev))) > > Are we losing A635 here? I don't see it in the condition below. a660_family takes care of it Konrad > >> + if (!info) >> + return ERR_PTR(-EINVAL); >> + >> + /* Assign these early so that we can use the is_aXYZ helpers */ >> + /* Numeric revision IDs (e.g. 630) */ >> + adreno_gpu->revn = info->revn; >> + /* New-style ADRENO_REV()-only */ >> + adreno_gpu->rev = info->rev; >> + /* Quirk data */ >> + adreno_gpu->info = info; >> + >> + if (adreno_is_a650(adreno_gpu) || adreno_is_a660_family(adreno_gpu)) >> adreno_gpu->base.hw_apriv = true; >> >> - a6xx_llc_slices_init(pdev, a6xx_gpu); >> + /* No LLCC on non-RPMh (and by extension, non-GMU) SoCs */ >> + if (!adreno_has_gmu_wrapper(adreno_gpu)) >> + a6xx_llc_slices_init(pdev, a6xx_gpu); >> >> ret = a6xx_set_supported_hw(&pdev->dev, config->rev); >> if (ret) { >> @@ -2082,7 +2272,10 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev) >> return ERR_PTR(ret); >> } >> >> - ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1); >> + if (adreno_has_gmu_wrapper(adreno_gpu)) >> + ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs_gmuwrapper, 1); >> + else >> + ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1); >> if (ret) { >> a6xx_destroy(&(a6xx_gpu->base.base)); >> return ERR_PTR(ret); >> @@ -2095,13 +2288,10 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev) >> if (adreno_is_a618(adreno_gpu) || adreno_is_7c3(adreno_gpu)) >> priv->gpu_clamp_to_idle = true; >> >> - /* Check if there is a GMU phandle and set it up */ >> - node = of_parse_phandle(pdev->dev.of_node, "qcom,gmu", 0); >> - >> - /* FIXME: How do we gracefully handle this? */ >> - BUG_ON(!node); >> - >> - ret = a6xx_gmu_init(a6xx_gpu, node); >> + if (adreno_has_gmu_wrapper(adreno_gpu)) >> + ret = a6xx_gmu_wrapper_init(a6xx_gpu, node); >> + else >> + ret = a6xx_gmu_init(a6xx_gpu, node); >> of_node_put(node); >> if (ret) { >> a6xx_destroy(&(a6xx_gpu->base.base)); >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >> index eea2e60ce3b7..51a7656072fa 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h >> @@ -76,6 +76,7 @@ int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state); >> void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state); >> >> int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node); >> +int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node); >> void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu); >> >> void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp, >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> index b7e217d00a22..e11e8a02ac22 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> @@ -1041,16 +1041,18 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu) >> /* Get the generic state from the adreno core */ >> adreno_gpu_state_get(gpu, &a6xx_state->base); >> >> - a6xx_get_gmu_registers(gpu, a6xx_state); >> + if (!adreno_has_gmu_wrapper(adreno_gpu)) { >> + a6xx_get_gmu_registers(gpu, a6xx_state); >> >> - a6xx_state->gmu_log = a6xx_snapshot_gmu_bo(a6xx_state, &a6xx_gpu->gmu.log); >> - a6xx_state->gmu_hfi = a6xx_snapshot_gmu_bo(a6xx_state, &a6xx_gpu->gmu.hfi); >> - a6xx_state->gmu_debug = a6xx_snapshot_gmu_bo(a6xx_state, &a6xx_gpu->gmu.debug); >> + a6xx_state->gmu_log = a6xx_snapshot_gmu_bo(a6xx_state, &a6xx_gpu->gmu.log); >> + a6xx_state->gmu_hfi = a6xx_snapshot_gmu_bo(a6xx_state, &a6xx_gpu->gmu.hfi); >> + a6xx_state->gmu_debug = a6xx_snapshot_gmu_bo(a6xx_state, &a6xx_gpu->gmu.debug); >> >> - a6xx_snapshot_gmu_hfi_history(gpu, a6xx_state); >> + a6xx_snapshot_gmu_hfi_history(gpu, a6xx_state); >> + } >> >> /* If GX isn't on the rest of the data isn't going to be accessible */ >> - if (!a6xx_gmu_gx_is_on(&a6xx_gpu->gmu)) >> + if (!adreno_has_gmu_wrapper(adreno_gpu) && !a6xx_gmu_gx_is_on(&a6xx_gpu->gmu)) >> return &a6xx_state->base; >> >> /* Get the banks of indexed registers */ >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h >> index b4f9b1343d63..2c0f0ef094cb 100644 >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h >> @@ -115,6 +115,7 @@ struct adreno_gpu { >> * code (a3xx_gpu.c) and stored in this common location. >> */ >> const unsigned int *reg_offsets; >> + bool gmu_is_wrapper; >> }; >> #define to_adreno_gpu(x) container_of(x, struct adreno_gpu, base) >> >> @@ -145,6 +146,11 @@ struct adreno_platform_config { >> >> bool adreno_cmp_rev(struct adreno_rev rev1, struct adreno_rev rev2); >> >> +static inline bool adreno_has_gmu_wrapper(struct adreno_gpu *gpu) >> +{ >> + return gpu->gmu_is_wrapper; >> +} >> + >> static inline bool adreno_is_a2xx(struct adreno_gpu *gpu) >> { >> return (gpu->revn < 300); >> >> -- >> 2.39.2 >> > >