| Message ID | 20241225090113.17027-1-bo.kong@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add AIE Driver | expand |
On 25/12/2024 10:00, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- > > Changes in v3: > 1. Remove not used include file, include only headers which AIE use > 2. Remove Makefile some private driver headers > > Changes in v2: > 1. Fix coding style Only? Although several of my comments were about coding style, I pointed out different issues lack totally fake CONFIG symbols, incorrect usage of singleton approach and more. Are they implemented? Both of your changelogs are very vague, so I say does not make the review process easier. > --- > drivers/media/platform/mediatek/Kconfig | 1 + > drivers/media/platform/mediatek/Makefile | 1 + > drivers/media/platform/mediatek/aie/Kconfig | 41 + > drivers/media/platform/mediatek/aie/Makefile | 8 + > drivers/media/platform/mediatek/aie/mtk_aie.h | 950 +++++ > .../media/platform/mediatek/aie/mtk_aie_53.c | 1398 +++++++ > .../media/platform/mediatek/aie/mtk_aie_drv.c | 3545 +++++++++++++++++ > 7 files changed, 5944 insertions(+) > create mode 100644 drivers/media/platform/mediatek/aie/Kconfig > create mode 100644 drivers/media/platform/mediatek/aie/Makefile > create mode 100644 drivers/media/platform/mediatek/aie/mtk_aie.h > create mode 100644 drivers/media/platform/mediatek/aie/mtk_aie_53.c > create mode 100644 drivers/media/platform/mediatek/aie/mtk_aie_drv.c > > diff --git a/drivers/media/platform/mediatek/Kconfig b/drivers/media/platform/mediatek/Kconfig > index 84104e2cd024..cd161272666b 100644 > --- a/drivers/media/platform/mediatek/Kconfig > +++ b/drivers/media/platform/mediatek/Kconfig > @@ -2,6 +2,7 @@ > > comment "Mediatek media platform drivers" > > +source "drivers/media/platform/mediatek/aie/Kconfig" > source "drivers/media/platform/mediatek/jpeg/Kconfig" > source "drivers/media/platform/mediatek/mdp/Kconfig" > source "drivers/media/platform/mediatek/vcodec/Kconfig" > diff --git a/drivers/media/platform/mediatek/Makefile b/drivers/media/platform/mediatek/Makefile > index 38e6ba917fe5..23a096fdf21c 100644 > --- a/drivers/media/platform/mediatek/Makefile > +++ b/drivers/media/platform/mediatek/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > +obj-y += aie/ > obj-y += jpeg/ > obj-y += mdp/ > obj-y += vcodec/ > diff --git a/drivers/media/platform/mediatek/aie/Kconfig b/drivers/media/platform/mediatek/aie/Kconfig > new file mode 100644 > index 000000000000..b7925cd69309 > --- /dev/null > +++ b/drivers/media/platform/mediatek/aie/Kconfig > @@ -0,0 +1,41 @@ > +config VIDEO_MTK_AIE > + tristate "MediaTek AI engine function" > + depends on OF > + select V4L2_MEM2MEM_DEV > + select VIDEOBUF2_DMA_CONTIG > + select MEDIA_CONTROLLER_REQUEST_API > + help > + Support the AI engine (AIE) feature > + > + AIE driver is a V4L2 memory-to-memory device driver which > + provides hardware accelerated face detection function, > + it can detect different sizes of faces in a raw image. > + > +config VIDEO_MTK_AIE_RESULT_IN_KERNEL > + bool "Operate AIE in kernel mode" > + depends on VIDEO_MTK_AIE > + default y > + help > + When this option is enabled, the MediaTek (MTK) AIE driver operates in > + kernel mode, which is the default mode. > + > + In kernel mode, the AIE driver's results are processed directly within > + the kernel space, enhancing performance and reliability. > + > + Disabling this option might compromise the AIE driver performance and stability. > + > + Unless you have specific needs for operating the driver in user mode, > + for example: unit test (UT), this option should remain enabled. > + > +config VIDEO_MTK_AIE_RESULT_IN_USER > + bool "Operate AIE in user mode" > + depends on VIDEO_MTK_AIE > + help > + Enabling this option sets the MediaTek (MTK) AIE driver to operate in > + user mode. > + > + In this mode, AIE driver result values are handled at user level, providing an > + organized manner to store multiple result values. > + > + Unless you understand the implications of operating in user mode, > + this option is usually recommended to be disabled. > \ No newline at end of file Your patches have patch warnings. > diff --git a/drivers/media/platform/mediatek/aie/Makefile b/drivers/media/platform/mediatek/aie/Makefile > new file mode 100644 > index 000000000000..15c1638a5064 > --- /dev/null > +++ b/drivers/media/platform/mediatek/aie/Makefile > @@ -0,0 +1,8 @@ > +# SPDX-License-Identifier: GPL-2.0 > +mtk-aie-$(CONFIG_VIDEO_MTK_AIE) += mtk_aie_53.o > +mtk-aie-$(CONFIG_VIDEO_MTK_AIE) += mtk_aie_drv.o > + > +obj-$(CONFIG_VIDEO_MTK_AIE) += mtk-aie.o > + > +ccflags-$(CONFIG_VIDEO_MTK_AIE) += -I$(srctree)/drivers/misc/mediatek/mtk-interconnect/ > +ccflags-$(CONFIG_VIDEO_MTK_AIE) += -I$(srctree)/drivers/media/platform/mtk-isp/mtk-vmm/ Drop both. You are not supposed to include other drivers private data structures. Encapsulation and interfaces are there for a purpose. > \ No newline at end of file Same here .... > + > +#define FLD_BLINK_WEIGHT_FOREST14_SIZE 6416 > +#define FLD_CV_SIZE 19392 > +#define FLD_FP_SIZE 80160 > +#define FLD_LEAFNODE_SIZE 4608000 > +#define FLD_TREE_KM02_SIZE 120000 > +#define FLD_TREE_KM13_SIZE 120000 > +#define FLD_OUTPUT_SIZE 112 > + > +#define FD_VERSION 1946050 > +#define ATTR_VERSION 1929401 Nothing improved, drop. Drivers do not have versions. I'll skip the rest. Best regards, Krzysztof
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > +#define FLD_PL_IN_BASE_ADDR_0_0 0x550 > +#define FLD_PL_IN_BASE_ADDR_0_1 0x554 > +#define FLD_PL_IN_BASE_ADDR_0_2 0x558 > +#define FLD_PL_IN_BASE_ADDR_0_3 0x55C > +#define FLD_PL_IN_BASE_ADDR_0_4 0x560 > +#define FLD_PL_IN_BASE_ADDR_0_5 0x564 > +#define FLD_PL_IN_BASE_ADDR_0_6 0x568 > +#define FLD_PL_IN_BASE_ADDR_0_7 0x56C > +#define FLD_PL_IN_BASE_ADDR_0_8 0x570 > +#define FLD_PL_IN_BASE_ADDR_0_9 0x574 > +#define FLD_PL_IN_BASE_ADDR_0_10 0x578 > +#define FLD_PL_IN_BASE_ADDR_0_11 0x57C > +#define FLD_PL_IN_BASE_ADDR_0_12 0x580 > +#define FLD_PL_IN_BASE_ADDR_0_13 0x584 > +#define FLD_PL_IN_BASE_ADDR_0_14 0x588 > +#define FLD_PL_IN_BASE_ADDR_0_15 0x58C > +#define FLD_PL_IN_BASE_ADDR_0_16 0x590 > +#define FLD_PL_IN_BASE_ADDR_0_17 0x594 > +#define FLD_PL_IN_BASE_ADDR_0_18 0x598 > +#define FLD_PL_IN_BASE_ADDR_0_19 0x59C > +#define FLD_PL_IN_BASE_ADDR_0_20 0x5A0 > +#define FLD_PL_IN_BASE_ADDR_0_21 0x5A4 > +#define FLD_PL_IN_BASE_ADDR_0_22 0x5A8 > +#define FLD_PL_IN_BASE_ADDR_0_23 0x5AC > +#define FLD_PL_IN_BASE_ADDR_0_24 0x5B0 > +#define FLD_PL_IN_BASE_ADDR_0_25 0x5B4 > +#define FLD_PL_IN_BASE_ADDR_0_26 0x5B8 > +#define FLD_PL_IN_BASE_ADDR_0_27 0x5BC > +#define FLD_PL_IN_BASE_ADDR_0_28 0x5C0 > +#define FLD_PL_IN_BASE_ADDR_0_29 0x5C4 > + [snip] > +void aie_execute(struct mtk_aie_dev *fd, struct aie_enq_info *aie_cfg) > +{ > + unsigned int loop_num = 0; > + unsigned int loop_reg_val = 0; > + unsigned int i = 0; > + > + if (aie_cfg->sel_mode == FDMODE) { > + writel(0x0, fd->fd_base + AIE_START_REG); > + writel(0x00000111, fd->fd_base + AIE_ENABLE_REG); > + loop_num = FD_LOOP_NUM / 3 * (aie_cfg->number_of_pyramid); > + loop_reg_val = (loop_num << 8) | > + (aie_cfg->number_of_pyramid - 1); > + writel(loop_reg_val, fd->fd_base + AIE_LOOP_REG); > + writel(0x1, fd->fd_base + AIE_INT_EN_REG); > + writel(fd->reg_cfg.rs_adr, > + fd->fd_base + AIE_RS_CON_BASE_ADR_REG); > + writel(fd->reg_cfg.fd_adr, > + fd->fd_base + AIE_FD_CON_BASE_ADR_REG); > + writel(fd->reg_cfg.yuv2rgb_adr, > + fd->fd_base + AIE_YUV2RGB_CON_BASE_ADR_REG); > + > + if (fd->variant->hw_version == 31) { > + writel(0x00000002, > + fd->fd_base + AIE_YUV2RGB_CON_BASE_ADR_MSB); > + writel(0x00000002, > + fd->fd_base + AIE_RS_CON_BASE_ADR_MSB); > + writel(0x00000002, > + fd->fd_base + AIE_FD_CON_BASE_ADR_MSB); > + } > + > + writel(0x1, fd->fd_base + AIE_START_REG); > + } else if (aie_cfg->sel_mode == ATTRIBUTEMODE) { > + writel(0x0, fd->fd_base + AIE_START_REG); > + writel(0x00000101, fd->fd_base + AIE_ENABLE_REG); > + writel(0x00001A00, fd->fd_base + AIE_LOOP_REG); > + writel(0x1, fd->fd_base + AIE_INT_EN_REG); > + writel(fd->reg_cfg.rs_adr, > + fd->fd_base + AIE_RS_CON_BASE_ADR_REG); > + writel(fd->reg_cfg.fd_adr, > + fd->fd_base + AIE_FD_CON_BASE_ADR_REG); > + writel(fd->reg_cfg.yuv2rgb_adr, > + fd->fd_base + AIE_YUV2RGB_CON_BASE_ADR_REG); > + > + if (fd->variant->hw_version == 31) { > + writel(0x00000002, > + fd->fd_base + AIE_YUV2RGB_CON_BASE_ADR_MSB); > + writel(0x00000002, > + fd->fd_base + AIE_RS_CON_BASE_ADR_MSB); > + writel(0x00000002, > + fd->fd_base + AIE_FD_CON_BASE_ADR_MSB); > + } > + > + writel(0x1, fd->fd_base + AIE_START_REG); > + } else if (aie_cfg->sel_mode == FLDMODE) { > + if (fd->variant->fld_enable) { > + writel(0x10, fd->fd_base + AIE_START_REG); > + writel(0x00011111, fd->fd_base + AIE_DMA_CTL_REG); > + writel(0x01111111, fd->fd_base + FLD_EN); > + writel(0x1, fd->fd_base + AIE_INT_EN_REG); > + for (i = 0; i < aie_cfg->fld_face_num; i++) { > + writel(aie_cfg->src_img_addr, > + fd->fd_base + FLD_BASE_ADDR_FACE_0 + > + i * 0x4); > + writel(aie_cfg->fld_input[i].fld_in_crop_x1 > + << 16 | > + aie_cfg->fld_input[i] > + .fld_in_crop_y1, > + fd->fd_base + fld_face_info_0[i]); > + writel(aie_cfg->fld_input[i].fld_in_crop_x2 > + << 16 | > + aie_cfg->fld_input[i] > + .fld_in_crop_y2, > + fd->fd_base + fld_face_info_1[i]); > + writel(aie_cfg->fld_input[i].fld_in_rip << 4 | > + aie_cfg->fld_input[i].fld_in_rop, > + fd->fd_base + fld_face_info_2[i]); > + } > + > + writel(aie_cfg->fld_face_num << 28 | FLD_FOREST << 16 | > + FLD_POINT, > + fd->fd_base + FLD_MODEL_PARA1); > + writel(13 << 16 | 0xfe9, > + fd->fd_base + FLD_MODEL_PARA14); > + > + writel(aie_cfg->src_img_width << 16 | > + aie_cfg->src_img_height, > + fd->fd_base + FLD_SRC_WD_HT); > + > + /*input settings*/ > + writel(0x007c003f, fd->fd_base + FLD_PL_IN_SIZE_0); > + writel(0x0040000f, fd->fd_base + FLD_PL_IN_STRIDE_0); > + writel(0x007c003f, fd->fd_base + FLD_PL_IN_SIZE_1); > + writel(0x0040000f, fd->fd_base + FLD_PL_IN_STRIDE_1); > + writel(0x0016003f, fd->fd_base + FLD_PL_IN_SIZE_2_0); > + writel(0x0040000f, fd->fd_base + FLD_PL_IN_STRIDE_2_0); > + writel(0x0013003f, fd->fd_base + FLD_PL_IN_SIZE_2_1); > + writel(0x0040000f, fd->fd_base + FLD_PL_IN_STRIDE_2_1); > + writel(0x0013003f, fd->fd_base + FLD_PL_IN_SIZE_2_2); > + writel(0x0040000f, fd->fd_base + FLD_PL_IN_STRIDE_2_2); > + writel(0x00a6001f, fd->fd_base + FLD_PL_IN_SIZE_3); > + writel(0x0020000f, fd->fd_base + FLD_PL_IN_STRIDE_3); > + > + /*output setting*/ > + writel((2400 * aie_cfg->fld_face_num - 1) << 16 | 127, > + fd->fd_base + FLD_SH_IN_SIZE_0); > + writel(0x0010000f, fd->fd_base + FLD_SH_IN_STRIDE_0); > + writel(fd->fld_para->fld_output_pa[0], > + fd->fd_base + FLD_TR_OUT_BASE_ADDR_0); > + writel((aie_cfg->fld_face_num - 1) << 16 | 0x6f, > + fd->fd_base + FLD_TR_OUT_SIZE_0); > + writel(0x0070000f, fd->fd_base + FLD_TR_OUT_STRIDE_0); > + writel(fd->fld_para->fld_output_pa[0], > + fd->fd_base + FLD_PP_OUT_BASE_ADDR_0); > + writel((aie_cfg->fld_face_num - 1) << 16 | 0x6f, > + fd->fd_base + FLD_PP_OUT_SIZE_0); > + writel(0x0070000f, fd->fd_base + FLD_PP_OUT_STRIDE_0); > + > + /*cv score*/ > + writel(0x00000001, fd->fd_base + FLD_BS_BIAS); > + writel(0x0000b835, > + fd->fd_base + FLD_CV_FM_RANGE_0); // 8E8 > + writel(0xffff5cba, > + fd->fd_base + FLD_CV_FM_RANGE_1); // 8EC > + writel(0x00005ed5, > + fd->fd_base + FLD_CV_PM_RANGE_0); // 8F0 > + writel(0xffff910d, > + fd->fd_base + FLD_CV_PM_RANGE_1); // 8F4 > + writel(0x0000031e, fd->fd_base + FLD_BS_RANGE_0); // 8F8 > + writel(0xfffffcae, fd->fd_base + FLD_BS_RANGE_1); // 8FC > + > + /* 6 steps */ > + writel(fd->fld_para->fld_step_pa[FLD_STEP_BLINK][14], > + fd->fd_base + FLD_BS_IN_BASE_ADDR_14); > + > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][0], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_0); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][1], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_1); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][2], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_2); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][3], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_3); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][4], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_4); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][5], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_5); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][6], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_6); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][7], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_7); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][8], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_8); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][9], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_9); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][10], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_10); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][11], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_11); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][12], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_12); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][13], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_13); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_CV][14], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_14); > + > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][0], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_0); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][1], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_1); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][2], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_2); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][3], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_3); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][4], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_4); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][5], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_5); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][6], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_6); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][7], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_7); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][8], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_8); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][9], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_9); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][10], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_10); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][11], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_11); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][12], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_12); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][13], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_13); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_FP][14], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_14); > + > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][0], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_0); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][1], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_1); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][2], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_2); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][3], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_3); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][4], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_4); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][5], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_5); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][6], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_6); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][7], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_7); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][8], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_8); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][9], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_9); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][10], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_10); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][11], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_11); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][12], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_12); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][13], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_13); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_LEAF][14], > + fd->fd_base + FLD_SH_IN_BASE_ADDR_14); > + > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][0], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_0); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][1], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_1); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][2], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_2); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][3], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_3); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][4], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_4); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][5], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_5); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][6], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_6); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][7], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_7); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][8], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_8); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][9], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_9); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][10], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_10); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][11], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_11); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][12], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_12); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][13], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_13); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][14], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_14); Use a for-loop to simplify these code: #define FLD_PL_IN_BASE_ADDR_0_(n) (0x550 + 4 * n) for (i = 0; i < 15; i++) writel(fd->fld_para->fld_step_pa[FLD_STEP_KM02][i], fd->fd_base + FLD_PL_IN_BASE_ADDR_0_(i)); Regards, CK > + > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][0], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_0); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][1], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_1); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][2], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_2); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][3], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_3); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][4], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_4); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][5], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_5); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][6], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_6); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][7], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_7); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][8], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_8); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][9], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_9); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][10], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_10); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][11], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_11); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][12], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_12); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][13], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_13); > + writel(fd->fld_para->fld_step_pa[FLD_STEP_KM13][14], > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_14); > + > + /* */ > + writel(0x22222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_0_7_MSB); > + writel(0x02222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_0_8_15_MSB); > + > + writel(0x22222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_0_7_MSB); > + writel(0x02222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_1_8_15_MSB); > + > + writel(0x22222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_0_7_MSB); > + writel(0x02222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_2_8_15_MSB); > + > + writel(0x22222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_0_7_MSB); > + writel(0x02222222, > + fd->fd_base + FLD_PL_IN_BASE_ADDR_3_8_15_MSB); > + > + writel(0x22222222, > + fd->fd_base + FLD_SH_IN_BASE_ADDR_0_7_MSB); > + writel(0x02222222, > + fd->fd_base + FLD_SH_IN_BASE_ADDR_8_15_MSB); > + > + writel(0x02000000, > + fd->fd_base + FLD_BS_IN_BASE_ADDR_8_15_MSB); > + > + writel(0x22222222, > + fd->fd_base + FLD_BASE_ADDR_FACE_0_7_MSB); > + writel(0x02222222, > + fd->fd_base + FLD_BASE_ADDR_FACE_8_14_MSB); > + writel(0x00000002, > + fd->fd_base + FLD_TR_OUT_BASE_ADDR_0_MSB); > + writel(0x00000002, > + fd->fd_base + FLD_PP_OUT_BASE_ADDR_0_MSB); > + > + /*fld mode + trigger start*/ > + writel(0x11, fd->fd_base + AIE_START_REG); > + } > + } > +} > +
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > diff --git a/drivers/media/platform/mediatek/aie/Kconfig b/drivers/media/platform/mediatek/aie/Kconfig > new file mode 100644 > index 000000000000..b7925cd69309 > --- /dev/null > +++ b/drivers/media/platform/mediatek/aie/Kconfig > @@ -0,0 +1,41 @@ > +config VIDEO_MTK_AIE > + tristate "MediaTek AI engine function" > + depends on OF > + select V4L2_MEM2MEM_DEV > + select VIDEOBUF2_DMA_CONTIG > + select MEDIA_CONTROLLER_REQUEST_API > + help > + Support the AI engine (AIE) feature > + > + AIE driver is a V4L2 memory-to-memory device driver which > + provides hardware accelerated face detection function, > + it can detect different sizes of faces in a raw image. > + > +config VIDEO_MTK_AIE_RESULT_IN_KERNEL This config is useless, so drop it. > + bool "Operate AIE in kernel mode" > + depends on VIDEO_MTK_AIE > + default y > + help > + When this option is enabled, the MediaTek (MTK) AIE driver operates in > + kernel mode, which is the default mode. > + > + In kernel mode, the AIE driver's results are processed directly within > + the kernel space, enhancing performance and reliability. > + > + Disabling this option might compromise the AIE driver performance and stability. > + > + Unless you have specific needs for operating the driver in user mode, > + for example: unit test (UT), this option should remain enabled. > + > +config VIDEO_MTK_AIE_RESULT_IN_USER Ditto. Regards, CK > + bool "Operate AIE in user mode" > + depends on VIDEO_MTK_AIE > + help > + Enabling this option sets the MediaTek (MTK) AIE driver to operate in > + user mode. > + > + In this mode, AIE driver result values are handled at user level, providing an > + organized manner to store multiple result values. > + > + Unless you understand the implications of operating in user mode, > + this option is usually recommended to be disabled. >
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > +static const unsigned int anchor_en_num[FD_LOOP_NUM] = { > + 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, > + 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, > + 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, > + 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5 > +}; > + A constant array full of '5'? Use a symbol to replace it. #define ANCHOR_EN_NUM 5 Regards, CK
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > +static const unsigned int fld_face_info_0[FLD_MAX_FRAME] = { > + FLD_INFO_0_FACE_0, FLD_INFO_0_FACE_1, FLD_INFO_0_FACE_2, > + FLD_INFO_0_FACE_3, FLD_INFO_0_FACE_4, FLD_INFO_0_FACE_5, > + FLD_INFO_0_FACE_6, FLD_INFO_0_FACE_7, FLD_INFO_0_FACE_8, > + FLD_INFO_0_FACE_9, FLD_INFO_0_FACE_10, FLD_INFO_0_FACE_11, > + FLD_INFO_0_FACE_12, FLD_INFO_0_FACE_13, FLD_INFO_0_FACE_14 > +}; > + > +static const unsigned int fld_face_info_1[FLD_MAX_FRAME] = { > + FLD_INFO_1_FACE_0, FLD_INFO_1_FACE_1, FLD_INFO_1_FACE_2, > + FLD_INFO_1_FACE_3, FLD_INFO_1_FACE_4, FLD_INFO_1_FACE_5, > + FLD_INFO_1_FACE_6, FLD_INFO_1_FACE_7, FLD_INFO_1_FACE_8, > + FLD_INFO_1_FACE_9, FLD_INFO_1_FACE_10, FLD_INFO_1_FACE_11, > + FLD_INFO_1_FACE_12, FLD_INFO_1_FACE_13, FLD_INFO_1_FACE_14 > +}; > + > +static const unsigned int fld_face_info_2[FLD_MAX_FRAME] = { > + FLD_INFO_2_FACE_0, FLD_INFO_2_FACE_1, FLD_INFO_2_FACE_2, > + FLD_INFO_2_FACE_3, FLD_INFO_2_FACE_4, FLD_INFO_2_FACE_5, > + FLD_INFO_2_FACE_6, FLD_INFO_2_FACE_7, FLD_INFO_2_FACE_8, > + FLD_INFO_2_FACE_9, FLD_INFO_2_FACE_10, FLD_INFO_2_FACE_11, > + FLD_INFO_2_FACE_12, FLD_INFO_2_FACE_13, FLD_INFO_2_FACE_14 > +}; Use a macro to replace these array: #define FLD_FACE_INFO(m, n) (0x440 + 0x4 * m + 0xc * n) Regards, CK
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > +static const struct mtk_aie_variant aie_31_drvdata = { > + .hw_version = 31, > + .fld_enable = 1, > + .y2r_cfg_size = 34, > + .rs_cfg_size = 30, > + .fd_cfg_size = 56, > +}; > + This is the first patch to add AIE driver. So it's not necessary to create SoC data. So drop hw_version and fld_enable. Define symbol for others: #define Y2R_CFG_SIZE 34 #define RS_CFG_SIZE 30 #define FD_CFG_SIZE 56 Regards, CK
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > +struct mtk_aie_ctx { > + struct mtk_aie_dev *fd_dev; > + struct device *dev; > + struct v4l2_fh fh; > + struct v4l2_ctrl_handler hdl; > + struct v4l2_pix_format_mplane src_fmt; > + struct v4l2_meta_format dst_fmt; > + struct v4l2_ctrl_aie_init user_init; > + struct v4l2_ctrl_aie_param user_param; struct v4l2_ctrl_aie_param is defined in future patch. When apply this patch, it would build fail. So reorder patch sequence to let struct v4l2_ctrl_aie_param defined first. Regards, CK > +}; > +
On Wed, 2024-12-25 at 17:00 +0800, bo.kong wrote: > From: Bo Kong <Bo.Kong@mediatek.com> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@mediatek.com> > --- [snip] > +static int mtk_aie_hw_connect(struct mtk_aie_dev *fd) > +{ > + if (mtk_aie_hw_enable(fd)) > + return -EINVAL; mtk_aie_hw_connect() just call mtk_aie_hw_enable(), and mtk_aie_hw_enable() just print some message and call aie_init(), so drop mtk_aie_hw_connect() and mtk_aie_hw_enable() and caller directly call aie_init(). > + > + return 0; > +} > + > +static void mtk_aie_hw_disconnect(struct mtk_aie_dev *fd) > +{ > + aie_uninit(fd); mtk_aie_hw_disconnect() just call aie_unnit(), so drop mtk_aie_hw_disconnect() and caller directly call aie_uninit(). Regards, CK > +} > +
From: Bo Kong <Bo.Kong@mediatek.com> AIE(AI Engine) is one of the units in mt8188 ISP which provides hardware accelerated face detection function. Bo Kong (4): arm64: dts: mt8188: add aie node media: dt-bindings: add MT8188 AIE media: mediatek: add MT8188 AIE driver uapi: linux: add MT8188 AIE .../bindings/media/mediatek,mt8188-aie.yaml | 97 + arch/arm64/boot/dts/mediatek/mt8188.dtsi | 33 + drivers/media/platform/mediatek/Kconfig | 1 + drivers/media/platform/mediatek/Makefile | 1 + drivers/media/platform/mediatek/aie/Kconfig | 41 + drivers/media/platform/mediatek/aie/Makefile | 8 + drivers/media/platform/mediatek/aie/mtk_aie.h | 950 +++++ .../media/platform/mediatek/aie/mtk_aie_53.c | 1398 +++++++ .../media/platform/mediatek/aie/mtk_aie_drv.c | 3545 +++++++++++++++++ include/uapi/linux/mtk_aie_v4l2_controls.h | 132 + include/uapi/linux/videodev2.h | 6 + 11 files changed, 6212 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8188-aie.yaml create mode 100644 drivers/media/platform/mediatek/aie/Kconfig create mode 100644 drivers/media/platform/mediatek/aie/Makefile create mode 100644 drivers/media/platform/mediatek/aie/mtk_aie.h create mode 100644 drivers/media/platform/mediatek/aie/mtk_aie_53.c create mode 100644 drivers/media/platform/mediatek/aie/mtk_aie_drv.c create mode 100644 include/uapi/linux/mtk_aie_v4l2_controls.h