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

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

On Thu, 2025-10-09 at 13:27 +0200, AngeloGioacchino Del Regno wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> 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.
> 

OK, I'll move this to [PATCH 2/20].

> The code, anyway, looks okay.

Thanks for the reviews.

Regards,
Jason-JH Lin

> 
> Cheers,
> Angelo