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

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

Il 27/08/25 13:37, Jason-JH Lin ha scritto:
> 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.
> 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. After that, the shift functions
> do more than just shift addresses, so the APIs were renamed to
> cmdq_convert_gce_addr() and cmdq_revert_gce_addr().
> 
> 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       | 22 ++++++++++++----------
>   include/linux/mailbox/mtk-cmdq-mailbox.h |  1 +
>   2 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index a9e8895d14df..373effbcfa40 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -94,20 +94,21 @@ 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;
>   	u32 gce_num;
>   };
>   
> -static inline u32 cmdq_reg_shift_addr(dma_addr_t addr, const struct gce_plat *pdata)
> +static inline u32 cmdq_convert_gce_addr(dma_addr_t addr, const struct gce_plat *pdata)

You are adding those functions in a previous commit - please, just give them a good
and right name from the beginning and don't change it anymore in any later commit.

The code, anyway, looks okay.

Cheers,
Angelo

>   {
> -	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)
> +static inline dma_addr_t cmdq_revert_gce_addr(u32 addr, const struct gce_plat *pdata)
>   {
> -	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);
>   
> @@ -254,7 +256,7 @@ static void cmdq_task_insert_into_thread(struct cmdq_task *task)
>   	struct cmdq_task *prev_task = list_last_entry(
>   			&thread->task_busy_list, typeof(*task), list_entry);
>   	u64 *prev_task_base = prev_task->pkt->va_base;
> -	u32 shift_addr = cmdq_reg_shift_addr(task->pa_base, task->cmdq->pdata);
> +	u32 shift_addr = cmdq_convert_gce_addr(task->pa_base, task->cmdq->pdata);
>   
>   	/* let previous task jump to this task */
>   	dma_sync_single_for_cpu(dev, prev_task->pa_base,
> @@ -326,7 +328,7 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
>   		return;
>   
>   	shift_addr = readl(thread->base + CMDQ_THR_CURR_ADDR);
> -	curr_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
> +	curr_pa = cmdq_revert_gce_addr(shift_addr, cmdq->pdata);
>   
>   	list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
>   				 list_entry) {
> @@ -477,9 +479,9 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
>   		 */
>   		WARN_ON(cmdq_thread_reset(cmdq, thread) < 0);
>   
> -		shift_addr = cmdq_reg_shift_addr(task->pa_base, cmdq->pdata);
> +		shift_addr = cmdq_convert_gce_addr(task->pa_base, cmdq->pdata);
>   		writel(shift_addr, thread->base + CMDQ_THR_CURR_ADDR);
> -		shift_addr = cmdq_reg_shift_addr(task->pa_base + pkt->cmd_buf_size, cmdq->pdata);
> +		shift_addr = cmdq_convert_gce_addr(task->pa_base + pkt->cmd_buf_size, cmdq->pdata);
>   		writel(shift_addr, thread->base + CMDQ_THR_END_ADDR);
>   
>   		writel(thread->priority, thread->base + CMDQ_THR_PRIORITY);
> @@ -488,9 +490,9 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data)
>   	} else {
>   		WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0);
>   		shift_addr = readl(thread->base + CMDQ_THR_CURR_ADDR);
> -		curr_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
> +		curr_pa = cmdq_revert_gce_addr(shift_addr, cmdq->pdata);
>   		shift_addr = readl(thread->base + CMDQ_THR_END_ADDR);
> -		end_pa = cmdq_reg_revert_addr(shift_addr, cmdq->pdata);
> +		end_pa = cmdq_revert_gce_addr(shift_addr, cmdq->pdata);
>   		/* check boundary */
>   		if (curr_pa == end_pa - CMDQ_INST_SIZE ||
>   		    curr_pa == end_pa) {
> 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 {