Re: [PATCH v5 3/4] uapi: linux: add MT8188 AIE

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Wed, 2025-04-23 at 04:02 +0000, Bo Kong (孔波) wrote:
> Hi,CK,
>     Thanks for the reviews.
> 
> 
> > > +/**
> > > + * struct aie_enq_info - V4L2 Kernelspace parameters.
> > > + *
> > > + * @sel_mode: select a mode(FDMODE, ATTRIBUTEMODE, FLDMODE) for
> > > current fd.
> > > + *           FDMODE: Face Detection.
> > > + *           ATTRIBUTEMODE: Gender and ethnicity detection
> > > + *           FLDMODE: Locations of eyebrows, eyes, ears, nose,and
> > > mouth
> > > + * @src_img_fmt: source image format.
> > > + * @src_img_width: the width of the source image.
> > > + * @src_img_height: the height of the source image.
> > > + * @src_img_stride: the stride of the source image.
> > > + * @pyramid_base_width: pyramid is the size of resizer, the width
> > > of the base pyramid.
> > > + * @pyramid_base_height: pyramid is the size of resizer, the width
> > > of the base pyramid.
> > > + * @number_of_pyramid: number of pyramid, min: 1, max: 3.
> > > + * @rotate_degree: the rotate degree of the image.
> > > + * @en_roi: enable roi(roi is a box diagram that selects a
> > > rectangle in a picture).
> > > + *          when en_roi is enable, AIE will return a rectangle
> > > face detection result
> > > + * @src_roi: roi params.
> > > + * @en_padding: enable padding, this is only used on the hardware
> > > of yuv to rgb.
> > > + *              and has noting to do with fd_mode
> > > + * @src_padding: padding params.
> > > + * @freq_level: frequency level, Get value from user space enque.
> > > + * @fld_face_num: the number of faces in fld.
> > > + *                user space tells driver the number of
> > > detections.
> > > + * @fld_input: fld input params.
> > > + * @src_img_addr: Source image address.
> > > + * @src_img_addr_uv: Source image address for UV plane.
> > > + * @fd_out: Face detection results.
> > > + * @attr_out: Attribute detection results.
> > > + * @fld_out: Array of facial landmark detection results for
> > > multiple frames.
> > > + * @irq_status: Interrupt request status.
> > > + */
> > > +struct aie_enq_info {
> > > +	__u32 sel_mode;
> > > +	__u32 src_img_fmt;
> > > +	__u32 src_img_width;
> > > +	__u32 src_img_height;
> > > +	__u32 src_img_stride;
> > > +	__u32 pyramid_base_width;
> > > +	__u32 pyramid_base_height;
> > > +	__u32 number_of_pyramid;
> > > +	__u32 rotate_degree;
> > > +	int en_roi;
> > > +	struct aie_roi_coordinate src_roi;
> > > +	int en_padding;
> > > +	struct aie_padding_size src_padding;
> > > +	unsigned int freq_level;
> > > +	unsigned int fld_face_num;
> > > +	struct v4l2_fld_crop_rip_rop fld_input[FLD_MAX_FRAME];
> > 
> > Above information is set by user space, so driver is not necessary to
> > return these information back to user space.
> > Drop these.
> > 
> 
> This is the V4L2 enqueue structure, which requires information sent
> from user space. It cannot be dropped; otherwise, the FLD input will
> not receive data from the user.

In struct v4l2_ctrl_aie_param, you have already define these parameter,
so it's not necessary to set these parameter by two interface to driver.
So drop these.

Regards,
CK

> 
> > > +	__u32 src_img_addr;
> > > +	__u32 src_img_addr_uv;
> > 
> > If user space program need to access source image buffer, it should
> > map it itself.
> > Do not pass this address information from driver.
> > Drop these.
> > 
> 
> This is where the user provides the addresses of the Y and UV
> components of the image for the AIE to use.
> 
> > > +	struct fd_result fd_out;
> > > +	struct attr_result attr_out;
> > > +	struct fld_result fld_out[FLD_MAX_FRAME];
> > 
> > Union these three structure because hardware would work in one mode
> > in one time.
> > 
> > > +	__u32 irq_status;
> > 
> > User space program should not process irq_status.
> > So drop this.
> > 
> 
> The irq status is provided for users to check the current status of the
> AIE. Users need to use it to monitor the AIE status in order to perform
> some custom operations in advance.
> 
> > > +};
> > > +