public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/msm/adreno: Support for CLX feature on A8xx
@ 2026-05-15 20:07 Akhil P Oommen
  2026-05-15 20:07 ` [PATCH 1/3] drm/msm/a6xx: Add HFI support for CLX feature Akhil P Oommen
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Akhil P Oommen @ 2026-05-15 20:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Konrad Dybcio, Dmitry Baryshkov,
	Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
	Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Akhil P Oommen

Add support for Current Limit Extension (CLX) feature found on a few A8x
GPUs. This feature is required to limit the peak current consumption to
avoid HW spec violation on GX/MX rails.

Add the necessary HFI interface support to pass the recommended CLX and
IFF/PCLX limits tables to the GMU. And enable this feature on both A840
and X285 GPU by adding the repective configurations to the gpu catalog.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
---
Akhil P Oommen (3):
      drm/msm/a6xx: Add HFI support for CLX feature
      drm/msm/a6xx: Enable CLX feature on A840
      drm/msm/a8xx: Enable CLX feature on Adreno X2-85

 drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 172 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h     |  15 +++
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c     |  71 ++++++++++++
 3 files changed, 258 insertions(+)
---
base-commit: b462608de92a7cac450781f9d8d4c7cf3ccf82db
change-id: 20260514-a8xx-clx-support-2ee492a88da1

Best regards,
-- 
Akhil P Oommen <akhilpo@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] drm/msm/a6xx: Add HFI support for CLX feature
  2026-05-15 20:07 [PATCH 0/3] drm/msm/adreno: Support for CLX feature on A8xx Akhil P Oommen
@ 2026-05-15 20:07 ` Akhil P Oommen
  2026-05-15 22:47   ` Claude review: " Claude Code Review Bot
  2026-05-15 20:07 ` [PATCH 2/3] drm/msm/a6xx: Enable CLX feature on A840 Akhil P Oommen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Akhil P Oommen @ 2026-05-15 20:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Konrad Dybcio, Dmitry Baryshkov,
	Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
	Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Akhil P Oommen

Add support for Current Limit Extension (CLX) feature found on a few A8x
GPUs. This feature is required to limit the peak current consumption to
avoid HW spec violation on GX/MX rails.

Add the necessary HFI interface support to pass the recommended CLX and
IFF/PCLX limits tables to the GMU. Per-GPU configuration is consumed
from the catalog entries.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 15 ++++++++
 drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 71 +++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index eb431e5e00b1..f6a3f1924bb3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -34,6 +34,19 @@ struct cpu_gpu_lock {
 	uint64_t regs[62];
 };
 
+struct a6xx_limits_tbl {
+	/** @tbl: List of limits table **/
+	struct a6xx_hfi_limits_tbl *tbl;
+	/** @count: Number of entries in the list **/
+	u32 count;
+};
+
+#define DECLARE_ADRENO_LIMITS_TABLE(name)	\
+static const struct a6xx_limits_tbl name = {	\
+	.tbl = name ## _tbl,			\
+	.count = ARRAY_SIZE(name ## _tbl),	\
+}
+
 /**
  * struct a6xx_info - a6xx specific information from device table
  *
@@ -54,6 +67,8 @@ struct a6xx_info {
 	u32 gmu_cgc_mode;
 	u32 prim_fifo_threshold;
 	const struct a6xx_bcm *bcms;
+	const struct a6xx_hfi_clx_table_v2_cmd *clx_tbl;
+	const struct a6xx_limits_tbl *limits_tbl;
 };
 
 struct a6xx_gpu {
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
index 487c2736f2b3..13ae34d0d898 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c
@@ -886,6 +886,73 @@ static int a6xx_hfi_enable_acd(struct a6xx_gmu *gmu)
 	return 0;
 }
 
+static int a6xx_hfi_enable_iff_pclx(struct a6xx_gmu *gmu, const struct a6xx_limits_tbl *limits)
+{
+	struct a6xx_hfi_table_entry *entry;
+	struct a6xx_hfi_table *tbl;
+	size_t entry_size;
+	size_t size;
+	int ret;
+
+	if (!limits)
+		return 0;
+
+	ret = a6xx_hfi_feature_ctrl_msg(gmu, HFI_FEATURE_IFF_PCLX, 1, 0);
+	if (ret) {
+		DRM_DEV_ERROR(gmu->dev, "Unable to enable IFF PCLX (%d)\n", ret);
+		return ret;
+	}
+
+	entry_size = limits->count * sizeof(struct a6xx_hfi_limits_tbl);
+	size = sizeof(*tbl) + sizeof(*entry) + entry_size;
+
+	tbl = kzalloc(size, GFP_KERNEL);
+	if (!tbl)
+		return -ENOMEM;
+
+	tbl->type = HFI_TABLE_LIMITS_MIT;
+	entry = &tbl->entry[0];
+	entry->count = limits->count;
+	entry->stride = sizeof(struct a6xx_hfi_limits_tbl) >> 2;
+
+	memcpy(entry->data, limits->tbl, entry_size);
+
+	ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_TABLE, tbl, size, NULL, 0);
+	if (ret)
+		DRM_DEV_ERROR(gmu->dev, "Unable to send PCLX table (%d)\n", ret);
+
+	kfree(tbl);
+	return ret;
+}
+
+static int a6xx_hfi_enable_clx(struct a6xx_gmu *gmu)
+{
+	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+	const struct a6xx_info *info = adreno_gpu->info->a6xx;
+	struct a6xx_hfi_clx_table_v2_cmd cmd = { 0 };
+	int ret;
+
+	if (!info->clx_tbl)
+		return 0;
+
+	ret = a6xx_hfi_feature_ctrl_msg(gmu, HFI_FEATURE_CLX, 1, 0);
+	if (ret) {
+		DRM_DEV_ERROR(gmu->dev, "Unable to enable CLX (%d)\n", ret);
+		return ret;
+	}
+
+	memcpy(&cmd, info->clx_tbl, sizeof(cmd));
+
+	ret = a6xx_hfi_send_msg(gmu, HFI_H2F_MSG_CLX_TBL, &cmd, sizeof(cmd), NULL, 0);
+	if (ret) {
+		DRM_DEV_ERROR(gmu->dev, "Unable to send CLX table (%d)\n", ret);
+		return ret;
+	}
+
+	return a6xx_hfi_enable_iff_pclx(gmu, info->limits_tbl);
+}
+
 static int a6xx_hfi_send_test(struct a6xx_gmu *gmu)
 {
 	struct a6xx_hfi_msg_test msg = { 0 };
@@ -987,6 +1054,10 @@ int a6xx_hfi_start(struct a6xx_gmu *gmu, int boot_state)
 	if (ret)
 		return ret;
 
+	ret = a6xx_hfi_enable_clx(gmu);
+	if (ret)
+		return ret;
+
 	ret = a6xx_hfi_enable_ifpc(gmu);
 	if (ret)
 		return ret;

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] drm/msm/a6xx: Enable CLX feature on A840
  2026-05-15 20:07 [PATCH 0/3] drm/msm/adreno: Support for CLX feature on A8xx Akhil P Oommen
  2026-05-15 20:07 ` [PATCH 1/3] drm/msm/a6xx: Add HFI support for CLX feature Akhil P Oommen
@ 2026-05-15 20:07 ` Akhil P Oommen
  2026-05-15 22:47   ` Claude review: " Claude Code Review Bot
  2026-05-15 20:07 ` [PATCH 3/3] drm/msm/a8xx: Enable CLX feature on Adreno X2-85 Akhil P Oommen
  2026-05-15 22:47 ` Claude review: drm/msm/adreno: Support for CLX feature on A8xx Claude Code Review Bot
  3 siblings, 1 reply; 8+ messages in thread
From: Akhil P Oommen @ 2026-05-15 20:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Konrad Dybcio, Dmitry Baryshkov,
	Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
	Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Akhil P Oommen

Add the A840 CLX domain table and the IFF/PCLX limits table to the
catalog. With the HFI plumbing in place, this enables the Current Limit
Extension (CLX) feature on Adreno 840.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 86 +++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
index 550ff3a9b82e..c503912a61c7 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
@@ -2108,6 +2108,90 @@ static const struct adreno_reglist_pipe a840_dyn_pwrup_reglist_regs[] = {
 };
 DECLARE_ADRENO_REGLIST_PIPE_LIST(a840_dyn_pwrup_reglist);
 
+static const struct a6xx_hfi_clx_table_v2_cmd a840_clx_tbl = {
+	.version = (2 << 16) | 1,
+	.domain = {
+		/* GX DOMAIN */
+		{
+			.data = CLX_DATA(60, 5, 1, 1),
+			.clxt = 0,
+			.clxh = 0,
+			.urg_mode = 1,
+			.lkg_en = 0,
+			.curr_budget = 100,
+		},
+		/* MXG DOMAIN */
+		{
+			.data = CLX_DATA(60, 1, 1, 1),
+			.clxt = 0,
+			.clxh = 0,
+			.urg_mode = 1,
+			.lkg_en = 0,
+			.curr_budget = 50,
+		},
+	},
+};
+
+struct a6xx_hfi_limits_tbl a840_limits_tbl[] = {
+	{
+		.feature_id = GMU_MIT_IFF,
+		.domain = GMU_GX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 1,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 12,
+			.mit_time = 5,
+			.curr_limit = 6000,
+		},
+	},
+	{
+		.feature_id = GMU_MIT_IFF,
+		.domain = GMU_MX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 1,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 9,
+			.mit_time = 2000,
+			.curr_limit = 6000,
+		},
+	},
+	{
+		.feature_id = GMU_MIT_PCLX,
+		.domain = GMU_GX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 1,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 7,
+			.mit_time = 3,
+			.curr_limit = 30000,
+		},
+	},
+	{
+		.feature_id = GMU_MIT_PCLX,
+		.domain = GMU_MX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 0,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 7,
+			.mit_time = 3,
+			.curr_limit = 6000,
+		},
+	},
+};
+DECLARE_ADRENO_LIMITS_TABLE(a840_limits);
+
 static const struct adreno_info a8xx_gpus[] = {
 	{
 		.chip_ids = ADRENO_CHIP_IDS(0x44070001),
@@ -2185,6 +2269,8 @@ static const struct adreno_info a8xx_gpus[] = {
 				},
 				{ /* sentinel */ },
 			},
+			.clx_tbl = &a840_clx_tbl,
+			.limits_tbl = &a840_limits,
 		},
 		.preempt_record_size = 19708 * SZ_1K,
 		.speedbins = ADRENO_SPEEDBINS(

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] drm/msm/a8xx: Enable CLX feature on Adreno X2-85
  2026-05-15 20:07 [PATCH 0/3] drm/msm/adreno: Support for CLX feature on A8xx Akhil P Oommen
  2026-05-15 20:07 ` [PATCH 1/3] drm/msm/a6xx: Add HFI support for CLX feature Akhil P Oommen
  2026-05-15 20:07 ` [PATCH 2/3] drm/msm/a6xx: Enable CLX feature on A840 Akhil P Oommen
@ 2026-05-15 20:07 ` Akhil P Oommen
  2026-05-15 22:47   ` Claude review: " Claude Code Review Bot
  2026-05-15 22:47 ` Claude review: drm/msm/adreno: Support for CLX feature on A8xx Claude Code Review Bot
  3 siblings, 1 reply; 8+ messages in thread
From: Akhil P Oommen @ 2026-05-15 20:07 UTC (permalink / raw)
  To: Rob Clark, Sean Paul, Konrad Dybcio, Dmitry Baryshkov,
	Abhinav Kumar, Jessica Zhang, Marijn Suijten, David Airlie,
	Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Akhil P Oommen

Add the CLX domain table and the IFF/PCLX limits tables to the catalog to
enable CLX feature support on Adreno X2-85 GPU present in the Glymur
chipset.

Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 86 +++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
index c503912a61c7..9ad13eb173de 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
@@ -1799,6 +1799,90 @@ static const struct adreno_reglist_pipe x285_dyn_pwrup_reglist_regs[] = {
 };
 DECLARE_ADRENO_REGLIST_PIPE_LIST(x285_dyn_pwrup_reglist);
 
+static const struct a6xx_hfi_clx_table_v2_cmd x285_clx_tbl = {
+	.version = (2 << 16) | 1,
+	.domain = {
+		/* GX DOMAIN */
+		{
+			.data = CLX_DATA(60, 11, 0, 1),
+			.clxt = 0,
+			.clxh = 0,
+			.urg_mode = 1,
+			.lkg_en = 0,
+			.curr_budget = 100,
+		},
+		/* MXG DOMAIN */
+		{
+			.data = CLX_DATA(60, 3, 1, 1),
+			.clxt = 0,
+			.clxh = 0,
+			.urg_mode = 1,
+			.lkg_en = 0,
+			.curr_budget = 100,
+		},
+	},
+};
+
+struct a6xx_hfi_limits_tbl x285_limits_tbl[] = {
+	{
+		.feature_id = GMU_MIT_IFF,
+		.domain = GMU_GX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 1,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 12,
+			.mit_time = 5,
+			.curr_limit = 18000,
+		},
+	},
+	{
+		.feature_id = GMU_MIT_IFF,
+		.domain = GMU_MX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 1,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 18,
+			.mit_time = 2000,
+			.curr_limit = 6000,
+		},
+	},
+	{
+		.feature_id = GMU_MIT_PCLX,
+		.domain = GMU_GX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 1,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 6,
+			.mit_time = 3,
+			.curr_limit = 66000,
+		},
+	},
+	{
+		.feature_id = GMU_MIT_PCLX,
+		.domain = GMU_MX_DOMAIN,
+		.feature_rev = 0,
+		.cfg = {
+			.enable = 0,
+			.msg_path = 0,
+			.lkg_en = 0,
+			.mode = 0,
+			.sid = 6,
+			.mit_time = 3,
+			.curr_limit = 18000,
+		},
+	},
+};
+DECLARE_ADRENO_LIMITS_TABLE(x285_limits);
+
 static const struct adreno_reglist_pipe a840_nonctxt_regs[] = {
 	{ REG_A8XX_CP_SMMU_STREAM_ID_LPAC, 0x00000101, BIT(PIPE_NONE) },
 	{ REG_A8XX_GRAS_DBG_ECO_CNTL, 0x00000800, BIT(PIPE_BV) | BIT(PIPE_BR) },
@@ -2227,6 +2311,8 @@ static const struct adreno_info a8xx_gpus[] = {
 				},
 				{ /* sentinel */ },
 			},
+			.clx_tbl = &x285_clx_tbl,
+			.limits_tbl = &x285_limits,
 		},
 		.speedbins = ADRENO_SPEEDBINS(
 			{ 0,   0 },

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Claude review: drm/msm/adreno: Support for CLX feature on A8xx
  2026-05-15 20:07 [PATCH 0/3] drm/msm/adreno: Support for CLX feature on A8xx Akhil P Oommen
                   ` (2 preceding siblings ...)
  2026-05-15 20:07 ` [PATCH 3/3] drm/msm/a8xx: Enable CLX feature on Adreno X2-85 Akhil P Oommen
@ 2026-05-15 22:47 ` Claude Code Review Bot
  3 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-15 22:47 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/msm/adreno: Support for CLX feature on A8xx
Author: Akhil P Oommen <akhilpo@oss.qualcomm.com>
Patches: 4
Reviewed: 2026-05-16T08:47:13.384907

---

This is a clean, well-structured 3-patch series adding Current Limit Extension (CLX) support for A8xx GPUs. The design follows established patterns in the driver: patch 1 adds the HFI plumbing (feature control + table messaging), patches 2 and 3 add per-GPU catalog data for A840 and X2-85 respectively. The code is straightforward, the size computation for the variable-length HFI table message is correct, and the series slots CLX into the right place in the `a6xx_hfi_start()` sequence (after ACD, before IFPC).

There are two concrete issues worth addressing, both minor. Overall the series looks good.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/msm/a6xx: Add HFI support for CLX feature
  2026-05-15 20:07 ` [PATCH 1/3] drm/msm/a6xx: Add HFI support for CLX feature Akhil P Oommen
@ 2026-05-15 22:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-15 22:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**The HFI plumbing is correct.** The `a6xx_hfi_enable_clx()` / `a6xx_hfi_enable_iff_pclx()` pair follows the same pattern as existing `a6xx_hfi_enable_acd()`. The size calculation for the dynamically-allocated `a6xx_hfi_table` is correct, and `a6xx_hfi_send_msg()` fills in the header word.

**Issue: `a6xx_limits_tbl.tbl` should be `const`-qualified.**

In `a6xx_gpu.h`:
```c
struct a6xx_limits_tbl {
	/** @tbl: List of limits table **/
	struct a6xx_hfi_limits_tbl *tbl;
	...
};
```

The `tbl` pointer should be `const struct a6xx_hfi_limits_tbl *tbl`, since:
1. The `DECLARE_ADRENO_LIMITS_TABLE` macro creates a `static const struct a6xx_limits_tbl`, so the compiler needs the member to accept a pointer to const data.
2. The data is only ever read (`memcpy` source in `a6xx_hfi_enable_iff_pclx`), never written.

Without `const`, the assignment from the `static const` table data in patches 2/3 will produce a warning (discarding `const` qualifier) — or worse, silently strip the constness. This should be:
```c
const struct a6xx_hfi_limits_tbl *tbl;
```

**Minor: memcpy from const source is fine.** The `memcpy(&cmd, info->clx_tbl, sizeof(cmd))` in `a6xx_hfi_enable_clx()` correctly copies the const catalog data into a stack-local `cmd` before `a6xx_hfi_send_msg` mutates the header — good pattern.

**Ordering is correct.** CLX is enabled after ACD and before IFPC in `a6xx_hfi_start()`, which makes sense — current limiting should be established before entering IFPC.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/msm/a6xx: Enable CLX feature on A840
  2026-05-15 20:07 ` [PATCH 2/3] drm/msm/a6xx: Enable CLX feature on A840 Akhil P Oommen
@ 2026-05-15 22:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-15 22:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Issue: `a840_limits_tbl` should be `static const`.**

```c
struct a6xx_hfi_limits_tbl a840_limits_tbl[] = {
```

This is missing both `static` and `const`. All other comparable catalog arrays in this file (e.g., `a840_dyn_pwrup_reglist_regs[]`) are `static const`. This table is only referenced through the `DECLARE_ADRENO_LIMITS_TABLE(a840_limits)` macro and should be:
```c
static const struct a6xx_hfi_limits_tbl a840_limits_tbl[] = {
```

Without `static`, the symbol is externally visible (unnecessary namespace pollution). Without `const`, the data goes into the writable data section instead of rodata. Also, as noted above, the `DECLARE_ADRENO_LIMITS_TABLE` macro assigns `name ## _tbl` to a member in a `static const` struct, so having the underlying array be non-const creates a type inconsistency.

**Data tables look reasonable.** Four limits entries (IFF for GX, IFF for MX, PCLX for GX, PCLX for MX) with the MX PCLX entry disabled (`.enable = 0`) — this is a sensible per-domain configuration. The CLX domain table has two domains (GX and MXG) with different current budgets (100 vs 50), which matches the asymmetric power characteristics.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/msm/a8xx: Enable CLX feature on Adreno X2-85
  2026-05-15 20:07 ` [PATCH 3/3] drm/msm/a8xx: Enable CLX feature on Adreno X2-85 Akhil P Oommen
@ 2026-05-15 22:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-05-15 22:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Same `static const` issue as patch 2:**

```c
struct a6xx_hfi_limits_tbl x285_limits_tbl[] = {
```

Should be `static const struct a6xx_hfi_limits_tbl x285_limits_tbl[]`.

**Data is structurally identical to A840 with different tuning values.** The X2-85 has higher current limits (18000 vs 6000 for IFF GX, 66000 vs 30000 for PCLX GX) and different SID values, and both CLX domains have `curr_budget = 100` (vs 100/50 for A840), consistent with a more power-hungry GPU.

**Minor observation:** Patches 2 and 3 place the X2-85 data tables *before* the A840 data tables in the source file (line 1799 vs 2108), which is fine since order doesn't matter for catalog entries, but it does mean the series adds X2-85 tables earlier in the file than A840 tables despite A840 being enabled first in the series. Not a problem, just mildly surprising when reading the diff.

---

**Summary of requested changes:**

1. **Patch 1**: Make `struct a6xx_limits_tbl::tbl` a `const` pointer: `const struct a6xx_hfi_limits_tbl *tbl`
2. **Patches 2 & 3**: Add `static const` to `a840_limits_tbl[]` and `x285_limits_tbl[]`

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-15 22:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 20:07 [PATCH 0/3] drm/msm/adreno: Support for CLX feature on A8xx Akhil P Oommen
2026-05-15 20:07 ` [PATCH 1/3] drm/msm/a6xx: Add HFI support for CLX feature Akhil P Oommen
2026-05-15 22:47   ` Claude review: " Claude Code Review Bot
2026-05-15 20:07 ` [PATCH 2/3] drm/msm/a6xx: Enable CLX feature on A840 Akhil P Oommen
2026-05-15 22:47   ` Claude review: " Claude Code Review Bot
2026-05-15 20:07 ` [PATCH 3/3] drm/msm/a8xx: Enable CLX feature on Adreno X2-85 Akhil P Oommen
2026-05-15 22:47   ` Claude review: " Claude Code Review Bot
2026-05-15 22:47 ` Claude review: drm/msm/adreno: Support for CLX feature on A8xx Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox