From: Akhil P Oommen <akhilpo@codeaurora.org>
To: Jordan Crouse <jcrouse@codeaurora.org>
Cc: devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, dianders@chromium.org,
mka@chromium.org, dri-devel@freedesktop.org,
freedreno@lists.freedesktop.org
Subject: Re: [PATCH v2 1/3] drm/msm: adreno: Make speed-bin support generic
Date: Wed, 2 Dec 2020 21:00:09 +0530 [thread overview]
Message-ID: <1911b3dc-407d-01a0-e4f2-72c0e331236b@codeaurora.org> (raw)
In-Reply-To: <20201130170231.GF16856@jcrouse1-lnx.qualcomm.com>
<< Resending since Jordan wasn't in the CC list >>
On 11/30/2020 10:32 PM, Jordan Crouse wrote:
> On Fri, Nov 27, 2020 at 06:19:44PM +0530, Akhil P Oommen wrote:
>> So far a530v2 gpu has support for detecting its supported opps
>> based on a fuse value called speed-bin. This patch makes this
>> support generic across gpu families. This is in preparation to
>> extend speed-bin support to a6x family.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
>> ---
>> Changes from v1:
>> 1. Added the changes to support a618 sku to the series.
>> 2. Avoid failing probe in case of an unsupported sku. (Rob)
>>
>> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 34 --------------
>> drivers/gpu/drm/msm/adreno/adreno_device.c | 4 ++
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 71 ++++++++++++++++++++++++++++++
>> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 5 +++
>> 4 files changed, 80 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
>> index 8fa5c91..7d42321 100644
>> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
>> @@ -1531,38 +1531,6 @@ static const struct adreno_gpu_funcs funcs = {
>> .get_timestamp = a5xx_get_timestamp,
>> };
>>
>> -static void check_speed_bin(struct device *dev)
>> -{
>> - struct nvmem_cell *cell;
>> - u32 val;
>> -
>> - /*
>> - * If the OPP table specifies a opp-supported-hw property then we have
>> - * to set something with dev_pm_opp_set_supported_hw() or the table
>> - * doesn't get populated so pick an arbitrary value that should
>> - * ensure the default frequencies are selected but not conflict with any
>> - * actual bins
>> - */
>> - val = 0x80;
>> -
>> - cell = nvmem_cell_get(dev, "speed_bin");
>> -
>> - if (!IS_ERR(cell)) {
>> - void *buf = nvmem_cell_read(cell, NULL);
>> -
>> - if (!IS_ERR(buf)) {
>> - u8 bin = *((u8 *) buf);
>> -
>> - val = (1 << bin);
>> - kfree(buf);
>> - }
>> -
>> - nvmem_cell_put(cell);
>> - }
>> -
>> - dev_pm_opp_set_supported_hw(dev, &val, 1);
>> -}
>> -
>> struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
>> {
>> struct msm_drm_private *priv = dev->dev_private;
>> @@ -1588,8 +1556,6 @@ struct msm_gpu *a5xx_gpu_init(struct drm_device *dev)
>>
>> a5xx_gpu->lm_leakage = 0x4E001A;
>>
>> - check_speed_bin(&pdev->dev);
>> -
>> ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 4);
>> if (ret) {
>> a5xx_destroy(&(a5xx_gpu->base.base));
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 87c8b03..e0ff16c 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -18,6 +18,8 @@ bool snapshot_debugbus = false;
>> MODULE_PARM_DESC(snapshot_debugbus, "Include debugbus sections in GPU devcoredump (if not fused off)");
>> module_param_named(snapshot_debugbus, snapshot_debugbus, bool, 0600);
>>
>> +const u32 a530v2_speedbins[] = {0, 1, 2, 3, 4, 5, 6, 7};
>> +
>> static const struct adreno_info gpulist[] = {
>> {
>> .rev = ADRENO_REV(2, 0, 0, 0),
>> @@ -163,6 +165,8 @@ static const struct adreno_info gpulist[] = {
>> ADRENO_QUIRK_FAULT_DETECT_MASK,
>> .init = a5xx_gpu_init,
>> .zapfw = "a530_zap.mdt",
>> + .speedbins = a530v2_speedbins,
>> + .speedbins_count = ARRAY_SIZE(a530v2_speedbins),
>> }, {
>> .rev = ADRENO_REV(5, 4, 0, 2),
>> .revn = 540,
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> index f21561d..b342fa4 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> @@ -14,6 +14,7 @@
>> #include <linux/pm_opp.h>
>> #include <linux/slab.h>
>> #include <linux/soc/qcom/mdt_loader.h>
>> +#include <linux/nvmem-consumer.h>
>> #include <soc/qcom/ocmem.h>
>> #include "adreno_gpu.h"
>> #include "msm_gem.h"
>> @@ -891,6 +892,69 @@ void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem)
>> adreno_ocmem->hdl);
>> }
>>
>> +static int adreno_set_supported_hw(struct device *dev,
>> + struct adreno_gpu *adreno_gpu)
>> +{
>> + u8 speedbins_count = adreno_gpu->info->speedbins_count;
>> + const u32 *speedbins = adreno_gpu->info->speedbins;
>> + struct nvmem_cell *cell;
>> + u32 bin, i;
>> + u32 val = 0;
>> + void *buf, *opp_table;
>> +
>> + cell = nvmem_cell_get(dev, "speed_bin");
>> + /*
>> + * -ENOENT means that the platform doesn't support speedbin which is
>> + * fine
>> + */
>> + if (PTR_ERR(cell) == -ENOENT)
>> + return 0;
>> + else if (IS_ERR(cell))
>> + return PTR_ERR(cell);
>> +
>> + if (!speedbins)
>> + goto done;
>> +
>> + buf = nvmem_cell_read(cell, NULL);
>> + if (IS_ERR(buf)) {
>> + nvmem_cell_put(cell);
>> + return PTR_ERR(buf);
>> + }
>> +
>> + bin = *((u32 *) buf);
>> +
>> + for (i = 0; i < speedbins_count; i++) {
>> + if (bin == speedbins[i]) {
>> + val = (1 << i);
>> + break;
>> + }
>> + }
>> +
>> + kfree(buf);
>> +done:
>> + nvmem_cell_put(cell);
>> +
>> + if (!val) {
>> + DRM_DEV_ERROR(dev,
>> + "missing support for speed-bin: %u. Some OPPs may not be supported by hardware",
>> + bin);
>> + val = ~0U;
>> + }
>> +
>> + opp_table = dev_pm_opp_set_supported_hw(dev, &val, 1);
>> + if (IS_ERR(opp_table))
>> + return PTR_ERR(opp_table);
>> +
>> + adreno_gpu->opp_table = opp_table;
>> + return 0;
>> +}
>> +
>> +static void adreno_put_supported_hw(struct opp_table *opp_table)
>> +{
>> + if (opp_table)
>> + dev_pm_opp_put_supported_hw(opp_table);
>> +}
>> +
>> int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>> struct adreno_gpu *adreno_gpu,
>> const struct adreno_gpu_funcs *funcs, int nr_rings)
>> @@ -899,6 +963,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>> struct adreno_platform_config *config = dev->platform_data;
>> struct msm_gpu_config adreno_gpu_config = { 0 };
>> struct msm_gpu *gpu = &adreno_gpu->base;
>> + int ret;
>>
>> adreno_gpu->funcs = funcs;
>> adreno_gpu->info = adreno_info(config->rev);
>> @@ -910,6 +975,10 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>>
>> adreno_gpu_config.nr_rings = nr_rings;
>>
>> + ret = adreno_set_supported_hw(dev, adreno_gpu);
>> + if (ret)
>> + return ret;
>> +
>
> I still don't understand why we are doing this here instead of a5xx_gpu.c and
> a6xx_gpu.c.
>
> Jordan
Could you please clarify why you prefer so?
-Akhil
>
>> adreno_get_pwrlevels(dev, gpu);
>>
>> pm_runtime_set_autosuspend_delay(dev,
>> @@ -936,4 +1005,6 @@ void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
>>
>> icc_put(gpu->icc_path);
>> icc_put(gpu->ocmem_icc_path);
>> +
>> + adreno_put_supported_hw(adreno_gpu->opp_table);
>> }
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> index c3775f7..a756ad7 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> @@ -55,6 +55,7 @@ struct adreno_reglist {
>> };
>>
>> extern const struct adreno_reglist a630_hwcg[], a640_hwcg[], a650_hwcg[];
>> +extern const u32 a618_speedbins[];
>>
>> struct adreno_info {
>> struct adreno_rev rev;
>> @@ -67,6 +68,8 @@ struct adreno_info {
>> const char *zapfw;
>> u32 inactive_period;
>> const struct adreno_reglist *hwcg;
>> + const u32 *speedbins;
>> + const u8 speedbins_count;
>> };
>>
>> const struct adreno_info *adreno_info(struct adreno_rev rev);
>> @@ -112,6 +115,8 @@ struct adreno_gpu {
>> * code (a3xx_gpu.c) and stored in this common location.
>> */
>> const unsigned int *reg_offsets;
>> +
>> + struct opp_table *opp_table;
>> };
>> #define to_adreno_gpu(x) container_of(x, struct adreno_gpu, base)
>>
>> --
>> 2.7.4
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
prev parent reply other threads:[~2020-12-02 15:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-27 12:49 [PATCH v2 1/3] drm/msm: adreno: Make speed-bin support generic Akhil P Oommen
2020-11-27 12:49 ` [PATCH v2 2/3] drm/msm: Add speed-bin support for a618 gpu Akhil P Oommen
2020-11-27 12:49 ` [PATCH v2 3/3] arm: dts: sc7180: Add support for gpu fuse Akhil P Oommen
2020-11-30 17:02 ` [PATCH v2 1/3] drm/msm: adreno: Make speed-bin support generic Jordan Crouse
2020-12-02 15:23 ` Akhil P Oommen
2020-12-02 16:30 ` Jordan Crouse
2020-12-03 16:02 ` Akhil P Oommen
2020-12-02 15:30 ` Akhil P Oommen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1911b3dc-407d-01a0-e4f2-72c0e331236b@codeaurora.org \
--to=akhilpo@codeaurora.org \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=dri-devel@freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=jcrouse@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mka@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).