Re: [PATCH v6 07/20] mailbox: mtk-cmdq: Add mminfra_offset configuration for DRAM transaction

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Mon, 2025-06-02 at 01:31 +0800, Jason-JH Lin wrote:
> The GCE in MT8196 is placed in MMINFRA and requires all addresses
> in GCE instructions for DRAM transactions to be IOVA.
> 
> Due to MMIO, if the GCE needs to access a hardware register at
> 0x1000_0000, but the SMMU is also mapping a DRAM block at 0x1000_0000,
> the MMINFRA will not know whether to write to the hardware register or
> the DRAM.

I don't know why you mention SMMU because GCE access DRAM without any iommu function.
It seems previous SoC may have the same problem, how is it solved in other SoC?

> To solve this, MMINFRA treats addresses greater than 2G as data paths
> and those less than 2G as config paths because the DRAM start address
> is currently at 2G (0x8000_0000). On the data path, MMINFRA remaps
> DRAM addresses by subtracting 2G, allowing SMMU to map DRAM addresses
> less than 2G.
> For example, if the DRAM start address 0x8000_0000 is mapped to
> IOVA=0x0, when GCE accesses IOVA=0x0, it must add a 2G offset to
> the address in the GCE instruction. MMINFRA will then see it as a
> data path (IOVA >= 2G) and subtract 2G, allowing GCE to access IOVA=0x0.
> 
> Since the MMINFRA remap subtracting 2G is done in hardware and cannot
> be configured by software, the address of DRAM in GCE instruction must
> always add 2G to ensure proper access.
> This 2G adjustment is referred to as mminfra_offset in the CMDQ driver.
> CMDQ helper can get the mminfra_offset from the cmdq_mbox_priv of
> cmdq_pkt and add the mminfra_offset to the DRAM address in GCE
> instructions.
> 
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
>  drivers/mailbox/mtk-cmdq-mailbox.c       | 6 ++++--
>  include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index e2ea12e9aecb..6f4b9879069e 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -94,6 +94,7 @@ struct cmdq {
>  struct gce_plat {
>  	u32 thread_nr;
>  	u8 shift;
> +	dma_addr_t mminfra_offset;
>  	bool control_by_sw;
>  	bool sw_ddr_en;
>  	bool gce_vm;
> @@ -102,12 +103,12 @@ struct gce_plat {
>  
>  static inline u32 cmdq_reg_shift_addr(dma_addr_t addr, const struct gce_plat *pdata)

This function does not just shift address.
So I would like this function name to be 'cmdq_iova_to_gce_addr'.

>  {
> -	return (addr >> pdata->shift);
> +	return ((addr + pdata->mminfra_offset) >> pdata->shift);
>  }
>  
>  static inline dma_addr_t cmdq_reg_revert_addr(u32 addr, const struct gce_plat *pdata)

I would like this function name to be 'cmdq_gce_addr_to_iova'.

Regards,
CK

>  {
> -	return ((dma_addr_t)addr << pdata->shift);
> +	return (((dma_addr_t)addr << pdata->shift) - pdata->mminfra_offset);
>  }
>  
>  void cmdq_get_mbox_priv(struct mbox_chan *chan, struct cmdq_mbox_priv *priv)
> @@ -115,6 +116,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->mminfra_offset = cmdq->pdata->mminfra_offset;
>  }
>  EXPORT_SYMBOL(cmdq_get_mbox_priv);
>  
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 73b70be4a8a7..07c1bfbdb8c4 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;
> +	dma_addr_t mminfra_offset;
>  };
>  
>  struct cmdq_pkt {