On 25/12/2024 10:00, bo.kong wrote: > From: Bo Kong <Bo.Kong@xxxxxxxxxxxx> > > Add a V4L2 sub-device driver for MT8188 AIE. > > Signed-off-by: Bo Kong <Bo.Kong@xxxxxxxxxxxx> > --- > > 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