Hi Tomasz, On Mon, 2019-12-02 at 18:48 +0900, Tomasz Figa wrote: > On Sat, Nov 30, 2019 at 1:55 AM Jerry-ch Chen > <Jerry-ch.Chen@xxxxxxxxxxxx> wrote: > > > > Hi Tomasz, > > > > On Wed, 2019-11-20 at 18:28 +0900, Tomasz Figa wrote: > > > On Wed, Nov 20, 2019 at 5:11 PM Jerry-ch Chen > > > <Jerry-ch.Chen@xxxxxxxxxxxx> wrote: > > > > > > > > Hi Tomasz, > > > > > > > > On Fri, 2019-10-25 at 11:52 +0800, Tomasz Figa wrote: > > > > > On Tue, Oct 15, 2019 at 11:16:15AM +0800, Jerry-ch Chen wrote: > > > > > > Hi Tomasz, > > > > > > > > > > > > On Fri, 2019-09-06 at 18:11 +0800, Jerry-ch Chen wrote: > > > > > > > From: Jerry-ch Chen <jerry-ch.chen@xxxxxxxxxxxx> > > > > > > > > > > > > > > This patch adds the driver of Face Detection (FD) unit in > > > > > > > Mediatek camera system, providing face detection function. > > > > > > > > > > > > > > The mtk-isp directory will contain drivers for multiple IP > > > > > > > blocks found in Mediatek ISP system. It will include ISP Pass 1 > > > > > > > driver (CAM), sensor interface driver, DIP driver and face > > > > > > > detection driver. > > > > > > > > > > > > > > Signed-off-by: Jerry-ch Chen <jerry-ch.chen@xxxxxxxxxxxx> > > > > > > > --- > > > > > > > drivers/media/platform/Kconfig | 2 + > > > > > > > drivers/media/platform/Makefile | 2 + > > > > > > > drivers/media/platform/mtk-isp/fd/Kconfig | 19 + > > > > > > > drivers/media/platform/mtk-isp/fd/Makefile | 5 + > > > > > > > drivers/media/platform/mtk-isp/fd/mtk_fd.h | 148 ++ > > > > > > > drivers/media/platform/mtk-isp/fd/mtk_fd_40.c | 1219 +++++++++++++++++ > > > > > > > include/uapi/linux/mtk-fd-v4l2-controls.h | 69 + > > > > > > > include/uapi/linux/v4l2-controls.h | 4 + > > > > > > > 8 files changed, 1468 insertions(+) > > > > > > > create mode 100644 drivers/media/platform/mtk-isp/fd/Kconfig > > > > > > > create mode 100644 drivers/media/platform/mtk-isp/fd/Makefile > > > > > > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd.h > > > > > > > create mode 100644 drivers/media/platform/mtk-isp/fd/mtk_fd_40.c > > > > > > > create mode 100644 include/uapi/linux/mtk-fd-v4l2-controls.h > > > > > > > > > > > > > > > > > [snip] [snip] > > > > > [snip] > > > > > > > > > > > > +/* Set the face angle and directions to be detected */ > > > > > > > +#define V4L2_CID_MTK_FD_DETECT_POSE (V4L2_CID_USER_MTK_FD_BASE + 1) > > > > > > > + > > > > > > > +/* Set image widths for an input image to be scaled down for face detection */ > > > > > > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH (V4L2_CID_USER_MTK_FD_BASE + 2) > > > > > > > + > > > > > > > +/* Set image heights for an input image to be scaled down for face detection */ > > > > > > > +#define V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT (V4L2_CID_USER_MTK_FD_BASE + 3) > > > > > > > + > > > > > > > +/* Set the length of scale down size array */ > > > > > > > +#define V4L2_CID_MTK_FD_SCALE_IMG_NUM (V4L2_CID_USER_MTK_FD_BASE + 4) > > > > > > > + > > > > > > > +/* Set the detection speed, usually reducing accuracy. */ > > > > > > > +#define V4L2_CID_MTK_FD_DETECT_SPEED (V4L2_CID_USER_MTK_FD_BASE + 5) > > > > > > > + > > > > > > > +/* Select the detection model or algorithm to be used. */ > > > > > > > +#define V4L2_CID_MTK_FD_DETECTION_MODEL (V4L2_CID_USER_MTK_FD_BASE + 6) > > > > > > > + > > > > > > > +/* We reserve 16 controls for this driver. */ > > > > > > > +#define V4L2_CID_MTK_FD_MAX 16 > > > > > > > + > > > > > > > > > > > > For these control IDs, I think the following should be remained as chip > > > > > > specific controls. > > > > > > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_WIDTH, > > > > > > V4L2_CID_MTK_FD_SCALE_DOWN_IMG_HEIGHT and V4L2_CID_MTK_FD_SCALE_IMG_NUM > > > > > > > > > > > > Hope there would be standardizing face detection api that cover the rest > > > > > > controls: V4L2_CID_MTK_FD_DETECT_POSE, V4L2_CID_MTK_FD_DETECT_SPEED and > > > > > > V4L2_CID_MTK_FD_DETECTION_MODEL > > > > > > > > > > > > Would you have any suggestions on how to propose the standard face > > > > > > detection apis? > > > > > > > > > > > > > > > > Given no follow up feedback from the community, I think we can keep them > > > > > as driver-specific, but should make sure that they have some reasonable > > > > > default values in case an application doesn't recognize them. > > > > > > > > > > Best regards, > > > > > Tomasz > > > > > > > > > Should I keep the file "mtk-fd-v4l2-controls.h" which defines the > > > > control ids under the folder "/include/uapi/linux"? > > > > > > We should define the CID base for the FD driver in v4l2-controls.h, > > > but the controls themselves should be only defined inside the driver. > > > > > > For example: > > > https://elixir.bootlin.com/linux/v5.4-rc8/source/include/uapi/linux/v4l2-controls.h#L178 > > > https://elixir.bootlin.com/linux/v5.4-rc8/source/drivers/media/i2c/adv7180.c#L181 > > > > > > Best regards, > > > Tomasz > > > > Appreciate for providing the example, > > Would it be fine for me to put the private CID in the mtk_fd.h(which is > > similar to before...) or follow the example to define inside > > mtk_fd_40.c?? > > > > The next version is almost ready, maybe I can send it when I ready in a > > few days? > > Since mtk_fd_40.c is the only place the definitions from mtk_fd.h are > used, I'd suggest just moving all the contents to the .c file. > > Best regards, > Tomasz Done, Thanks and best regards, Jerry