Re: [PATCH 0/3] media: mediatek: Add support MT8188 AIE

[AngeloGioacchino Del Regno]

Il 17/07/24 14:41, 20220614094956 created ha scritto:
> From: Yelian Wang <yelian.wang@xxxxxxxxxxxx>
>
> *** BLURB HERE ***
>
> This patch series add YAML DT binding and V4L2 sub-device driver
> for mediatek MT8188 AIE. AIE is the ISP unit in the SoC，it's used
> to detect faces on an image stored in dram. Mainly used for the
> camera's Face Detection function of MT8188.

Leaving the fact that this driver is *far* from being upstream quality, there's
something missing that is essential for reviewers to even try to help you here.

What does this hardware do, in depth?

I get it, it's giving out "face detection" functionality, okay, but what are we
supposed to feed to it? An image? A stream? Encoded? Decoded? Size limitations?

What is it supposed to output?
An image? A stream? Polygons? With or without text?

....too many questions, and I'm afraid that this should not even be a V4L2 driver,
or a generous part of it should not, anyway... maybe.

With such a huge driver, you should at least provide an extensive overview of what
you are trying to implement and how the user(/kernel)-facing part is supposed to
work.

Regards,
Angelo

> This series is based on linux-next, tag: next-20240716
>
> Yelian Wang (3):
>    media: dt-bindings: add MT8188 AIE
>    uapi: linux: add MT8188 AIE
>    media: mediatek: add MT8188 AIE driver
>
>   .../bindings/media/mediatek-aie.yaml          |   99 +
>   drivers/media/platform/mediatek/Kconfig       |    1 +
>   drivers/media/platform/mediatek/Makefile      |    1 +
>   drivers/media/platform/mediatek/aie/Kconfig   |   13 +
>   drivers/media/platform/mediatek/aie/Makefile  |    5 +
>   drivers/media/platform/mediatek/aie/mtk_aie.h | 1012 +++++
>   .../media/platform/mediatek/aie/mtk_aie_53.c  | 2031 +++++++++
>   .../media/platform/mediatek/aie/mtk_aie_drv.c | 3613 +++++++++++++++++
>   include/uapi/linux/mtk_aie_v4l2_controls.h    |  130 +
>   include/uapi/linux/videodev2.h                |    6 +
>   10 files changed, 6911 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/media/mediatek-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