diff mbox series

[v5,06/27] dt-bindings: mediatek: Add binding for mt8192 IOMMU

Message ID 20201209080102.26626-7-yong.wu@mediatek.com
State Not Applicable, archived
Headers show
Series MT8192 IOMMU support | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success

Commit Message

Yong Wu (吴勇) Dec. 9, 2020, 8 a.m. UTC 
This patch adds decriptions for mt8192 IOMMU and SMI.

mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
table format. The M4U-SMI HW diagram is as below:

                          EMI
                           |
                          M4U
                           |
                      ------------
                       SMI Common
                      ------------
                           |
  +-------+------+------+----------------------+-------+
  |       |      |      |       ......         |       |
  |       |      |      |                      |       |
larb0   larb1  larb2  larb4     ......      larb19   larb20
disp0   disp1   mdp    vdec                   IPE      IPE

All the connections are HW fixed, SW can NOT adjust it.

mt8192 M4U support 0~16GB iova range. we preassign different engines
into different iova ranges:

domain-id  module     iova-range                  larbs
   0       disp        0 ~ 4G                      larb0/1
   1       vcodec      4G ~ 8G                     larb4/5/7
   2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
   3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
   4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5

The iova range for CCU0/1(camera control unit) is HW requirement.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
 include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
 2 files changed, 257 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h

Comments

Krzysztof Kozlowski Dec. 9, 2020, 12:13 p.m. UTC | #1 
On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> This patch adds decriptions for mt8192 IOMMU and SMI.
> 
> mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> table format. The M4U-SMI HW diagram is as below:
> 
>                           EMI
>                            |
>                           M4U
>                            |
>                       ------------
>                        SMI Common
>                       ------------
>                            |
>   +-------+------+------+----------------------+-------+
>   |       |      |      |       ......         |       |
>   |       |      |      |                      |       |
> larb0   larb1  larb2  larb4     ......      larb19   larb20
> disp0   disp1   mdp    vdec                   IPE      IPE
> 
> All the connections are HW fixed, SW can NOT adjust it.
> 
> mt8192 M4U support 0~16GB iova range. we preassign different engines
> into different iova ranges:
> 
> domain-id  module     iova-range                  larbs
>    0       disp        0 ~ 4G                      larb0/1
>    1       vcodec      4G ~ 8G                     larb4/5/7
>    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
>    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
>    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> 
> The iova range for CCU0/1(camera control unit) is HW requirement.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
>  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
>  2 files changed, 257 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> 

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
Tomasz Figa Dec. 23, 2020, 8:18 a.m. UTC | #2 
On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> This patch adds decriptions for mt8192 IOMMU and SMI.
> 
> mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> table format. The M4U-SMI HW diagram is as below:
> 
>                           EMI
>                            |
>                           M4U
>                            |
>                       ------------
>                        SMI Common
>                       ------------
>                            |
>   +-------+------+------+----------------------+-------+
>   |       |      |      |       ......         |       |
>   |       |      |      |                      |       |
> larb0   larb1  larb2  larb4     ......      larb19   larb20
> disp0   disp1   mdp    vdec                   IPE      IPE
> 
> All the connections are HW fixed, SW can NOT adjust it.
> 
> mt8192 M4U support 0~16GB iova range. we preassign different engines
> into different iova ranges:
> 
> domain-id  module     iova-range                  larbs
>    0       disp        0 ~ 4G                      larb0/1
>    1       vcodec      4G ~ 8G                     larb4/5/7
>    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20

Why do we preassign these addresses in DT? Shouldn't it be a user's or
integrator's decision to split the 16 GB address range into sub-ranges
and define which larbs those sub-ranges are shared with?

Best regards,
Tomasz

>    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
>    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> 
> The iova range for CCU0/1(camera control unit) is HW requirement.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
>  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
>  2 files changed, 257 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> 
> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index ba6626347381..0f26fe14c8e2 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -76,6 +76,7 @@ properties:
>            - mediatek,mt8167-m4u  # generation two
>            - mediatek,mt8173-m4u  # generation two
>            - mediatek,mt8183-m4u  # generation two
> +          - mediatek,mt8192-m4u  # generation two
>  
>        - description: mt7623 generation one
>          items:
> @@ -115,7 +116,11 @@ properties:
>        dt-binding/memory/mt6779-larb-port.h for mt6779,
>        dt-binding/memory/mt8167-larb-port.h for mt8167,
>        dt-binding/memory/mt8173-larb-port.h for mt8173,
> -      dt-binding/memory/mt8183-larb-port.h for mt8183.
> +      dt-binding/memory/mt8183-larb-port.h for mt8183,
> +      dt-binding/memory/mt8192-larb-port.h for mt8192.
> +
> +  power-domains:
> +    maxItems: 1
>  
>  required:
>    - compatible
> @@ -133,11 +138,22 @@ allOf:
>                - mediatek,mt2701-m4u
>                - mediatek,mt2712-m4u
>                - mediatek,mt8173-m4u
> +              - mediatek,mt8192-m4u
>  
>      then:
>        required:
>          - clocks
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - mediatek,mt8192-m4u
> +
> +    then:
> +      required:
> +        - power-domains
> +
>  additionalProperties: false
>  
>  examples:
> diff --git a/include/dt-bindings/memory/mt8192-larb-port.h b/include/dt-bindings/memory/mt8192-larb-port.h
> new file mode 100644
> index 000000000000..ec1ac2ba7094
> --- /dev/null
> +++ b/include/dt-bindings/memory/mt8192-larb-port.h
> @@ -0,0 +1,240 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + *
> + * Author: Chao Hao <chao.hao@mediatek.com>
> + * Author: Yong Wu <yong.wu@mediatek.com>
> + */
> +#ifndef _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_
> +#define _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_
> +
> +#include <dt-bindings/memory/mtk-smi-larb-port.h>
> +
> +/*
> + * MM IOMMU:
> + * domain 0: display: larb0, larb1.
> + * domain 1: vcodec: larb4, larb5, larb7.
> + * domain 2: CAM/MDP: larb2, larb9, larb11, larb13, larb14, larb16,
> + *           larb17, larb18, larb19, larb20,
> + * domain 3: CCU0: larb13 - port9/10.
> + * domain 4: CCU1: larb14 - port4/5.
> + *
> + * larb3/6/8/10/12/15 is null.
> + */
> +
> +/* larb0 */
> +#define M4U_PORT_L0_DISP_POSTMASK0		MTK_M4U_DOM_ID(0, 0, 0)
> +#define M4U_PORT_L0_OVL_RDMA0_HDR		MTK_M4U_DOM_ID(0, 0, 1)
> +#define M4U_PORT_L0_OVL_RDMA0			MTK_M4U_DOM_ID(0, 0, 2)
> +#define M4U_PORT_L0_DISP_RDMA0			MTK_M4U_DOM_ID(0, 0, 3)
> +#define M4U_PORT_L0_DISP_WDMA0			MTK_M4U_DOM_ID(0, 0, 4)
> +#define M4U_PORT_L0_DISP_FAKE0			MTK_M4U_DOM_ID(0, 0, 5)
> +
> +/* larb1 */
> +#define M4U_PORT_L1_OVL_2L_RDMA0_HDR		MTK_M4U_DOM_ID(0, 1, 0)
> +#define M4U_PORT_L1_OVL_2L_RDMA2_HDR		MTK_M4U_DOM_ID(0, 1, 1)
> +#define M4U_PORT_L1_OVL_2L_RDMA0		MTK_M4U_DOM_ID(0, 1, 2)
> +#define M4U_PORT_L1_OVL_2L_RDMA2		MTK_M4U_DOM_ID(0, 1, 3)
> +#define M4U_PORT_L1_DISP_MDP_RDMA4		MTK_M4U_DOM_ID(0, 1, 4)
> +#define M4U_PORT_L1_DISP_RDMA4			MTK_M4U_DOM_ID(0, 1, 5)
> +#define M4U_PORT_L1_DISP_UFBC_WDMA0		MTK_M4U_DOM_ID(0, 1, 6)
> +#define M4U_PORT_L1_DISP_FAKE1			MTK_M4U_DOM_ID(0, 1, 7)
> +
> +/* larb2 */
> +#define M4U_PORT_L2_MDP_RDMA0			MTK_M4U_DOM_ID(2, 2, 0)
> +#define M4U_PORT_L2_MDP_RDMA1			MTK_M4U_DOM_ID(2, 2, 1)
> +#define M4U_PORT_L2_MDP_WROT0			MTK_M4U_DOM_ID(2, 2, 2)
> +#define M4U_PORT_L2_MDP_WROT1			MTK_M4U_DOM_ID(2, 2, 3)
> +#define M4U_PORT_L2_MDP_DISP_FAKE0		MTK_M4U_DOM_ID(2, 2, 4)
> +
> +/* larb3: null */
> +
> +/* larb4 */
> +#define M4U_PORT_L4_VDEC_MC_EXT			MTK_M4U_DOM_ID(1, 4, 0)
> +#define M4U_PORT_L4_VDEC_UFO_EXT		MTK_M4U_DOM_ID(1, 4, 1)
> +#define M4U_PORT_L4_VDEC_PP_EXT			MTK_M4U_DOM_ID(1, 4, 2)
> +#define M4U_PORT_L4_VDEC_PRED_RD_EXT		MTK_M4U_DOM_ID(1, 4, 3)
> +#define M4U_PORT_L4_VDEC_PRED_WR_EXT		MTK_M4U_DOM_ID(1, 4, 4)
> +#define M4U_PORT_L4_VDEC_PPWRAP_EXT		MTK_M4U_DOM_ID(1, 4, 5)
> +#define M4U_PORT_L4_VDEC_TILE_EXT		MTK_M4U_DOM_ID(1, 4, 6)
> +#define M4U_PORT_L4_VDEC_VLD_EXT		MTK_M4U_DOM_ID(1, 4, 7)
> +#define M4U_PORT_L4_VDEC_VLD2_EXT		MTK_M4U_DOM_ID(1, 4, 8)
> +#define M4U_PORT_L4_VDEC_AVC_MV_EXT		MTK_M4U_DOM_ID(1, 4, 9)
> +#define M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT	MTK_M4U_DOM_ID(1, 4, 10)
> +
> +/* larb5 */
> +#define M4U_PORT_L5_VDEC_LAT0_VLD_EXT		MTK_M4U_DOM_ID(1, 5, 0)
> +#define M4U_PORT_L5_VDEC_LAT0_VLD2_EXT		MTK_M4U_DOM_ID(1, 5, 1)
> +#define M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT	MTK_M4U_DOM_ID(1, 5, 2)
> +#define M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT	MTK_M4U_DOM_ID(1, 5, 3)
> +#define M4U_PORT_L5_VDEC_LAT0_TILE_EXT		MTK_M4U_DOM_ID(1, 5, 4)
> +#define M4U_PORT_L5_VDEC_LAT0_WDMA_EXT		MTK_M4U_DOM_ID(1, 5, 5)
> +#define M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT	MTK_M4U_DOM_ID(1, 5, 6)
> +#define M4U_PORT_L5_VDEC_UFO_ENC_EXT		MTK_M4U_DOM_ID(1, 5, 7)
> +
> +/* larb6: null */
> +
> +/* larb7 */
> +#define M4U_PORT_L7_VENC_RCPU			MTK_M4U_DOM_ID(1, 7, 0)
> +#define M4U_PORT_L7_VENC_REC			MTK_M4U_DOM_ID(1, 7, 1)
> +#define M4U_PORT_L7_VENC_BSDMA			MTK_M4U_DOM_ID(1, 7, 2)
> +#define M4U_PORT_L7_VENC_SV_COMV		MTK_M4U_DOM_ID(1, 7, 3)
> +#define M4U_PORT_L7_VENC_RD_COMV		MTK_M4U_DOM_ID(1, 7, 4)
> +#define M4U_PORT_L7_VENC_CUR_LUMA		MTK_M4U_DOM_ID(1, 7, 5)
> +#define M4U_PORT_L7_VENC_CUR_CHROMA		MTK_M4U_DOM_ID(1, 7, 6)
> +#define M4U_PORT_L7_VENC_REF_LUMA		MTK_M4U_DOM_ID(1, 7, 7)
> +#define M4U_PORT_L7_VENC_REF_CHROMA		MTK_M4U_DOM_ID(1, 7, 8)
> +#define M4U_PORT_L7_JPGENC_Y_RDMA		MTK_M4U_DOM_ID(1, 7, 9)
> +#define M4U_PORT_L7_JPGENC_Q_RDMA		MTK_M4U_DOM_ID(1, 7, 10)
> +#define M4U_PORT_L7_JPGENC_C_TABLE		MTK_M4U_DOM_ID(1, 7, 11)
> +#define M4U_PORT_L7_JPGENC_BSDMA		MTK_M4U_DOM_ID(1, 7, 12)
> +#define M4U_PORT_L7_VENC_SUB_R_LUMA		MTK_M4U_DOM_ID(1, 7, 13)
> +#define M4U_PORT_L7_VENC_SUB_W_LUMA		MTK_M4U_DOM_ID(1, 7, 14)
> +
> +/* larb8: null */
> +
> +/* larb9 */
> +#define M4U_PORT_L9_IMG_IMGI_D1			MTK_M4U_DOM_ID(2, 9, 0)
> +#define M4U_PORT_L9_IMG_IMGBI_D1		MTK_M4U_DOM_ID(2, 9, 1)
> +#define M4U_PORT_L9_IMG_DMGI_D1			MTK_M4U_DOM_ID(2, 9, 2)
> +#define M4U_PORT_L9_IMG_DEPI_D1			MTK_M4U_DOM_ID(2, 9, 3)
> +#define M4U_PORT_L9_IMG_ICE_D1			MTK_M4U_DOM_ID(2, 9, 4)
> +#define M4U_PORT_L9_IMG_SMTI_D1			MTK_M4U_DOM_ID(2, 9, 5)
> +#define M4U_PORT_L9_IMG_SMTO_D2			MTK_M4U_DOM_ID(2, 9, 6)
> +#define M4U_PORT_L9_IMG_SMTO_D1			MTK_M4U_DOM_ID(2, 9, 7)
> +#define M4U_PORT_L9_IMG_CRZO_D1			MTK_M4U_DOM_ID(2, 9, 8)
> +#define M4U_PORT_L9_IMG_IMG3O_D1		MTK_M4U_DOM_ID(2, 9, 9)
> +#define M4U_PORT_L9_IMG_VIPI_D1			MTK_M4U_DOM_ID(2, 9, 10)
> +#define M4U_PORT_L9_IMG_SMTI_D5			MTK_M4U_DOM_ID(2, 9, 11)
> +#define M4U_PORT_L9_IMG_TIMGO_D1		MTK_M4U_DOM_ID(2, 9, 12)
> +#define M4U_PORT_L9_IMG_UFBC_W0			MTK_M4U_DOM_ID(2, 9, 13)
> +#define M4U_PORT_L9_IMG_UFBC_R0			MTK_M4U_DOM_ID(2, 9, 14)
> +
> +/* larb10: null */
> +
> +/* larb11 */
> +#define M4U_PORT_L11_IMG_IMGI_D1		MTK_M4U_DOM_ID(2, 11, 0)
> +#define M4U_PORT_L11_IMG_IMGBI_D1		MTK_M4U_DOM_ID(2, 11, 1)
> +#define M4U_PORT_L11_IMG_DMGI_D1		MTK_M4U_DOM_ID(2, 11, 2)
> +#define M4U_PORT_L11_IMG_DEPI_D1		MTK_M4U_DOM_ID(2, 11, 3)
> +#define M4U_PORT_L11_IMG_ICE_D1			MTK_M4U_DOM_ID(2, 11, 4)
> +#define M4U_PORT_L11_IMG_SMTI_D1		MTK_M4U_DOM_ID(2, 11, 5)
> +#define M4U_PORT_L11_IMG_SMTO_D2		MTK_M4U_DOM_ID(2, 11, 6)
> +#define M4U_PORT_L11_IMG_SMTO_D1		MTK_M4U_DOM_ID(2, 11, 7)
> +#define M4U_PORT_L11_IMG_CRZO_D1		MTK_M4U_DOM_ID(2, 11, 8)
> +#define M4U_PORT_L11_IMG_IMG3O_D1		MTK_M4U_DOM_ID(2, 11, 9)
> +#define M4U_PORT_L11_IMG_VIPI_D1		MTK_M4U_DOM_ID(2, 11, 10)
> +#define M4U_PORT_L11_IMG_SMTI_D5		MTK_M4U_DOM_ID(2, 11, 11)
> +#define M4U_PORT_L11_IMG_TIMGO_D1		MTK_M4U_DOM_ID(2, 11, 12)
> +#define M4U_PORT_L11_IMG_UFBC_W0		MTK_M4U_DOM_ID(2, 11, 13)
> +#define M4U_PORT_L11_IMG_UFBC_R0		MTK_M4U_DOM_ID(2, 11, 14)
> +#define M4U_PORT_L11_IMG_WPE_RDMA1		MTK_M4U_DOM_ID(2, 11, 15)
> +#define M4U_PORT_L11_IMG_WPE_RDMA0		MTK_M4U_DOM_ID(2, 11, 16)
> +#define M4U_PORT_L11_IMG_WPE_WDMA		MTK_M4U_DOM_ID(2, 11, 17)
> +#define M4U_PORT_L11_IMG_MFB_RDMA0		MTK_M4U_DOM_ID(2, 11, 18)
> +#define M4U_PORT_L11_IMG_MFB_RDMA1		MTK_M4U_DOM_ID(2, 11, 19)
> +#define M4U_PORT_L11_IMG_MFB_RDMA2		MTK_M4U_DOM_ID(2, 11, 20)
> +#define M4U_PORT_L11_IMG_MFB_RDMA3		MTK_M4U_DOM_ID(2, 11, 21)
> +#define M4U_PORT_L11_IMG_MFB_RDMA4		MTK_M4U_DOM_ID(2, 11, 22)
> +#define M4U_PORT_L11_IMG_MFB_RDMA5		MTK_M4U_DOM_ID(2, 11, 23)
> +#define M4U_PORT_L11_IMG_MFB_WDMA0		MTK_M4U_DOM_ID(2, 11, 24)
> +#define M4U_PORT_L11_IMG_MFB_WDMA1		MTK_M4U_DOM_ID(2, 11, 25)
> +
> +/* larb12: null */
> +
> +/* larb13 */
> +#define M4U_PORT_L13_CAM_MRAWI			MTK_M4U_DOM_ID(2, 13, 0)
> +#define M4U_PORT_L13_CAM_MRAWO0			MTK_M4U_DOM_ID(2, 13, 1)
> +#define M4U_PORT_L13_CAM_MRAWO1			MTK_M4U_DOM_ID(2, 13, 2)
> +#define M4U_PORT_L13_CAM_CAMSV1			MTK_M4U_DOM_ID(2, 13, 3)
> +#define M4U_PORT_L13_CAM_CAMSV2			MTK_M4U_DOM_ID(2, 13, 4)
> +#define M4U_PORT_L13_CAM_CAMSV3			MTK_M4U_DOM_ID(2, 13, 5)
> +#define M4U_PORT_L13_CAM_CAMSV4			MTK_M4U_DOM_ID(2, 13, 6)
> +#define M4U_PORT_L13_CAM_CAMSV5			MTK_M4U_DOM_ID(2, 13, 7)
> +#define M4U_PORT_L13_CAM_CAMSV6			MTK_M4U_DOM_ID(2, 13, 8)
> +#define M4U_PORT_L13_CAM_CCUI			MTK_M4U_DOM_ID(3, 13, 9)
> +#define M4U_PORT_L13_CAM_CCUO			MTK_M4U_DOM_ID(3, 13, 10)
> +#define M4U_PORT_L13_CAM_FAKE			MTK_M4U_DOM_ID(2, 13, 11)
> +
> +/* larb14 */
> +#define M4U_PORT_L14_CAM_RESERVE1		MTK_M4U_DOM_ID(2, 14, 0)
> +#define M4U_PORT_L14_CAM_RESERVE2		MTK_M4U_DOM_ID(2, 14, 1)
> +#define M4U_PORT_L14_CAM_RESERVE3		MTK_M4U_DOM_ID(2, 14, 2)
> +#define M4U_PORT_L14_CAM_CAMSV0			MTK_M4U_DOM_ID(2, 14, 3)
> +#define M4U_PORT_L14_CAM_CCUI			MTK_M4U_DOM_ID(4, 14, 4)
> +#define M4U_PORT_L14_CAM_CCUO			MTK_M4U_DOM_ID(4, 14, 5)
> +
> +/* larb15: null */
> +
> +/* larb16 */
> +#define M4U_PORT_L16_CAM_IMGO_R1_A		MTK_M4U_DOM_ID(2, 16, 0)
> +#define M4U_PORT_L16_CAM_RRZO_R1_A		MTK_M4U_DOM_ID(2, 16, 1)
> +#define M4U_PORT_L16_CAM_CQI_R1_A		MTK_M4U_DOM_ID(2, 16, 2)
> +#define M4U_PORT_L16_CAM_BPCI_R1_A		MTK_M4U_DOM_ID(2, 16, 3)
> +#define M4U_PORT_L16_CAM_YUVO_R1_A		MTK_M4U_DOM_ID(2, 16, 4)
> +#define M4U_PORT_L16_CAM_UFDI_R2_A		MTK_M4U_DOM_ID(2, 16, 5)
> +#define M4U_PORT_L16_CAM_RAWI_R2_A		MTK_M4U_DOM_ID(2, 16, 6)
> +#define M4U_PORT_L16_CAM_RAWI_R3_A		MTK_M4U_DOM_ID(2, 16, 7)
> +#define M4U_PORT_L16_CAM_AAO_R1_A		MTK_M4U_DOM_ID(2, 16, 8)
> +#define M4U_PORT_L16_CAM_AFO_R1_A		MTK_M4U_DOM_ID(2, 16, 9)
> +#define M4U_PORT_L16_CAM_FLKO_R1_A		MTK_M4U_DOM_ID(2, 16, 10)
> +#define M4U_PORT_L16_CAM_LCESO_R1_A		MTK_M4U_DOM_ID(2, 16, 11)
> +#define M4U_PORT_L16_CAM_CRZO_R1_A		MTK_M4U_DOM_ID(2, 16, 12)
> +#define M4U_PORT_L16_CAM_LTMSO_R1_A		MTK_M4U_DOM_ID(2, 16, 13)
> +#define M4U_PORT_L16_CAM_RSSO_R1_A		MTK_M4U_DOM_ID(2, 16, 14)
> +#define M4U_PORT_L16_CAM_AAHO_R1_A		MTK_M4U_DOM_ID(2, 16, 15)
> +#define M4U_PORT_L16_CAM_LSCI_R1_A		MTK_M4U_DOM_ID(2, 16, 16)
> +
> +/* larb17 */
> +#define M4U_PORT_L17_CAM_IMGO_R1_B		MTK_M4U_DOM_ID(2, 17, 0)
> +#define M4U_PORT_L17_CAM_RRZO_R1_B		MTK_M4U_DOM_ID(2, 17, 1)
> +#define M4U_PORT_L17_CAM_CQI_R1_B		MTK_M4U_DOM_ID(2, 17, 2)
> +#define M4U_PORT_L17_CAM_BPCI_R1_B		MTK_M4U_DOM_ID(2, 17, 3)
> +#define M4U_PORT_L17_CAM_YUVO_R1_B		MTK_M4U_DOM_ID(2, 17, 4)
> +#define M4U_PORT_L17_CAM_UFDI_R2_B		MTK_M4U_DOM_ID(2, 17, 5)
> +#define M4U_PORT_L17_CAM_RAWI_R2_B		MTK_M4U_DOM_ID(2, 17, 6)
> +#define M4U_PORT_L17_CAM_RAWI_R3_B		MTK_M4U_DOM_ID(2, 17, 7)
> +#define M4U_PORT_L17_CAM_AAO_R1_B		MTK_M4U_DOM_ID(2, 17, 8)
> +#define M4U_PORT_L17_CAM_AFO_R1_B		MTK_M4U_DOM_ID(2, 17, 9)
> +#define M4U_PORT_L17_CAM_FLKO_R1_B		MTK_M4U_DOM_ID(2, 17, 10)
> +#define M4U_PORT_L17_CAM_LCESO_R1_B		MTK_M4U_DOM_ID(2, 17, 11)
> +#define M4U_PORT_L17_CAM_CRZO_R1_B		MTK_M4U_DOM_ID(2, 17, 12)
> +#define M4U_PORT_L17_CAM_LTMSO_R1_B		MTK_M4U_DOM_ID(2, 17, 13)
> +#define M4U_PORT_L17_CAM_RSSO_R1_B		MTK_M4U_DOM_ID(2, 17, 14)
> +#define M4U_PORT_L17_CAM_AAHO_R1_B		MTK_M4U_DOM_ID(2, 17, 15)
> +#define M4U_PORT_L17_CAM_LSCI_R1_B		MTK_M4U_DOM_ID(2, 17, 16)
> +
> +/* larb18 */
> +#define M4U_PORT_L18_CAM_IMGO_R1_C		MTK_M4U_DOM_ID(2, 18, 0)
> +#define M4U_PORT_L18_CAM_RRZO_R1_C		MTK_M4U_DOM_ID(2, 18, 1)
> +#define M4U_PORT_L18_CAM_CQI_R1_C		MTK_M4U_DOM_ID(2, 18, 2)
> +#define M4U_PORT_L18_CAM_BPCI_R1_C		MTK_M4U_DOM_ID(2, 18, 3)
> +#define M4U_PORT_L18_CAM_YUVO_R1_C		MTK_M4U_DOM_ID(2, 18, 4)
> +#define M4U_PORT_L18_CAM_UFDI_R2_C		MTK_M4U_DOM_ID(2, 18, 5)
> +#define M4U_PORT_L18_CAM_RAWI_R2_C		MTK_M4U_DOM_ID(2, 18, 6)
> +#define M4U_PORT_L18_CAM_RAWI_R3_C		MTK_M4U_DOM_ID(2, 18, 7)
> +#define M4U_PORT_L18_CAM_AAO_R1_C		MTK_M4U_DOM_ID(2, 18, 8)
> +#define M4U_PORT_L18_CAM_AFO_R1_C		MTK_M4U_DOM_ID(2, 18, 9)
> +#define M4U_PORT_L18_CAM_FLKO_R1_C		MTK_M4U_DOM_ID(2, 18, 10)
> +#define M4U_PORT_L18_CAM_LCESO_R1_C		MTK_M4U_DOM_ID(2, 18, 11)
> +#define M4U_PORT_L18_CAM_CRZO_R1_C		MTK_M4U_DOM_ID(2, 18, 12)
> +#define M4U_PORT_L18_CAM_LTMSO_R1_C		MTK_M4U_DOM_ID(2, 18, 13)
> +#define M4U_PORT_L18_CAM_RSSO_R1_C		MTK_M4U_DOM_ID(2, 18, 14)
> +#define M4U_PORT_L18_CAM_AAHO_R1_C		MTK_M4U_DOM_ID(2, 18, 15)
> +#define M4U_PORT_L18_CAM_LSCI_R1_C		MTK_M4U_DOM_ID(2, 18, 16)
> +
> +/* larb19 */
> +#define M4U_PORT_L19_IPE_DVS_RDMA		MTK_M4U_DOM_ID(2, 19, 0)
> +#define M4U_PORT_L19_IPE_DVS_WDMA		MTK_M4U_DOM_ID(2, 19, 1)
> +#define M4U_PORT_L19_IPE_DVP_RDMA		MTK_M4U_DOM_ID(2, 19, 2)
> +#define M4U_PORT_L19_IPE_DVP_WDMA		MTK_M4U_DOM_ID(2, 19, 3)
> +
> +/* larb20 */
> +#define M4U_PORT_L20_IPE_FDVT_RDA		MTK_M4U_DOM_ID(2, 20, 0)
> +#define M4U_PORT_L20_IPE_FDVT_RDB		MTK_M4U_DOM_ID(2, 20, 1)
> +#define M4U_PORT_L20_IPE_FDVT_WRA		MTK_M4U_DOM_ID(2, 20, 2)
> +#define M4U_PORT_L20_IPE_FDVT_WRB		MTK_M4U_DOM_ID(2, 20, 3)
> +#define M4U_PORT_L20_IPE_RSC_RDMA0		MTK_M4U_DOM_ID(2, 20, 4)
> +#define M4U_PORT_L20_IPE_RSC_WDMA		MTK_M4U_DOM_ID(2, 20, 5)
> +
> +#endif
> -- 
> 2.18.0
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
Yong Wu (吴勇) Dec. 24, 2020, 11:35 a.m. UTC | #3 
On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > This patch adds decriptions for mt8192 IOMMU and SMI.
> > 
> > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > table format. The M4U-SMI HW diagram is as below:
> > 
> >                           EMI
> >                            |
> >                           M4U
> >                            |
> >                       ------------
> >                        SMI Common
> >                       ------------
> >                            |
> >   +-------+------+------+----------------------+-------+
> >   |       |      |      |       ......         |       |
> >   |       |      |      |                      |       |
> > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > disp0   disp1   mdp    vdec                   IPE      IPE
> > 
> > All the connections are HW fixed, SW can NOT adjust it.
> > 
> > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > into different iova ranges:
> > 
> > domain-id  module     iova-range                  larbs
> >    0       disp        0 ~ 4G                      larb0/1
> >    1       vcodec      4G ~ 8G                     larb4/5/7
> >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> 
> Why do we preassign these addresses in DT? Shouldn't it be a user's or
> integrator's decision to split the 16 GB address range into sub-ranges
> and define which larbs those sub-ranges are shared with?

The problem is that we can't split the 16GB range with the larb as unit.
The example is the below ccu0(larb13 port9/10) is a independent
range(domain), the others ports in larb13 is in another domain.

disp/vcodec/cam/mdp don't have special iova requirement, they could
access any range. vcodec also can locate 8G~12G. it don't care about
where its iova locate. here I preassign like this following with our
internal project setting.

Why set this in DT?, this is only for simplifying the code. Assume we
put it in the platform data. We have up to 32 larbs, each larb has up to
32 ports, each port may be in different iommu domains. we should have a
big array for this..however we only use a macro to get the domain in the
DT method.

When replying this mail, I happen to see there is a "dev->dev_range_map"
which has "dma-range" information, I think I could use this value to get
which domain the device belong to. then no need put domid in DT. I will
test this.

Thanks.
> 
> Best regards,
> Tomasz
> 
> >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > 
> > The iova range for CCU0/1(camera control unit) is HW requirement.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> >  2 files changed, 257 insertions(+), 1 deletion(-)
> >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > 
[snip]
Tomasz Figa Jan. 13, 2021, 5:30 a.m. UTC | #4 
On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
>
> On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > >
> > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > table format. The M4U-SMI HW diagram is as below:
> > >
> > >                           EMI
> > >                            |
> > >                           M4U
> > >                            |
> > >                       ------------
> > >                        SMI Common
> > >                       ------------
> > >                            |
> > >   +-------+------+------+----------------------+-------+
> > >   |       |      |      |       ......         |       |
> > >   |       |      |      |                      |       |
> > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > disp0   disp1   mdp    vdec                   IPE      IPE
> > >
> > > All the connections are HW fixed, SW can NOT adjust it.
> > >
> > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > into different iova ranges:
> > >
> > > domain-id  module     iova-range                  larbs
> > >    0       disp        0 ~ 4G                      larb0/1
> > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> >
> > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > integrator's decision to split the 16 GB address range into sub-ranges
> > and define which larbs those sub-ranges are shared with?
>
> The problem is that we can't split the 16GB range with the larb as unit.
> The example is the below ccu0(larb13 port9/10) is a independent
> range(domain), the others ports in larb13 is in another domain.
>
> disp/vcodec/cam/mdp don't have special iova requirement, they could
> access any range. vcodec also can locate 8G~12G. it don't care about
> where its iova locate. here I preassign like this following with our
> internal project setting.

Let me try to understand this a bit more. Given the split you're
proposing, is there actually any isolation enforced between particular
domains? For example, if I program vcodec to with a DMA address from
the 0-4G range, would the IOMMU actually generate a fault, even if
disp had some memory mapped at that address?

>
> Why set this in DT?, this is only for simplifying the code. Assume we
> put it in the platform data. We have up to 32 larbs, each larb has up to
> 32 ports, each port may be in different iommu domains. we should have a
> big array for this..however we only use a macro to get the domain in the
> DT method.
>
> When replying this mail, I happen to see there is a "dev->dev_range_map"
> which has "dma-range" information, I think I could use this value to get
> which domain the device belong to. then no need put domid in DT. I will
> test this.

My feeling is that the only part that needs to be enforced statically
is the reserved IOVA range for CCUs. The other ranges should be
determined dynamically, although I think I need to understand better
how the hardware and your proposed design work to tell what would be
likely the best choice here.

Best regards,
Tomasz

>
> Thanks.
> >
> > Best regards,
> > Tomasz
> >
> > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > >
> > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > >
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > >
> [snip]
Yong Wu (吴勇) Jan. 13, 2021, 6:45 a.m. UTC | #5 
On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > > >
> > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > > table format. The M4U-SMI HW diagram is as below:
> > > >
> > > >                           EMI
> > > >                            |
> > > >                           M4U
> > > >                            |
> > > >                       ------------
> > > >                        SMI Common
> > > >                       ------------
> > > >                            |
> > > >   +-------+------+------+----------------------+-------+
> > > >   |       |      |      |       ......         |       |
> > > >   |       |      |      |                      |       |
> > > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > > disp0   disp1   mdp    vdec                   IPE      IPE
> > > >
> > > > All the connections are HW fixed, SW can NOT adjust it.
> > > >
> > > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > > into different iova ranges:
> > > >
> > > > domain-id  module     iova-range                  larbs
> > > >    0       disp        0 ~ 4G                      larb0/1
> > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > >
> > > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > > integrator's decision to split the 16 GB address range into sub-ranges
> > > and define which larbs those sub-ranges are shared with?
> >
> > The problem is that we can't split the 16GB range with the larb as unit.
> > The example is the below ccu0(larb13 port9/10) is a independent
> > range(domain), the others ports in larb13 is in another domain.
> >
> > disp/vcodec/cam/mdp don't have special iova requirement, they could
> > access any range. vcodec also can locate 8G~12G. it don't care about
> > where its iova locate. here I preassign like this following with our
> > internal project setting.
> 
> Let me try to understand this a bit more. Given the split you're
> proposing, is there actually any isolation enforced between particular
> domains? For example, if I program vcodec to with a DMA address from
> the 0-4G range, would the IOMMU actually generate a fault, even if
> disp had some memory mapped at that address?

In this case. we will get fault in current SW setting.

> 
> >
> > Why set this in DT?, this is only for simplifying the code. Assume we
> > put it in the platform data. We have up to 32 larbs, each larb has up to
> > 32 ports, each port may be in different iommu domains. we should have a
> > big array for this..however we only use a macro to get the domain in the
> > DT method.
> >
> > When replying this mail, I happen to see there is a "dev->dev_range_map"
> > which has "dma-range" information, I think I could use this value to get
> > which domain the device belong to. then no need put domid in DT. I will
> > test this.
> 
> My feeling is that the only part that needs to be enforced statically
> is the reserved IOVA range for CCUs. The other ranges should be
> determined dynamically, although I think I need to understand better
> how the hardware and your proposed design work to tell what would be
> likely the best choice here.

I have removed the domid patch in v6. and get the domain id in [27/33]
in v6..

About the other ranges should be dynamical, the commit message [30/33]
of v6 should be helpful. the problem is that we have a bank_sel setting
for the iova[32:33]. currently we preassign this value. thus, all the
ranges are fixed. If you adjust this setting, you can let vcodec access
0~4G.

Currently we have no interface to adjust this setting. Suppose we add a
new interface for this. It would be something like:

   int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
  
   Then, all the MM drivers should call it before the HW works every
time, and its implement will be a bit complex since we aren't sure if
the larb has power at that time. the important thing is that the MM
devices have already not known which larb it connects with as we plan to
delete "mediatek,larb" in their dtsi nodes.

   In current design, the MM device don't need care about it and 4GB
range is enough for them.

> 
> Best regards,
> Tomasz
> 
> >
> > Thanks.
> > >
> > > Best regards,
> > > Tomasz
> > >
> > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > >
> > > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > > >
> > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > > >
> > [snip]
Tomasz Figa Jan. 20, 2021, 4:15 a.m. UTC | #6 
On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
>
> On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> > On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > >
> > > On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > > > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > > > >
> > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > > > table format. The M4U-SMI HW diagram is as below:
> > > > >
> > > > >                           EMI
> > > > >                            |
> > > > >                           M4U
> > > > >                            |
> > > > >                       ------------
> > > > >                        SMI Common
> > > > >                       ------------
> > > > >                            |
> > > > >   +-------+------+------+----------------------+-------+
> > > > >   |       |      |      |       ......         |       |
> > > > >   |       |      |      |                      |       |
> > > > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > > > disp0   disp1   mdp    vdec                   IPE      IPE
> > > > >
> > > > > All the connections are HW fixed, SW can NOT adjust it.
> > > > >
> > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > > > into different iova ranges:
> > > > >
> > > > > domain-id  module     iova-range                  larbs
> > > > >    0       disp        0 ~ 4G                      larb0/1
> > > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > > >
> > > > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > > > integrator's decision to split the 16 GB address range into sub-ranges
> > > > and define which larbs those sub-ranges are shared with?
> > >
> > > The problem is that we can't split the 16GB range with the larb as unit.
> > > The example is the below ccu0(larb13 port9/10) is a independent
> > > range(domain), the others ports in larb13 is in another domain.
> > >
> > > disp/vcodec/cam/mdp don't have special iova requirement, they could
> > > access any range. vcodec also can locate 8G~12G. it don't care about
> > > where its iova locate. here I preassign like this following with our
> > > internal project setting.
> >
> > Let me try to understand this a bit more. Given the split you're
> > proposing, is there actually any isolation enforced between particular
> > domains? For example, if I program vcodec to with a DMA address from
> > the 0-4G range, would the IOMMU actually generate a fault, even if
> > disp had some memory mapped at that address?
>
> In this case. we will get fault in current SW setting.
>

Okay, thanks.

> >
> > >
> > > Why set this in DT?, this is only for simplifying the code. Assume we
> > > put it in the platform data. We have up to 32 larbs, each larb has up to
> > > 32 ports, each port may be in different iommu domains. we should have a
> > > big array for this..however we only use a macro to get the domain in the
> > > DT method.
> > >
> > > When replying this mail, I happen to see there is a "dev->dev_range_map"
> > > which has "dma-range" information, I think I could use this value to get
> > > which domain the device belong to. then no need put domid in DT. I will
> > > test this.
> >
> > My feeling is that the only part that needs to be enforced statically
> > is the reserved IOVA range for CCUs. The other ranges should be
> > determined dynamically, although I think I need to understand better
> > how the hardware and your proposed design work to tell what would be
> > likely the best choice here.
>
> I have removed the domid patch in v6. and get the domain id in [27/33]
> in v6..
>
> About the other ranges should be dynamical, the commit message [30/33]
> of v6 should be helpful. the problem is that we have a bank_sel setting
> for the iova[32:33]. currently we preassign this value. thus, all the
> ranges are fixed. If you adjust this setting, you can let vcodec access
> 0~4G.

Okay, so it sounds like we effectively have four 4G address spaces and
we can assign the master devices to them. I guess each of these
address spaces makes for an IOMMU group.

It's fine to pre-assign the devices to those groups for now, but it
definitely shouldn't be hardcoded in DT, because it depends on the use
case of the device. I'll take a look at v6, but it sounds like it
should be fine if it doesn't take the address space assignment from DT
anymore.

>
> Currently we have no interface to adjust this setting. Suppose we add a
> new interface for this. It would be something like:
>
>    int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
>
>    Then, all the MM drivers should call it before the HW works every
> time, and its implement will be a bit complex since we aren't sure if
> the larb has power at that time. the important thing is that the MM
> devices have already not known which larb it connects with as we plan to
> delete "mediatek,larb" in their dtsi nodes.

From the practical point of view, it doesn't look like setting this on
a per-larb basis would make much sense. The reason to switch the
bank_sel would be to decide which MM devices can share the same
address space. This is a security aspect, because it effectively
determines which devices are isolated from each other.

That said, I agree that for now we can just start with a fixed
assignment. We can think of the API if there is a need to adjust the
assignment.

>
>    In current design, the MM device don't need care about it and 4GB
> range is enough for them.
>

Actually, is the current assignment correct?

domain-id  module     iova-range                  larbs
   0       disp        0 ~ 4G                      larb0/1
   1       vcodec      4G ~ 8G                     larb4/5/7
   2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
   3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
   4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5

Wouldn't CCU0 and CCU1 conflict with disp? Should perhaps disp be
assigned 12G ~ 16G instead?

Best regards,
Tomasz

> >
> > Best regards,
> > Tomasz
> >
> > >
> > > Thanks.
> > > >
> > > > Best regards,
> > > > Tomasz
> > > >
> > > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > > >
> > > > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > > > >
> > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > > > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > > > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > > > >
> > > [snip]
>
Yong Wu (吴勇) Jan. 20, 2021, 7:07 a.m. UTC | #7 
On Wed, 2021-01-20 at 13:15 +0900, Tomasz Figa wrote:
> On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> > > On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > >
> > > > On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > > > > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > > > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > > > > >
> > > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > > > > table format. The M4U-SMI HW diagram is as below:
> > > > > >
> > > > > >                           EMI
> > > > > >                            |
> > > > > >                           M4U
> > > > > >                            |
> > > > > >                       ------------
> > > > > >                        SMI Common
> > > > > >                       ------------
> > > > > >                            |
> > > > > >   +-------+------+------+----------------------+-------+
> > > > > >   |       |      |      |       ......         |       |
> > > > > >   |       |      |      |                      |       |
> > > > > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > > > > disp0   disp1   mdp    vdec                   IPE      IPE
> > > > > >
> > > > > > All the connections are HW fixed, SW can NOT adjust it.
> > > > > >
> > > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > > > > into different iova ranges:
> > > > > >
> > > > > > domain-id  module     iova-range                  larbs
> > > > > >    0       disp        0 ~ 4G                      larb0/1
> > > > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > > > >
> > > > > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > > > > integrator's decision to split the 16 GB address range into sub-ranges
> > > > > and define which larbs those sub-ranges are shared with?
> > > >
> > > > The problem is that we can't split the 16GB range with the larb as unit.
> > > > The example is the below ccu0(larb13 port9/10) is a independent
> > > > range(domain), the others ports in larb13 is in another domain.
> > > >
> > > > disp/vcodec/cam/mdp don't have special iova requirement, they could
> > > > access any range. vcodec also can locate 8G~12G. it don't care about
> > > > where its iova locate. here I preassign like this following with our
> > > > internal project setting.
> > >
> > > Let me try to understand this a bit more. Given the split you're
> > > proposing, is there actually any isolation enforced between particular
> > > domains? For example, if I program vcodec to with a DMA address from
> > > the 0-4G range, would the IOMMU actually generate a fault, even if
> > > disp had some memory mapped at that address?
> >
> > In this case. we will get fault in current SW setting.
> >
> 
> Okay, thanks.
> 
> > >
> > > >
> > > > Why set this in DT?, this is only for simplifying the code. Assume we
> > > > put it in the platform data. We have up to 32 larbs, each larb has up to
> > > > 32 ports, each port may be in different iommu domains. we should have a
> > > > big array for this..however we only use a macro to get the domain in the
> > > > DT method.
> > > >
> > > > When replying this mail, I happen to see there is a "dev->dev_range_map"
> > > > which has "dma-range" information, I think I could use this value to get
> > > > which domain the device belong to. then no need put domid in DT. I will
> > > > test this.
> > >
> > > My feeling is that the only part that needs to be enforced statically
> > > is the reserved IOVA range for CCUs. The other ranges should be
> > > determined dynamically, although I think I need to understand better
> > > how the hardware and your proposed design work to tell what would be
> > > likely the best choice here.
> >
> > I have removed the domid patch in v6. and get the domain id in [27/33]
> > in v6..
> >
> > About the other ranges should be dynamical, the commit message [30/33]
> > of v6 should be helpful. the problem is that we have a bank_sel setting
> > for the iova[32:33]. currently we preassign this value. thus, all the
> > ranges are fixed. If you adjust this setting, you can let vcodec access
> > 0~4G.
> 
> Okay, so it sounds like we effectively have four 4G address spaces and
> we can assign the master devices to them. I guess each of these
> address spaces makes for an IOMMU group.

Yes. Each a address spaces is an IOMMU group.

> 
> It's fine to pre-assign the devices to those groups for now, but it
> definitely shouldn't be hardcoded in DT, because it depends on the use
> case of the device. I'll take a look at v6, but it sounds like it
> should be fine if it doesn't take the address space assignment from DT
> anymore.

Thanks very much for your review.

> 
> >
> > Currently we have no interface to adjust this setting. Suppose we add a
> > new interface for this. It would be something like:
> >
> >    int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
> >
> >    Then, all the MM drivers should call it before the HW works every
> > time, and its implement will be a bit complex since we aren't sure if
> > the larb has power at that time. the important thing is that the MM
> > devices have already not known which larb it connects with as we plan to
> > delete "mediatek,larb" in their dtsi nodes.
> 
> From the practical point of view, it doesn't look like setting this on
> a per-larb basis would make much sense. The reason to switch the
> bank_sel would be to decide which MM devices can share the same
> address space. This is a security aspect, because it effectively
> determines which devices are isolated from each other.
> 
> That said, I agree that for now we can just start with a fixed
> assignment. We can think of the API if there is a need to adjust the
> assignment.

Sorry for here. I forgot a thing here. that interface above still will
not be helpful. If we don't divide the whole 16GB ranges into 4
regions(let all the other ranges be dynamical), It won't work since we
can only adjust bank_sel with the larb as unit. This is a problem. there
are many ports in a larb. Take a example, the address for vcodec read
port is 32bits while the address for vcodec write port is 33bit, then it
will fail since we only have one bank_sel setting for one larb. Thus we
have to use current design.

> 
> >
> >    In current design, the MM device don't need care about it and 4GB
> > range is enough for them.
> >
> 
> Actually, is the current assignment correct?

Oh. In the code (patch [32/33] of v6), I put CCU0/1 in the cam/mdp
region which start at 8G since CCU0/1 is a module of camera.

> 
> domain-id  module     iova-range                  larbs
>    0       disp        0 ~ 4G                      larb0/1
>    1       vcodec      4G ~ 8G                     larb4/5/7
>    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
>    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
>    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> 
> Wouldn't CCU0 and CCU1 conflict with disp? 

About the conflict, I use patch [29/33] of v6 for this. I will reserve
this special iova region when the full domain(0-4G in this example)
initialize.

> Should perhaps disp be assigned 12G ~ 16G instead?

I think no need put it to 12G-16G, In previous SoC, we have only 4GB
ranges for whole MM engines. currently only cam/mdp domain exclude 128M
for CCU. it should be something wrong if this is not enough.

> 
> Best regards,
> Tomasz
> 
> > >
> > > Best regards,
> > > Tomasz
> > >
> > > >
> > > > Thanks.
> > > > >
> > > > > Best regards,
> > > > > Tomasz
> > > > >
> > > > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > > > >
> > > > > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > > > > >
> > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > ---
> > > > > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > > > > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > > > > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > > > > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > > > > >
> > > > [snip]
> >
Tomasz Figa Jan. 25, 2021, 4:18 a.m. UTC | #8 
On Wed, Jan 20, 2021 at 4:08 PM Yong Wu <yong.wu@mediatek.com> wrote:
>
> On Wed, 2021-01-20 at 13:15 +0900, Tomasz Figa wrote:
> > On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > >
> > > On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> > > > On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > > >
> > > > > On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > > > > > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > > > > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > > > > > >
> > > > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > > > > > table format. The M4U-SMI HW diagram is as below:
> > > > > > >
> > > > > > >                           EMI
> > > > > > >                            |
> > > > > > >                           M4U
> > > > > > >                            |
> > > > > > >                       ------------
> > > > > > >                        SMI Common
> > > > > > >                       ------------
> > > > > > >                            |
> > > > > > >   +-------+------+------+----------------------+-------+
> > > > > > >   |       |      |      |       ......         |       |
> > > > > > >   |       |      |      |                      |       |
> > > > > > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > > > > > disp0   disp1   mdp    vdec                   IPE      IPE
> > > > > > >
> > > > > > > All the connections are HW fixed, SW can NOT adjust it.
> > > > > > >
> > > > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > > > > > into different iova ranges:
> > > > > > >
> > > > > > > domain-id  module     iova-range                  larbs
> > > > > > >    0       disp        0 ~ 4G                      larb0/1
> > > > > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > > > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > > > > >
> > > > > > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > > > > > integrator's decision to split the 16 GB address range into sub-ranges
> > > > > > and define which larbs those sub-ranges are shared with?
> > > > >
> > > > > The problem is that we can't split the 16GB range with the larb as unit.
> > > > > The example is the below ccu0(larb13 port9/10) is a independent
> > > > > range(domain), the others ports in larb13 is in another domain.
> > > > >
> > > > > disp/vcodec/cam/mdp don't have special iova requirement, they could
> > > > > access any range. vcodec also can locate 8G~12G. it don't care about
> > > > > where its iova locate. here I preassign like this following with our
> > > > > internal project setting.
> > > >
> > > > Let me try to understand this a bit more. Given the split you're
> > > > proposing, is there actually any isolation enforced between particular
> > > > domains? For example, if I program vcodec to with a DMA address from
> > > > the 0-4G range, would the IOMMU actually generate a fault, even if
> > > > disp had some memory mapped at that address?
> > >
> > > In this case. we will get fault in current SW setting.
> > >
> >
> > Okay, thanks.
> >
> > > >
> > > > >
> > > > > Why set this in DT?, this is only for simplifying the code. Assume we
> > > > > put it in the platform data. We have up to 32 larbs, each larb has up to
> > > > > 32 ports, each port may be in different iommu domains. we should have a
> > > > > big array for this..however we only use a macro to get the domain in the
> > > > > DT method.
> > > > >
> > > > > When replying this mail, I happen to see there is a "dev->dev_range_map"
> > > > > which has "dma-range" information, I think I could use this value to get
> > > > > which domain the device belong to. then no need put domid in DT. I will
> > > > > test this.
> > > >
> > > > My feeling is that the only part that needs to be enforced statically
> > > > is the reserved IOVA range for CCUs. The other ranges should be
> > > > determined dynamically, although I think I need to understand better
> > > > how the hardware and your proposed design work to tell what would be
> > > > likely the best choice here.
> > >
> > > I have removed the domid patch in v6. and get the domain id in [27/33]
> > > in v6..
> > >
> > > About the other ranges should be dynamical, the commit message [30/33]
> > > of v6 should be helpful. the problem is that we have a bank_sel setting
> > > for the iova[32:33]. currently we preassign this value. thus, all the
> > > ranges are fixed. If you adjust this setting, you can let vcodec access
> > > 0~4G.
> >
> > Okay, so it sounds like we effectively have four 4G address spaces and
> > we can assign the master devices to them. I guess each of these
> > address spaces makes for an IOMMU group.
>
> Yes. Each a address spaces is an IOMMU group.
>
> >
> > It's fine to pre-assign the devices to those groups for now, but it
> > definitely shouldn't be hardcoded in DT, because it depends on the use
> > case of the device. I'll take a look at v6, but it sounds like it
> > should be fine if it doesn't take the address space assignment from DT
> > anymore.
>
> Thanks very much for your review.
>

Hmm, I had a look at v6 and it still has the address spaces hardcoded
in the DTS. Could we move the fixed assignment to the MTK IOMMU driver
code instead, so that it can be easily adjusted as the kernel code
evolves without the need to update the DTS?

> >
> > >
> > > Currently we have no interface to adjust this setting. Suppose we add a
> > > new interface for this. It would be something like:
> > >
> > >    int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
> > >
> > >    Then, all the MM drivers should call it before the HW works every
> > > time, and its implement will be a bit complex since we aren't sure if
> > > the larb has power at that time. the important thing is that the MM
> > > devices have already not known which larb it connects with as we plan to
> > > delete "mediatek,larb" in their dtsi nodes.
> >
> > From the practical point of view, it doesn't look like setting this on
> > a per-larb basis would make much sense. The reason to switch the
> > bank_sel would be to decide which MM devices can share the same
> > address space. This is a security aspect, because it effectively
> > determines which devices are isolated from each other.
> >
> > That said, I agree that for now we can just start with a fixed
> > assignment. We can think of the API if there is a need to adjust the
> > assignment.
>
> Sorry for here. I forgot a thing here. that interface above still will
> not be helpful. If we don't divide the whole 16GB ranges into 4
> regions(let all the other ranges be dynamical), It won't work since we
> can only adjust bank_sel with the larb as unit. This is a problem. there
> are many ports in a larb. Take a example, the address for vcodec read
> port is 32bits while the address for vcodec write port is 33bit, then it
> will fail since we only have one bank_sel setting for one larb.

That's exactly why I proposed to have the API operate based on the
struct device, rather than individual DMA ports. Although I guess the
CCU case is different, because it's the same larb as the camera.

Anyway, I agree that we don't have to come up with such an API right now.

> Thus we
> have to use current design.
>
> >
> > >
> > >    In current design, the MM device don't need care about it and 4GB
> > > range is enough for them.
> > >
> >
> > Actually, is the current assignment correct?
>
> Oh. In the code (patch [32/33] of v6), I put CCU0/1 in the cam/mdp
> region which start at 8G since CCU0/1 is a module of camera.
>
> >
> > domain-id  module     iova-range                  larbs
> >    0       disp        0 ~ 4G                      larb0/1
> >    1       vcodec      4G ~ 8G                     larb4/5/7
> >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> >
> > Wouldn't CCU0 and CCU1 conflict with disp?
>
> About the conflict, I use patch [29/33] of v6 for this. I will reserve
> this special iova region when the full domain(0-4G in this example)
> initialize.
>
> > Should perhaps disp be assigned 12G ~ 16G instead?
>
> I think no need put it to 12G-16G, In previous SoC, we have only 4GB
> ranges for whole MM engines. currently only cam/mdp domain exclude 128M
> for CCU. it should be something wrong if this is not enough.
>

Indeed, space is not a problem, but from the security point of view
it's undesirable. I believe CCU would be running proprietary firmware,
so it should be isolated as much as possible from other components.
And, after all, why waste the remaining 4G of address space?

Best regards,
Tomasz

> >
> > Best regards,
> > Tomasz
> >
> > > >
> > > > Best regards,
> > > > Tomasz
> > > >
> > > > >
> > > > > Thanks.
> > > > > >
> > > > > > Best regards,
> > > > > > Tomasz
> > > > > >
> > > > > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > > > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > > > > >
> > > > > > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > > > > > >
> > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > > ---
> > > > > > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > > > > > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > > > > > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > > > > > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > > > > > >
> > > > > [snip]
> > >
>
Yong Wu (吴勇) Jan. 25, 2021, 7:33 a.m. UTC | #9 
On Mon, 2021-01-25 at 13:18 +0900, Tomasz Figa wrote:
> On Wed, Jan 20, 2021 at 4:08 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > On Wed, 2021-01-20 at 13:15 +0900, Tomasz Figa wrote:
> > > On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > >
> > > > On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> > > > > On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > > > >
> > > > > > On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > > > > > > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > > > > > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > > > > > > >
> > > > > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > > > > > > table format. The M4U-SMI HW diagram is as below:
> > > > > > > >
> > > > > > > >                           EMI
> > > > > > > >                            |
> > > > > > > >                           M4U
> > > > > > > >                            |
> > > > > > > >                       ------------
> > > > > > > >                        SMI Common
> > > > > > > >                       ------------
> > > > > > > >                            |
> > > > > > > >   +-------+------+------+----------------------+-------+
> > > > > > > >   |       |      |      |       ......         |       |
> > > > > > > >   |       |      |      |                      |       |
> > > > > > > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > > > > > > disp0   disp1   mdp    vdec                   IPE      IPE
> > > > > > > >
> > > > > > > > All the connections are HW fixed, SW can NOT adjust it.
> > > > > > > >
> > > > > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > > > > > > into different iova ranges:
> > > > > > > >
> > > > > > > > domain-id  module     iova-range                  larbs
> > > > > > > >    0       disp        0 ~ 4G                      larb0/1
> > > > > > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > > > > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > > > > > >
> > > > > > > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > > > > > > integrator's decision to split the 16 GB address range into sub-ranges
> > > > > > > and define which larbs those sub-ranges are shared with?
> > > > > >
> > > > > > The problem is that we can't split the 16GB range with the larb as unit.
> > > > > > The example is the below ccu0(larb13 port9/10) is a independent
> > > > > > range(domain), the others ports in larb13 is in another domain.
> > > > > >
> > > > > > disp/vcodec/cam/mdp don't have special iova requirement, they could
> > > > > > access any range. vcodec also can locate 8G~12G. it don't care about
> > > > > > where its iova locate. here I preassign like this following with our
> > > > > > internal project setting.
> > > > >
> > > > > Let me try to understand this a bit more. Given the split you're
> > > > > proposing, is there actually any isolation enforced between particular
> > > > > domains? For example, if I program vcodec to with a DMA address from
> > > > > the 0-4G range, would the IOMMU actually generate a fault, even if
> > > > > disp had some memory mapped at that address?
> > > >
> > > > In this case. we will get fault in current SW setting.
> > > >
> > >
> > > Okay, thanks.
> > >
> > > > >
> > > > > >
> > > > > > Why set this in DT?, this is only for simplifying the code. Assume we
> > > > > > put it in the platform data. We have up to 32 larbs, each larb has up to
> > > > > > 32 ports, each port may be in different iommu domains. we should have a
> > > > > > big array for this..however we only use a macro to get the domain in the
> > > > > > DT method.
> > > > > >
> > > > > > When replying this mail, I happen to see there is a "dev->dev_range_map"
> > > > > > which has "dma-range" information, I think I could use this value to get
> > > > > > which domain the device belong to. then no need put domid in DT. I will
> > > > > > test this.
> > > > >
> > > > > My feeling is that the only part that needs to be enforced statically
> > > > > is the reserved IOVA range for CCUs. The other ranges should be
> > > > > determined dynamically, although I think I need to understand better
> > > > > how the hardware and your proposed design work to tell what would be
> > > > > likely the best choice here.
> > > >
> > > > I have removed the domid patch in v6. and get the domain id in [27/33]
> > > > in v6..
> > > >
> > > > About the other ranges should be dynamical, the commit message [30/33]
> > > > of v6 should be helpful. the problem is that we have a bank_sel setting
> > > > for the iova[32:33]. currently we preassign this value. thus, all the
> > > > ranges are fixed. If you adjust this setting, you can let vcodec access
> > > > 0~4G.
> > >
> > > Okay, so it sounds like we effectively have four 4G address spaces and
> > > we can assign the master devices to them. I guess each of these
> > > address spaces makes for an IOMMU group.
> >
> > Yes. Each a address spaces is an IOMMU group.
> >
> > >
> > > It's fine to pre-assign the devices to those groups for now, but it
> > > definitely shouldn't be hardcoded in DT, because it depends on the use
> > > case of the device. I'll take a look at v6, but it sounds like it
> > > should be fine if it doesn't take the address space assignment from DT
> > > anymore.
> >
> > Thanks very much for your review.
> >
> 
> Hmm, I had a look at v6 and it still has the address spaces hardcoded
> in the DTS. 

sorry. I didn't get here. where do you mean. or help reply in v6.

I only added the preassign list as comment in the file
(dt-binding/memory/mt8192-larb-port.h). I thought our iommu consumer may
need it when they use these ports. they need add dma-ranges property if
its iova is over 4GB.

> Could we move the fixed assignment to the MTK IOMMU driver code instead,
> so that it can be easily adjusted as the kernel code
> evolves without the need to update the DTS?
> 
> > >
> > > >
> > > > Currently we have no interface to adjust this setting. Suppose we add a
> > > > new interface for this. It would be something like:
> > > >
> > > >    int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
> > > >
> > > >    Then, all the MM drivers should call it before the HW works every
> > > > time, and its implement will be a bit complex since we aren't sure if
> > > > the larb has power at that time. the important thing is that the MM
> > > > devices have already not known which larb it connects with as we plan to
> > > > delete "mediatek,larb" in their dtsi nodes.
> > >
> > > From the practical point of view, it doesn't look like setting this on
> > > a per-larb basis would make much sense. The reason to switch the
> > > bank_sel would be to decide which MM devices can share the same
> > > address space. This is a security aspect, because it effectively
> > > determines which devices are isolated from each other.
> > >
> > > That said, I agree that for now we can just start with a fixed
> > > assignment. We can think of the API if there is a need to adjust the
> > > assignment.
> >
> > Sorry for here. I forgot a thing here. that interface above still will
> > not be helpful. If we don't divide the whole 16GB ranges into 4
> > regions(let all the other ranges be dynamical), It won't work since we
> > can only adjust bank_sel with the larb as unit. This is a problem. there
> > are many ports in a larb. Take a example, the address for vcodec read
> > port is 32bits while the address for vcodec write port is 33bit, then it
> > will fail since we only have one bank_sel setting for one larb.
> 
> That's exactly why I proposed to have the API operate based on the
> struct device, rather than individual DMA ports. Although I guess the
> CCU case is different, because it's the same larb as the camera.
> 
> Anyway, I agree that we don't have to come up with such an API right now.

Thanks for the confirm.

> 
> > Thus we
> > have to use current design.
> >
> > >
> > > >
> > > >    In current design, the MM device don't need care about it and 4GB
> > > > range is enough for them.
> > > >
> > >
> > > Actually, is the current assignment correct?
> >
> > Oh. In the code (patch [32/33] of v6), I put CCU0/1 in the cam/mdp
> > region which start at 8G since CCU0/1 is a module of camera.
> >
> > >
> > > domain-id  module     iova-range                  larbs
> > >    0       disp        0 ~ 4G                      larb0/1
> > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > >
> > > Wouldn't CCU0 and CCU1 conflict with disp?
> >
> > About the conflict, I use patch [29/33] of v6 for this. I will reserve
> > this special iova region when the full domain(0-4G in this example)
> > initialize.
> >
> > > Should perhaps disp be assigned 12G ~ 16G instead?
> >
> > I think no need put it to 12G-16G, In previous SoC, we have only 4GB
> > ranges for whole MM engines. currently only cam/mdp domain exclude 128M
> > for CCU. it should be something wrong if this is not enough.
> >
> 
> Indeed, space is not a problem, but from the security point of view
> it's undesirable. I believe CCU would be running proprietary firmware,
> so it should be isolated as much as possible from other components.

CCU are in the same larb with camera. Thus, it also need locate the same
iova range with camera.

> And, after all, why waste the remaining 4G of address space?
> 
> Best regards,
> Tomasz
> 
> > >
> > > Best regards,
> > > Tomasz
> > >
> > > > >
> > > > > Best regards,
> > > > > Tomasz
> > > > >
> > > > > >
> > > > > > Thanks.
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Tomasz
> > > > > > >
> > > > > > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > > > > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > > > > > >
> > > > > > > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > > > > > > >
> > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > > > ---
> > > > > > > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > > > > > > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > > > > > > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > > > > > > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > > > > > > >
> > > > > > [snip]
> > > >
> >
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Yong Wu (吴勇) Feb. 1, 2021, 5:36 a.m. UTC | #10 
On Fri, 2021-01-29 at 20:45 +0900, Tomasz Figa wrote:
> On Mon, Jan 25, 2021 at 4:34 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > On Mon, 2021-01-25 at 13:18 +0900, Tomasz Figa wrote:
> > > On Wed, Jan 20, 2021 at 4:08 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > >
> > > > On Wed, 2021-01-20 at 13:15 +0900, Tomasz Figa wrote:
> > > > > On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > > > >
> > > > > > On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> > > > > > > On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> > > > > > > > > On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> > > > > > > > > > This patch adds decriptions for mt8192 IOMMU and SMI.
> > > > > > > > > >
> > > > > > > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> > > > > > > > > > table format. The M4U-SMI HW diagram is as below:
> > > > > > > > > >
> > > > > > > > > >                           EMI
> > > > > > > > > >                            |
> > > > > > > > > >                           M4U
> > > > > > > > > >                            |
> > > > > > > > > >                       ------------
> > > > > > > > > >                        SMI Common
> > > > > > > > > >                       ------------
> > > > > > > > > >                            |
> > > > > > > > > >   +-------+------+------+----------------------+-------+
> > > > > > > > > >   |       |      |      |       ......         |       |
> > > > > > > > > >   |       |      |      |                      |       |
> > > > > > > > > > larb0   larb1  larb2  larb4     ......      larb19   larb20
> > > > > > > > > > disp0   disp1   mdp    vdec                   IPE      IPE
> > > > > > > > > >
> > > > > > > > > > All the connections are HW fixed, SW can NOT adjust it.
> > > > > > > > > >
> > > > > > > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines
> > > > > > > > > > into different iova ranges:
> > > > > > > > > >
> > > > > > > > > > domain-id  module     iova-range                  larbs
> > > > > > > > > >    0       disp        0 ~ 4G                      larb0/1
> > > > > > > > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > > > > > > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > > > > > > > >
> > > > > > > > > Why do we preassign these addresses in DT? Shouldn't it be a user's or
> > > > > > > > > integrator's decision to split the 16 GB address range into sub-ranges
> > > > > > > > > and define which larbs those sub-ranges are shared with?
> > > > > > > >
> > > > > > > > The problem is that we can't split the 16GB range with the larb as unit.
> > > > > > > > The example is the below ccu0(larb13 port9/10) is a independent
> > > > > > > > range(domain), the others ports in larb13 is in another domain.
> > > > > > > >
> > > > > > > > disp/vcodec/cam/mdp don't have special iova requirement, they could
> > > > > > > > access any range. vcodec also can locate 8G~12G. it don't care about
> > > > > > > > where its iova locate. here I preassign like this following with our
> > > > > > > > internal project setting.
> > > > > > >
> > > > > > > Let me try to understand this a bit more. Given the split you're
> > > > > > > proposing, is there actually any isolation enforced between particular
> > > > > > > domains? For example, if I program vcodec to with a DMA address from
> > > > > > > the 0-4G range, would the IOMMU actually generate a fault, even if
> > > > > > > disp had some memory mapped at that address?
> > > > > >
> > > > > > In this case. we will get fault in current SW setting.
> > > > > >
> > > > >
> > > > > Okay, thanks.
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Why set this in DT?, this is only for simplifying the code. Assume we
> > > > > > > > put it in the platform data. We have up to 32 larbs, each larb has up to
> > > > > > > > 32 ports, each port may be in different iommu domains. we should have a
> > > > > > > > big array for this..however we only use a macro to get the domain in the
> > > > > > > > DT method.
> > > > > > > >
> > > > > > > > When replying this mail, I happen to see there is a "dev->dev_range_map"
> > > > > > > > which has "dma-range" information, I think I could use this value to get
> > > > > > > > which domain the device belong to. then no need put domid in DT. I will
> > > > > > > > test this.
> > > > > > >
> > > > > > > My feeling is that the only part that needs to be enforced statically
> > > > > > > is the reserved IOVA range for CCUs. The other ranges should be
> > > > > > > determined dynamically, although I think I need to understand better
> > > > > > > how the hardware and your proposed design work to tell what would be
> > > > > > > likely the best choice here.
> > > > > >
> > > > > > I have removed the domid patch in v6. and get the domain id in [27/33]
> > > > > > in v6..
> > > > > >
> > > > > > About the other ranges should be dynamical, the commit message [30/33]
> > > > > > of v6 should be helpful. the problem is that we have a bank_sel setting
> > > > > > for the iova[32:33]. currently we preassign this value. thus, all the
> > > > > > ranges are fixed. If you adjust this setting, you can let vcodec access
> > > > > > 0~4G.
> > > > >
> > > > > Okay, so it sounds like we effectively have four 4G address spaces and
> > > > > we can assign the master devices to them. I guess each of these
> > > > > address spaces makes for an IOMMU group.
> > > >
> > > > Yes. Each a address spaces is an IOMMU group.
> > > >
> > > > >
> > > > > It's fine to pre-assign the devices to those groups for now, but it
> > > > > definitely shouldn't be hardcoded in DT, because it depends on the use
> > > > > case of the device. I'll take a look at v6, but it sounds like it
> > > > > should be fine if it doesn't take the address space assignment from DT
> > > > > anymore.
> > > >
> > > > Thanks very much for your review.
> > > >
> > >
> > > Hmm, I had a look at v6 and it still has the address spaces hardcoded
> > > in the DTS.
> >
> > sorry. I didn't get here. where do you mean. or help reply in v6.
> >
> > I only added the preassign list as comment in the file
> > (dt-binding/memory/mt8192-larb-port.h). I thought our iommu consumer may
> > need it when they use these ports. they need add dma-ranges property if
> > its iova is over 4GB.
> 
> That's exactly the problem. v6 simply replaced one way to describe the
> policy (domain ID) with another (dma-ranges). However, DT is not the
> right place to describe policies, because it's the place to describe
> hardware in a way agnostic from policies and use cases. In other
> words, DT must not impose using the hardware in one way or another.
> 
> For example, we have two different companies that want to ship
> products based on this SoC - A and B. Company A may want to put MDP
> and camera in the same address space, but company B instead would
> prefer MDP to be in the same address space as video. Because this
> decision is stored in DT, one of them will have to change and rebuild
> their kernel and maintain a downstream patch.

We have already got the domain id from the device's dma-ranges of DT. In
this case, we don't need rebuild the kernel, right? they only need
update the dma-ranges in DT.

> 
> My suggestion to follow here would be to:
>  - stop using dma-ranges for this purpose,

Assume we have already adjusted the iova of engine A to 4G~8G with the
below array, if it don't use dma-ranges in DT, It will abort at [1]
since we have already updated the domain->geometer.aperture_start/end to
4G~8G and the default base/size in this function always are 0/4G.

[1]
https://elixir.bootlin.com/linux/v5.11-rc1/source/drivers/iommu/dma-iommu.c#L345

>  - add an array in the MTK IOMMU driver that has a default map between
> larbs and domains, e.g.
> 
> static u8 mt8192_domain_map[NUM_DOMAINS][NUM_LARBS] = {
>    [0] = { 0 , 1, 0xff },
>    [1] = { 4, 5, 7, 0xff },
>    [2] = { 2, 9, 11, 13, 14, 16, 17, 18, 19, 20, 0xff }, 
> };

If this simple array work, I won't add dom_id in DT at the begginning.

As you may already know, we determine the domain by port number within
the larb rather that the larb number. If using the array, It should be
something like:

static u8 mt8192_domain_map[NUM_DOMAINS][NUM_LARBS_MAX] = {  
    /* Each a bit represent a port. ~0 means all ports in domain 0. */
    [0] = {~0, ~0, },   /* larb0/1 in domain 0 */
    [1] = { 0, 0, 0, 0, ~0, ~0, 0, ~0, }, /* larb4/5/7 in domain1 */
    [2] = { 0, 0, ~0 ....}, 
    /* CCU0: larb13 bit9/10  */
    [3] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, BIT(9) | BIT(10)}
    /* CCU1: larb14 port4/5*/
    [4] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, BIT(4) | BIT(5)},
};

This array looks a bit complex. I didn't like it before.

> 
>  - add a kernel command line parameter that allows overriding of this map, e.g.
> 
> mtk_iommu.domain_map="0:0,1:1:4,5,7:2:2,9,11,13,14,16,17,18,19,20"
> 
> would be equivalent to the array above. Same could be also given by a
> Kconfig entry if one can't or doesn't want to add extra command line
> parameters.
> 
> Would something like this work?
> 
> >
> > > Could we move the fixed assignment to the MTK IOMMU driver code instead,
> > > so that it can be easily adjusted as the kernel code
> > > evolves without the need to update the DTS?
> > >
> > > > >
> > > > > >
> > > > > > Currently we have no interface to adjust this setting. Suppose we add a
> > > > > > new interface for this. It would be something like:
> > > > > >
> > > > > >    int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
> > > > > >
> > > > > >    Then, all the MM drivers should call it before the HW works every
> > > > > > time, and its implement will be a bit complex since we aren't sure if
> > > > > > the larb has power at that time. the important thing is that the MM
> > > > > > devices have already not known which larb it connects with as we plan to
> > > > > > delete "mediatek,larb" in their dtsi nodes.
> > > > >
> > > > > From the practical point of view, it doesn't look like setting this on
> > > > > a per-larb basis would make much sense. The reason to switch the
> > > > > bank_sel would be to decide which MM devices can share the same
> > > > > address space. This is a security aspect, because it effectively
> > > > > determines which devices are isolated from each other.
> > > > >
> > > > > That said, I agree that for now we can just start with a fixed
> > > > > assignment. We can think of the API if there is a need to adjust the
> > > > > assignment.
> > > >
> > > > Sorry for here. I forgot a thing here. that interface above still will
> > > > not be helpful. If we don't divide the whole 16GB ranges into 4
> > > > regions(let all the other ranges be dynamical), It won't work since we
> > > > can only adjust bank_sel with the larb as unit. This is a problem. there
> > > > are many ports in a larb. Take a example, the address for vcodec read
> > > > port is 32bits while the address for vcodec write port is 33bit, then it
> > > > will fail since we only have one bank_sel setting for one larb.
> > >
> > > That's exactly why I proposed to have the API operate based on the
> > > struct device, rather than individual DMA ports. Although I guess the
> > > CCU case is different, because it's the same larb as the camera.
> > >
> > > Anyway, I agree that we don't have to come up with such an API right now.
> >
> > Thanks for the confirm.
> >
> > >
> > > > Thus we
> > > > have to use current design.
> > > >
> > > > >
> > > > > >
> > > > > >    In current design, the MM device don't need care about it and 4GB
> > > > > > range is enough for them.
> > > > > >
> > > > >
> > > > > Actually, is the current assignment correct?
> > > >
> > > > Oh. In the code (patch [32/33] of v6), I put CCU0/1 in the cam/mdp
> > > > region which start at 8G since CCU0/1 is a module of camera.
> > > >
> > > > >
> > > > > domain-id  module     iova-range                  larbs
> > > > >    0       disp        0 ~ 4G                      larb0/1
> > > > >    1       vcodec      4G ~ 8G                     larb4/5/7
> > > > >    2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> > > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > > >
> > > > > Wouldn't CCU0 and CCU1 conflict with disp?
> > > >
> > > > About the conflict, I use patch [29/33] of v6 for this. I will reserve
> > > > this special iova region when the full domain(0-4G in this example)
> > > > initialize.
> > > >
> > > > > Should perhaps disp be assigned 12G ~ 16G instead?
> > > >
> > > > I think no need put it to 12G-16G, In previous SoC, we have only 4GB
> > > > ranges for whole MM engines. currently only cam/mdp domain exclude 128M
> > > > for CCU. it should be something wrong if this is not enough.
> > > >
> > >
> > > Indeed, space is not a problem, but from the security point of view
> > > it's undesirable. I believe CCU would be running proprietary firmware,
> > > so it should be isolated as much as possible from other components.
> >
> > CCU are in the same larb with camera. Thus, it also need locate the same
> > iova range with camera.
> 
> What are larb13 and larb14 used by besides CCU? Is it possible to put
> them in a separate address space from other camera larbs?

I may not following you here. What's the benefit for this? The problem
is that larb13-port-9/10 and larb14-port4/5 should be a separate address
space, the others can not occupy their ranges.


In the end, the dma-range can not be omited in two case:
a) the iova over 4GB.
b) the special engine that can only support a special range.
right?

Actually we support the device adjust their dma-ranges like from 4G~8G
to 8G~12G. but we also have our limitation. How about I reword comment
in the mtxxxx-larb-port.h like:

/*
 * MM IOMMU supports 16GB dma address. We seperate it to four banks:
 * 0 ~ 4G; 4G ~ 8G; 8G ~12G; 12G ~ 16G. we could adjust these master
 * locate any banks. BUT
 * 1) Make sure all the ports in a larb are in one bank.
 * 2) The iova of any master can NOT cross the 4G/8G/12G boundary.
 * 3) If there is some special master require a special iova range,
 *    Make sure the other port in that larb locate in the same bank.
 *
 * This is the suggested mapping in this SoC:
 * 
 * modules      iova-range             larbs-ports
 * display       0 ~ 4G                   larb0/1
 * vcodec        4G ~ 8G                  larb4/5/7
 * cam/mdp       8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
 * CCU0    0x2_4000_0000 ~ 0x2_43ff_ffff     larb13: port 9/10
 * CCU1    0x2_4400_0000 ~ 0x2_47ff_ffff     larb14: port 4/5
 *
 * In this SoC, CCU have a special iova range requirement, that means
larb13/larb14 always need locate 8G ~ 16G.
 *
 */

We list our limitation and suggesting. If someone would like adjust the
dma-ranges, then he should make sure it follow these rules and guarantee
the drivers works, I means If a iova-A comes from another iommu domain,
the master should map it again in its own domain to make sure the HW
works.

How about this? or still think the array is better?

About the command line, because I have fixed the CCU0/1 in 8G~12G in the
driver code, and this is possible to be adjusted in command-line. This
is only for larb13/14. and could be added when necessary.(currently I
think we no need this).

And in the code I will add 12 ~ 16G support.

> 
> Best regards,
> Tomasz
> 
> >
> > > And, after all, why waste the remaining 4G of address space?
> > >
> > > Best regards,
> > > Tomasz
> > >
> > > > >
> > > > > Best regards,
> > > > > Tomasz
> > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Tomasz
> > > > > > >
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Tomasz
> > > > > > > > >
> > > > > > > > > >    3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> > > > > > > > > >    4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> > > > > > > > > >
> > > > > > > > > > The iova range for CCU0/1(camera control unit) is HW requirement.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > > > > > > ---
> > > > > > > > > >  .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> > > > > > > > > >  include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> > > > > > > > > >  2 files changed, 257 insertions(+), 1 deletion(-)
> > > > > > > > > >  create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> > > > > > > > > >
> > > > > > > > [snip]
> > > > > >
> > > >
> > >
> > > _______________________________________________
> > > Linux-mediatek mailing list
> > > Linux-mediatek@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >
Robin Murphy Feb. 1, 2021, 10:44 a.m. UTC | #11 
On 2021-01-29 11:45, Tomasz Figa wrote:
> On Mon, Jan 25, 2021 at 4:34 PM Yong Wu <yong.wu@mediatek.com> wrote:
>>
>> On Mon, 2021-01-25 at 13:18 +0900, Tomasz Figa wrote:
>>> On Wed, Jan 20, 2021 at 4:08 PM Yong Wu <yong.wu@mediatek.com> wrote:
>>>>
>>>> On Wed, 2021-01-20 at 13:15 +0900, Tomasz Figa wrote:
>>>>> On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
>>>>>>
>>>>>> On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
>>>>>>> On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
>>>>>>>>
>>>>>>>> On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
>>>>>>>>> On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
>>>>>>>>>> This patch adds decriptions for mt8192 IOMMU and SMI.
>>>>>>>>>>
>>>>>>>>>> mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
>>>>>>>>>> table format. The M4U-SMI HW diagram is as below:
>>>>>>>>>>
>>>>>>>>>>                            EMI
>>>>>>>>>>                             |
>>>>>>>>>>                            M4U
>>>>>>>>>>                             |
>>>>>>>>>>                        ------------
>>>>>>>>>>                         SMI Common
>>>>>>>>>>                        ------------
>>>>>>>>>>                             |
>>>>>>>>>>    +-------+------+------+----------------------+-------+
>>>>>>>>>>    |       |      |      |       ......         |       |
>>>>>>>>>>    |       |      |      |                      |       |
>>>>>>>>>> larb0   larb1  larb2  larb4     ......      larb19   larb20
>>>>>>>>>> disp0   disp1   mdp    vdec                   IPE      IPE
>>>>>>>>>>
>>>>>>>>>> All the connections are HW fixed, SW can NOT adjust it.
>>>>>>>>>>
>>>>>>>>>> mt8192 M4U support 0~16GB iova range. we preassign different engines
>>>>>>>>>> into different iova ranges:
>>>>>>>>>>
>>>>>>>>>> domain-id  module     iova-range                  larbs
>>>>>>>>>>     0       disp        0 ~ 4G                      larb0/1
>>>>>>>>>>     1       vcodec      4G ~ 8G                     larb4/5/7
>>>>>>>>>>     2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
>>>>>>>>>
>>>>>>>>> Why do we preassign these addresses in DT? Shouldn't it be a user's or
>>>>>>>>> integrator's decision to split the 16 GB address range into sub-ranges
>>>>>>>>> and define which larbs those sub-ranges are shared with?
>>>>>>>>
>>>>>>>> The problem is that we can't split the 16GB range with the larb as unit.
>>>>>>>> The example is the below ccu0(larb13 port9/10) is a independent
>>>>>>>> range(domain), the others ports in larb13 is in another domain.
>>>>>>>>
>>>>>>>> disp/vcodec/cam/mdp don't have special iova requirement, they could
>>>>>>>> access any range. vcodec also can locate 8G~12G. it don't care about
>>>>>>>> where its iova locate. here I preassign like this following with our
>>>>>>>> internal project setting.
>>>>>>>
>>>>>>> Let me try to understand this a bit more. Given the split you're
>>>>>>> proposing, is there actually any isolation enforced between particular
>>>>>>> domains? For example, if I program vcodec to with a DMA address from
>>>>>>> the 0-4G range, would the IOMMU actually generate a fault, even if
>>>>>>> disp had some memory mapped at that address?
>>>>>>
>>>>>> In this case. we will get fault in current SW setting.
>>>>>>
>>>>>
>>>>> Okay, thanks.
>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Why set this in DT?, this is only for simplifying the code. Assume we
>>>>>>>> put it in the platform data. We have up to 32 larbs, each larb has up to
>>>>>>>> 32 ports, each port may be in different iommu domains. we should have a
>>>>>>>> big array for this..however we only use a macro to get the domain in the
>>>>>>>> DT method.
>>>>>>>>
>>>>>>>> When replying this mail, I happen to see there is a "dev->dev_range_map"
>>>>>>>> which has "dma-range" information, I think I could use this value to get
>>>>>>>> which domain the device belong to. then no need put domid in DT. I will
>>>>>>>> test this.
>>>>>>>
>>>>>>> My feeling is that the only part that needs to be enforced statically
>>>>>>> is the reserved IOVA range for CCUs. The other ranges should be
>>>>>>> determined dynamically, although I think I need to understand better
>>>>>>> how the hardware and your proposed design work to tell what would be
>>>>>>> likely the best choice here.
>>>>>>
>>>>>> I have removed the domid patch in v6. and get the domain id in [27/33]
>>>>>> in v6..
>>>>>>
>>>>>> About the other ranges should be dynamical, the commit message [30/33]
>>>>>> of v6 should be helpful. the problem is that we have a bank_sel setting
>>>>>> for the iova[32:33]. currently we preassign this value. thus, all the
>>>>>> ranges are fixed. If you adjust this setting, you can let vcodec access
>>>>>> 0~4G.
>>>>>
>>>>> Okay, so it sounds like we effectively have four 4G address spaces and
>>>>> we can assign the master devices to them. I guess each of these
>>>>> address spaces makes for an IOMMU group.
>>>>
>>>> Yes. Each a address spaces is an IOMMU group.
>>>>
>>>>>
>>>>> It's fine to pre-assign the devices to those groups for now, but it
>>>>> definitely shouldn't be hardcoded in DT, because it depends on the use
>>>>> case of the device. I'll take a look at v6, but it sounds like it
>>>>> should be fine if it doesn't take the address space assignment from DT
>>>>> anymore.
>>>>
>>>> Thanks very much for your review.
>>>>
>>>
>>> Hmm, I had a look at v6 and it still has the address spaces hardcoded
>>> in the DTS.
>>
>> sorry. I didn't get here. where do you mean. or help reply in v6.
>>
>> I only added the preassign list as comment in the file
>> (dt-binding/memory/mt8192-larb-port.h). I thought our iommu consumer may
>> need it when they use these ports. they need add dma-ranges property if
>> its iova is over 4GB.
> 
> That's exactly the problem. v6 simply replaced one way to describe the
> policy (domain ID) with another (dma-ranges). However, DT is not the
> right place to describe policies, because it's the place to describe
> hardware in a way agnostic from policies and use cases. In other
> words, DT must not impose using the hardware in one way or another.
> 
> For example, we have two different companies that want to ship
> products based on this SoC - A and B. Company A may want to put MDP
> and camera in the same address space, but company B instead would
> prefer MDP to be in the same address space as video. Because this
> decision is stored in DT, one of them will have to change and rebuild
> their kernel and maintain a downstream patch.

Well, in most cases Company A and Company B will be building their own 
products around the SoC, so will each have their own downstream DTS 
anyway. Even if they're buying complete hardware from an OEM and just 
shipping it with custom software configurations, it's still quite likely 
that they'd have their own DTS tweaks for branding and possibly other 
firmware-related things.

Also note that expected use-cases frequently *are* reflected in DT - 
pretty much every use of the "linux,shared-dma-pool" reserved-memory 
binding, for instance. In fact memory carveouts in general are usually 
just software policy rather than any kind of hardware or firmware 
description. There are also plenty of DT properties for actual hardware 
that imply "this is how you need to configure me" rather than just "this 
is what I can do", so the distinction between "describing the platform" 
and "telling software what to do" isn't as clear-cut as we'd like it to be.

While I'm also not entirely convinced that "dma-ranges" is the perfect 
tool for the job - as opposed to less abstraction via a larb property or 
extra IOMMU specifier cell - it is at least descriptive to the DMA and 
IOMMU subsystems as well as the driver, and can draw a direct parallel 
to how some PCI host bridge drivers handle inbound windows. In many 
cases those just need to be programmed *somehow*, so "dma-ranges" is set 
in the DTS or inserted by the bootloader, and the kernel driver parses 
that then programs the hardware to match. It seems like we're doing a 
directly comparable thing here.

Robin.

> My suggestion to follow here would be to:
>   - stop using dma-ranges for this purpose,
>   - add an array in the MTK IOMMU driver that has a default map between
> larbs and domains, e.g.
> 
> static u8 mt8192_domain_map[NUM_DOMAINS][NUM_LARBS] = {
>     [0] = { 0 , 1, 0xff },
>     [1] = { 4, 5, 7, 0xff },
>     [2] = { 2, 9, 11, 13, 14, 16, 17, 18, 19, 20, 0xff },
> };
> 
>   - add a kernel command line parameter that allows overriding of this map, e.g.
> 
> mtk_iommu.domain_map="0:0,1:1:4,5,7:2:2,9,11,13,14,16,17,18,19,20"
> 
> would be equivalent to the array above. Same could be also given by a
> Kconfig entry if one can't or doesn't want to add extra command line
> parameters.
> 
> Would something like this work?
> 
>>
>>> Could we move the fixed assignment to the MTK IOMMU driver code instead,
>>> so that it can be easily adjusted as the kernel code
>>> evolves without the need to update the DTS?
>>>
>>>>>
>>>>>>
>>>>>> Currently we have no interface to adjust this setting. Suppose we add a
>>>>>> new interface for this. It would be something like:
>>>>>>
>>>>>>     int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
>>>>>>
>>>>>>     Then, all the MM drivers should call it before the HW works every
>>>>>> time, and its implement will be a bit complex since we aren't sure if
>>>>>> the larb has power at that time. the important thing is that the MM
>>>>>> devices have already not known which larb it connects with as we plan to
>>>>>> delete "mediatek,larb" in their dtsi nodes.
>>>>>
>>>>>  From the practical point of view, it doesn't look like setting this on
>>>>> a per-larb basis would make much sense. The reason to switch the
>>>>> bank_sel would be to decide which MM devices can share the same
>>>>> address space. This is a security aspect, because it effectively
>>>>> determines which devices are isolated from each other.
>>>>>
>>>>> That said, I agree that for now we can just start with a fixed
>>>>> assignment. We can think of the API if there is a need to adjust the
>>>>> assignment.
>>>>
>>>> Sorry for here. I forgot a thing here. that interface above still will
>>>> not be helpful. If we don't divide the whole 16GB ranges into 4
>>>> regions(let all the other ranges be dynamical), It won't work since we
>>>> can only adjust bank_sel with the larb as unit. This is a problem. there
>>>> are many ports in a larb. Take a example, the address for vcodec read
>>>> port is 32bits while the address for vcodec write port is 33bit, then it
>>>> will fail since we only have one bank_sel setting for one larb.
>>>
>>> That's exactly why I proposed to have the API operate based on the
>>> struct device, rather than individual DMA ports. Although I guess the
>>> CCU case is different, because it's the same larb as the camera.
>>>
>>> Anyway, I agree that we don't have to come up with such an API right now.
>>
>> Thanks for the confirm.
>>
>>>
>>>> Thus we
>>>> have to use current design.
>>>>
>>>>>
>>>>>>
>>>>>>     In current design, the MM device don't need care about it and 4GB
>>>>>> range is enough for them.
>>>>>>
>>>>>
>>>>> Actually, is the current assignment correct?
>>>>
>>>> Oh. In the code (patch [32/33] of v6), I put CCU0/1 in the cam/mdp
>>>> region which start at 8G since CCU0/1 is a module of camera.
>>>>
>>>>>
>>>>> domain-id  module     iova-range                  larbs
>>>>>     0       disp        0 ~ 4G                      larb0/1
>>>>>     1       vcodec      4G ~ 8G                     larb4/5/7
>>>>>     2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
>>>>>     3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
>>>>>     4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
>>>>>
>>>>> Wouldn't CCU0 and CCU1 conflict with disp?
>>>>
>>>> About the conflict, I use patch [29/33] of v6 for this. I will reserve
>>>> this special iova region when the full domain(0-4G in this example)
>>>> initialize.
>>>>
>>>>> Should perhaps disp be assigned 12G ~ 16G instead?
>>>>
>>>> I think no need put it to 12G-16G, In previous SoC, we have only 4GB
>>>> ranges for whole MM engines. currently only cam/mdp domain exclude 128M
>>>> for CCU. it should be something wrong if this is not enough.
>>>>
>>>
>>> Indeed, space is not a problem, but from the security point of view
>>> it's undesirable. I believe CCU would be running proprietary firmware,
>>> so it should be isolated as much as possible from other components.
>>
>> CCU are in the same larb with camera. Thus, it also need locate the same
>> iova range with camera.
> 
> What are larb13 and larb14 used by besides CCU? Is it possible to put
> them in a separate address space from other camera larbs?
> 
> Best regards,
> Tomasz
> 
>>
>>> And, after all, why waste the remaining 4G of address space?
>>>
>>> Best regards,
>>> Tomasz
>>>
>>>>>
>>>>> Best regards,
>>>>> Tomasz
>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Tomasz
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Tomasz
>>>>>>>>>
>>>>>>>>>>     3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
>>>>>>>>>>     4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
>>>>>>>>>>
>>>>>>>>>> The iova range for CCU0/1(camera control unit) is HW requirement.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>>>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>>>>> ---
>>>>>>>>>>   .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
>>>>>>>>>>   include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
>>>>>>>>>>   2 files changed, 257 insertions(+), 1 deletion(-)
>>>>>>>>>>   create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
>>>>>>>>>>
>>>>>>>> [snip]
>>>>>>
>>>>
>>>
>>> _______________________________________________
>>> Linux-mediatek mailing list
>>> Linux-mediatek@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
Tomasz Figa Feb. 9, 2021, 11:05 a.m. UTC | #12 
On Mon, Feb 1, 2021 at 7:44 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-01-29 11:45, Tomasz Figa wrote:
> > On Mon, Jan 25, 2021 at 4:34 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >>
> >> On Mon, 2021-01-25 at 13:18 +0900, Tomasz Figa wrote:
> >>> On Wed, Jan 20, 2021 at 4:08 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >>>>
> >>>> On Wed, 2021-01-20 at 13:15 +0900, Tomasz Figa wrote:
> >>>>> On Wed, Jan 13, 2021 at 3:45 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >>>>>>
> >>>>>> On Wed, 2021-01-13 at 14:30 +0900, Tomasz Figa wrote:
> >>>>>>> On Thu, Dec 24, 2020 at 8:35 PM Yong Wu <yong.wu@mediatek.com> wrote:
> >>>>>>>>
> >>>>>>>> On Wed, 2020-12-23 at 17:18 +0900, Tomasz Figa wrote:
> >>>>>>>>> On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote:
> >>>>>>>>>> This patch adds decriptions for mt8192 IOMMU and SMI.
> >>>>>>>>>>
> >>>>>>>>>> mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation
> >>>>>>>>>> table format. The M4U-SMI HW diagram is as below:
> >>>>>>>>>>
> >>>>>>>>>>                            EMI
> >>>>>>>>>>                             |
> >>>>>>>>>>                            M4U
> >>>>>>>>>>                             |
> >>>>>>>>>>                        ------------
> >>>>>>>>>>                         SMI Common
> >>>>>>>>>>                        ------------
> >>>>>>>>>>                             |
> >>>>>>>>>>    +-------+------+------+----------------------+-------+
> >>>>>>>>>>    |       |      |      |       ......         |       |
> >>>>>>>>>>    |       |      |      |                      |       |
> >>>>>>>>>> larb0   larb1  larb2  larb4     ......      larb19   larb20
> >>>>>>>>>> disp0   disp1   mdp    vdec                   IPE      IPE
> >>>>>>>>>>
> >>>>>>>>>> All the connections are HW fixed, SW can NOT adjust it.
> >>>>>>>>>>
> >>>>>>>>>> mt8192 M4U support 0~16GB iova range. we preassign different engines
> >>>>>>>>>> into different iova ranges:
> >>>>>>>>>>
> >>>>>>>>>> domain-id  module     iova-range                  larbs
> >>>>>>>>>>     0       disp        0 ~ 4G                      larb0/1
> >>>>>>>>>>     1       vcodec      4G ~ 8G                     larb4/5/7
> >>>>>>>>>>     2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> >>>>>>>>>
> >>>>>>>>> Why do we preassign these addresses in DT? Shouldn't it be a user's or
> >>>>>>>>> integrator's decision to split the 16 GB address range into sub-ranges
> >>>>>>>>> and define which larbs those sub-ranges are shared with?
> >>>>>>>>
> >>>>>>>> The problem is that we can't split the 16GB range with the larb as unit.
> >>>>>>>> The example is the below ccu0(larb13 port9/10) is a independent
> >>>>>>>> range(domain), the others ports in larb13 is in another domain.
> >>>>>>>>
> >>>>>>>> disp/vcodec/cam/mdp don't have special iova requirement, they could
> >>>>>>>> access any range. vcodec also can locate 8G~12G. it don't care about
> >>>>>>>> where its iova locate. here I preassign like this following with our
> >>>>>>>> internal project setting.
> >>>>>>>
> >>>>>>> Let me try to understand this a bit more. Given the split you're
> >>>>>>> proposing, is there actually any isolation enforced between particular
> >>>>>>> domains? For example, if I program vcodec to with a DMA address from
> >>>>>>> the 0-4G range, would the IOMMU actually generate a fault, even if
> >>>>>>> disp had some memory mapped at that address?
> >>>>>>
> >>>>>> In this case. we will get fault in current SW setting.
> >>>>>>
> >>>>>
> >>>>> Okay, thanks.
> >>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Why set this in DT?, this is only for simplifying the code. Assume we
> >>>>>>>> put it in the platform data. We have up to 32 larbs, each larb has up to
> >>>>>>>> 32 ports, each port may be in different iommu domains. we should have a
> >>>>>>>> big array for this..however we only use a macro to get the domain in the
> >>>>>>>> DT method.
> >>>>>>>>
> >>>>>>>> When replying this mail, I happen to see there is a "dev->dev_range_map"
> >>>>>>>> which has "dma-range" information, I think I could use this value to get
> >>>>>>>> which domain the device belong to. then no need put domid in DT. I will
> >>>>>>>> test this.
> >>>>>>>
> >>>>>>> My feeling is that the only part that needs to be enforced statically
> >>>>>>> is the reserved IOVA range for CCUs. The other ranges should be
> >>>>>>> determined dynamically, although I think I need to understand better
> >>>>>>> how the hardware and your proposed design work to tell what would be
> >>>>>>> likely the best choice here.
> >>>>>>
> >>>>>> I have removed the domid patch in v6. and get the domain id in [27/33]
> >>>>>> in v6..
> >>>>>>
> >>>>>> About the other ranges should be dynamical, the commit message [30/33]
> >>>>>> of v6 should be helpful. the problem is that we have a bank_sel setting
> >>>>>> for the iova[32:33]. currently we preassign this value. thus, all the
> >>>>>> ranges are fixed. If you adjust this setting, you can let vcodec access
> >>>>>> 0~4G.
> >>>>>
> >>>>> Okay, so it sounds like we effectively have four 4G address spaces and
> >>>>> we can assign the master devices to them. I guess each of these
> >>>>> address spaces makes for an IOMMU group.
> >>>>
> >>>> Yes. Each a address spaces is an IOMMU group.
> >>>>
> >>>>>
> >>>>> It's fine to pre-assign the devices to those groups for now, but it
> >>>>> definitely shouldn't be hardcoded in DT, because it depends on the use
> >>>>> case of the device. I'll take a look at v6, but it sounds like it
> >>>>> should be fine if it doesn't take the address space assignment from DT
> >>>>> anymore.
> >>>>
> >>>> Thanks very much for your review.
> >>>>
> >>>
> >>> Hmm, I had a look at v6 and it still has the address spaces hardcoded
> >>> in the DTS.
> >>
> >> sorry. I didn't get here. where do you mean. or help reply in v6.
> >>
> >> I only added the preassign list as comment in the file
> >> (dt-binding/memory/mt8192-larb-port.h). I thought our iommu consumer may
> >> need it when they use these ports. they need add dma-ranges property if
> >> its iova is over 4GB.
> >
> > That's exactly the problem. v6 simply replaced one way to describe the
> > policy (domain ID) with another (dma-ranges). However, DT is not the
> > right place to describe policies, because it's the place to describe
> > hardware in a way agnostic from policies and use cases. In other
> > words, DT must not impose using the hardware in one way or another.
> >
> > For example, we have two different companies that want to ship
> > products based on this SoC - A and B. Company A may want to put MDP
> > and camera in the same address space, but company B instead would
> > prefer MDP to be in the same address space as video. Because this
> > decision is stored in DT, one of them will have to change and rebuild
> > their kernel and maintain a downstream patch.
>
> Well, in most cases Company A and Company B will be building their own
> products around the SoC, so will each have their own downstream DTS
> anyway. Even if they're buying complete hardware from an OEM and just
> shipping it with custom software configurations, it's still quite likely
> that they'd have their own DTS tweaks for branding and possibly other
> firmware-related things.

Isn't this exactly the opposite of what we aim for with upstreaming
the code to mainline? Sure, that is how things look in practice, but
that's not because people want it to be this way, but rather because
it's impossible to do things without changing mainline in a way that
possibly breaks others. The example of this IOMMU configuration is
exactly this.

>
> Also note that expected use-cases frequently *are* reflected in DT -
> pretty much every use of the "linux,shared-dma-pool" reserved-memory
> binding, for instance. In fact memory carveouts in general are usually
> just software policy rather than any kind of hardware or firmware
> description.

Arguably those shouldn't be described in DT either, unless they are
implied by actual reservations done in the hardware and/or firmware.

> There are also plenty of DT properties for actual hardware
> that imply "this is how you need to configure me" rather than just "this
> is what I can do", so the distinction between "describing the platform"
> and "telling software what to do" isn't as clear-cut as we'd like it to be.

That's again not how things should be done and prevents others from
using the hardware the way they want without downstreaming things.

>
> While I'm also not entirely convinced that "dma-ranges" is the perfect
> tool for the job - as opposed to less abstraction via a larb property or
> extra IOMMU specifier cell - it is at least descriptive to the DMA and
> IOMMU subsystems as well as the driver, and can draw a direct parallel
> to how some PCI host bridge drivers handle inbound windows. In many
> cases those just need to be programmed *somehow*, so "dma-ranges" is set
> in the DTS or inserted by the bootloader, and the kernel driver parses
> that then programs the hardware to match. It seems like we're doing a
> directly comparable thing here.

Well, ultimately it's up to the subsystem maintainer to decide, so
feel free to ignore my grumbling, but I'd say that the fact that one
thing is not done right doesn't imply that it's okay for new things to
be done wrong. I agree that technically the current approach is not
incorrect, but I feel like practically it's a yet another small step
making us farther from being able to use mainline in production.

Best regards,
Tomasz

>
> Robin.
>
> > My suggestion to follow here would be to:
> >   - stop using dma-ranges for this purpose,
> >   - add an array in the MTK IOMMU driver that has a default map between
> > larbs and domains, e.g.
> >
> > static u8 mt8192_domain_map[NUM_DOMAINS][NUM_LARBS] = {
> >     [0] = { 0 , 1, 0xff },
> >     [1] = { 4, 5, 7, 0xff },
> >     [2] = { 2, 9, 11, 13, 14, 16, 17, 18, 19, 20, 0xff },
> > };
> >
> >   - add a kernel command line parameter that allows overriding of this map, e.g.
> >
> > mtk_iommu.domain_map="0:0,1:1:4,5,7:2:2,9,11,13,14,16,17,18,19,20"
> >
> > would be equivalent to the array above. Same could be also given by a
> > Kconfig entry if one can't or doesn't want to add extra command line
> > parameters.
> >
> > Would something like this work?
> >
> >>
> >>> Could we move the fixed assignment to the MTK IOMMU driver code instead,
> >>> so that it can be easily adjusted as the kernel code
> >>> evolves without the need to update the DTS?
> >>>
> >>>>>
> >>>>>>
> >>>>>> Currently we have no interface to adjust this setting. Suppose we add a
> >>>>>> new interface for this. It would be something like:
> >>>>>>
> >>>>>>     int mtk_smi_larb_config_banksel(struct device *larb, int banksel)
> >>>>>>
> >>>>>>     Then, all the MM drivers should call it before the HW works every
> >>>>>> time, and its implement will be a bit complex since we aren't sure if
> >>>>>> the larb has power at that time. the important thing is that the MM
> >>>>>> devices have already not known which larb it connects with as we plan to
> >>>>>> delete "mediatek,larb" in their dtsi nodes.
> >>>>>
> >>>>>  From the practical point of view, it doesn't look like setting this on
> >>>>> a per-larb basis would make much sense. The reason to switch the
> >>>>> bank_sel would be to decide which MM devices can share the same
> >>>>> address space. This is a security aspect, because it effectively
> >>>>> determines which devices are isolated from each other.
> >>>>>
> >>>>> That said, I agree that for now we can just start with a fixed
> >>>>> assignment. We can think of the API if there is a need to adjust the
> >>>>> assignment.
> >>>>
> >>>> Sorry for here. I forgot a thing here. that interface above still will
> >>>> not be helpful. If we don't divide the whole 16GB ranges into 4
> >>>> regions(let all the other ranges be dynamical), It won't work since we
> >>>> can only adjust bank_sel with the larb as unit. This is a problem. there
> >>>> are many ports in a larb. Take a example, the address for vcodec read
> >>>> port is 32bits while the address for vcodec write port is 33bit, then it
> >>>> will fail since we only have one bank_sel setting for one larb.
> >>>
> >>> That's exactly why I proposed to have the API operate based on the
> >>> struct device, rather than individual DMA ports. Although I guess the
> >>> CCU case is different, because it's the same larb as the camera.
> >>>
> >>> Anyway, I agree that we don't have to come up with such an API right now.
> >>
> >> Thanks for the confirm.
> >>
> >>>
> >>>> Thus we
> >>>> have to use current design.
> >>>>
> >>>>>
> >>>>>>
> >>>>>>     In current design, the MM device don't need care about it and 4GB
> >>>>>> range is enough for them.
> >>>>>>
> >>>>>
> >>>>> Actually, is the current assignment correct?
> >>>>
> >>>> Oh. In the code (patch [32/33] of v6), I put CCU0/1 in the cam/mdp
> >>>> region which start at 8G since CCU0/1 is a module of camera.
> >>>>
> >>>>>
> >>>>> domain-id  module     iova-range                  larbs
> >>>>>     0       disp        0 ~ 4G                      larb0/1
> >>>>>     1       vcodec      4G ~ 8G                     larb4/5/7
> >>>>>     2       cam/mdp     8G ~ 12G             larb2/9/11/13/14/16/17/18/19/20
> >>>>>     3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> >>>>>     4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> >>>>>
> >>>>> Wouldn't CCU0 and CCU1 conflict with disp?
> >>>>
> >>>> About the conflict, I use patch [29/33] of v6 for this. I will reserve
> >>>> this special iova region when the full domain(0-4G in this example)
> >>>> initialize.
> >>>>
> >>>>> Should perhaps disp be assigned 12G ~ 16G instead?
> >>>>
> >>>> I think no need put it to 12G-16G, In previous SoC, we have only 4GB
> >>>> ranges for whole MM engines. currently only cam/mdp domain exclude 128M
> >>>> for CCU. it should be something wrong if this is not enough.
> >>>>
> >>>
> >>> Indeed, space is not a problem, but from the security point of view
> >>> it's undesirable. I believe CCU would be running proprietary firmware,
> >>> so it should be isolated as much as possible from other components.
> >>
> >> CCU are in the same larb with camera. Thus, it also need locate the same
> >> iova range with camera.
> >
> > What are larb13 and larb14 used by besides CCU? Is it possible to put
> > them in a separate address space from other camera larbs?
> >
> > Best regards,
> > Tomasz
> >
> >>
> >>> And, after all, why waste the remaining 4G of address space?
> >>>
> >>> Best regards,
> >>> Tomasz
> >>>
> >>>>>
> >>>>> Best regards,
> >>>>> Tomasz
> >>>>>
> >>>>>>>
> >>>>>>> Best regards,
> >>>>>>> Tomasz
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks.
> >>>>>>>>>
> >>>>>>>>> Best regards,
> >>>>>>>>> Tomasz
> >>>>>>>>>
> >>>>>>>>>>     3       CCU0    0x4000_0000 ~ 0x43ff_ffff     larb13: port 9/10
> >>>>>>>>>>     4       CCU1    0x4400_0000 ~ 0x47ff_ffff     larb14: port 4/5
> >>>>>>>>>>
> >>>>>>>>>> The iova range for CCU0/1(camera control unit) is HW requirement.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> >>>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
> >>>>>>>>>> ---
> >>>>>>>>>>   .../bindings/iommu/mediatek,iommu.yaml        |  18 +-
> >>>>>>>>>>   include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++
> >>>>>>>>>>   2 files changed, 257 insertions(+), 1 deletion(-)
> >>>>>>>>>>   create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h
> >>>>>>>>>>
> >>>>>>>> [snip]
> >>>>>>
> >>>>
> >>>
> >>> _______________________________________________
> >>> Linux-mediatek mailing list
> >>> Linux-mediatek@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >>
> > _______________________________________________
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index ba6626347381..0f26fe14c8e2 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -76,6 +76,7 @@  properties:
           - mediatek,mt8167-m4u  # generation two
           - mediatek,mt8173-m4u  # generation two
           - mediatek,mt8183-m4u  # generation two
+          - mediatek,mt8192-m4u  # generation two
 
       - description: mt7623 generation one
         items:
@@ -115,7 +116,11 @@  properties:
       dt-binding/memory/mt6779-larb-port.h for mt6779,
       dt-binding/memory/mt8167-larb-port.h for mt8167,
       dt-binding/memory/mt8173-larb-port.h for mt8173,
-      dt-binding/memory/mt8183-larb-port.h for mt8183.
+      dt-binding/memory/mt8183-larb-port.h for mt8183,
+      dt-binding/memory/mt8192-larb-port.h for mt8192.
+
+  power-domains:
+    maxItems: 1
 
 required:
   - compatible
@@ -133,11 +138,22 @@  allOf:
               - mediatek,mt2701-m4u
               - mediatek,mt2712-m4u
               - mediatek,mt8173-m4u
+              - mediatek,mt8192-m4u
 
     then:
       required:
         - clocks
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mediatek,mt8192-m4u
+
+    then:
+      required:
+        - power-domains
+
 additionalProperties: false
 
 examples:
diff --git a/include/dt-bindings/memory/mt8192-larb-port.h b/include/dt-bindings/memory/mt8192-larb-port.h
new file mode 100644
index 000000000000..ec1ac2ba7094
--- /dev/null
+++ b/include/dt-bindings/memory/mt8192-larb-port.h
@@ -0,0 +1,240 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 MediaTek Inc.
+ *
+ * Author: Chao Hao <chao.hao@mediatek.com>
+ * Author: Yong Wu <yong.wu@mediatek.com>
+ */
+#ifndef _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_
+#define _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_
+
+#include <dt-bindings/memory/mtk-smi-larb-port.h>
+
+/*
+ * MM IOMMU:
+ * domain 0: display: larb0, larb1.
+ * domain 1: vcodec: larb4, larb5, larb7.
+ * domain 2: CAM/MDP: larb2, larb9, larb11, larb13, larb14, larb16,
+ *           larb17, larb18, larb19, larb20,
+ * domain 3: CCU0: larb13 - port9/10.
+ * domain 4: CCU1: larb14 - port4/5.
+ *
+ * larb3/6/8/10/12/15 is null.
+ */
+
+/* larb0 */
+#define M4U_PORT_L0_DISP_POSTMASK0		MTK_M4U_DOM_ID(0, 0, 0)
+#define M4U_PORT_L0_OVL_RDMA0_HDR		MTK_M4U_DOM_ID(0, 0, 1)
+#define M4U_PORT_L0_OVL_RDMA0			MTK_M4U_DOM_ID(0, 0, 2)
+#define M4U_PORT_L0_DISP_RDMA0			MTK_M4U_DOM_ID(0, 0, 3)
+#define M4U_PORT_L0_DISP_WDMA0			MTK_M4U_DOM_ID(0, 0, 4)
+#define M4U_PORT_L0_DISP_FAKE0			MTK_M4U_DOM_ID(0, 0, 5)
+
+/* larb1 */
+#define M4U_PORT_L1_OVL_2L_RDMA0_HDR		MTK_M4U_DOM_ID(0, 1, 0)
+#define M4U_PORT_L1_OVL_2L_RDMA2_HDR		MTK_M4U_DOM_ID(0, 1, 1)
+#define M4U_PORT_L1_OVL_2L_RDMA0		MTK_M4U_DOM_ID(0, 1, 2)
+#define M4U_PORT_L1_OVL_2L_RDMA2		MTK_M4U_DOM_ID(0, 1, 3)
+#define M4U_PORT_L1_DISP_MDP_RDMA4		MTK_M4U_DOM_ID(0, 1, 4)
+#define M4U_PORT_L1_DISP_RDMA4			MTK_M4U_DOM_ID(0, 1, 5)
+#define M4U_PORT_L1_DISP_UFBC_WDMA0		MTK_M4U_DOM_ID(0, 1, 6)
+#define M4U_PORT_L1_DISP_FAKE1			MTK_M4U_DOM_ID(0, 1, 7)
+
+/* larb2 */
+#define M4U_PORT_L2_MDP_RDMA0			MTK_M4U_DOM_ID(2, 2, 0)
+#define M4U_PORT_L2_MDP_RDMA1			MTK_M4U_DOM_ID(2, 2, 1)
+#define M4U_PORT_L2_MDP_WROT0			MTK_M4U_DOM_ID(2, 2, 2)
+#define M4U_PORT_L2_MDP_WROT1			MTK_M4U_DOM_ID(2, 2, 3)
+#define M4U_PORT_L2_MDP_DISP_FAKE0		MTK_M4U_DOM_ID(2, 2, 4)
+
+/* larb3: null */
+
+/* larb4 */
+#define M4U_PORT_L4_VDEC_MC_EXT			MTK_M4U_DOM_ID(1, 4, 0)
+#define M4U_PORT_L4_VDEC_UFO_EXT		MTK_M4U_DOM_ID(1, 4, 1)
+#define M4U_PORT_L4_VDEC_PP_EXT			MTK_M4U_DOM_ID(1, 4, 2)
+#define M4U_PORT_L4_VDEC_PRED_RD_EXT		MTK_M4U_DOM_ID(1, 4, 3)
+#define M4U_PORT_L4_VDEC_PRED_WR_EXT		MTK_M4U_DOM_ID(1, 4, 4)
+#define M4U_PORT_L4_VDEC_PPWRAP_EXT		MTK_M4U_DOM_ID(1, 4, 5)
+#define M4U_PORT_L4_VDEC_TILE_EXT		MTK_M4U_DOM_ID(1, 4, 6)
+#define M4U_PORT_L4_VDEC_VLD_EXT		MTK_M4U_DOM_ID(1, 4, 7)
+#define M4U_PORT_L4_VDEC_VLD2_EXT		MTK_M4U_DOM_ID(1, 4, 8)
+#define M4U_PORT_L4_VDEC_AVC_MV_EXT		MTK_M4U_DOM_ID(1, 4, 9)
+#define M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT	MTK_M4U_DOM_ID(1, 4, 10)
+
+/* larb5 */
+#define M4U_PORT_L5_VDEC_LAT0_VLD_EXT		MTK_M4U_DOM_ID(1, 5, 0)
+#define M4U_PORT_L5_VDEC_LAT0_VLD2_EXT		MTK_M4U_DOM_ID(1, 5, 1)
+#define M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT	MTK_M4U_DOM_ID(1, 5, 2)
+#define M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT	MTK_M4U_DOM_ID(1, 5, 3)
+#define M4U_PORT_L5_VDEC_LAT0_TILE_EXT		MTK_M4U_DOM_ID(1, 5, 4)
+#define M4U_PORT_L5_VDEC_LAT0_WDMA_EXT		MTK_M4U_DOM_ID(1, 5, 5)
+#define M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT	MTK_M4U_DOM_ID(1, 5, 6)
+#define M4U_PORT_L5_VDEC_UFO_ENC_EXT		MTK_M4U_DOM_ID(1, 5, 7)
+
+/* larb6: null */
+
+/* larb7 */
+#define M4U_PORT_L7_VENC_RCPU			MTK_M4U_DOM_ID(1, 7, 0)
+#define M4U_PORT_L7_VENC_REC			MTK_M4U_DOM_ID(1, 7, 1)
+#define M4U_PORT_L7_VENC_BSDMA			MTK_M4U_DOM_ID(1, 7, 2)
+#define M4U_PORT_L7_VENC_SV_COMV		MTK_M4U_DOM_ID(1, 7, 3)
+#define M4U_PORT_L7_VENC_RD_COMV		MTK_M4U_DOM_ID(1, 7, 4)
+#define M4U_PORT_L7_VENC_CUR_LUMA		MTK_M4U_DOM_ID(1, 7, 5)
+#define M4U_PORT_L7_VENC_CUR_CHROMA		MTK_M4U_DOM_ID(1, 7, 6)
+#define M4U_PORT_L7_VENC_REF_LUMA		MTK_M4U_DOM_ID(1, 7, 7)
+#define M4U_PORT_L7_VENC_REF_CHROMA		MTK_M4U_DOM_ID(1, 7, 8)
+#define M4U_PORT_L7_JPGENC_Y_RDMA		MTK_M4U_DOM_ID(1, 7, 9)
+#define M4U_PORT_L7_JPGENC_Q_RDMA		MTK_M4U_DOM_ID(1, 7, 10)
+#define M4U_PORT_L7_JPGENC_C_TABLE		MTK_M4U_DOM_ID(1, 7, 11)
+#define M4U_PORT_L7_JPGENC_BSDMA		MTK_M4U_DOM_ID(1, 7, 12)
+#define M4U_PORT_L7_VENC_SUB_R_LUMA		MTK_M4U_DOM_ID(1, 7, 13)
+#define M4U_PORT_L7_VENC_SUB_W_LUMA		MTK_M4U_DOM_ID(1, 7, 14)
+
+/* larb8: null */
+
+/* larb9 */
+#define M4U_PORT_L9_IMG_IMGI_D1			MTK_M4U_DOM_ID(2, 9, 0)
+#define M4U_PORT_L9_IMG_IMGBI_D1		MTK_M4U_DOM_ID(2, 9, 1)
+#define M4U_PORT_L9_IMG_DMGI_D1			MTK_M4U_DOM_ID(2, 9, 2)
+#define M4U_PORT_L9_IMG_DEPI_D1			MTK_M4U_DOM_ID(2, 9, 3)
+#define M4U_PORT_L9_IMG_ICE_D1			MTK_M4U_DOM_ID(2, 9, 4)
+#define M4U_PORT_L9_IMG_SMTI_D1			MTK_M4U_DOM_ID(2, 9, 5)
+#define M4U_PORT_L9_IMG_SMTO_D2			MTK_M4U_DOM_ID(2, 9, 6)
+#define M4U_PORT_L9_IMG_SMTO_D1			MTK_M4U_DOM_ID(2, 9, 7)
+#define M4U_PORT_L9_IMG_CRZO_D1			MTK_M4U_DOM_ID(2, 9, 8)
+#define M4U_PORT_L9_IMG_IMG3O_D1		MTK_M4U_DOM_ID(2, 9, 9)
+#define M4U_PORT_L9_IMG_VIPI_D1			MTK_M4U_DOM_ID(2, 9, 10)
+#define M4U_PORT_L9_IMG_SMTI_D5			MTK_M4U_DOM_ID(2, 9, 11)
+#define M4U_PORT_L9_IMG_TIMGO_D1		MTK_M4U_DOM_ID(2, 9, 12)
+#define M4U_PORT_L9_IMG_UFBC_W0			MTK_M4U_DOM_ID(2, 9, 13)
+#define M4U_PORT_L9_IMG_UFBC_R0			MTK_M4U_DOM_ID(2, 9, 14)
+
+/* larb10: null */
+
+/* larb11 */
+#define M4U_PORT_L11_IMG_IMGI_D1		MTK_M4U_DOM_ID(2, 11, 0)
+#define M4U_PORT_L11_IMG_IMGBI_D1		MTK_M4U_DOM_ID(2, 11, 1)
+#define M4U_PORT_L11_IMG_DMGI_D1		MTK_M4U_DOM_ID(2, 11, 2)
+#define M4U_PORT_L11_IMG_DEPI_D1		MTK_M4U_DOM_ID(2, 11, 3)
+#define M4U_PORT_L11_IMG_ICE_D1			MTK_M4U_DOM_ID(2, 11, 4)
+#define M4U_PORT_L11_IMG_SMTI_D1		MTK_M4U_DOM_ID(2, 11, 5)
+#define M4U_PORT_L11_IMG_SMTO_D2		MTK_M4U_DOM_ID(2, 11, 6)
+#define M4U_PORT_L11_IMG_SMTO_D1		MTK_M4U_DOM_ID(2, 11, 7)
+#define M4U_PORT_L11_IMG_CRZO_D1		MTK_M4U_DOM_ID(2, 11, 8)
+#define M4U_PORT_L11_IMG_IMG3O_D1		MTK_M4U_DOM_ID(2, 11, 9)
+#define M4U_PORT_L11_IMG_VIPI_D1		MTK_M4U_DOM_ID(2, 11, 10)
+#define M4U_PORT_L11_IMG_SMTI_D5		MTK_M4U_DOM_ID(2, 11, 11)
+#define M4U_PORT_L11_IMG_TIMGO_D1		MTK_M4U_DOM_ID(2, 11, 12)
+#define M4U_PORT_L11_IMG_UFBC_W0		MTK_M4U_DOM_ID(2, 11, 13)
+#define M4U_PORT_L11_IMG_UFBC_R0		MTK_M4U_DOM_ID(2, 11, 14)
+#define M4U_PORT_L11_IMG_WPE_RDMA1		MTK_M4U_DOM_ID(2, 11, 15)
+#define M4U_PORT_L11_IMG_WPE_RDMA0		MTK_M4U_DOM_ID(2, 11, 16)
+#define M4U_PORT_L11_IMG_WPE_WDMA		MTK_M4U_DOM_ID(2, 11, 17)
+#define M4U_PORT_L11_IMG_MFB_RDMA0		MTK_M4U_DOM_ID(2, 11, 18)
+#define M4U_PORT_L11_IMG_MFB_RDMA1		MTK_M4U_DOM_ID(2, 11, 19)
+#define M4U_PORT_L11_IMG_MFB_RDMA2		MTK_M4U_DOM_ID(2, 11, 20)
+#define M4U_PORT_L11_IMG_MFB_RDMA3		MTK_M4U_DOM_ID(2, 11, 21)
+#define M4U_PORT_L11_IMG_MFB_RDMA4		MTK_M4U_DOM_ID(2, 11, 22)
+#define M4U_PORT_L11_IMG_MFB_RDMA5		MTK_M4U_DOM_ID(2, 11, 23)
+#define M4U_PORT_L11_IMG_MFB_WDMA0		MTK_M4U_DOM_ID(2, 11, 24)
+#define M4U_PORT_L11_IMG_MFB_WDMA1		MTK_M4U_DOM_ID(2, 11, 25)
+
+/* larb12: null */
+
+/* larb13 */
+#define M4U_PORT_L13_CAM_MRAWI			MTK_M4U_DOM_ID(2, 13, 0)
+#define M4U_PORT_L13_CAM_MRAWO0			MTK_M4U_DOM_ID(2, 13, 1)
+#define M4U_PORT_L13_CAM_MRAWO1			MTK_M4U_DOM_ID(2, 13, 2)
+#define M4U_PORT_L13_CAM_CAMSV1			MTK_M4U_DOM_ID(2, 13, 3)
+#define M4U_PORT_L13_CAM_CAMSV2			MTK_M4U_DOM_ID(2, 13, 4)
+#define M4U_PORT_L13_CAM_CAMSV3			MTK_M4U_DOM_ID(2, 13, 5)
+#define M4U_PORT_L13_CAM_CAMSV4			MTK_M4U_DOM_ID(2, 13, 6)
+#define M4U_PORT_L13_CAM_CAMSV5			MTK_M4U_DOM_ID(2, 13, 7)
+#define M4U_PORT_L13_CAM_CAMSV6			MTK_M4U_DOM_ID(2, 13, 8)
+#define M4U_PORT_L13_CAM_CCUI			MTK_M4U_DOM_ID(3, 13, 9)
+#define M4U_PORT_L13_CAM_CCUO			MTK_M4U_DOM_ID(3, 13, 10)
+#define M4U_PORT_L13_CAM_FAKE			MTK_M4U_DOM_ID(2, 13, 11)
+
+/* larb14 */
+#define M4U_PORT_L14_CAM_RESERVE1		MTK_M4U_DOM_ID(2, 14, 0)
+#define M4U_PORT_L14_CAM_RESERVE2		MTK_M4U_DOM_ID(2, 14, 1)
+#define M4U_PORT_L14_CAM_RESERVE3		MTK_M4U_DOM_ID(2, 14, 2)
+#define M4U_PORT_L14_CAM_CAMSV0			MTK_M4U_DOM_ID(2, 14, 3)
+#define M4U_PORT_L14_CAM_CCUI			MTK_M4U_DOM_ID(4, 14, 4)
+#define M4U_PORT_L14_CAM_CCUO			MTK_M4U_DOM_ID(4, 14, 5)
+
+/* larb15: null */
+
+/* larb16 */
+#define M4U_PORT_L16_CAM_IMGO_R1_A		MTK_M4U_DOM_ID(2, 16, 0)
+#define M4U_PORT_L16_CAM_RRZO_R1_A		MTK_M4U_DOM_ID(2, 16, 1)
+#define M4U_PORT_L16_CAM_CQI_R1_A		MTK_M4U_DOM_ID(2, 16, 2)
+#define M4U_PORT_L16_CAM_BPCI_R1_A		MTK_M4U_DOM_ID(2, 16, 3)
+#define M4U_PORT_L16_CAM_YUVO_R1_A		MTK_M4U_DOM_ID(2, 16, 4)
+#define M4U_PORT_L16_CAM_UFDI_R2_A		MTK_M4U_DOM_ID(2, 16, 5)
+#define M4U_PORT_L16_CAM_RAWI_R2_A		MTK_M4U_DOM_ID(2, 16, 6)
+#define M4U_PORT_L16_CAM_RAWI_R3_A		MTK_M4U_DOM_ID(2, 16, 7)
+#define M4U_PORT_L16_CAM_AAO_R1_A		MTK_M4U_DOM_ID(2, 16, 8)
+#define M4U_PORT_L16_CAM_AFO_R1_A		MTK_M4U_DOM_ID(2, 16, 9)
+#define M4U_PORT_L16_CAM_FLKO_R1_A		MTK_M4U_DOM_ID(2, 16, 10)
+#define M4U_PORT_L16_CAM_LCESO_R1_A		MTK_M4U_DOM_ID(2, 16, 11)
+#define M4U_PORT_L16_CAM_CRZO_R1_A		MTK_M4U_DOM_ID(2, 16, 12)
+#define M4U_PORT_L16_CAM_LTMSO_R1_A		MTK_M4U_DOM_ID(2, 16, 13)
+#define M4U_PORT_L16_CAM_RSSO_R1_A		MTK_M4U_DOM_ID(2, 16, 14)
+#define M4U_PORT_L16_CAM_AAHO_R1_A		MTK_M4U_DOM_ID(2, 16, 15)
+#define M4U_PORT_L16_CAM_LSCI_R1_A		MTK_M4U_DOM_ID(2, 16, 16)
+
+/* larb17 */
+#define M4U_PORT_L17_CAM_IMGO_R1_B		MTK_M4U_DOM_ID(2, 17, 0)
+#define M4U_PORT_L17_CAM_RRZO_R1_B		MTK_M4U_DOM_ID(2, 17, 1)
+#define M4U_PORT_L17_CAM_CQI_R1_B		MTK_M4U_DOM_ID(2, 17, 2)
+#define M4U_PORT_L17_CAM_BPCI_R1_B		MTK_M4U_DOM_ID(2, 17, 3)
+#define M4U_PORT_L17_CAM_YUVO_R1_B		MTK_M4U_DOM_ID(2, 17, 4)
+#define M4U_PORT_L17_CAM_UFDI_R2_B		MTK_M4U_DOM_ID(2, 17, 5)
+#define M4U_PORT_L17_CAM_RAWI_R2_B		MTK_M4U_DOM_ID(2, 17, 6)
+#define M4U_PORT_L17_CAM_RAWI_R3_B		MTK_M4U_DOM_ID(2, 17, 7)
+#define M4U_PORT_L17_CAM_AAO_R1_B		MTK_M4U_DOM_ID(2, 17, 8)
+#define M4U_PORT_L17_CAM_AFO_R1_B		MTK_M4U_DOM_ID(2, 17, 9)
+#define M4U_PORT_L17_CAM_FLKO_R1_B		MTK_M4U_DOM_ID(2, 17, 10)
+#define M4U_PORT_L17_CAM_LCESO_R1_B		MTK_M4U_DOM_ID(2, 17, 11)
+#define M4U_PORT_L17_CAM_CRZO_R1_B		MTK_M4U_DOM_ID(2, 17, 12)
+#define M4U_PORT_L17_CAM_LTMSO_R1_B		MTK_M4U_DOM_ID(2, 17, 13)
+#define M4U_PORT_L17_CAM_RSSO_R1_B		MTK_M4U_DOM_ID(2, 17, 14)
+#define M4U_PORT_L17_CAM_AAHO_R1_B		MTK_M4U_DOM_ID(2, 17, 15)
+#define M4U_PORT_L17_CAM_LSCI_R1_B		MTK_M4U_DOM_ID(2, 17, 16)
+
+/* larb18 */
+#define M4U_PORT_L18_CAM_IMGO_R1_C		MTK_M4U_DOM_ID(2, 18, 0)
+#define M4U_PORT_L18_CAM_RRZO_R1_C		MTK_M4U_DOM_ID(2, 18, 1)
+#define M4U_PORT_L18_CAM_CQI_R1_C		MTK_M4U_DOM_ID(2, 18, 2)
+#define M4U_PORT_L18_CAM_BPCI_R1_C		MTK_M4U_DOM_ID(2, 18, 3)
+#define M4U_PORT_L18_CAM_YUVO_R1_C		MTK_M4U_DOM_ID(2, 18, 4)
+#define M4U_PORT_L18_CAM_UFDI_R2_C		MTK_M4U_DOM_ID(2, 18, 5)
+#define M4U_PORT_L18_CAM_RAWI_R2_C		MTK_M4U_DOM_ID(2, 18, 6)
+#define M4U_PORT_L18_CAM_RAWI_R3_C		MTK_M4U_DOM_ID(2, 18, 7)
+#define M4U_PORT_L18_CAM_AAO_R1_C		MTK_M4U_DOM_ID(2, 18, 8)
+#define M4U_PORT_L18_CAM_AFO_R1_C		MTK_M4U_DOM_ID(2, 18, 9)
+#define M4U_PORT_L18_CAM_FLKO_R1_C		MTK_M4U_DOM_ID(2, 18, 10)
+#define M4U_PORT_L18_CAM_LCESO_R1_C		MTK_M4U_DOM_ID(2, 18, 11)
+#define M4U_PORT_L18_CAM_CRZO_R1_C		MTK_M4U_DOM_ID(2, 18, 12)
+#define M4U_PORT_L18_CAM_LTMSO_R1_C		MTK_M4U_DOM_ID(2, 18, 13)
+#define M4U_PORT_L18_CAM_RSSO_R1_C		MTK_M4U_DOM_ID(2, 18, 14)
+#define M4U_PORT_L18_CAM_AAHO_R1_C		MTK_M4U_DOM_ID(2, 18, 15)
+#define M4U_PORT_L18_CAM_LSCI_R1_C		MTK_M4U_DOM_ID(2, 18, 16)
+
+/* larb19 */
+#define M4U_PORT_L19_IPE_DVS_RDMA		MTK_M4U_DOM_ID(2, 19, 0)
+#define M4U_PORT_L19_IPE_DVS_WDMA		MTK_M4U_DOM_ID(2, 19, 1)
+#define M4U_PORT_L19_IPE_DVP_RDMA		MTK_M4U_DOM_ID(2, 19, 2)
+#define M4U_PORT_L19_IPE_DVP_WDMA		MTK_M4U_DOM_ID(2, 19, 3)
+
+/* larb20 */
+#define M4U_PORT_L20_IPE_FDVT_RDA		MTK_M4U_DOM_ID(2, 20, 0)
+#define M4U_PORT_L20_IPE_FDVT_RDB		MTK_M4U_DOM_ID(2, 20, 1)
+#define M4U_PORT_L20_IPE_FDVT_WRA		MTK_M4U_DOM_ID(2, 20, 2)
+#define M4U_PORT_L20_IPE_FDVT_WRB		MTK_M4U_DOM_ID(2, 20, 3)
+#define M4U_PORT_L20_IPE_RSC_RDMA0		MTK_M4U_DOM_ID(2, 20, 4)
+#define M4U_PORT_L20_IPE_RSC_WDMA		MTK_M4U_DOM_ID(2, 20, 5)
+
+#endif