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

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

On Fri, 2025-06-27 at 09:49 +0000, CK Hu (胡俊光) wrote:
> 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.

Yes, actually it does,'t have much to do with SMMU. It's because GCE of
MT8196 is placed in MMINFRA. MMINFRA has a rule to determine that if
the address going out from GCE < 2G, it should go to the config path
(HW register), and if the address >= 2G, it should go ti the data path
(DRAM).

Since the GCE of MT8196 uses IOMMU, the SMMU might map the IOVA to a
position between 0 and 2G. To prevent MMINFRA from sending the IOVA
intended for 0-2G to the config path, the GCE needs to add this 2G
`mminfra_offset` to the DRAM address. MMINFRA will then uniformly
subtract 2G from the address going to DRAM to access the actual IOVA.
This subtraction of 2G is a hardware-bound rule of MMINFRA and cannot
be modified by software.

> It seems previous SoC may .have the same problem, how is it solved in
> other SoC?
> 

Previous SoCs used GCE without IOMMU and accessed PA, and the PA of
DRAM was usually placed after 2G, so there was no need for this rule to
determine whether to go to DRAM, thus avoiding this issue.

> > 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'.
> 

Sure, but this not only for the iova, so I'll rename it as
`cmdq_convert_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'.

And rename this as `cmdq_revert_gce_addr`.

Regards,
Jason-JH Lin

> 
> Regards,
> CK