[PATCH v3 0/6] Support for GPU ACD feature on Adreno X1-85

[PATCH v3 0/6] Support for GPU ACD feature on Adreno X1-85

[Akhil P Oommen]

This series adds support for ACD feature for Adreno GPU which helps to
lower the power consumption on GX rail and also sometimes is a requirement
to enable higher GPU frequencies. At high level, following are the
sequences required for ACD feature:
	1. Identify the ACD level data for each regulator corner
	2. Send a message to AOSS to switch voltage plan
	3. Send a table with ACD level information to GMU during every
	gpu wake up

For (1), it is better to keep ACD level data in devicetree because this
value depends on the process node, voltage margins etc which are
chipset specific. For instance, same GPU HW IP on a different chipset
would have a different set of values. So, a new schema which extends
opp-v2 is created to add a new property called "qcom,opp-acd-level".

ACD support is dynamically detected based on the presence of
"qcom,opp-acd-level" property in GPU's opp table. Also, qmp node should be
present under GMU node in devicetree for communication with AOSS.

The devicetree patch in this series adds the acd-level data for X1-85
GPU present in Snapdragon X1 Elite chipset.

The last two devicetree patches are for Bjorn and all the rest for
Rob Clark.

---
Changes in v3:
- Rebased on top of v6.13-rc4 since X1E doesn't boot properly with msm-next
- Update patternProperties regex (Krzysztof)
- Update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml
- Update the new dt properties' description
- Do not move qmp_get() to acd probe (Konrad)
- New patches: patch#2, #3 and #6
- Link to v2: https://lore.kernel.org/r/20241021-gpu-acd-v2-0-9c25a62803bc@quicinc.com

Changes in v2:
- Removed RFC tag for the series
- Improve documentation for the new dt bindings (Krzysztof)
- Add fallback compatible string for opp-table (Krzysztof)
- Link to v1: https://lore.kernel.org/r/20241012-gpu-acd-v1-0-1e5e91aa95b6@quicinc.com

---
Akhil P Oommen (6):
      drm/msm/adreno: Add support for ACD
      drm/msm: a6x: Rework qmp_get() error handling
      drm/msm/adreno: Add module param to disable ACD
      dt-bindings: opp: Add v2-qcom-adreno vendor bindings
      arm64: dts: qcom: x1e80100: Add ACD levels for GPU
      arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU

 .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 arch/arm64/boot/dts/qcom/x1e80100.dtsi             | 25 +++++-
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c              | 96 ++++++++++++++++++---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h              |  1 +
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 36 ++++++++
 drivers/gpu/drm/msm/adreno/a6xx_hfi.h              | 21 +++++
 drivers/gpu/drm/msm/adreno/adreno_device.c         |  4 +
 8 files changed, 268 insertions(+), 13 deletions(-)
---
base-commit: dbfac60febfa806abb2d384cb6441e77335d2799
change-id: 20240724-gpu-acd-6c1dc5dcf516

Best regards,
-- 
Akhil P Oommen <quic_akhilpo@quicinc.com>

[PATCH v3 1/6] drm/msm/adreno: Add support for ACD

[Akhil P Oommen]

ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce
the power consumption. In some chipsets, it is also a requirement to
support higher GPU frequencies. This patch adds support for GPU ACD by
sending necessary data to GMU and AOSS. The feature support for the
chipset is detected based on devicetree data.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 84 ++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  1 +
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 +++++++++++++++
 drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++
 4 files changed, 132 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 14db7376c712..2689e79aefa5 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1021,14 +1021,6 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 
 	gmu->hung = false;
 
-	/* Notify AOSS about the ACD state (unimplemented for now => disable it) */
-	if (!IS_ERR(gmu->qmp)) {
-		ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}",
-			       0 /* Hardcode ACD to be disabled for now */);
-		if (ret)
-			dev_err(gmu->dev, "failed to send GPU ACD state\n");
-	}
-
 	/* Turn on the resources */
 	pm_runtime_get_sync(gmu->dev);
 
@@ -1476,6 +1468,68 @@ static int a6xx_gmu_pwrlevels_probe(struct a6xx_gmu *gmu)
 	return a6xx_gmu_rpmh_votes_init(gmu);
 }
 
+static int a6xx_gmu_acd_probe(struct a6xx_gmu *gmu)
+{
+	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
+	struct a6xx_hfi_acd_table *cmd = &gmu->acd_table;
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+	struct msm_gpu *gpu = &adreno_gpu->base;
+	int ret, i, cmd_idx = 0;
+
+	cmd->version = 1;
+	cmd->stride = 1;
+	cmd->enable_by_level = 0;
+
+	/* Skip freq = 0 and parse acd-level for rest of the OPPs */
+	for (i = 1; i < gmu->nr_gpu_freqs; i++) {
+		struct dev_pm_opp *opp;
+		struct device_node *np;
+		unsigned long freq;
+		u32 val;
+
+		freq = gmu->gpu_freqs[i];
+		opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, freq, true);
+		np = dev_pm_opp_get_of_node(opp);
+
+		ret = of_property_read_u32(np, "qcom,opp-acd-level", &val);
+		of_node_put(np);
+		dev_pm_opp_put(opp);
+		if (ret == -EINVAL)
+			continue;
+		else if (ret) {
+			DRM_DEV_ERROR(gmu->dev, "Unable to read acd level for freq %lu\n", freq);
+			return ret;
+		}
+
+		cmd->enable_by_level |= BIT(i);
+		cmd->data[cmd_idx++] = val;
+	}
+
+	cmd->num_levels = cmd_idx;
+
+	/* It is a problem if qmp node is unavailable when ACD is required */
+	if (cmd->enable_by_level && IS_ERR_OR_NULL(gmu->qmp)) {
+		DRM_DEV_ERROR(gmu->dev, "Unable to send ACD state to AOSS\n");
+		return -EINVAL;
+	}
+
+	/* Otherwise, nothing to do if qmp is unavailable */
+	if (IS_ERR_OR_NULL(gmu->qmp))
+		return 0;
+
+	/*
+	 * Notify AOSS about the ACD state. AOSS is supposed to assume that ACD is disabled on
+	 * system reset. So it is harmless if we couldn't notify 'OFF' state
+	 */
+	ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}", !!cmd->enable_by_level);
+	if (ret && cmd->enable_by_level) {
+		DRM_DEV_ERROR(gmu->dev, "Failed to send ACD state to AOSS\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int a6xx_gmu_clocks_probe(struct a6xx_gmu *gmu)
 {
 	int ret = devm_clk_bulk_get_all(gmu->dev, &gmu->clocks);
@@ -1793,7 +1847,7 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	gmu->qmp = qmp_get(gmu->dev);
 	if (IS_ERR(gmu->qmp) && adreno_is_a7xx(adreno_gpu)) {
 		ret = PTR_ERR(gmu->qmp);
-		goto remove_device_link;
+		goto detach_gxpd;
 	}
 
 	init_completion(&gmu->pd_gate);
@@ -1809,6 +1863,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	/* Get the power levels for the GMU and GPU */
 	a6xx_gmu_pwrlevels_probe(gmu);
 
+	ret = a6xx_gmu_acd_probe(gmu);
+	if (ret)
+		goto detach_gxpd;
+
 	/* Set up the HFI queues */
 	a6xx_hfi_init(gmu);
 
@@ -1819,7 +1877,13 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 
 	return 0;
 
-remove_device_link:
+detach_gxpd:
+	if (!IS_ERR_OR_NULL(gmu->gxpd))
+		dev_pm_domain_detach(gmu->gxpd, false);
+
+	if (!IS_ERR_OR_NULL(gmu->qmp))
+		qmp_put(gmu->qmp);
+
 	device_link_del(link);
 
 detach_cxpd:
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index b4a79f88ccf4..87d225b08e9b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -81,6 +81,7 @@ struct a6xx_gmu {
 	int nr_gpu_freqs;
 	unsigned long gpu_freqs[16];
 	u32 gx_arc_votes[16];
+	struct a6xx_hfi_acd_table acd_table;
 
 	int nr_gmu_freqs;
 	unsigned long gmu_freqs[4];
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
index cb8844ed46b2..3c183c1c6266 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -702,6 +702,38 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
 		NULL, 0);
 }
 
+#define HFI_FEATURE_ACD 12
+
+static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu)
+{
+	struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table;
+	struct a6xx_hfi_msg_feature_ctrl msg = {
+		.feature = HFI_FEATURE_ACD,
+		.enable = 1,
+		.data = 0,
+	};
+	int ret;
+
+	if (!acd_table->enable_by_level)
+		return 0;
+
+	/* Enable ACD feature at GMU */
+	ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, sizeof(msg), NULL, 0);
+	if (ret) {
+		DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret);
+		return ret;
+	}
+
+	/* Send ACD table to GMU */
+	ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), NULL, 0);
+	if (ret) {
+		DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int a6xx_hfi_send_test(struct a6xx_gmu *gmu)
 {
 	struct a6xx_hfi_msg_test msg = { 0 };
@@ -799,6 +831,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state)
 	if (ret)
 		return ret;
 
+	ret = a6xx_hfi_enable_acd(gmu);
+	if (ret)
+		return ret;
+
 	ret = a6xx_hfi_send_core_fw_start(gmu);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
index 528110169398..51864c8ad0e6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
@@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test {
 	u32 header;
 };
 
+#define HFI_H2F_MSG_ACD 7
+#define MAX_ACD_STRIDE 2
+
+struct a6xx_hfi_acd_table {
+	u32 header;
+	u32 version;
+	u32 enable_by_level;
+	u32 stride;
+	u32 num_levels;
+	u32 data[16 * MAX_ACD_STRIDE];
+};
+
 #define HFI_H2F_MSG_START 10
 
 struct a6xx_hfi_msg_start {
 	u32 header;
 };
 
+#define HFI_H2F_FEATURE_CTRL 11
+
+struct a6xx_hfi_msg_feature_ctrl {
+	u32 header;
+	u32 feature;
+	u32 enable;
+	u32 data;
+};
+
 #define HFI_H2F_MSG_CORE_FW_START 14
 
 struct a6xx_hfi_msg_core_fw_start {

-- 
2.45.2

Re: [PATCH v3 1/6] drm/msm/adreno: Add support for ACD

[neil.armstrong]

On 30/12/2024 22:11, Akhil P Oommen wrote:
> ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce
> the power consumption. In some chipsets, it is also a requirement to
> support higher GPU frequencies. This patch adds support for GPU ACD by
> sending necessary data to GMU and AOSS. The feature support for the
> chipset is detected based on devicetree data.
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>   drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 84 ++++++++++++++++++++++++++++++-----
>   drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  1 +
>   drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 +++++++++++++++
>   drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++
>   4 files changed, 132 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 14db7376c712..2689e79aefa5 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -1021,14 +1021,6 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
>   
>   	gmu->hung = false;
>   
> -	/* Notify AOSS about the ACD state (unimplemented for now => disable it) */
> -	if (!IS_ERR(gmu->qmp)) {
> -		ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}",
> -			       0 /* Hardcode ACD to be disabled for now */);
> -		if (ret)
> -			dev_err(gmu->dev, "failed to send GPU ACD state\n");
> -	}
> -
>   	/* Turn on the resources */
>   	pm_runtime_get_sync(gmu->dev);
>   
> @@ -1476,6 +1468,68 @@ static int a6xx_gmu_pwrlevels_probe(struct a6xx_gmu *gmu)
>   	return a6xx_gmu_rpmh_votes_init(gmu);
>   }
>   
> +static int a6xx_gmu_acd_probe(struct a6xx_gmu *gmu)
> +{
> +	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
> +	struct a6xx_hfi_acd_table *cmd = &gmu->acd_table;
> +	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> +	struct msm_gpu *gpu = &adreno_gpu->base;
> +	int ret, i, cmd_idx = 0;
> +
> +	cmd->version = 1;
> +	cmd->stride = 1;
> +	cmd->enable_by_level = 0;
> +
> +	/* Skip freq = 0 and parse acd-level for rest of the OPPs */
> +	for (i = 1; i < gmu->nr_gpu_freqs; i++) {
> +		struct dev_pm_opp *opp;
> +		struct device_node *np;
> +		unsigned long freq;
> +		u32 val;
> +
> +		freq = gmu->gpu_freqs[i];
> +		opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, freq, true);
> +		np = dev_pm_opp_get_of_node(opp);
> +
> +		ret = of_property_read_u32(np, "qcom,opp-acd-level", &val);
> +		of_node_put(np);
> +		dev_pm_opp_put(opp);
> +		if (ret == -EINVAL)
> +			continue;
> +		else if (ret) {
> +			DRM_DEV_ERROR(gmu->dev, "Unable to read acd level for freq %lu\n", freq);
> +			return ret;
> +		}
> +
> +		cmd->enable_by_level |= BIT(i);
> +		cmd->data[cmd_idx++] = val;
> +	}
> +
> +	cmd->num_levels = cmd_idx;
> +
> +	/* It is a problem if qmp node is unavailable when ACD is required */
> +	if (cmd->enable_by_level && IS_ERR_OR_NULL(gmu->qmp)) {
> +		DRM_DEV_ERROR(gmu->dev, "Unable to send ACD state to AOSS\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Otherwise, nothing to do if qmp is unavailable */
> +	if (IS_ERR_OR_NULL(gmu->qmp))
> +		return 0;
> +
> +	/*
> +	 * Notify AOSS about the ACD state. AOSS is supposed to assume that ACD is disabled on
> +	 * system reset. So it is harmless if we couldn't notify 'OFF' state
> +	 */
> +	ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}", !!cmd->enable_by_level);
> +	if (ret && cmd->enable_by_level) {
> +		DRM_DEV_ERROR(gmu->dev, "Failed to send ACD state to AOSS\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int a6xx_gmu_clocks_probe(struct a6xx_gmu *gmu)
>   {
>   	int ret = devm_clk_bulk_get_all(gmu->dev, &gmu->clocks);
> @@ -1793,7 +1847,7 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>   	gmu->qmp = qmp_get(gmu->dev);
>   	if (IS_ERR(gmu->qmp) && adreno_is_a7xx(adreno_gpu)) {
>   		ret = PTR_ERR(gmu->qmp);
> -		goto remove_device_link;
> +		goto detach_gxpd;
>   	}
>   
>   	init_completion(&gmu->pd_gate);
> @@ -1809,6 +1863,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>   	/* Get the power levels for the GMU and GPU */
>   	a6xx_gmu_pwrlevels_probe(gmu);
>   
> +	ret = a6xx_gmu_acd_probe(gmu);
> +	if (ret)
> +		goto detach_gxpd;
> +
>   	/* Set up the HFI queues */
>   	a6xx_hfi_init(gmu);
>   
> @@ -1819,7 +1877,13 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
>   
>   	return 0;
>   
> -remove_device_link:
> +detach_gxpd:
> +	if (!IS_ERR_OR_NULL(gmu->gxpd))
> +		dev_pm_domain_detach(gmu->gxpd, false);
> +
> +	if (!IS_ERR_OR_NULL(gmu->qmp))
> +		qmp_put(gmu->qmp);
> +
>   	device_link_del(link);
>   
>   detach_cxpd:
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index b4a79f88ccf4..87d225b08e9b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -81,6 +81,7 @@ struct a6xx_gmu {
>   	int nr_gpu_freqs;
>   	unsigned long gpu_freqs[16];
>   	u32 gx_arc_votes[16];
> +	struct a6xx_hfi_acd_table acd_table;
>   
>   	int nr_gmu_freqs;
>   	unsigned long gmu_freqs[4];
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> index cb8844ed46b2..3c183c1c6266 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -702,6 +702,38 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu)
>   		NULL, 0);
>   }
>   
> +#define HFI_FEATURE_ACD 12
> +
> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu)
> +{
> +	struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table;
> +	struct a6xx_hfi_msg_feature_ctrl msg = {
> +		.feature = HFI_FEATURE_ACD,
> +		.enable = 1,
> +		.data = 0,
> +	};
> +	int ret;
> +
> +	if (!acd_table->enable_by_level)
> +		return 0;
> +
> +	/* Enable ACD feature at GMU */
> +	ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg, sizeof(msg), NULL, 0);
> +	if (ret) {
> +		DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	/* Send ACD table to GMU */
> +	ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), NULL, 0);


Seems you still don't send the proper acd_table

Neil

> +	if (ret) {
> +		DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int a6xx_hfi_send_test(struct a6xx_gmu *gmu)
>   {
>   	struct a6xx_hfi_msg_test msg = { 0 };
> @@ -799,6 +831,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state)
>   	if (ret)
>   		return ret;
>   
> +	ret = a6xx_hfi_enable_acd(gmu);
> +	if (ret)
> +		return ret;
> +
>   	ret = a6xx_hfi_send_core_fw_start(gmu);
>   	if (ret)
>   		return ret;
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
> index 528110169398..51864c8ad0e6 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test {
>   	u32 header;
>   };
>   
> +#define HFI_H2F_MSG_ACD 7
> +#define MAX_ACD_STRIDE 2
> +
> +struct a6xx_hfi_acd_table {
> +	u32 header;
> +	u32 version;
> +	u32 enable_by_level;
> +	u32 stride;
> +	u32 num_levels;
> +	u32 data[16 * MAX_ACD_STRIDE];
> +};
> +
>   #define HFI_H2F_MSG_START 10
>   
>   struct a6xx_hfi_msg_start {
>   	u32 header;
>   };
>   
> +#define HFI_H2F_FEATURE_CTRL 11
> +
> +struct a6xx_hfi_msg_feature_ctrl {
> +	u32 header;
> +	u32 feature;
> +	u32 enable;
> +	u32 data;
> +};
> +
>   #define HFI_H2F_MSG_CORE_FW_START 14
>   
>   struct a6xx_hfi_msg_core_fw_start {
> 

Re: [PATCH v3 1/6] drm/msm/adreno: Add support for ACD

[Akhil P Oommen]

On 12/31/2024 3:18 PM, neil.armstrong@linaro.org wrote:
> On 30/12/2024 22:11, Akhil P Oommen wrote:
>> ACD a.k.a Adaptive Clock Distribution is a feature which helps to reduce
>> the power consumption. In some chipsets, it is also a requirement to
>> support higher GPU frequencies. This patch adds support for GPU ACD by
>> sending necessary data to GMU and AOSS. The feature support for the
>> chipset is detected based on devicetree data.
>>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 84 +++++++++++++++++++++++++
>> +++++-----
>>   drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  1 +
>>   drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 36 +++++++++++++++
>>   drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 21 +++++++++
>>   4 files changed, 132 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/
>> msm/adreno/a6xx_gmu.c
>> index 14db7376c712..2689e79aefa5 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>> @@ -1021,14 +1021,6 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
>>         gmu->hung = false;
>>   -    /* Notify AOSS about the ACD state (unimplemented for now =>
>> disable it) */
>> -    if (!IS_ERR(gmu->qmp)) {
>> -        ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}",
>> -                   0 /* Hardcode ACD to be disabled for now */);
>> -        if (ret)
>> -            dev_err(gmu->dev, "failed to send GPU ACD state\n");
>> -    }
>> -
>>       /* Turn on the resources */
>>       pm_runtime_get_sync(gmu->dev);
>>   @@ -1476,6 +1468,68 @@ static int a6xx_gmu_pwrlevels_probe(struct
>> a6xx_gmu *gmu)
>>       return a6xx_gmu_rpmh_votes_init(gmu);
>>   }
>>   +static int a6xx_gmu_acd_probe(struct a6xx_gmu *gmu)
>> +{
>> +    struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
>> +    struct a6xx_hfi_acd_table *cmd = &gmu->acd_table;
>> +    struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
>> +    struct msm_gpu *gpu = &adreno_gpu->base;
>> +    int ret, i, cmd_idx = 0;
>> +
>> +    cmd->version = 1;
>> +    cmd->stride = 1;
>> +    cmd->enable_by_level = 0;
>> +
>> +    /* Skip freq = 0 and parse acd-level for rest of the OPPs */
>> +    for (i = 1; i < gmu->nr_gpu_freqs; i++) {
>> +        struct dev_pm_opp *opp;
>> +        struct device_node *np;
>> +        unsigned long freq;
>> +        u32 val;
>> +
>> +        freq = gmu->gpu_freqs[i];
>> +        opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, freq, true);
>> +        np = dev_pm_opp_get_of_node(opp);
>> +
>> +        ret = of_property_read_u32(np, "qcom,opp-acd-level", &val);
>> +        of_node_put(np);
>> +        dev_pm_opp_put(opp);
>> +        if (ret == -EINVAL)
>> +            continue;
>> +        else if (ret) {
>> +            DRM_DEV_ERROR(gmu->dev, "Unable to read acd level for
>> freq %lu\n", freq);
>> +            return ret;
>> +        }
>> +
>> +        cmd->enable_by_level |= BIT(i);
>> +        cmd->data[cmd_idx++] = val;
>> +    }
>> +
>> +    cmd->num_levels = cmd_idx;
>> +
>> +    /* It is a problem if qmp node is unavailable when ACD is
>> required */
>> +    if (cmd->enable_by_level && IS_ERR_OR_NULL(gmu->qmp)) {
>> +        DRM_DEV_ERROR(gmu->dev, "Unable to send ACD state to AOSS\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Otherwise, nothing to do if qmp is unavailable */
>> +    if (IS_ERR_OR_NULL(gmu->qmp))
>> +        return 0;
>> +
>> +    /*
>> +     * Notify AOSS about the ACD state. AOSS is supposed to assume
>> that ACD is disabled on
>> +     * system reset. So it is harmless if we couldn't notify 'OFF' state
>> +     */
>> +    ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}", !!
>> cmd->enable_by_level);
>> +    if (ret && cmd->enable_by_level) {
>> +        DRM_DEV_ERROR(gmu->dev, "Failed to send ACD state to AOSS\n");
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int a6xx_gmu_clocks_probe(struct a6xx_gmu *gmu)
>>   {
>>       int ret = devm_clk_bulk_get_all(gmu->dev, &gmu->clocks);
>> @@ -1793,7 +1847,7 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu,
>> struct device_node *node)
>>       gmu->qmp = qmp_get(gmu->dev);
>>       if (IS_ERR(gmu->qmp) && adreno_is_a7xx(adreno_gpu)) {
>>           ret = PTR_ERR(gmu->qmp);
>> -        goto remove_device_link;
>> +        goto detach_gxpd;
>>       }
>>         init_completion(&gmu->pd_gate);
>> @@ -1809,6 +1863,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu,
>> struct device_node *node)
>>       /* Get the power levels for the GMU and GPU */
>>       a6xx_gmu_pwrlevels_probe(gmu);
>>   +    ret = a6xx_gmu_acd_probe(gmu);
>> +    if (ret)
>> +        goto detach_gxpd;
>> +
>>       /* Set up the HFI queues */
>>       a6xx_hfi_init(gmu);
>>   @@ -1819,7 +1877,13 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu,
>> struct device_node *node)
>>         return 0;
>>   -remove_device_link:
>> +detach_gxpd:
>> +    if (!IS_ERR_OR_NULL(gmu->gxpd))
>> +        dev_pm_domain_detach(gmu->gxpd, false);
>> +
>> +    if (!IS_ERR_OR_NULL(gmu->qmp))
>> +        qmp_put(gmu->qmp);
>> +
>>       device_link_del(link);
>>     detach_cxpd:
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/
>> msm/adreno/a6xx_gmu.h
>> index b4a79f88ccf4..87d225b08e9b 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
>> @@ -81,6 +81,7 @@ struct a6xx_gmu {
>>       int nr_gpu_freqs;
>>       unsigned long gpu_freqs[16];
>>       u32 gx_arc_votes[16];
>> +    struct a6xx_hfi_acd_table acd_table;
>>         int nr_gmu_freqs;
>>       unsigned long gmu_freqs[4];
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/
>> msm/adreno/a6xx_hfi.c
>> index cb8844ed46b2..3c183c1c6266 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
>> @@ -702,6 +702,38 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu
>> *gmu)
>>           NULL, 0);
>>   }
>>   +#define HFI_FEATURE_ACD 12
>> +
>> +static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu)
>> +{
>> +    struct a6xx_hfi_acd_table *acd_table = &gmu->acd_table;
>> +    struct a6xx_hfi_msg_feature_ctrl msg = {
>> +        .feature = HFI_FEATURE_ACD,
>> +        .enable = 1,
>> +        .data = 0,
>> +    };
>> +    int ret;
>> +
>> +    if (!acd_table->enable_by_level)
>> +        return 0;
>> +
>> +    /* Enable ACD feature at GMU */
>> +    ret = a6xx_hfi_send_msg(gmu, HFI_H2F_FEATURE_CTRL, &msg,
>> sizeof(msg), NULL, 0);
>> +    if (ret) {
>> +        DRM_DEV_ERROR(gmu->dev, "Unable to enable ACD (%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    /* Send ACD table to GMU */
>> +    ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg),
>> NULL, 0);
> 
> 
> Seems you still don't send the proper acd_table

Aah! I forgot this one. Usually the end-to-end validation is done by HW
folks during Bringups. But I think I can do some additional validation
on the gmu fw side. Will check that and post Rev-4.

-Akhil.

> 
> Neil
> 
>> +    if (ret) {
>> +        DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int a6xx_hfi_send_test(struct a6xx_gmu *gmu)
>>   {
>>       struct a6xx_hfi_msg_test msg = { 0 };
>> @@ -799,6 +831,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int
>> boot_state)
>>       if (ret)
>>           return ret;
>>   +    ret = a6xx_hfi_enable_acd(gmu);
>> +    if (ret)
>> +        return ret;
>> +
>>       ret = a6xx_hfi_send_core_fw_start(gmu);
>>       if (ret)
>>           return ret;
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/
>> msm/adreno/a6xx_hfi.h
>> index 528110169398..51864c8ad0e6 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
>> @@ -151,12 +151,33 @@ struct a6xx_hfi_msg_test {
>>       u32 header;
>>   };
>>   +#define HFI_H2F_MSG_ACD 7
>> +#define MAX_ACD_STRIDE 2
>> +
>> +struct a6xx_hfi_acd_table {
>> +    u32 header;
>> +    u32 version;
>> +    u32 enable_by_level;
>> +    u32 stride;
>> +    u32 num_levels;
>> +    u32 data[16 * MAX_ACD_STRIDE];
>> +};
>> +
>>   #define HFI_H2F_MSG_START 10
>>     struct a6xx_hfi_msg_start {
>>       u32 header;
>>   };
>>   +#define HFI_H2F_FEATURE_CTRL 11
>> +
>> +struct a6xx_hfi_msg_feature_ctrl {
>> +    u32 header;
>> +    u32 feature;
>> +    u32 enable;
>> +    u32 data;
>> +};
>> +
>>   #define HFI_H2F_MSG_CORE_FW_START 14
>>     struct a6xx_hfi_msg_core_fw_start {
>>
> 

[PATCH v3 2/6] drm/msm: a6x: Rework qmp_get() error handling

[Akhil P Oommen]

Fix the following for qmp_get() errors:

1. Correctly handle probe defer for A6x GPUs
2. Ignore other errors because those are okay when GPU ACD is
not required. They are checked again during gpu acd probe.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 2689e79aefa5..1f213a0fc61b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1844,9 +1844,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 		goto detach_cxpd;
 	}
 
+	/* Other errors are handled during GPU ACD probe */
 	gmu->qmp = qmp_get(gmu->dev);
-	if (IS_ERR(gmu->qmp) && adreno_is_a7xx(adreno_gpu)) {
-		ret = PTR_ERR(gmu->qmp);
+	if (PTR_ERR_OR_ZERO(gmu->qmp) == -EPROBE_DEFER) {
+		ret = -EPROBE_DEFER;
 		goto detach_gxpd;
 	}
 

-- 
2.45.2

Re: [PATCH v3 2/6] drm/msm: a6x: Rework qmp_get() error handling

[Konrad Dybcio]

On 30.12.2024 10:11 PM, Akhil P Oommen wrote:
> Fix the following for qmp_get() errors:
> 
> 1. Correctly handle probe defer for A6x GPUs
> 2. Ignore other errors because those are okay when GPU ACD is
> not required. They are checked again during gpu acd probe.
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---

I think this looks right

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

[PATCH v3 3/6] drm/msm/adreno: Add module param to disable ACD

[Akhil P Oommen]

Add a module param to disable ACD which will help to quickly rule it
out for any GPU issues.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 7 +++++++
 drivers/gpu/drm/msm/adreno/adreno_device.c | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 1f213a0fc61b..ce08eed572c8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1475,6 +1475,13 @@ static int a6xx_gmu_acd_probe(struct a6xx_gmu *gmu)
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
 	struct msm_gpu *gpu = &adreno_gpu->base;
 	int ret, i, cmd_idx = 0;
+	extern bool disable_acd;
+
+	/* Skip ACD probe if requested via module param */
+	if (disable_acd) {
+		DRM_DEV_ERROR(gmu->dev, "Skipping GPU ACD probe\n");
+		return 0;
+	}
 
 	cmd->version = 1;
 	cmd->stride = 1;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 236b25c094cd..f4552b8c6767 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -24,6 +24,10 @@ int enable_preemption = -1;
 MODULE_PARM_DESC(enable_preemption, "Enable preemption (A7xx only) (1=on , 0=disable, -1=auto (default))");
 module_param(enable_preemption, int, 0600);
 
+bool disable_acd;
+MODULE_PARM_DESC(disable_acd, "Forcefully disable GPU ACD");
+module_param_unsafe(disable_acd, bool, 0600);
+
 extern const struct adreno_gpulist a2xx_gpulist;
 extern const struct adreno_gpulist a3xx_gpulist;
 extern const struct adreno_gpulist a4xx_gpulist;

-- 
2.45.2

Re: [PATCH v3 3/6] drm/msm/adreno: Add module param to disable ACD

[Konrad Dybcio]

On 30.12.2024 10:11 PM, Akhil P Oommen wrote:
> Add a module param to disable ACD which will help to quickly rule it
> out for any GPU issues.
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---

Is that something useful during internal development, or do we
see ACD causing issues in the wild?

If the latter, would that be caused by e.g. outdated firmware?

Konrad

Re: [PATCH v3 3/6] drm/msm/adreno: Add module param to disable ACD

[Akhil P Oommen]

On 12/31/2024 4:02 PM, Konrad Dybcio wrote:
> On 30.12.2024 10:11 PM, Akhil P Oommen wrote:
>> Add a module param to disable ACD which will help to quickly rule it
>> out for any GPU issues.
>>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>> ---
> 
> Is that something useful during internal development, or do we
> see ACD causing issues in the wild?
> 
> If the latter, would that be caused by e.g. outdated firmware?

It is rare to see ACD issues in production because there is pretty
stringent testing done during development. Still, disabling ACD is one
of the first thing we try because debugging HW spec violation is a
nightmare.

Regarding firmware, yeah, it is possible but rare in production.

-Akhil.

> 
> Konrad

[PATCH v3 4/6] dt-bindings: opp: Add v2-qcom-adreno vendor bindings

[Akhil P Oommen]

Add a new schema which extends opp-v2 to support a new vendor specific
property required for Adreno GPUs found in Qualcomm's SoCs. The new
property called "qcom,opp-acd-level" carries a u32 value recommended
for each opp needs to be shared to GMU during runtime.

Also, update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 98 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
new file mode 100644
index 000000000000..b7874f43aaf6
--- /dev/null
+++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/opp/opp-v2-qcom-adreno.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Adreno compatible OPP supply
+
+description:
+  Adreno GPUs present in Qualcomm's Snapdragon chipsets uses an OPP specific
+  ACD related information tailored for the specific chipset. This binding
+  provides the information needed to describe such a hardware value.
+
+maintainers:
+  - Rob Clark <robdclark@gmail.com>
+
+allOf:
+  - $ref: opp-v2-base.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: operating-points-v2-adreno
+      - const: operating-points-v2
+
+patternProperties:
+  '^opp-[0-9]+$':
+    type: object
+    additionalProperties: false
+
+    properties:
+      opp-hz: true
+
+      opp-level: true
+
+      opp-peak-kBps: true
+
+      opp-supported-hw: true
+
+      qcom,opp-acd-level:
+        description: |
+          A positive value representing the ACD (Adaptive Clock Distribution,
+          a fancy name for clk throttling during voltage droop) level associated
+          with this OPP node. This value is shared to a co-processor inside GPU
+          (called Graphics Management Unit a.k.a GMU) during wake up. It may not
+          be present for some OPPs and GMU will disable ACD while transitioning
+          to that OPP. This value encodes a voltage threshold, delay cycles &
+          calibration margins which are identified by characterization of the
+          SoC. So, it doesn't have any unit. This data is passed to GMU firmware
+          via 'HFI_H2F_MSG_ACD' packet.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - opp-hz
+      - opp-level
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    gpu_opp_table: opp-table {
+        compatible = "operating-points-v2-adreno", "operating-points-v2";
+
+        opp-687000000 {
+            opp-hz = /bits/ 64 <687000000>;
+            opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+            opp-peak-kBps = <8171875>;
+            qcom,opp-acd-level = <0x882e5ffd>;
+        };
+
+        opp-550000000 {
+            opp-hz = /bits/ 64 <550000000>;
+            opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
+            opp-peak-kBps = <6074219>;
+            qcom,opp-acd-level = <0xc0285ffd>;
+        };
+
+        opp-390000000 {
+            opp-hz = /bits/ 64 <390000000>;
+            opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+            opp-peak-kBps = <3000000>;
+            qcom,opp-acd-level = <0xc0285ffd>;
+        };
+
+        opp-300000000 {
+            opp-hz = /bits/ 64 <300000000>;
+            opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
+            opp-peak-kBps = <2136719>;
+            /* Intentionally left out qcom,opp-acd-level property here */
+        };
+
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 910305c11e8a..f7119623e1f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7292,6 +7292,7 @@ S:	Maintained
 B:	https://gitlab.freedesktop.org/drm/msm/-/issues
 T:	git https://gitlab.freedesktop.org/drm/msm.git
 F:	Documentation/devicetree/bindings/display/msm/gpu.yaml
+F:	Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
 F:	drivers/gpu/drm/msm/adreno/
 F:	drivers/gpu/drm/msm/msm_gpu.*
 F:	drivers/gpu/drm/msm/msm_gpu_devfreq.*

-- 
2.45.2

Re: [PATCH v3 4/6] dt-bindings: opp: Add v2-qcom-adreno vendor bindings

[Rob Herring (Arm)]

On Tue, 31 Dec 2024 02:41:05 +0530, Akhil P Oommen wrote:
> Add a new schema which extends opp-v2 to support a new vendor specific
> property required for Adreno GPUs found in Qualcomm's SoCs. The new
> property called "qcom,opp-acd-level" carries a u32 value recommended
> for each opp needs to be shared to GMU during runtime.
> 
> Also, update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml.
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---
>  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 98 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml: ignoring, error parsing file
Traceback (most recent call last):
  File "/usr/bin/yamllint", line 33, in <module>
    sys.exit(load_entry_point('yamllint==1.29.0', 'console_scripts', 'yamllint')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yamllint/cli.py", line 228, in run
    prob_level = show_problems(problems, file, args_format=args.format,
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/yamllint/cli.py", line 113, in show_problems
    for problem in problems:
  File "/usr/lib/python3/dist-packages/yamllint/linter.py", line 200, in _run
    for problem in get_cosmetic_problems(buffer, conf, filepath):
  File "/usr/lib/python3/dist-packages/yamllint/linter.py", line 137, in get_cosmetic_problems
    for problem in rule.check(rule_conf,
  File "/usr/lib/python3/dist-packages/yamllint/rules/indentation.py", line 583, in check
    yield from _check(conf, token, prev, next, nextnext, context)
  File "/usr/lib/python3/dist-packages/yamllint/rules/indentation.py", line 344, in _check
    if expected < 0:
       ^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'NoneType' and 'int'
./Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml:97:1: did not find expected key
make[2]: *** Deleting file 'Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.example.dts'
Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml:97:1: did not find expected key
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1506: dt_binding_check] Error 2
make: *** [Makefile:251: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241231-gpu-acd-v3-4-3ba73660e9ca@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 4/6] dt-bindings: opp: Add v2-qcom-adreno vendor bindings

[Konrad Dybcio]

On 30.12.2024 11:25 PM, Rob Herring (Arm) wrote:
> 
> On Tue, 31 Dec 2024 02:41:05 +0530, Akhil P Oommen wrote:
>> Add a new schema which extends opp-v2 to support a new vendor specific
>> property required for Adreno GPUs found in Qualcomm's SoCs. The new
>> property called "qcom,opp-acd-level" carries a u32 value recommended
>> for each opp needs to be shared to GMU during runtime.
>>
>> Also, update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml.
>>
>> Cc: Rob Clark <robdclark@gmail.com>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>> ---
>>  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
>>  MAINTAINERS                                        |  1 +
>>  2 files changed, 98 insertions(+)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml: ignoring, error parsing file
> Traceback (most recent call last):
>   File "/usr/bin/yamllint", line 33, in <module>
>     sys.exit(load_entry_point('yamllint==1.29.0', 'console_scripts', 'yamllint')())
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/usr/lib/python3/dist-packages/yamllint/cli.py", line 228, in run
>     prob_level = show_problems(problems, file, args_format=args.format,

You need to shift the closing '}':

diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
index b7874f43aaf6..46fbffaf0a61 100644
--- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
+++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
@@ -93,5 +93,4 @@ examples:
             opp-peak-kBps = <2136719>;
             /* Intentionally left out qcom,opp-acd-level property here */
         };
-
-};
+    };


Konrad

Re: [PATCH v3 4/6] dt-bindings: opp: Add v2-qcom-adreno vendor bindings

[Akhil P Oommen]

On 12/31/2024 3:09 PM, Konrad Dybcio wrote:
> On 30.12.2024 11:25 PM, Rob Herring (Arm) wrote:
>>
>> On Tue, 31 Dec 2024 02:41:05 +0530, Akhil P Oommen wrote:
>>> Add a new schema which extends opp-v2 to support a new vendor specific
>>> property required for Adreno GPUs found in Qualcomm's SoCs. The new
>>> property called "qcom,opp-acd-level" carries a u32 value recommended
>>> for each opp needs to be shared to GMU during runtime.
>>>
>>> Also, update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml.
>>>
>>> Cc: Rob Clark <robdclark@gmail.com>
>>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
>>> ---
>>>  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
>>>  MAINTAINERS                                        |  1 +
>>>  2 files changed, 98 insertions(+)
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml: ignoring, error parsing file
>> Traceback (most recent call last):
>>   File "/usr/bin/yamllint", line 33, in <module>
>>     sys.exit(load_entry_point('yamllint==1.29.0', 'console_scripts', 'yamllint')())
>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>   File "/usr/lib/python3/dist-packages/yamllint/cli.py", line 228, in run
>>     prob_level = show_problems(problems, file, args_format=args.format,
> 
> You need to shift the closing '}':
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml b/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
> index b7874f43aaf6..46fbffaf0a61 100644
> --- a/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
> +++ b/Documentation/devicetree/bindings/opp/opp-v2-qcom-adreno.yaml
> @@ -93,5 +93,4 @@ examples:
>              opp-peak-kBps = <2136719>;
>              /* Intentionally left out qcom,opp-acd-level property here */
>          };
> -
> -};
> +    };
> 

Thanks. Will update.

-Akhil

> 
> Konrad

[PATCH v3 5/6] arm64: dts: qcom: x1e80100: Add ACD levels for GPU

[Akhil P Oommen]

Update GPU node to include acd level values.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 88805629ed2b..2cf16f904aaa 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -3335,60 +3335,69 @@ zap-shader {
 			};
 
 			gpu_opp_table: opp-table {
-				compatible = "operating-points-v2";
+				compatible = "operating-points-v2-adreno", "operating-points-v2";
 
 				opp-1100000000 {
 					opp-hz = /bits/ 64 <1100000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
 					opp-peak-kBps = <16500000>;
+					qcom,opp-acd-level = <0xa82a5ffd>;
 				};
 
 				opp-1000000000 {
 					opp-hz = /bits/ 64 <1000000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_TURBO>;
 					opp-peak-kBps = <14398438>;
+					qcom,opp-acd-level = <0xa82b5ffd>;
 				};
 
 				opp-925000000 {
 					opp-hz = /bits/ 64 <925000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
 					opp-peak-kBps = <14398438>;
+					qcom,opp-acd-level = <0xa82b5ffd>;
 				};
 
 				opp-800000000 {
 					opp-hz = /bits/ 64 <800000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
 					opp-peak-kBps = <12449219>;
+					qcom,opp-acd-level = <0xa82c5ffd>;
 				};
 
 				opp-744000000 {
 					opp-hz = /bits/ 64 <744000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L2>;
 					opp-peak-kBps = <10687500>;
+					qcom,opp-acd-level = <0x882e5ffd>;
 				};
 
 				opp-687000000 {
 					opp-hz = /bits/ 64 <687000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
 					opp-peak-kBps = <8171875>;
+					qcom,opp-acd-level = <0x882e5ffd>;
 				};
 
 				opp-550000000 {
 					opp-hz = /bits/ 64 <550000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
 					opp-peak-kBps = <6074219>;
+					qcom,opp-acd-level = <0xc0285ffd>;
 				};
 
 				opp-390000000 {
 					opp-hz = /bits/ 64 <390000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
 					opp-peak-kBps = <3000000>;
+					qcom,opp-acd-level = <0xc0285ffd>;
 				};
 
 				opp-300000000 {
 					opp-hz = /bits/ 64 <300000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
 					opp-peak-kBps = <2136719>;
+					qcom,opp-acd-level = <0xc02b5ffd>;
 				};
 			};
 		};

-- 
2.45.2

[PATCH v3 6/6] arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU

[Akhil P Oommen]

Now that we have ACD support for GPU, add additional OPPs up to
Turbo L3 which are supported across all existing SKUs.

Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 2cf16f904aaa..2e03afb85822 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -3337,6 +3337,20 @@ zap-shader {
 			gpu_opp_table: opp-table {
 				compatible = "operating-points-v2-adreno", "operating-points-v2";
 
+				opp-1250000000 {
+					opp-hz = /bits/ 64 <1250000000>;
+					opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L3>;
+					opp-peak-kBps = <16500000>;
+					qcom,opp-acd-level = <0xa82a5ffd>;
+				};
+
+				opp-1175000000 {
+					opp-hz = /bits/ 64 <1175000000>;
+					opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L2>;
+					opp-peak-kBps = <14398438>;
+					qcom,opp-acd-level = <0xa82a5ffd>;
+				};
+
 				opp-1100000000 {
 					opp-hz = /bits/ 64 <1100000000>;
 					opp-level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;

-- 
2.45.2

Re: [PATCH v3 6/6] arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU

[Konrad Dybcio]

On 30.12.2024 10:11 PM, Akhil P Oommen wrote:
> Now that we have ACD support for GPU, add additional OPPs up to
> Turbo L3 which are supported across all existing SKUs.
> 
> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v3 0/6] Support for GPU ACD feature on Adreno X1-85

[Maya Matuszczyk]

Hi,
I've applied this series for testing, and I've no performance
increase, and some screen corruption, there's some lines(mostly white)
on my yoga slim 7x that appear on the bottom of the screen. When I
move my cursor in swaywm over it, the lines get occluded by the cursor
and screenshots don't show these lines.

Best Regards,
Maya Matuszczyk

pon., 30 gru 2024 o 22:11 Akhil P Oommen <quic_akhilpo@quicinc.com> napisał(a):
>
> This series adds support for ACD feature for Adreno GPU which helps to
> lower the power consumption on GX rail and also sometimes is a requirement
> to enable higher GPU frequencies. At high level, following are the
> sequences required for ACD feature:
>         1. Identify the ACD level data for each regulator corner
>         2. Send a message to AOSS to switch voltage plan
>         3. Send a table with ACD level information to GMU during every
>         gpu wake up
>
> For (1), it is better to keep ACD level data in devicetree because this
> value depends on the process node, voltage margins etc which are
> chipset specific. For instance, same GPU HW IP on a different chipset
> would have a different set of values. So, a new schema which extends
> opp-v2 is created to add a new property called "qcom,opp-acd-level".
>
> ACD support is dynamically detected based on the presence of
> "qcom,opp-acd-level" property in GPU's opp table. Also, qmp node should be
> present under GMU node in devicetree for communication with AOSS.
>
> The devicetree patch in this series adds the acd-level data for X1-85
> GPU present in Snapdragon X1 Elite chipset.
>
> The last two devicetree patches are for Bjorn and all the rest for
> Rob Clark.
>
> ---
> Changes in v3:
> - Rebased on top of v6.13-rc4 since X1E doesn't boot properly with msm-next
> - Update patternProperties regex (Krzysztof)
> - Update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml
> - Update the new dt properties' description
> - Do not move qmp_get() to acd probe (Konrad)
> - New patches: patch#2, #3 and #6
> - Link to v2: https://lore.kernel.org/r/20241021-gpu-acd-v2-0-9c25a62803bc@quicinc.com
>
> Changes in v2:
> - Removed RFC tag for the series
> - Improve documentation for the new dt bindings (Krzysztof)
> - Add fallback compatible string for opp-table (Krzysztof)
> - Link to v1: https://lore.kernel.org/r/20241012-gpu-acd-v1-0-1e5e91aa95b6@quicinc.com
>
> ---
> Akhil P Oommen (6):
>       drm/msm/adreno: Add support for ACD
>       drm/msm: a6x: Rework qmp_get() error handling
>       drm/msm/adreno: Add module param to disable ACD
>       dt-bindings: opp: Add v2-qcom-adreno vendor bindings
>       arm64: dts: qcom: x1e80100: Add ACD levels for GPU
>       arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU
>
>  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi             | 25 +++++-
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c              | 96 ++++++++++++++++++---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.h              |  1 +
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 36 ++++++++
>  drivers/gpu/drm/msm/adreno/a6xx_hfi.h              | 21 +++++
>  drivers/gpu/drm/msm/adreno/adreno_device.c         |  4 +
>  8 files changed, 268 insertions(+), 13 deletions(-)
> ---
> base-commit: dbfac60febfa806abb2d384cb6441e77335d2799
> change-id: 20240724-gpu-acd-6c1dc5dcf516
>
> Best regards,
> --
> Akhil P Oommen <quic_akhilpo@quicinc.com>
>
>

Re: [PATCH v3 0/6] Support for GPU ACD feature on Adreno X1-85

[Rob Clark]

fwiw, I did see some perf boost (was mainly looking at gfxbench aztec
ruins vk high/normal, and also a separate mesa MR that fixes some LRZ
issues with turnip, so I don't remember how much boost was related to
which offhand)..  I've not seen corruption yet (gnome-shell / f41),
although what you describe sounds cache-line(ish) and could be just
timing related.  You could limit max freq via
/sys/devices/platform/soc@0/3d00000.gpu/devfreq/3d00000.gpu/max_freq
and see if that "fixes" things.  I don't really expect this patchset
to introduce these sorts of issues, but maybe the increased freq
exposes some preexisting conditions?

BR,
-R

On Sun, Jan 5, 2025 at 9:56 AM Maya Matuszczyk <maccraft123mc@gmail.com> wrote:
>
> Hi,
> I've applied this series for testing, and I've no performance
> increase, and some screen corruption, there's some lines(mostly white)
> on my yoga slim 7x that appear on the bottom of the screen. When I
> move my cursor in swaywm over it, the lines get occluded by the cursor
> and screenshots don't show these lines.
>
> Best Regards,
> Maya Matuszczyk
>
> pon., 30 gru 2024 o 22:11 Akhil P Oommen <quic_akhilpo@quicinc.com> napisał(a):
> >
> > This series adds support for ACD feature for Adreno GPU which helps to
> > lower the power consumption on GX rail and also sometimes is a requirement
> > to enable higher GPU frequencies. At high level, following are the
> > sequences required for ACD feature:
> >         1. Identify the ACD level data for each regulator corner
> >         2. Send a message to AOSS to switch voltage plan
> >         3. Send a table with ACD level information to GMU during every
> >         gpu wake up
> >
> > For (1), it is better to keep ACD level data in devicetree because this
> > value depends on the process node, voltage margins etc which are
> > chipset specific. For instance, same GPU HW IP on a different chipset
> > would have a different set of values. So, a new schema which extends
> > opp-v2 is created to add a new property called "qcom,opp-acd-level".
> >
> > ACD support is dynamically detected based on the presence of
> > "qcom,opp-acd-level" property in GPU's opp table. Also, qmp node should be
> > present under GMU node in devicetree for communication with AOSS.
> >
> > The devicetree patch in this series adds the acd-level data for X1-85
> > GPU present in Snapdragon X1 Elite chipset.
> >
> > The last two devicetree patches are for Bjorn and all the rest for
> > Rob Clark.
> >
> > ---
> > Changes in v3:
> > - Rebased on top of v6.13-rc4 since X1E doesn't boot properly with msm-next
> > - Update patternProperties regex (Krzysztof)
> > - Update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml
> > - Update the new dt properties' description
> > - Do not move qmp_get() to acd probe (Konrad)
> > - New patches: patch#2, #3 and #6
> > - Link to v2: https://lore.kernel.org/r/20241021-gpu-acd-v2-0-9c25a62803bc@quicinc.com
> >
> > Changes in v2:
> > - Removed RFC tag for the series
> > - Improve documentation for the new dt bindings (Krzysztof)
> > - Add fallback compatible string for opp-table (Krzysztof)
> > - Link to v1: https://lore.kernel.org/r/20241012-gpu-acd-v1-0-1e5e91aa95b6@quicinc.com
> >
> > ---
> > Akhil P Oommen (6):
> >       drm/msm/adreno: Add support for ACD
> >       drm/msm: a6x: Rework qmp_get() error handling
> >       drm/msm/adreno: Add module param to disable ACD
> >       dt-bindings: opp: Add v2-qcom-adreno vendor bindings
> >       arm64: dts: qcom: x1e80100: Add ACD levels for GPU
> >       arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU
> >
> >  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi             | 25 +++++-
> >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c              | 96 ++++++++++++++++++---
> >  drivers/gpu/drm/msm/adreno/a6xx_gmu.h              |  1 +
> >  drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 36 ++++++++
> >  drivers/gpu/drm/msm/adreno/a6xx_hfi.h              | 21 +++++
> >  drivers/gpu/drm/msm/adreno/adreno_device.c         |  4 +
> >  8 files changed, 268 insertions(+), 13 deletions(-)
> > ---
> > base-commit: dbfac60febfa806abb2d384cb6441e77335d2799
> > change-id: 20240724-gpu-acd-6c1dc5dcf516
> >
> > Best regards,
> > --
> > Akhil P Oommen <quic_akhilpo@quicinc.com>
> >
> >

Re: [PATCH v3 0/6] Support for GPU ACD feature on Adreno X1-85

[Akhil P Oommen]

On Sun, Jan 05, 2025 at 04:55:42PM -0800, Rob Clark wrote:
> fwiw, I did see some perf boost (was mainly looking at gfxbench aztec
> ruins vk high/normal, and also a separate mesa MR that fixes some LRZ
> issues with turnip, so I don't remember how much boost was related to
> which offhand)..  I've not seen corruption yet (gnome-shell / f41),
> although what you describe sounds cache-line(ish) and could be just
> timing related.  You could limit max freq via
> /sys/devices/platform/soc@0/3d00000.gpu/devfreq/3d00000.gpu/max_freq
> and see if that "fixes" things.  I don't really expect this patchset
> to introduce these sorts of issues, but maybe the increased freq
> exposes some preexisting conditions?

Actually, ACD related issues may show up as weird glitches in HW because
of HW spec violation. These issues are very very rare in production
devices though. And the behavior may vary between devices due to silicon
variations.

@Maya, thanks for testing this series. Sorry, one of my patch is buggy.
Could you please apply the below diff and check once?

--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -725,7 +725,7 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu)
        }

        /* Send ACD table to GMU */
-       ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), NULL, 0);
+       ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, acd_table, sizeof(*acd_table), NULL, 0);
        if (ret) {
                DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret);
                return ret;

--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -109,7 +109,7 @@ static int a6xx_hfi_wait_for_ack(struct a6xx_gmu *gmu, u32 id, u32 seqnum,

        /* Wait for a response */
        ret = gmu_poll_timeout(gmu, REG_A6XX_GMU_GMU2HOST_INTR_INFO, val,
-               val & A6XX_GMU_GMU2HOST_INTR_INFO_MSGQ, 100, 5000);
+               val & A6XX_GMU_GMU2HOST_INTR_INFO_MSGQ, 100, 1000000);

        if (ret) {
                DRM_DEV_ERROR(gmu->dev,


-Akhil
> 
> BR,
> -R
> 
> On Sun, Jan 5, 2025 at 9:56 AM Maya Matuszczyk <maccraft123mc@gmail.com> wrote:
> >
> > Hi,
> > I've applied this series for testing, and I've no performance
> > increase, and some screen corruption, there's some lines(mostly white)
> > on my yoga slim 7x that appear on the bottom of the screen. When I
> > move my cursor in swaywm over it, the lines get occluded by the cursor
> > and screenshots don't show these lines.
> >
> > Best Regards,
> > Maya Matuszczyk
> >
> > pon., 30 gru 2024 o 22:11 Akhil P Oommen <quic_akhilpo@quicinc.com> napisał(a):
> > >
> > > This series adds support for ACD feature for Adreno GPU which helps to
> > > lower the power consumption on GX rail and also sometimes is a requirement
> > > to enable higher GPU frequencies. At high level, following are the
> > > sequences required for ACD feature:
> > >         1. Identify the ACD level data for each regulator corner
> > >         2. Send a message to AOSS to switch voltage plan
> > >         3. Send a table with ACD level information to GMU during every
> > >         gpu wake up
> > >
> > > For (1), it is better to keep ACD level data in devicetree because this
> > > value depends on the process node, voltage margins etc which are
> > > chipset specific. For instance, same GPU HW IP on a different chipset
> > > would have a different set of values. So, a new schema which extends
> > > opp-v2 is created to add a new property called "qcom,opp-acd-level".
> > >
> > > ACD support is dynamically detected based on the presence of
> > > "qcom,opp-acd-level" property in GPU's opp table. Also, qmp node should be
> > > present under GMU node in devicetree for communication with AOSS.
> > >
> > > The devicetree patch in this series adds the acd-level data for X1-85
> > > GPU present in Snapdragon X1 Elite chipset.
> > >
> > > The last two devicetree patches are for Bjorn and all the rest for
> > > Rob Clark.
> > >
> > > ---
> > > Changes in v3:
> > > - Rebased on top of v6.13-rc4 since X1E doesn't boot properly with msm-next
> > > - Update patternProperties regex (Krzysztof)
> > > - Update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml
> > > - Update the new dt properties' description
> > > - Do not move qmp_get() to acd probe (Konrad)
> > > - New patches: patch#2, #3 and #6
> > > - Link to v2: https://lore.kernel.org/r/20241021-gpu-acd-v2-0-9c25a62803bc@quicinc.com
> > >
> > > Changes in v2:
> > > - Removed RFC tag for the series
> > > - Improve documentation for the new dt bindings (Krzysztof)
> > > - Add fallback compatible string for opp-table (Krzysztof)
> > > - Link to v1: https://lore.kernel.org/r/20241012-gpu-acd-v1-0-1e5e91aa95b6@quicinc.com
> > >
> > > ---
> > > Akhil P Oommen (6):
> > >       drm/msm/adreno: Add support for ACD
> > >       drm/msm: a6x: Rework qmp_get() error handling
> > >       drm/msm/adreno: Add module param to disable ACD
> > >       dt-bindings: opp: Add v2-qcom-adreno vendor bindings
> > >       arm64: dts: qcom: x1e80100: Add ACD levels for GPU
> > >       arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU
> > >
> > >  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
> > >  MAINTAINERS                                        |  1 +
> > >  arch/arm64/boot/dts/qcom/x1e80100.dtsi             | 25 +++++-
> > >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c              | 96 ++++++++++++++++++---
> > >  drivers/gpu/drm/msm/adreno/a6xx_gmu.h              |  1 +
> > >  drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 36 ++++++++
> > >  drivers/gpu/drm/msm/adreno/a6xx_hfi.h              | 21 +++++
> > >  drivers/gpu/drm/msm/adreno/adreno_device.c         |  4 +
> > >  8 files changed, 268 insertions(+), 13 deletions(-)
> > > ---
> > > base-commit: dbfac60febfa806abb2d384cb6441e77335d2799
> > > change-id: 20240724-gpu-acd-6c1dc5dcf516
> > >
> > > Best regards,
> > > --
> > > Akhil P Oommen <quic_akhilpo@quicinc.com>
> > >
> > >

Re: [PATCH v3 0/6] Support for GPU ACD feature on Adreno X1-85

[Maya Matuszczyk]

pon., 6 sty 2025 o 20:55 Akhil P Oommen <quic_akhilpo@quicinc.com> napisał(a):
>
> On Sun, Jan 05, 2025 at 04:55:42PM -0800, Rob Clark wrote:
> > fwiw, I did see some perf boost (was mainly looking at gfxbench aztec
> > ruins vk high/normal, and also a separate mesa MR that fixes some LRZ
> > issues with turnip, so I don't remember how much boost was related to
> > which offhand)..  I've not seen corruption yet (gnome-shell / f41),
> > although what you describe sounds cache-line(ish) and could be just
> > timing related.  You could limit max freq via
> > /sys/devices/platform/soc@0/3d00000.gpu/devfreq/3d00000.gpu/max_freq
> > and see if that "fixes" things.  I don't really expect this patchset
> > to introduce these sorts of issues, but maybe the increased freq
> > exposes some preexisting conditions?
>
> Actually, ACD related issues may show up as weird glitches in HW because
> of HW spec violation. These issues are very very rare in production
> devices though. And the behavior may vary between devices due to silicon
> variations.
>
> @Maya, thanks for testing this series. Sorry, one of my patch is buggy.
> Could you please apply the below diff and check once?
>
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -725,7 +725,7 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu)
>         }
>
>         /* Send ACD table to GMU */
> -       ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, &msg, sizeof(msg), NULL, 0);
> +       ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_ACD, acd_table, sizeof(*acd_table), NULL, 0);
>         if (ret) {
>                 DRM_DEV_ERROR(gmu->dev, "Unable to ACD table (%d)\n", ret);
>                 return ret;
>
> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
> @@ -109,7 +109,7 @@ static int a6xx_hfi_wait_for_ack(struct a6xx_gmu *gmu, u32 id, u32 seqnum,
>
>         /* Wait for a response */
>         ret = gmu_poll_timeout(gmu, REG_A6XX_GMU_GMU2HOST_INTR_INFO, val,
> -               val & A6XX_GMU_GMU2HOST_INTR_INFO_MSGQ, 100, 5000);
> +               val & A6XX_GMU_GMU2HOST_INTR_INFO_MSGQ, 100, 1000000);
>
>         if (ret) {
>                 DRM_DEV_ERROR(gmu->dev,
>

With this change on top of this patch series I'm seeing an increase in
vkmark scores roughly in line with increased frequency.



>
> -Akhil
> >
> > BR,
> > -R
> >
> > On Sun, Jan 5, 2025 at 9:56 AM Maya Matuszczyk <maccraft123mc@gmail.com> wrote:
> > >
> > > Hi,
> > > I've applied this series for testing, and I've no performance
> > > increase, and some screen corruption, there's some lines(mostly white)
> > > on my yoga slim 7x that appear on the bottom of the screen. When I
> > > move my cursor in swaywm over it, the lines get occluded by the cursor
> > > and screenshots don't show these lines.
> > >
> > > Best Regards,
> > > Maya Matuszczyk
> > >
> > > pon., 30 gru 2024 o 22:11 Akhil P Oommen <quic_akhilpo@quicinc.com> napisał(a):
> > > >
> > > > This series adds support for ACD feature for Adreno GPU which helps to
> > > > lower the power consumption on GX rail and also sometimes is a requirement
> > > > to enable higher GPU frequencies. At high level, following are the
> > > > sequences required for ACD feature:
> > > >         1. Identify the ACD level data for each regulator corner
> > > >         2. Send a message to AOSS to switch voltage plan
> > > >         3. Send a table with ACD level information to GMU during every
> > > >         gpu wake up
> > > >
> > > > For (1), it is better to keep ACD level data in devicetree because this
> > > > value depends on the process node, voltage margins etc which are
> > > > chipset specific. For instance, same GPU HW IP on a different chipset
> > > > would have a different set of values. So, a new schema which extends
> > > > opp-v2 is created to add a new property called "qcom,opp-acd-level".
> > > >
> > > > ACD support is dynamically detected based on the presence of
> > > > "qcom,opp-acd-level" property in GPU's opp table. Also, qmp node should be
> > > > present under GMU node in devicetree for communication with AOSS.
> > > >
> > > > The devicetree patch in this series adds the acd-level data for X1-85
> > > > GPU present in Snapdragon X1 Elite chipset.
> > > >
> > > > The last two devicetree patches are for Bjorn and all the rest for
> > > > Rob Clark.
> > > >
> > > > ---
> > > > Changes in v3:
> > > > - Rebased on top of v6.13-rc4 since X1E doesn't boot properly with msm-next
> > > > - Update patternProperties regex (Krzysztof)
> > > > - Update MAINTAINERS file include the new opp-v2-qcom-adreno.yaml
> > > > - Update the new dt properties' description
> > > > - Do not move qmp_get() to acd probe (Konrad)
> > > > - New patches: patch#2, #3 and #6
> > > > - Link to v2: https://lore.kernel.org/r/20241021-gpu-acd-v2-0-9c25a62803bc@quicinc.com
> > > >
> > > > Changes in v2:
> > > > - Removed RFC tag for the series
> > > > - Improve documentation for the new dt bindings (Krzysztof)
> > > > - Add fallback compatible string for opp-table (Krzysztof)
> > > > - Link to v1: https://lore.kernel.org/r/20241012-gpu-acd-v1-0-1e5e91aa95b6@quicinc.com
> > > >
> > > > ---
> > > > Akhil P Oommen (6):
> > > >       drm/msm/adreno: Add support for ACD
> > > >       drm/msm: a6x: Rework qmp_get() error handling
> > > >       drm/msm/adreno: Add module param to disable ACD
> > > >       dt-bindings: opp: Add v2-qcom-adreno vendor bindings
> > > >       arm64: dts: qcom: x1e80100: Add ACD levels for GPU
> > > >       arm64: dts: qcom: x1e80100: Add OPPs up to Turbo L3 for GPU
> > > >
> > > >  .../bindings/opp/opp-v2-qcom-adreno.yaml           | 97 ++++++++++++++++++++++
> > > >  MAINTAINERS                                        |  1 +
> > > >  arch/arm64/boot/dts/qcom/x1e80100.dtsi             | 25 +++++-
> > > >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c              | 96 ++++++++++++++++++---
> > > >  drivers/gpu/drm/msm/adreno/a6xx_gmu.h              |  1 +
> > > >  drivers/gpu/drm/msm/adreno/a6xx_hfi.c              | 36 ++++++++
> > > >  drivers/gpu/drm/msm/adreno/a6xx_hfi.h              | 21 +++++
> > > >  drivers/gpu/drm/msm/adreno/adreno_device.c         |  4 +
> > > >  8 files changed, 268 insertions(+), 13 deletions(-)
> > > > ---
> > > > base-commit: dbfac60febfa806abb2d384cb6441e77335d2799
> > > > change-id: 20240724-gpu-acd-6c1dc5dcf516
> > > >
> > > > Best regards,
> > > > --
> > > > Akhil P Oommen <quic_akhilpo@quicinc.com>
> > > >
> > > >