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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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;
>   };
>   
>