| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Feb 2023 15:46:37 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Rob Clark <robdclark@...il.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
Sean Paul <sean@...rly.run>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
Akhil P Oommen <quic_akhilpo@...cinc.com>,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Rob Clark <robdclark@...omium.org>
Subject: Re: [PATCH v3 05/15] drm/msm/a6xx: Introduce GMU wrapper support
On 23.02.2023 15:43, Dmitry Baryshkov wrote:
> On Thu, 23 Feb 2023 at 14:06, Konrad Dybcio <konrad.dybcio@...aro.org> 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@...aro.org>
>> ---
>> 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
>>
>
>
Powered by blists - more mailing lists