* [PATCH v3 2/2] arm: dts: sc7180: Add support for gpu fuse
2020-12-07 10:42 [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu Akhil P Oommen
@ 2020-12-07 10:42 ` Akhil P Oommen
2020-12-11 11:55 ` Akhil P Oommen
2020-12-11 11:54 ` [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu Akhil P Oommen
2021-01-04 18:07 ` Jordan Crouse
2 siblings, 1 reply; 5+ messages in thread
From: Akhil P Oommen @ 2020-12-07 10:42 UTC (permalink / raw)
To: freedreno, devicetree
Cc: robh, dri-devel, linux-arm-msm, linux-kernel, jcrouse, mka,
robdclark, dianders
Add support for gpu fuse to help identify the supported opps.
Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 6678f1e..8cae3eb 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -675,6 +675,11 @@
reg = <0x25b 0x1>;
bits = <1 3>;
};
+
+ gpu_speed_bin: gpu_speed_bin@1d2 {
+ reg = <0x1d2 0x2>;
+ bits = <5 8>;
+ };
};
sdhc_1: sdhci@7c4000 {
@@ -1907,52 +1912,69 @@
operating-points-v2 = <&gpu_opp_table>;
qcom,gmu = <&gmu>;
+ nvmem-cells = <&gpu_speed_bin>;
+ nvmem-cell-names = "speed_bin";
+
interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
interconnect-names = "gfx-mem";
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
+ opp-825000000 {
+ opp-hz = /bits/ 64 <825000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
+ opp-peak-kBps = <8532000>;
+ opp-supported-hw = <0x04>;
+ };
+
opp-800000000 {
opp-hz = /bits/ 64 <800000000>;
opp-level = <RPMH_REGULATOR_LEVEL_TURBO>;
opp-peak-kBps = <8532000>;
+ opp-supported-hw = <0x07>;
};
opp-650000000 {
opp-hz = /bits/ 64 <650000000>;
opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
opp-peak-kBps = <7216000>;
+ opp-supported-hw = <0x07>;
};
opp-565000000 {
opp-hz = /bits/ 64 <565000000>;
opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
opp-peak-kBps = <5412000>;
+ opp-supported-hw = <0x07>;
};
opp-430000000 {
opp-hz = /bits/ 64 <430000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
opp-peak-kBps = <5412000>;
+ opp-supported-hw = <0x07>;
};
opp-355000000 {
opp-hz = /bits/ 64 <355000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
opp-peak-kBps = <3072000>;
+ opp-supported-hw = <0x07>;
};
opp-267000000 {
opp-hz = /bits/ 64 <267000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
opp-peak-kBps = <3072000>;
+ opp-supported-hw = <0x07>;
};
opp-180000000 {
opp-hz = /bits/ 64 <180000000>;
opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
opp-peak-kBps = <1804000>;
+ opp-supported-hw = <0x07>;
};
};
};
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] arm: dts: sc7180: Add support for gpu fuse
2020-12-07 10:42 ` [PATCH v3 2/2] arm: dts: sc7180: Add support for gpu fuse Akhil P Oommen
@ 2020-12-11 11:55 ` Akhil P Oommen
0 siblings, 0 replies; 5+ messages in thread
From: Akhil P Oommen @ 2020-12-11 11:55 UTC (permalink / raw)
To: freedreno, devicetree, Rob Herring, Jordan Crouse, Rob Clark
Cc: linux-arm-msm, linux-kernel, dianders, mka, dri-devel
On 12/7/2020 4:12 PM, Akhil P Oommen wrote:
> Add support for gpu fuse to help identify the supported opps.
>
> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 6678f1e..8cae3eb 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -675,6 +675,11 @@
> reg = <0x25b 0x1>;
> bits = <1 3>;
> };
> +
> + gpu_speed_bin: gpu_speed_bin@1d2 {
> + reg = <0x1d2 0x2>;
> + bits = <5 8>;
> + };
> };
>
> sdhc_1: sdhci@7c4000 {
> @@ -1907,52 +1912,69 @@
> operating-points-v2 = <&gpu_opp_table>;
> qcom,gmu = <&gmu>;
>
> + nvmem-cells = <&gpu_speed_bin>;
> + nvmem-cell-names = "speed_bin";
> +
> interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
> interconnect-names = "gfx-mem";
>
> gpu_opp_table: opp-table {
> compatible = "operating-points-v2";
>
> + opp-825000000 {
> + opp-hz = /bits/ 64 <825000000>;
> + opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
> + opp-peak-kBps = <8532000>;
> + opp-supported-hw = <0x04>;
> + };
> +
> opp-800000000 {
> opp-hz = /bits/ 64 <800000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_TURBO>;
> opp-peak-kBps = <8532000>;
> + opp-supported-hw = <0x07>;
> };
>
> opp-650000000 {
> opp-hz = /bits/ 64 <650000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
> opp-peak-kBps = <7216000>;
> + opp-supported-hw = <0x07>;
> };
>
> opp-565000000 {
> opp-hz = /bits/ 64 <565000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
> opp-peak-kBps = <5412000>;
> + opp-supported-hw = <0x07>;
> };
>
> opp-430000000 {
> opp-hz = /bits/ 64 <430000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> opp-peak-kBps = <5412000>;
> + opp-supported-hw = <0x07>;
> };
>
> opp-355000000 {
> opp-hz = /bits/ 64 <355000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> opp-peak-kBps = <3072000>;
> + opp-supported-hw = <0x07>;
> };
>
> opp-267000000 {
> opp-hz = /bits/ 64 <267000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> opp-peak-kBps = <3072000>;
> + opp-supported-hw = <0x07>;
> };
>
> opp-180000000 {
> opp-hz = /bits/ 64 <180000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> opp-peak-kBps = <1804000>;
> + opp-supported-hw = <0x07>;
> };
> };
> };
>
A gentle ping.
-Akhil.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu
2020-12-07 10:42 [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu Akhil P Oommen
2020-12-07 10:42 ` [PATCH v3 2/2] arm: dts: sc7180: Add support for gpu fuse Akhil P Oommen
@ 2020-12-11 11:54 ` Akhil P Oommen
2021-01-04 18:07 ` Jordan Crouse
2 siblings, 0 replies; 5+ messages in thread
From: Akhil P Oommen @ 2020-12-11 11:54 UTC (permalink / raw)
To: freedreno, devicetree, Jordan Crouse, Rob Clark
Cc: linux-arm-msm, linux-kernel, dianders, mka, dri-devel
On 12/7/2020 4:12 PM, Akhil P Oommen wrote:
> Some GPUs support different max frequencies depending on the platform.
> To identify the correct variant, we should check the gpu speedbin
> fuse value. Add support for this speedbin detection to a6xx family
> along with the required fuse details for a618 gpu.
>
> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> ---
> Changes from v2:
> 1. Made the changes a6xx specific to save space.
> 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/a6xx_gpu.c | 74 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +
> 2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 1306618..6304578 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -10,10 +10,13 @@
>
> #include <linux/bitfield.h>
> #include <linux/devfreq.h>
> +#include <linux/nvmem-consumer.h>
> #include <linux/soc/qcom/llcc-qcom.h>
>
> #define GPU_PAS_ID 13
>
> +const u32 a618_speedbins[] = {0, 169, 174};
> +
> static inline bool _a6xx_check_idle(struct msm_gpu *gpu)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> @@ -1208,6 +1211,10 @@ static void a6xx_destroy(struct msm_gpu *gpu)
> a6xx_gmu_remove(a6xx_gpu);
>
> adreno_gpu_cleanup(adreno_gpu);
> +
> + if (a6xx_gpu->opp_table)
> + dev_pm_opp_put_supported_hw(a6xx_gpu->opp_table);
> +
> kfree(a6xx_gpu);
> }
>
> @@ -1264,6 +1271,67 @@ static uint32_t a6xx_get_rptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
> return ring->memptrs->rptr = gpu_read(gpu, REG_A6XX_CP_RB_RPTR);
> }
>
> +static u32 fuse_to_supp_hw(struct device *dev, u32 revn, u32 fuse)
> +{
> + int i;
> +
> + if (revn == 618) {
> + for (i = 0; i < ARRAY_SIZE(a618_speedbins); i++) {
> + if (fuse == a618_speedbins[i])
> + return (1 << i);
> + }
> + }
> +
> + DRM_DEV_ERROR(dev,
> + "missing support for speed-bin: %u. Some OPPs may not be supported by hardware",
> + fuse);
> + return ~0U;
> +}
> +
> +static int a6xx_set_supported_hw(struct device *dev, struct a6xx_gpu *a6xx_gpu,
> + u32 revn)
> +{
> +
> + struct opp_table *opp_table;
> + struct nvmem_cell *cell;
> + u32 supp_hw = ~0U;
> + void *buf;
> +
> + 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)) {
> + DRM_DEV_ERROR(dev,
> + "failed to read speed-bin. Some OPPs may not be supported by hardware");
> + goto done;
> + }
> +
> + buf = nvmem_cell_read(cell, NULL);
> + if (IS_ERR(buf)) {
> + nvmem_cell_put(cell);
> + DRM_DEV_ERROR(dev,
> + "failed to read speed-bin. Some OPPs may not be supported by hardware");
> + goto done;
> + }
> +
> + supp_hw = fuse_to_supp_hw(dev, revn, *((u32 *) buf));
> +
> + kfree(buf);
> + nvmem_cell_put(cell);
> +
> +done:
> + opp_table = dev_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> + if (IS_ERR(opp_table))
> + return PTR_ERR(opp_table);
> +
> + a6xx_gpu->opp_table = opp_table;
> + return 0;
> +}
> +
> static const struct adreno_gpu_funcs funcs = {
> .base = {
> .get_param = adreno_get_param,
> @@ -1325,6 +1393,12 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> a6xx_llc_slices_init(pdev, a6xx_gpu);
>
> + ret = a6xx_set_supported_hw(&pdev->dev, a6xx_gpu, info->revn);
> + if (ret) {
> + a6xx_destroy(&(a6xx_gpu->base.base));
> + return ERR_PTR(ret);
> + }
> +
> ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1);
> 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 e793d32..ce0610c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> @@ -33,6 +33,8 @@ struct a6xx_gpu {
> void *llc_slice;
> void *htw_llc_slice;
> bool have_mmu500;
> +
> + struct opp_table *opp_table;
> };
>
> #define to_a6xx_gpu(x) container_of(x, struct a6xx_gpu, base)
>
A gentle ping.
-Akhil.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu
2020-12-07 10:42 [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu Akhil P Oommen
2020-12-07 10:42 ` [PATCH v3 2/2] arm: dts: sc7180: Add support for gpu fuse Akhil P Oommen
2020-12-11 11:54 ` [PATCH v3 1/2] drm/msm: Add speed-bin support to a618 gpu Akhil P Oommen
@ 2021-01-04 18:07 ` Jordan Crouse
2 siblings, 0 replies; 5+ messages in thread
From: Jordan Crouse @ 2021-01-04 18:07 UTC (permalink / raw)
To: Akhil P Oommen
Cc: freedreno, devicetree, robh, dri-devel, linux-arm-msm,
linux-kernel, mka, robdclark, dianders
On Mon, Dec 07, 2020 at 04:12:07PM +0530, Akhil P Oommen wrote:
> Some GPUs support different max frequencies depending on the platform.
> To identify the correct variant, we should check the gpu speedbin
> fuse value. Add support for this speedbin detection to a6xx family
> along with the required fuse details for a618 gpu.
>
> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> ---
> Changes from v2:
> 1. Made the changes a6xx specific to save space.
> 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/a6xx_gpu.c | 74 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +
> 2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 1306618..6304578 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -10,10 +10,13 @@
>
> #include <linux/bitfield.h>
> #include <linux/devfreq.h>
> +#include <linux/nvmem-consumer.h>
> #include <linux/soc/qcom/llcc-qcom.h>
>
> #define GPU_PAS_ID 13
>
> +const u32 a618_speedbins[] = {0, 169, 174};
This still feels too generic to me - this could easily be something like:
static u32 a618_get_speed_bin(int rev)
{
if (rev == 0)
return 0;
else if (rev == 169)
return 1;
else if (rev == 174)
return 2;
return UINT_MAX;
}
I know Akhil can see a future where there might be other "pro" targets but it is
unclear to me when those will see upstream support and even if they do a
handful of inline-able functions still seem better to me than a group of const
arrays.
> +
> static inline bool _a6xx_check_idle(struct msm_gpu *gpu)
> {
> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> @@ -1208,6 +1211,10 @@ static void a6xx_destroy(struct msm_gpu *gpu)
> a6xx_gmu_remove(a6xx_gpu);
>
> adreno_gpu_cleanup(adreno_gpu);
> +
> + if (a6xx_gpu->opp_table)
> + dev_pm_opp_put_supported_hw(a6xx_gpu->opp_table);
> +
> kfree(a6xx_gpu);
> }
>
> @@ -1264,6 +1271,67 @@ static uint32_t a6xx_get_rptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
> return ring->memptrs->rptr = gpu_read(gpu, REG_A6XX_CP_RB_RPTR);
> }
>
> +static u32 fuse_to_supp_hw(struct device *dev, u32 revn, u32 fuse)
> +{
> + int i;
> +
> + if (revn == 618) {
> + for (i = 0; i < ARRAY_SIZE(a618_speedbins); i++) {
> + if (fuse == a618_speedbins[i])
> + return (1 << i);
> + }
> + }
u32 val = UINT_MAX;
if (revn == 618)
val = a618_get_speed_bin(fuse);
if (val == UINT_MAX)
DRM_DEV_ERROR(dev, "Missing support for speed-bin %u. Some OPPs may not
be supported by hardware", fuse);
return val;
> +
> + DRM_DEV_ERROR(dev,
> + "missing support for speed-bin: %u. Some OPPs may not be supported by hardware",
> + fuse);
> + return ~0U;
> +}
> +
> +static int a6xx_set_supported_hw(struct device *dev, struct a6xx_gpu *a6xx_gpu,
> + u32 revn)
> +{
> +
> + struct opp_table *opp_table;
> + struct nvmem_cell *cell;
> + u32 supp_hw = ~0U;
> + void *buf;
> +
> + 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)) {
> + DRM_DEV_ERROR(dev,
> + "failed to read speed-bin. Some OPPs may not be supported by hardware");
> + goto done;
> + }
> +
> + buf = nvmem_cell_read(cell, NULL);
> + if (IS_ERR(buf)) {
> + nvmem_cell_put(cell);
> + DRM_DEV_ERROR(dev,
> + "failed to read speed-bin. Some OPPs may not be supported by hardware");
> + goto done;
> + }
> +
> + supp_hw = fuse_to_supp_hw(dev, revn, *((u32 *) buf));
> +
> + kfree(buf);
> + nvmem_cell_put(cell);
> +
> +done:
> + opp_table = dev_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> + if (IS_ERR(opp_table))
> + return PTR_ERR(opp_table);
> +
> + a6xx_gpu->opp_table = opp_table;
> + return 0;
> +}
Beyond the comments above, I think the rest of this is reasonable.
Jordan
> +
> static const struct adreno_gpu_funcs funcs = {
> .base = {
> .get_param = adreno_get_param,
> @@ -1325,6 +1393,12 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> a6xx_llc_slices_init(pdev, a6xx_gpu);
>
> + ret = a6xx_set_supported_hw(&pdev->dev, a6xx_gpu, info->revn);
> + if (ret) {
> + a6xx_destroy(&(a6xx_gpu->base.base));
> + return ERR_PTR(ret);
> + }
> +
> ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1);
> 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 e793d32..ce0610c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> @@ -33,6 +33,8 @@ struct a6xx_gpu {
> void *llc_slice;
> void *htw_llc_slice;
> bool have_mmu500;
> +
> + struct opp_table *opp_table;
> };
>
> #define to_a6xx_gpu(x) container_of(x, struct a6xx_gpu, base)
> --
> 2.7.4
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread