Hi, Laurent, > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx] > Sent: Tuesday, January 22, 2019 1:22 PM > To: Zhi, Yong <yong.zhi@xxxxxxxxx> > Cc: linux-media@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx; > tfiga@xxxxxxxxxxxx; Mani, Rajmohan <rajmohan.mani@xxxxxxxxx>; > Toivonen, Tuukka <tuukka.toivonen@xxxxxxxxx>; Hu, Jerry W > <jerry.w.hu@xxxxxxxxx>; Qiu, Tian Shu <tian.shu.qiu@xxxxxxxxx>; > hans.verkuil@xxxxxxxxx; mchehab@xxxxxxxxxx; Cao, Bingbu > <bingbu.cao@xxxxxxxxx> > Subject: Re: [PATCH v8 15/17] media: v4l: Add Intel IPU3 meta buffer formats > > Hi Yong, > > On Thu, Jan 10, 2019 at 06:35:11PM +0000, Zhi, Yong wrote: > > On Tuesday, December 11, 2018 6:59 AM, Laurent Pinchart wrote: > > > On Friday, 7 December 2018 03:03:40 EET Yong Zhi wrote: > > >> Add IPU3-specific meta formats for processing parameters and 3A > > >> statistics. > > >> > > >> V4L2_META_FMT_IPU3_PARAMS > > >> V4L2_META_FMT_IPU3_STAT_3A > > >> > > >> Signed-off-by: Yong Zhi <yong.zhi@xxxxxxxxx> > > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > > > > My Reviewed-by tag was related to the format part only (v4l2-ioctl.c > > > and > > > videodev2.h) :-) Please see below for more comments about the > > > documentation. > > > > > >> --- > > >> Documentation/media/uapi/v4l/meta-formats.rst | 1 + > > >> .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst | 178 > ++++++++++++++++++ > > >> drivers/media/v4l2-core/v4l2-ioctl.c | 2 + > > >> include/uapi/linux/videodev2.h | 4 + > > >> 4 files changed, 185 insertions(+) create mode 100644 > > >> Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst > > [snip] > > > >> diff --git > > >> a/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst > > >> b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst new file > > >> mode > > >> 100644 > > >> index 000000000000..8cd30ffbf8b8 > > >> --- /dev/null > > >> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst > > [snip] > > > >> +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters > > >> +for all > > >> above. > > > > > > I would write it as "The > > > > > > By the way why "4a" when the documentation talks about 3A ? > > > Shouldn't the structure be called ipu3_uapi_3a_config ? > > > > > > > The 4th "a" refers to the AWB filter response config. > > But the automatic algorithms are still automatic white balance, automatic > exposure and automatic focus, right, with ipu3_uapi_awb_fr_raw_buffer > being part of AWB, right ? > That's right, we still call it ipu3_uapi_stats_3a instead of ipu3_uapi_stats_4a. I have no problem to rename ipu3_uapi_4a_config to ipu3_uapi_3a_config. I can resend the patch with this update if no one opposes, thanks!! > > >> + > > >> +.. code-block:: c > > >> + > > >> + struct ipu3_uapi_stats_3a { > > >> + struct ipu3_uapi_awb_raw_buffer awb_raw_buffer; > > >> + struct ipu3_uapi_ae_raw_buffer_aligned > > >> ae_raw_buffer[IPU3_UAPI_MAX_STRIPES]; > > >> + struct ipu3_uapi_af_raw_buffer > > >> af_raw_buffer; > > >> + struct ipu3_uapi_awb_fr_raw_buffer awb_fr_raw_buffer; > > >> + struct ipu3_uapi_4a_config stats_4a_config; > > >> + __u32 ae_join_buffers; > > >> + __u8 padding[28]; > > >> + struct ipu3_uapi_stats_3a_bubble_info_per_stripe > > >> stats_3a_bubble_per_stripe; > > >> + struct ipu3_uapi_ff_status stats_3a_status; > > >> + }; > > >> > > >> +.. c:type:: ipu3_uapi_params > > [snip] > > -- > Regards, > > Laurent Pinchart
