[PATCH v2 0/3] MediaTek mt6572 GCE support

[PATCH v2 0/3] MediaTek mt6572 GCE support

[Roman Vivchar via B4 Relay]

This patch series adds support for the Global Command Engine found in
the mt6572 SoC.

Notably, the patch 2 introduces legacy ISA format, which is used on
mt6572, 6582 and 6592 SoCs. mt6577 doesn't have the GCE at all and
mt6595 is much more close to 6795/8173.
Patch 3 adds mt6572 platform data and legacy_isa forwarding to the
instruction JIT.

This format was reverse engineered from the Android HAL and uses 22-bit
offset with 2-bit subsystem ID, unlike the newer 16-bit offset and 8-bit
subsystem ID.
The mt6572 HAL includes workarounds for WFE and EOC instructions, emitting
another WFE as barrier at WFE, and JUMP +8 at EOC.
mt6582 and 6592 HAL dropped WFE barrier, but still have JUMP +8.

This sounds very much like some kind of software mitigations for hardware
bugs, but during testing on 6572 nothing bad happened so far. These
additional instructions are not included in the patches.

Note that while GCE can be used for e.g. mt6592 UFOE (which is at
0x14013000, so outside of the u16 range), currently the driver still uses
u16 for the offset, because I don't have any 6592 device.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
Changes in v2:
- Move dt-bindings to mailbox directory from gce (Krzysztof)
- Clarify that only v2 and v3 GCEs support PA writes
- Link to v1: https://patch.msgid.link/20260728-6572-gce-v1-0-750bba32689e@protonmail.com

---
Roman Vivchar (3):
      dt-bindings: mailbox: mediatek,gce-mailbox: add mt6572
      soc: mediatek: cmdq-helper: add legacy GCE ISA support
      mailbox: mtk-cmdq: add mt6572 support

 .../bindings/mailbox/mediatek,gce-mailbox.yaml     |  1 +
 drivers/mailbox/mtk-cmdq-mailbox.c                 | 11 +++++
 drivers/soc/mediatek/mtk-cmdq-helper.c             | 57 +++++++++++++++-------
 include/dt-bindings/mailbox/mediatek,mt6572-gce.h  | 52 ++++++++++++++++++++
 include/linux/mailbox/mtk-cmdq-mailbox.h           |  1 +
 5 files changed, 104 insertions(+), 18 deletions(-)
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260727-6572-gce-6a571ef4af09

Best regards,
--  
Roman Vivchar <rva333@protonmail.com>

[PATCH v2 1/3] dt-bindings: mailbox: mediatek,gce-mailbox: add mt6572

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

Add a compatible string and a header for the GCE constants.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
There's also 'other' subsystem which covers 0x16XXXXXX range, however
mt6572 doesn't seem to have any MMIO there. It's not included in this
patch.
---
 .../bindings/mailbox/mediatek,gce-mailbox.yaml     |  1 +
 include/dt-bindings/mailbox/mediatek,mt6572-gce.h  | 52 ++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
index 587126d03fc6..d6e93010bb94 100644
--- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
@@ -18,6 +18,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - mediatek,mt6572-gce
           - mediatek,mt6779-gce
           - mediatek,mt8173-gce
           - mediatek,mt8183-gce
diff --git a/include/dt-bindings/mailbox/mediatek,mt6572-gce.h b/include/dt-bindings/mailbox/mediatek,mt6572-gce.h
new file mode 100644
index 000000000000..94eb6ea97542
--- /dev/null
+++ b/include/dt-bindings/mailbox/mediatek,mt6572-gce.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2026 Roman Vivchar
+ */
+
+#ifndef _DT_BINDINGS_GCE_MT6572_H
+#define _DT_BINDINGS_GCE_MT6572_H
+
+/* GCE HW thread priority */
+#define CMDQ_THR_PRIO_NORMAL		0
+#define CMDQ_THR_PRIO_HIGHEST		1
+
+/* GCE SUBSYS */
+#define SUBSYS_14XXXXXX			0
+#define SUBSYS_15XXXXXX			1
+
+/* GCE HW EVENT */
+#define CMDQ_EVENT_MUTEX0		0
+#define CMDQ_EVENT_MUTEX1		1
+#define CMDQ_EVENT_MUTEX2		2
+#define CMDQ_EVENT_MUTEX3		3
+#define CMDQ_EVENT_MUTEX4		4
+#define CMDQ_EVENT_MUTEX5		5
+#define CMDQ_EVENT_MUTEX6		6
+#define CMDQ_EVENT_MUTEX7		7
+
+#define CMDQ_EVENT_DISP_WDMA_EOF	12
+#define CMDQ_EVENT_DISP_RDMA_EOF	13
+#define CMDQ_EVENT_DISP_BLS_EOF		14
+#define CMDQ_EVENT_DISP_COLOR_EOF	15
+#define CMDQ_EVENT_DISP_OVL_EOF		16
+#define CMDQ_EVENT_MDP_TDSHP_EOF	17
+#define CMDQ_EVENT_MDP_RSZ1_EOF		18
+#define CMDQ_EVENT_MDP_RSZ0_EOF		19
+#define CMDQ_EVENT_MDP_RDMA_EOF		20
+#define CMDQ_EVENT_MDP_WDMA_EOF		21
+#define CMDQ_EVENT_MDP_WROT_EOF		22
+
+#define CMDQ_EVENT_MDP_WROT_SOF		24
+#define CMDQ_EVENT_MDP_RSZ0_SOF		25
+#define CMDQ_EVENT_MDP_RSZ1_SOF		26
+#define CMDQ_EVENT_DISP_OVL_SOF		27
+#define CMDQ_EVENT_MDP_WDMA_SOF		28
+#define CMDQ_EVENT_MDP_RDMA_SOF		29
+#define CMDQ_EVENT_DISP_WDMA_SOF	30
+#define CMDQ_EVENT_DISP_COLOR_SOF	31
+#define CMDQ_EVENT_MDP_TDSHP_SOF	32
+#define CMDQ_EVENT_DISP_BLS_SOF		33
+#define CMDQ_EVENT_DISP_RDMA_SOF	34
+#define CMDQ_EVENT_CAM_MDP		35
+
+#endif

-- 
2.55.0

[PATCH v2 2/3] soc: mediatek: cmdq-helper: add legacy GCE ISA support

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

Some SoCs, such as mt6572, have different WRITE and POLL instruction
encoding. Instead of 16-bit offset and 8-bit subsystem ID, old GCEs have
22-bit offset and 2-bit subsystem ID.

Extend cmdq_instruction struct with additional union and struct to hold
the legacy format of instruction encoding. Add legacy_isa to select
legacy GCE ISA when writing instruction. Finally, add special handing
for legacy_isa cases.

Also correct comment about PA writes, because it's incorrect for legacy
ISA. Only v2 (mt8167, 8173) and v3 (mt8168, 6768, 6779, 6785, etc) have
GPRs and SPRs for PA writes.

While mt6572 GCE supports 22-bit offsets, the parameter type for offset
remains 16 bit, because all mt6572 GCE mmsys consumers are in the 0xffff
range.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
This change doesn't bring any functional change for mt6572. mt6572 uses
subsystem ID 0 for display, alongside with 16-bit offsets, so layout is
still compatible with v2/v3 GCEs. However, it's better to properly
describe layout.
---
 drivers/soc/mediatek/mtk-cmdq-helper.c   | 57 ++++++++++++++++++++++----------
 include/linux/mailbox/mtk-cmdq-mailbox.h |  1 +
 2 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index f8ee6c9ade89..fb82f2a85a21 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -31,18 +31,27 @@ struct cmdq_instruction {
 		};
 	};
 	union {
-		u16 offset;
-		u16 event;
-		u16 reg_dst;
-	};
-	union {
-		u8 subsys;
 		struct {
-			u8 sop:5;
-			u8 arg_c_t:1;
-			u8 src_t:1;
-			u8 dst_t:1;
-		};
+			union {
+				u16 offset;
+				u16 event;
+				u16 reg_dst;
+			};
+			union {
+				u8 subsys;
+				struct {
+					u8 sop:5;
+					u8 arg_c_t:1;
+					u8 src_t:1;
+					u8 dst_t:1;
+				};
+			};
+		} __packed;
+
+		struct {
+			u32 offset_legacy:22;
+			u32 subsys_legacy:2;
+		} __packed;
 	};
 	u8 op;
 };
@@ -86,7 +95,7 @@ int cmdq_dev_get_client_reg(struct device *dev,
 		client_reg->subsys = CMDQ_SUBSYS_INVALID;
 
 		/*
-		 * All GCEs support writing register PA with mask without subsys,
+		 * GCE v2/v3 support writing register PA with mask without subsys,
 		 * but this requires extra GCE instructions to convert the PA into
 		 * a format that GCE can handle, which is less performance than
 		 * directly using subsys. Therefore, when subsys is available,
@@ -219,10 +228,16 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value)
 {
 	struct cmdq_instruction inst = {
 		.op = CMDQ_CODE_WRITE,
-		.value = value,
-		.offset = offset,
-		.subsys = subsys
+		.value = value
 	};
+
+	if (pkt->priv.legacy_isa) {
+		inst.offset_legacy = offset;
+		inst.subsys_legacy = subsys;
+	} else {
+		inst.offset = offset;
+		inst.subsys = subsys;
+	}
 	return cmdq_pkt_append_command(pkt, inst);
 }
 EXPORT_SYMBOL(cmdq_pkt_write);
@@ -459,10 +474,16 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
 {
 	struct cmdq_instruction inst = {
 		.op = CMDQ_CODE_POLL,
-		.value = value,
-		.offset = offset,
-		.subsys = subsys
+		.value = value
 	};
+
+	if (pkt->priv.legacy_isa) {
+		inst.offset_legacy = offset;
+		inst.subsys_legacy = subsys;
+	} else {
+		inst.offset = offset;
+		inst.subsys = subsys;
+	}
 	return cmdq_pkt_append_command(pkt, inst);
 }
 EXPORT_SYMBOL(cmdq_pkt_poll);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 07c1bfbdb8c4..9fcebc4ca864 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -72,6 +72,7 @@ struct cmdq_cb_data {
 
 struct cmdq_mbox_priv {
 	u8 shift_pa;
+	bool legacy_isa;
 	dma_addr_t mminfra_offset;
 };
 

-- 
2.55.0

Re: [PATCH v2 2/3] soc: mediatek: cmdq-helper: add legacy GCE ISA support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing Hardware Capability Validation for Legacy ISA APIs
--

commit 28dadad6fede624ec82b707f4b97ba89645a0db7
Author: Roman Vivchar <rva333@protonmail.com>

soc: mediatek: cmdq-helper: add legacy GCE ISA support

Some SoCs, such as mt6572, have different WRITE and POLL instruction
encoding. Instead of 16-bit offset and 8-bit subsystem ID, old GCEs have
22-bit offset and 2-bit subsystem ID.

> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index f8ee6c9ade898..fb82f2a85a210 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c

[ ... ]

> @@ -86,7 +95,7 @@ int cmdq_dev_get_client_reg(struct device *dev,
>  		client_reg->subsys = CMDQ_SUBSYS_INVALID;
>  
>  		/*
> -		 * All GCEs support writing register PA with mask without subsys,
> +		 * GCE v2/v3 support writing register PA with mask without subsys,
>  		 * but this requires extra GCE instructions to convert the PA into
>  		 * a format that GCE can handle, which is less performance than
>  		 * directly using subsys. Therefore, when subsys is available,

[Severity: Medium]
When a client device on a legacy GCE is missing the mediatek,gce-client-reg
property, cmdq_dev_get_client_reg() defaults to assigning cmdq_pkt_write_pa
as the write function:

drivers/soc/mediatek/mtk-cmdq-helper.c:cmdq_dev_get_client_reg() {
    ...
		/*
		 * GCE v2/v3 support writing register PA with mask without subsys,
		 * but this requires extra GCE instructions to convert the PA into
		 * a format that GCE can handle, which is less performance than
		 * directly using subsys. Therefore, when subsys is available,
		 * we prefer to use subsys for writing register PA.
		 */
		client_reg->pkt_write = cmdq_pkt_write_pa;
    ...
}

However, cmdq_pkt_write_pa() unconditionally uses SPRs, which legacy GCE
hardware does not support:

drivers/soc/mediatek/mtk-cmdq-helper.c:cmdq_pkt_write_pa() {
    ...
	err = cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX0, CMDQ_ADDR_HIGH(pa_base));
    ...
}

Does this code need a check for pkt->priv.legacy_isa in cmdq_pkt_write_pa()
to prevent generating unsupported instructions that could cause silent queue
corruption or a GCE hardware fault?

[ ... ]

> @@ -459,10 +474,16 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>  {
>  	struct cmdq_instruction inst = {
>  		.op = CMDQ_CODE_POLL,
> -		.value = value,
> -		.offset = offset,
> -		.subsys = subsys
> +		.value = value
>  	};
> +
> +	if (pkt->priv.legacy_isa) {
> +		inst.offset_legacy = offset;
> +		inst.subsys_legacy = subsys;
> +	} else {
> +		inst.offset = offset;
> +		inst.subsys = subsys;
> +	}
>  	return cmdq_pkt_append_command(pkt, inst);
>  }

[Severity: Medium]
Similarly, cmdq_pkt_poll_addr() unconditionally relies on GPR registers, which
legacy hardware does not implement:

drivers/soc/mediatek/mtk-cmdq-helper.c:cmdq_pkt_poll_addr() {
    ...
	inst.op = CMDQ_CODE_POLL;
	inst.dst_t = CMDQ_REG_TYPE;
	inst.sop = CMDQ_POLL_ADDR_GPR;
    ...
}

Should there be a capability validation for legacy_isa in
cmdq_pkt_poll_addr() as well to avoid generating invalid microcode
instructions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-6572-gce-v2-0-412a5b037290@protonmail.com?part=2

Re: [PATCH v2 2/3] soc: mediatek: cmdq-helper: add legacy GCE ISA support

[AngeloGioacchino Del Regno]

On 9/7/26 20:03, Roman Vivchar via B4 Relay wrote:
> From: Roman Vivchar <rva333@protonmail.com>
> 
> Some SoCs, such as mt6572, have different WRITE and POLL instruction
> encoding. Instead of 16-bit offset and 8-bit subsystem ID, old GCEs have
> 22-bit offset and 2-bit subsystem ID.
> 
> Extend cmdq_instruction struct with additional union and struct to hold
> the legacy format of instruction encoding. Add legacy_isa to select
> legacy GCE ISA when writing instruction. Finally, add special handing
> for legacy_isa cases.
> 
> Also correct comment about PA writes, because it's incorrect for legacy
> ISA. Only v2 (mt8167, 8173) and v3 (mt8168, 6768, 6779, 6785, etc) have
> GPRs and SPRs for PA writes.
> 
> While mt6572 GCE supports 22-bit offsets, the parameter type for offset
> remains 16 bit, because all mt6572 GCE mmsys consumers are in the 0xffff
> range.
> 
> Signed-off-by: Roman Vivchar <rva333@protonmail.com>

Verified in the lab on multiple MediaTek boards.

... but I either need an ack on patch 3/3 from mailbox maintainers, or they need
an ack from me on this patch to pick everything.

I'm fine with anyone picking this, so...

Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Cheers,
Angelo


> ---
> This change doesn't bring any functional change for mt6572. mt6572 uses
> subsystem ID 0 for display, alongside with 16-bit offsets, so layout is
> still compatible with v2/v3 GCEs. However, it's better to properly
> describe layout.
> ---
>   drivers/soc/mediatek/mtk-cmdq-helper.c   | 57 ++++++++++++++++++++++----------
>   include/linux/mailbox/mtk-cmdq-mailbox.h |  1 +
>   2 files changed, 40 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index f8ee6c9ade89..fb82f2a85a21 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -31,18 +31,27 @@ struct cmdq_instruction {
>   		};
>   	};
>   	union {
> -		u16 offset;
> -		u16 event;
> -		u16 reg_dst;
> -	};
> -	union {
> -		u8 subsys;
>   		struct {
> -			u8 sop:5;
> -			u8 arg_c_t:1;
> -			u8 src_t:1;
> -			u8 dst_t:1;
> -		};
> +			union {
> +				u16 offset;
> +				u16 event;
> +				u16 reg_dst;
> +			};
> +			union {
> +				u8 subsys;
> +				struct {
> +					u8 sop:5;
> +					u8 arg_c_t:1;
> +					u8 src_t:1;
> +					u8 dst_t:1;
> +				};
> +			};
> +		} __packed;
> +
> +		struct {
> +			u32 offset_legacy:22;
> +			u32 subsys_legacy:2;
> +		} __packed;
>   	};
>   	u8 op;
>   };
> @@ -86,7 +95,7 @@ int cmdq_dev_get_client_reg(struct device *dev,
>   		client_reg->subsys = CMDQ_SUBSYS_INVALID;
>   
>   		/*
> -		 * All GCEs support writing register PA with mask without subsys,
> +		 * GCE v2/v3 support writing register PA with mask without subsys,
>   		 * but this requires extra GCE instructions to convert the PA into
>   		 * a format that GCE can handle, which is less performance than
>   		 * directly using subsys. Therefore, when subsys is available,
> @@ -219,10 +228,16 @@ int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value)
>   {
>   	struct cmdq_instruction inst = {
>   		.op = CMDQ_CODE_WRITE,
> -		.value = value,
> -		.offset = offset,
> -		.subsys = subsys
> +		.value = value
>   	};
> +
> +	if (pkt->priv.legacy_isa) {
> +		inst.offset_legacy = offset;
> +		inst.subsys_legacy = subsys;
> +	} else {
> +		inst.offset = offset;
> +		inst.subsys = subsys;
> +	}
>   	return cmdq_pkt_append_command(pkt, inst);
>   }
>   EXPORT_SYMBOL(cmdq_pkt_write);
> @@ -459,10 +474,16 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
>   {
>   	struct cmdq_instruction inst = {
>   		.op = CMDQ_CODE_POLL,
> -		.value = value,
> -		.offset = offset,
> -		.subsys = subsys
> +		.value = value
>   	};
> +
> +	if (pkt->priv.legacy_isa) {
> +		inst.offset_legacy = offset;
> +		inst.subsys_legacy = subsys;
> +	} else {
> +		inst.offset = offset;
> +		inst.subsys = subsys;
> +	}
>   	return cmdq_pkt_append_command(pkt, inst);
>   }
>   EXPORT_SYMBOL(cmdq_pkt_poll);
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 07c1bfbdb8c4..9fcebc4ca864 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -72,6 +72,7 @@ struct cmdq_cb_data {
>   
>   struct cmdq_mbox_priv {
>   	u8 shift_pa;
> +	bool legacy_isa;
>   	dma_addr_t mminfra_offset;
>   };
>   
> 

[PATCH v2 3/3] mailbox: mtk-cmdq: add mt6572 support

[Roman Vivchar via B4 Relay]

From: Roman Vivchar <rva333@protonmail.com>

Add legacy_isa field forwarding to the cmdq helper and a platform data
for the mt6572 GCE.

Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
 drivers/mailbox/mtk-cmdq-mailbox.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index e523c84b4808..5e7bc52baaa1 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -99,6 +99,7 @@ struct gce_plat {
 	bool control_by_sw;
 	bool sw_ddr_en;
 	bool gce_vm;
+	bool legacy_isa;
 	u32 gce_num;
 };
 
@@ -119,6 +120,7 @@ void cmdq_get_mbox_priv(struct mbox_chan *chan, struct cmdq_mbox_priv *priv)
 	struct cmdq *cmdq = container_of(chan->mbox, struct cmdq, mbox);
 
 	priv->shift_pa = cmdq->pdata->shift;
+	priv->legacy_isa = cmdq->pdata->legacy_isa;
 	priv->mminfra_offset = cmdq->pdata->mminfra_offset;
 }
 EXPORT_SYMBOL(cmdq_get_mbox_priv);
@@ -787,6 +789,14 @@ static const struct dev_pm_ops cmdq_pm_ops = {
 			   cmdq_runtime_resume, NULL)
 };
 
+static const struct gce_plat gce_plat_mt6572 = {
+	.thread_nr = 14,
+	.shift = 0,
+	.control_by_sw = false,
+	.legacy_isa = true,
+	.gce_num = 1
+};
+
 static const struct gce_plat gce_plat_mt6779 = {
 	.thread_nr = 24,
 	.shift = 3,
@@ -848,6 +858,7 @@ static const struct gce_plat gce_plat_mt8196 = {
 };
 
 static const struct of_device_id cmdq_of_ids[] = {
+	{.compatible = "mediatek,mt6572-gce", .data = (void *)&gce_plat_mt6572},
 	{.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_mt6779},
 	{.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_mt8173},
 	{.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_mt8183},

-- 
2.55.0

Re: [PATCH v2 3/3] mailbox: mtk-cmdq: add mt6572 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Functions generating SPR/GPR-based instructions lack safety checks for legacy_isa, potentially causing GCE hardware lockups on MT6572.
--

commit 4694eedf10a37d189a86ca417a39efa7d5e2b2dd
Author: Roman Vivchar <rva333@protonmail.com>

mailbox: mtk-cmdq: add mt6572 support

Add legacy_isa field forwarding to the cmdq helper and a platform data
for the mt6572 GCE.

> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index e523c84b48088..5e7bc52baaa13 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
[ ... ]
> @@ -787,6 +789,14 @@ static const struct dev_pm_ops cmdq_pm_ops = {
>  			   cmdq_runtime_resume, NULL)
>  };
>  
> +static const struct gce_plat gce_plat_mt6572 = {
> +	.thread_nr = 14,
> +	.shift = 0,
> +	.control_by_sw = false,
> +	.legacy_isa = true,
> +	.gce_num = 1
> +};
> +

[Severity: High]
Does enabling legacy ISA expose missing safety checks in the cmdq helper?

Looking at drivers/soc/mediatek/mtk-cmdq-helper.c, functions like
cmdq_pkt_write_pa() and cmdq_pkt_poll_addr() appear to generate SPR and
GPR-based instructions without checking for legacy hardware capabilities:

drivers/soc/mediatek/mtk-cmdq-helper.c:cmdq_pkt_write_pa() {
    ...
    err = cmdq_pkt_assign(pkt, CMDQ_THR_SPR_IDX0, CMDQ_ADDR_HIGH(pa_base));
    ...
}

drivers/soc/mediatek/mtk-cmdq-helper.c:cmdq_pkt_poll_addr() {
    ...
    inst.sop = CMDQ_POLL_ADDR_GPR;
    ...
}

If a client driver on MT6572 calls these functions, would it silently emit
structurally invalid instructions to the command buffer?

Since the legacy ISA on MT6572 does not support SPR or GPR instructions,
could this result in an unrecoverable GCE hardware lockup when flushed?

Would it make sense to check pkt->priv.legacy_isa in these functions and
return -EOPNOTSUPP to prevent hardware lockups?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-6572-gce-v2-0-412a5b037290@protonmail.com?part=3

Re: [PATCH v2 0/3] MediaTek mt6572 GCE support

[AngeloGioacchino Del Regno]

On 9/7/26 20:03, Roman Vivchar via B4 Relay wrote:
> This patch series adds support for the Global Command Engine found in
> the mt6572 SoC.
> 
> Notably, the patch 2 introduces legacy ISA format, which is used on
> mt6572, 6582 and 6592 SoCs. mt6577 doesn't have the GCE at all and
> mt6595 is much more close to 6795/8173.
> Patch 3 adds mt6572 platform data and legacy_isa forwarding to the
> instruction JIT.
> 
> This format was reverse engineered from the Android HAL and uses 22-bit
> offset with 2-bit subsystem ID, unlike the newer 16-bit offset and 8-bit
> subsystem ID.
> The mt6572 HAL includes workarounds for WFE and EOC instructions, emitting
> another WFE as barrier at WFE, and JUMP +8 at EOC.
> mt6582 and 6592 HAL dropped WFE barrier, but still have JUMP +8.
> 
> This sounds very much like some kind of software mitigations for hardware
> bugs, but during testing on 6572 nothing bad happened so far. These
> additional instructions are not included in the patches.
> 
> Note that while GCE can be used for e.g. mt6592 UFOE (which is at
> 0x14013000, so outside of the u16 range), currently the driver still uses
> u16 for the offset, because I don't have any 6592 device.
> 
> Signed-off-by: Roman Vivchar <rva333@protonmail.com>

Series is

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
> Changes in v2:
> - Move dt-bindings to mailbox directory from gce (Krzysztof)
> - Clarify that only v2 and v3 GCEs support PA writes
> - Link to v1: https://patch.msgid.link/20260728-6572-gce-v1-0-750bba32689e@protonmail.com
> 
> ---
> Roman Vivchar (3):
>        dt-bindings: mailbox: mediatek,gce-mailbox: add mt6572
>        soc: mediatek: cmdq-helper: add legacy GCE ISA support
>        mailbox: mtk-cmdq: add mt6572 support
> 
>   .../bindings/mailbox/mediatek,gce-mailbox.yaml     |  1 +
>   drivers/mailbox/mtk-cmdq-mailbox.c                 | 11 +++++
>   drivers/soc/mediatek/mtk-cmdq-helper.c             | 57 +++++++++++++++-------
>   include/dt-bindings/mailbox/mediatek,mt6572-gce.h  | 52 ++++++++++++++++++++
>   include/linux/mailbox/mtk-cmdq-mailbox.h           |  1 +
>   5 files changed, 104 insertions(+), 18 deletions(-)
> ---
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
> change-id: 20260727-6572-gce-6a571ef4af09
> 
> Best regards,
> --
> Roman Vivchar <rva333@protonmail.com>