Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-12-01 Thread Sakari Ailus
Hi Yong,

On Thu, Nov 29, 2018 at 11:06:23PM +, Zhi, Yong wrote:
> Hi, Sakari,
> 
> > -Original Message-
> > From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> > Sent: Thursday, November 29, 2018 4:46 PM
> > To: Zhi, Yong 
> > Cc: linux-media@vger.kernel.org; tf...@chromium.org;
> > mche...@kernel.org; hans.verk...@cisco.com;
> > laurent.pinch...@ideasonboard.com; Mani, Rajmohan
> > ; Zheng, Jian Xu ; Hu,
> > Jerry W ; Toivonen, Tuukka
> > ; Qiu, Tian Shu ; Cao,
> > Bingbu ; Li, Chao C 
> > Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> > 
> > Hi Yong,
> > 
> > On Fri, Nov 16, 2018 at 10:37:00PM +, Zhi, Yong wrote:
> > ...
> > > > > +/**
> > > > > + * struct ipu3_uapi_shd_grid_config - Bayer shading(darkening)
> > > > > +correction
> > > > > + *
> > > > > + * @width:   Grid horizontal dimensions, u8, [8, 128], default 73
> > > > > + * @height:  Grid vertical dimensions, u8, [8, 128], default 56
> > > > > + * @block_width_log2:Log2 of the width of the grid cell in 
> > > > > pixel
> > > > count
> > > > > + *   u4, [0, 15], default value 5.
> > > > > + * @__reserved0: reserved
> > > > > + * @block_height_log2:   Log2 of the height of the grid cell in 
> > > > > pixel
> > > > count
> > > > > + *   u4, [0, 15], default value 6.
> > > > > + * @__reserved1: reserved
> > > > > + * @grid_height_per_slice:   SHD_MAX_CELLS_PER_SET/width.
> > > > > + *   (with SHD_MAX_CELLS_PER_SET =
> > 146).
> > > > > + * @x_start: X value of top left corner of sensor relative to ROI
> > > > > + *   u12, [-4096, 0]. default 0, only negative values.
> > > > > + * @y_start: Y value of top left corner of sensor relative to ROI
> > > > > + *   u12, [-4096, 0]. default 0, only negative values.
> > > >
> > > > I suppose u12 is incorrect here, if the value is signed --- and
> > > > negative (sign bit) if not 0?
> > > >
> > >
> > > The value will be written to 13 bit register, should use s12.0.
> > 
> > If you have s12, that means the most significant bit is the sign bit. So if 
> > the
> > smallest value is -4096, you'd need s13.
> > 
> > But where is the sign bit, i.e. is this either s13 or s16?
> > 
> 
> The notation of s12.0 means 13 bit with fraction bit as 0 right? 

In s12.0, bit 11 is the sign bit, and bits 10--0 are the integer part. The
smallest number that can be represented is thus -2048 (not -4096).

> 
> > >
> > > > > + */
> > > > > +struct ipu3_uapi_shd_grid_config {
> > > > > + /* reg 0 */
> > > > > + __u8 width;
> > > > > + __u8 height;
> > > > > + __u8 block_width_log2:3;
> > > > > + __u8 __reserved0:1;
> > > > > + __u8 block_height_log2:3;
> > > > > + __u8 __reserved1:1;
> > > > > + __u8 grid_height_per_slice;
> > > > > + /* reg 1 */
> > > > > + __s16 x_start;
> > > > > + __s16 y_start;
> > > > > +} __packed;
> > 
> > ...
> > 
> > > > > +/**
> > > > > + * struct ipu3_uapi_iefd_cux2_1 - Calculate power of non-directed
> > denoise
> > > > > + * element apply.
> > > > > + * @x0: X0 point of Config Unit, u9.0, default 0.
> > > > > + * @x1: X1 point of Config Unit, u9.0, default 0.
> > > > > + * @a01: Slope A of Config Unit, s4.4, default 0.
> > > >
> > > > The field is marked unsigned below. Which one is correct?
> > > >
> > >
> > > They are both correct, however, s4.4 is the internal representation
> > > used by CU, the inputs are unsigned, I will add a note in v8, same
> > > applies to the few other places as you commented.
> > 
> > I still find this rather confusing. Is there a sign bit or is there not?
> > 
> 
> It's unsigned number from driver perspective, all CU inputs are unsigned,
> however, they will be "converted" to signed for FW/HW to use. I have to
> consult FW expert if more clarification is needed.

I think that would be good to have; if you somehow convert an unsigned
integer to a negative number, there's more than just the type cast there.

-- 
Kind regards,

Sakari Ailus
sakari.ai...@linux.intel.com


RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-29 Thread Zhi, Yong
Hi, Sakari,

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> Sent: Thursday, November 29, 2018 4:46 PM
> To: Zhi, Yong 
> Cc: linux-media@vger.kernel.org; tf...@chromium.org;
> mche...@kernel.org; hans.verk...@cisco.com;
> laurent.pinch...@ideasonboard.com; Mani, Rajmohan
> ; Zheng, Jian Xu ; Hu,
> Jerry W ; Toivonen, Tuukka
> ; Qiu, Tian Shu ; Cao,
> Bingbu ; Li, Chao C 
> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> 
> Hi Yong,
> 
> On Fri, Nov 16, 2018 at 10:37:00PM +, Zhi, Yong wrote:
> ...
> > > > +/**
> > > > + * struct ipu3_uapi_shd_grid_config - Bayer shading(darkening)
> > > > +correction
> > > > + *
> > > > + * @width: Grid horizontal dimensions, u8, [8, 128], default 73
> > > > + * @height:Grid vertical dimensions, u8, [8, 128], default 56
> > > > + * @block_width_log2:  Log2 of the width of the grid cell in pixel
> > > count
> > > > + * u4, [0, 15], default value 5.
> > > > + * @__reserved0:   reserved
> > > > + * @block_height_log2: Log2 of the height of the grid cell in pixel
> > > count
> > > > + * u4, [0, 15], default value 6.
> > > > + * @__reserved1:   reserved
> > > > + * @grid_height_per_slice: SHD_MAX_CELLS_PER_SET/width.
> > > > + * (with SHD_MAX_CELLS_PER_SET =
> 146).
> > > > + * @x_start:   X value of top left corner of sensor relative to ROI
> > > > + * u12, [-4096, 0]. default 0, only negative values.
> > > > + * @y_start:   Y value of top left corner of sensor relative to ROI
> > > > + * u12, [-4096, 0]. default 0, only negative values.
> > >
> > > I suppose u12 is incorrect here, if the value is signed --- and
> > > negative (sign bit) if not 0?
> > >
> >
> > The value will be written to 13 bit register, should use s12.0.
> 
> If you have s12, that means the most significant bit is the sign bit. So if 
> the
> smallest value is -4096, you'd need s13.
> 
> But where is the sign bit, i.e. is this either s13 or s16?
> 

The notation of s12.0 means 13 bit with fraction bit as 0 right? 

> >
> > > > + */
> > > > +struct ipu3_uapi_shd_grid_config {
> > > > +   /* reg 0 */
> > > > +   __u8 width;
> > > > +   __u8 height;
> > > > +   __u8 block_width_log2:3;
> > > > +   __u8 __reserved0:1;
> > > > +   __u8 block_height_log2:3;
> > > > +   __u8 __reserved1:1;
> > > > +   __u8 grid_height_per_slice;
> > > > +   /* reg 1 */
> > > > +   __s16 x_start;
> > > > +   __s16 y_start;
> > > > +} __packed;
> 
> ...
> 
> > > > +/**
> > > > + * struct ipu3_uapi_iefd_cux2_1 - Calculate power of non-directed
> denoise
> > > > + *   element apply.
> > > > + * @x0: X0 point of Config Unit, u9.0, default 0.
> > > > + * @x1: X1 point of Config Unit, u9.0, default 0.
> > > > + * @a01: Slope A of Config Unit, s4.4, default 0.
> > >
> > > The field is marked unsigned below. Which one is correct?
> > >
> >
> > They are both correct, however, s4.4 is the internal representation
> > used by CU, the inputs are unsigned, I will add a note in v8, same
> > applies to the few other places as you commented.
> 
> I still find this rather confusing. Is there a sign bit or is there not?
> 

It's unsigned number from driver perspective, all CU inputs are unsigned, 
however, they will be "converted" to signed for FW/HW to use. I have to consult 
FW expert if more clarification is needed.

> >
> > > > + * @__reserved1: reserved
> > > > + * @b01: offset B0 of Config Unit, u7.0, default 0.
> > > > + * @__reserved2: reserved
> > > > + */
> > > > +struct ipu3_uapi_iefd_cux2_1 {
> > > > +   __u32 x0:9;
> > > > +   __u32 x1:9;
> > > > +   __u32 a01:9;
> > > > +   __u32 __reserved1:5;
> > > > +
> > > > +   __u32 b01:8;
> > > > +   __u32 __reserved2:24;
> > > > +} __packed;
> > > > +
> > > > +/**
> > > > + * struct ipu3_uapi_iefd_cux4 - Calculate power of non-directed
> > > sharpening
> > > > + * element.
> > > > + *
> > &g

RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-28 Thread Mani, Rajmohan
Hi Hans,

> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> 
> On 11/07/18 00:27, Mani, Rajmohan wrote:
> > Hi Mauro,
> >
> > Thanks for the reviews.
> >
> >> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> >>
> >> Hi Mauro,
> >>
> >> On Fri, Nov 2, 2018 at 10:49 PM Mauro Carvalho Chehab
> >>  wrote:
> >>>
> >>> Em Mon, 29 Oct 2018 15:22:57 -0700
> >>> Yong Zhi  escreveu:
> >> [snip]
> >>>> +struct ipu3_uapi_awb_config_s {
> >>>> + __u16 rgbs_thr_gr;
> >>>> + __u16 rgbs_thr_r;
> >>>> + __u16 rgbs_thr_gb;
> >>>> + __u16 rgbs_thr_b;
> >>>> + struct ipu3_uapi_grid_config grid; }
> >>>> +__attribute__((aligned(32))) __packed;
> >>>
> >>> Hmm... Kernel defines a macro for aligned attribute:
> >>>
> >>> include/linux/compiler_types.h:#define __aligned(x)
> >> __attribute__((aligned(x)))
> >>>
> >>
> >> First, thanks for review!
> >>
> >> Maybe I missed something, but last time I checked, it wasn't
> >> accessible from UAPI headers in userspace.
> >
> > Ack. We see that's still the case.
> >
> >>
> >>> I'm not a gcc expert, but it sounds weird to first ask it to align
> >>> with 32 bits and then have __packed (with means that pads should be
> >>> removed).
> >>>
> >>> In other words, I *guess* is it should either be __packed or
> >>> __aligned(32).
> >>>
> >>> Not that it would do any difference, in practice, as this specific
> >>> struct has a size with is multiple of 32 bits, but let's do the
> >>> right annotation here, not mixing two incompatible alignment
> requirements.
> >>>
> >>
> >> My understanding was that __packed makes the compiler not insert any
> >> alignment between particular fields of the struct, while __aligned
> >> makes the whole struct be aligned at given boundary, if placed in
> >> another struct. If I didn't miss anything, having both should make perfect
> sense here.
> >
> > Ack
> >
> > I also recall that as part of addressing review comments  (from Hans
> > and Sakari), on earlier versions of this patch series, we added
> > __packed attribute to all structs to ensure the size of the structs remains 
> > the
> same between 32 and 64 bit builds.
> >
> > The addition of structure members of the name padding[x] in some of
> > the structs ensures that respective members are aligned at 32 byte
> > boundaries, while the overall size of the structs remain the same between 32
> and 64 bit builds.
> 
> I recommend that this is documented in the header. It's not a common
> construction so an explanation will help.

Ack.

> 
> Regards,
> 
>   Hans
> 
> >
> > Thanks
> > Raj
> >
> >>
> >> Best regards,
> >> Tomasz



RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-16 Thread Zhi, Yong
Hi, Sakari,

Thanks for the thorough review.

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> Sent: Friday, November 2, 2018 8:03 AM
> To: Zhi, Yong 
> Cc: linux-media@vger.kernel.org; tf...@chromium.org;
> mche...@kernel.org; hans.verk...@cisco.com;
> laurent.pinch...@ideasonboard.com; Mani, Rajmohan
> ; Zheng, Jian Xu ; Hu,
> Jerry W ; Toivonen, Tuukka
> ; Qiu, Tian Shu ; Cao,
> Bingbu ; Li, Chao C 
> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> 
> Hi Yong,
> 
> Thanks for the update! I went through this again... a few comments below
> but I'd say they're mostly pretty minor issues.
> 
> On Mon, Oct 29, 2018 at 03:22:57PM -0700, Yong Zhi wrote:
> > These meta formats are used on Intel IPU3 ImgU video queues
> > to carry 3A statistics and ISP pipeline parameters.
> >
> > V4L2_META_FMT_IPU3_3A
> > V4L2_META_FMT_IPU3_PARAMS
> >
> > Signed-off-by: Yong Zhi 
> > Signed-off-by: Chao C Li 
> > Signed-off-by: Rajmohan Mani 
> > ---
> >  Documentation/media/uapi/v4l/meta-formats.rst  |1 +
> >  .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++
> >  include/uapi/linux/intel-ipu3.h| 2819 
> > 
> >  3 files changed, 3001 insertions(+)
> >  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-
> ipu3.rst
> >  create mode 100644 include/uapi/linux/intel-ipu3.h
> >
> > diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> b/Documentation/media/uapi/v4l/meta-formats.rst
> > index cf971d5..eafc534 100644
> > --- a/Documentation/media/uapi/v4l/meta-formats.rst
> > +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> > @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata`
> interface only.
> >  .. toctree::
> >  :maxdepth: 1
> >
> > +pixfmt-meta-intel-ipu3
> >  pixfmt-meta-d4xx
> >  pixfmt-meta-uvc
> >  pixfmt-meta-vsp1-hgo
> > 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 000..23b945b
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > @@ -0,0 +1,181 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _intel-ipu3:
> 
> Instead, to avoid a warning from Sphinx, replace the line with these:
> 
> .. _v4l2-meta-fmt-ipu3-params:
> .. _v4l2-meta-fmt-ipu3-stat-3a:
> 

Ack.

> > +
> >
> +***
> ***
> > +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A
> ('ip3s')
> >
> +***
> ***
> > +
> > +.. c:type:: ipu3_uapi_stats_3a
> > +
> > +3A statistics
> > +=
> > +
> > +For IPU3 ImgU, the 3A statistics accelerators collect different statistics 
> > over
> > +an input bayer frame. Those statistics, defined in data struct
> > +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu
> 3a stat"
> > +video node, which are then passed to user space for statistics analysis
> > +using :c:type:`v4l2_meta_format` interface.
> > +
> > +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green,
> Blue and
> 
> Extra whitespace at the end of the line.
> 

Ack.

> > +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter
> response,
> > +and AE (Auto-exposure) histogram.
> > +
> > +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all
> above.
> > +
> > +
> > +.. code-block:: c
> > +
> > +
> > + struct ipu3_uapi_stats_3a {
> > +   struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> > +__attribute__((aligned(32)));
> > +   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;
> 
> I think you could just unwrap these, even if it causes them to be over 80
> characters per line. They display better in a web browser that way. Or
> alternatively align the wrapped lines to the same column.
> 

Ack.

> > +   struct ipu3_u

Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-15 Thread Hans Verkuil
On 10/29/18 23:22, Yong Zhi wrote:
> These meta formats are used on Intel IPU3 ImgU video queues
> to carry 3A statistics and ISP pipeline parameters.
> 
> V4L2_META_FMT_IPU3_3A
> V4L2_META_FMT_IPU3_PARAMS
> 
> Signed-off-by: Yong Zhi 
> Signed-off-by: Chao C Li 
> Signed-off-by: Rajmohan Mani 
> ---
>  Documentation/media/uapi/v4l/meta-formats.rst  |1 +
>  .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++
>  include/uapi/linux/intel-ipu3.h| 2819 
> 
>  3 files changed, 3001 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
>  create mode 100644 include/uapi/linux/intel-ipu3.h
> 
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
> b/Documentation/media/uapi/v4l/meta-formats.rst
> index cf971d5..eafc534 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface 
> only.
>  .. toctree::
>  :maxdepth: 1
>  
> +pixfmt-meta-intel-ipu3
>  pixfmt-meta-d4xx
>  pixfmt-meta-uvc
>  pixfmt-meta-vsp1-hgo
> 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 000..23b945b
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> @@ -0,0 +1,181 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _intel-ipu3:
> +
> +**
> +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> +**
> +
> +.. c:type:: ipu3_uapi_stats_3a
> +
> +3A statistics
> +=
> +
> +For IPU3 ImgU, the 3A statistics accelerators collect different statistics 
> over
> +an input bayer frame. Those statistics, defined in data struct
> +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a 
> stat"

I'd rephrase this:

are obtained from the "ipu3-imgu 3a stat" metadata capture video node,

> +video node, which are then passed to user space for statistics analysis
> +using :c:type:`v4l2_meta_format` interface.
> +
> +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green, Blue 
> and 
> +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter 
> response,
> +and AE (Auto-exposure) histogram.
> +
> +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all 
> above.
> +
> +
> +.. code-block:: c
> +
> +
> + struct ipu3_uapi_stats_3a {
> + struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> +  __attribute__((aligned(32)));
> + 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;
> + } __packed;
> +
> +
> +.. c:type:: ipu3_uapi_params
> +
> +Pipeline parameters
> +===
> +
> +IPU3 pipeline has a number of image processing stages, each of which takes a
> +set of parameters as input. The major stages of pipelines are shown here:
> +
> +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> +
> +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> +
> +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> +
> +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> +
> +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> +
> +Correction -> XNR3 -> TNR -> DDR
> +
> +The table below presents a description of the above algorithms.
> +
> + 
> ===
> +Name  Description
> + 
> ===
> +Optical Black Correction Optical Black Correction block subtracts a 
> pre-defined
> +  value from the respective pixel values to obtain better
> +  image quality.
> +  Defined in :c:type:`ipu3_uapi_obgrid_param`.
> +Linearization This algo block uses linearization parameters 
> to
> +  address non-linearity sensor effects. The Lookup table
> +  table is defined in
> +  :c:type:`ipu3_uapi_isp_lin_vmem_params`.
> +SHD   Lens shading correction is used to correct spatial
> +  non-uniformity of the pixel response due to optical
> +  lens shading. This is done by applying a different gain
> +  

Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-15 Thread Hans Verkuil
On 11/07/18 00:27, Mani, Rajmohan wrote:
> Hi Mauro,
> 
> Thanks for the reviews.
> 
>> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
>>
>> Hi Mauro,
>>
>> On Fri, Nov 2, 2018 at 10:49 PM Mauro Carvalho Chehab
>>  wrote:
>>>
>>> Em Mon, 29 Oct 2018 15:22:57 -0700
>>> Yong Zhi  escreveu:
>> [snip]
>>>> +struct ipu3_uapi_awb_config_s {
>>>> + __u16 rgbs_thr_gr;
>>>> + __u16 rgbs_thr_r;
>>>> + __u16 rgbs_thr_gb;
>>>> + __u16 rgbs_thr_b;
>>>> + struct ipu3_uapi_grid_config grid; }
>>>> +__attribute__((aligned(32))) __packed;
>>>
>>> Hmm... Kernel defines a macro for aligned attribute:
>>>
>>> include/linux/compiler_types.h:#define __aligned(x)
>> __attribute__((aligned(x)))
>>>
>>
>> First, thanks for review!
>>
>> Maybe I missed something, but last time I checked, it wasn't accessible from
>> UAPI headers in userspace.
> 
> Ack. We see that's still the case.
> 
>>
>>> I'm not a gcc expert, but it sounds weird to first ask it to align
>>> with 32 bits and then have __packed (with means that pads should be
>>> removed).
>>>
>>> In other words, I *guess* is it should either be __packed or
>>> __aligned(32).
>>>
>>> Not that it would do any difference, in practice, as this specific
>>> struct has a size with is multiple of 32 bits, but let's do the right
>>> annotation here, not mixing two incompatible alignment requirements.
>>>
>>
>> My understanding was that __packed makes the compiler not insert any
>> alignment between particular fields of the struct, while __aligned makes the
>> whole struct be aligned at given boundary, if placed in another struct. If I
>> didn't miss anything, having both should make perfect sense here.
> 
> Ack
> 
> I also recall that as part of addressing review comments  (from Hans and 
> Sakari),
> on earlier versions of this patch series, we added __packed attribute to all 
> structs
> to ensure the size of the structs remains the same between 32 and 64 bit 
> builds.
> 
> The addition of structure members of the name padding[x] in some of the 
> structs
> ensures that respective members are aligned at 32 byte boundaries, while the
> overall size of the structs remain the same between 32 and 64 bit builds.

I recommend that this is documented in the header. It's not a common 
construction
so an explanation will help.

Regards,

Hans

> 
> Thanks
> Raj
> 
>>
>> Best regards,
>> Tomasz



RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-06 Thread Mani, Rajmohan
Hi Mauro,

Thanks for the reviews.

> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> 
> Hi Mauro,
> 
> On Fri, Nov 2, 2018 at 10:49 PM Mauro Carvalho Chehab
>  wrote:
> >
> > Em Mon, 29 Oct 2018 15:22:57 -0700
> > Yong Zhi  escreveu:
> [snip]
> > > +struct ipu3_uapi_awb_config_s {
> > > + __u16 rgbs_thr_gr;
> > > + __u16 rgbs_thr_r;
> > > + __u16 rgbs_thr_gb;
> > > + __u16 rgbs_thr_b;
> > > + struct ipu3_uapi_grid_config grid; }
> > > +__attribute__((aligned(32))) __packed;
> >
> > Hmm... Kernel defines a macro for aligned attribute:
> >
> > include/linux/compiler_types.h:#define __aligned(x)
> __attribute__((aligned(x)))
> >
> 
> First, thanks for review!
> 
> Maybe I missed something, but last time I checked, it wasn't accessible from
> UAPI headers in userspace.

Ack. We see that's still the case.

> 
> > I'm not a gcc expert, but it sounds weird to first ask it to align
> > with 32 bits and then have __packed (with means that pads should be
> > removed).
> >
> > In other words, I *guess* is it should either be __packed or
> > __aligned(32).
> >
> > Not that it would do any difference, in practice, as this specific
> > struct has a size with is multiple of 32 bits, but let's do the right
> > annotation here, not mixing two incompatible alignment requirements.
> >
> 
> My understanding was that __packed makes the compiler not insert any
> alignment between particular fields of the struct, while __aligned makes the
> whole struct be aligned at given boundary, if placed in another struct. If I
> didn't miss anything, having both should make perfect sense here.

Ack

I also recall that as part of addressing review comments  (from Hans and 
Sakari),
on earlier versions of this patch series, we added __packed attribute to all 
structs
to ensure the size of the structs remains the same between 32 and 64 bit builds.

The addition of structure members of the name padding[x] in some of the structs
ensures that respective members are aligned at 32 byte boundaries, while the
overall size of the structs remain the same between 32 and 64 bit builds.

Thanks
Raj

> 
> Best regards,
> Tomasz


RE: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-06 Thread Zhi, Yong
Hi, Mauro,

Thanks for your review.

> -Original Message-
> From: Mauro Carvalho Chehab [mailto:mchehab+sams...@kernel.org]
> Sent: Friday, November 2, 2018 6:49 AM
> To: Zhi, Yong 
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com;
> tf...@chromium.org; hans.verk...@cisco.com;
> laurent.pinch...@ideasonboard.com; Mani, Rajmohan
> ; Zheng, Jian Xu ; Hu,
> Jerry W ; Toivonen, Tuukka
> ; Qiu, Tian Shu ; Cao,
> Bingbu ; Li, Chao C 
> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
> 
> Em Mon, 29 Oct 2018 15:22:57 -0700
> Yong Zhi  escreveu:
> 
> > These meta formats are used on Intel IPU3 ImgU video queues
> > to carry 3A statistics and ISP pipeline parameters.
> 
> Just minor things. See below.
> 
> >
> > V4L2_META_FMT_IPU3_3A
> > V4L2_META_FMT_IPU3_PARAMS
> >
> > Signed-off-by: Yong Zhi 
> > Signed-off-by: Chao C Li 
> > Signed-off-by: Rajmohan Mani 
> > ---
> >  Documentation/media/uapi/v4l/meta-formats.rst  |1 +
> >  .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++
> 
> I would actually prefer to have those two changes merged together with
> patch 1, as it makes easier for review.
> 
> >  include/uapi/linux/intel-ipu3.h| 2819 
> > 
> 
> This one makes sense to have a separate patch.
> 

Ack, will re-group the three files as suggested.

> >  3 files changed, 3001 insertions(+)
> >  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-
> ipu3.rst
> >  create mode 100644 include/uapi/linux/intel-ipu3.h
> >
> > diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> b/Documentation/media/uapi/v4l/meta-formats.rst
> > index cf971d5..eafc534 100644
> > --- a/Documentation/media/uapi/v4l/meta-formats.rst
> > +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> > @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata`
> interface only.
> >  .. toctree::
> >  :maxdepth: 1
> >
> > +pixfmt-meta-intel-ipu3
> >  pixfmt-meta-d4xx
> >  pixfmt-meta-uvc
> >  pixfmt-meta-vsp1-hgo
> > 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 000..23b945b
> > --- /dev/null
> > +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> > @@ -0,0 +1,181 @@
> > +.. -*- coding: utf-8; mode: rst -*-
> > +
> > +.. _intel-ipu3:
> > +
> >
> +***
> ***
> > +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A
> ('ip3s')
> >
> +***
> ***
> > +
> > +.. c:type:: ipu3_uapi_stats_3a
> > +
> > +3A statistics
> > +=
> > +
> > +For IPU3 ImgU, the 3A statistics accelerators collect different statistics 
> > over
> > +an input bayer frame. Those statistics, defined in data struct
> > +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu
> 3a stat"
> > +video node, which are then passed to user space for statistics analysis
> > +using :c:type:`v4l2_meta_format` interface.
> > +
> > +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green,
> Blue and
> > +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter
> response,
> > +and AE (Auto-exposure) histogram.
> > +
> > +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all
> above.
> > +
> > +
> > +.. code-block:: c
> > +
> > +
> > + struct ipu3_uapi_stats_3a {
> > +   struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> > +__attribute__((aligned(32)));
> > +   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;
> > + } __packed;
> > +
> > +
> > +.. c:type:: ipu3_uapi_params
> > +
> > +Pipeline parameters
> > +===
> > +
> > +IPU3 pip

Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-02 Thread Tomasz Figa
Hi Mauro,

On Fri, Nov 2, 2018 at 10:49 PM Mauro Carvalho Chehab
 wrote:
>
> Em Mon, 29 Oct 2018 15:22:57 -0700
> Yong Zhi  escreveu:
[snip]
> > +struct ipu3_uapi_awb_config_s {
> > + __u16 rgbs_thr_gr;
> > + __u16 rgbs_thr_r;
> > + __u16 rgbs_thr_gb;
> > + __u16 rgbs_thr_b;
> > + struct ipu3_uapi_grid_config grid;
> > +} __attribute__((aligned(32))) __packed;
>
> Hmm... Kernel defines a macro for aligned attribute:
>
> include/linux/compiler_types.h:#define __aligned(x) 
> __attribute__((aligned(x)))
>

First, thanks for review!

Maybe I missed something, but last time I checked, it wasn't
accessible from UAPI headers in userspace.

> I'm not a gcc expert, but it sounds weird to first ask it to align
> with 32 bits and then have __packed (with means that pads should be
> removed).
>
> In other words, I *guess* is it should either be __packed
> or __aligned(32).
>
> Not that it would do any difference, in practice, as this
> specific struct has a size with is multiple of 32 bits, but
> let's do the right annotation here, not mixing two incompatible
> alignment requirements.
>

My understanding was that __packed makes the compiler not insert any
alignment between particular fields of the struct, while __aligned
makes the whole struct be aligned at given boundary, if placed in
another struct. If I didn't miss anything, having both should make
perfect sense here.

Best regards,
Tomasz


Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-02 Thread Mauro Carvalho Chehab
Em Mon, 29 Oct 2018 15:22:57 -0700
Yong Zhi  escreveu:

> These meta formats are used on Intel IPU3 ImgU video queues
> to carry 3A statistics and ISP pipeline parameters.

Just minor things. See below.

> 
> V4L2_META_FMT_IPU3_3A
> V4L2_META_FMT_IPU3_PARAMS
> 
> Signed-off-by: Yong Zhi 
> Signed-off-by: Chao C Li 
> Signed-off-by: Rajmohan Mani 
> ---
>  Documentation/media/uapi/v4l/meta-formats.rst  |1 +
>  .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++

I would actually prefer to have those two changes merged together with
patch 1, as it makes easier for review.

>  include/uapi/linux/intel-ipu3.h| 2819 
> 

This one makes sense to have a separate patch.

>  3 files changed, 3001 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
>  create mode 100644 include/uapi/linux/intel-ipu3.h
> 
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
> b/Documentation/media/uapi/v4l/meta-formats.rst
> index cf971d5..eafc534 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface 
> only.
>  .. toctree::
>  :maxdepth: 1
>  
> +pixfmt-meta-intel-ipu3
>  pixfmt-meta-d4xx
>  pixfmt-meta-uvc
>  pixfmt-meta-vsp1-hgo
> 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 000..23b945b
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> @@ -0,0 +1,181 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _intel-ipu3:
> +
> +**
> +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> +**
> +
> +.. c:type:: ipu3_uapi_stats_3a
> +
> +3A statistics
> +=
> +
> +For IPU3 ImgU, the 3A statistics accelerators collect different statistics 
> over
> +an input bayer frame. Those statistics, defined in data struct
> +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a 
> stat"
> +video node, which are then passed to user space for statistics analysis
> +using :c:type:`v4l2_meta_format` interface.
> +
> +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green, Blue 
> and 
> +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter 
> response,
> +and AE (Auto-exposure) histogram.
> +
> +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all 
> above.
> +
> +
> +.. code-block:: c
> +
> +
> + struct ipu3_uapi_stats_3a {
> + struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> +  __attribute__((aligned(32)));
> + 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;
> + } __packed;
> +
> +
> +.. c:type:: ipu3_uapi_params
> +
> +Pipeline parameters
> +===
> +
> +IPU3 pipeline has a number of image processing stages, each of which takes a
> +set of parameters as input. The major stages of pipelines are shown here:
> +
> +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> +
> +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> +
> +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> +
> +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> +
> +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> +
> +Correction -> XNR3 -> TNR -> DDR
> +
> +The table below presents a description of the above algorithms.
> +
> + 
> ===
> +Name  Description
> + 
> ===
> +Optical Black Correction Optical Black Correction block subtracts a 
> pre-defined
> +  value from the respective pixel values to obtain better
> +  image quality.
> +  Defined in :c:type:`ipu3_uapi_obgrid_param`.
> +Linearization This algo block uses linearization parameters 
> to
> +  address non-linearity sensor effects. The Lookup table
> +  table is defined in
> +  :c:type:`ipu3_uapi_isp_lin_vmem_params`.
> +SHD   Lens shading correction is used to correct spatial
> +  non-uniformity of 

Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-02 Thread Sakari Ailus
Hi Yong,

Thanks for the update! I went through this again... a few comments below
but I'd say they're mostly pretty minor issues.

On Mon, Oct 29, 2018 at 03:22:57PM -0700, Yong Zhi wrote:
> These meta formats are used on Intel IPU3 ImgU video queues
> to carry 3A statistics and ISP pipeline parameters.
> 
> V4L2_META_FMT_IPU3_3A
> V4L2_META_FMT_IPU3_PARAMS
> 
> Signed-off-by: Yong Zhi 
> Signed-off-by: Chao C Li 
> Signed-off-by: Rajmohan Mani 
> ---
>  Documentation/media/uapi/v4l/meta-formats.rst  |1 +
>  .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++
>  include/uapi/linux/intel-ipu3.h| 2819 
> 
>  3 files changed, 3001 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
>  create mode 100644 include/uapi/linux/intel-ipu3.h
> 
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
> b/Documentation/media/uapi/v4l/meta-formats.rst
> index cf971d5..eafc534 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface 
> only.
>  .. toctree::
>  :maxdepth: 1
>  
> +pixfmt-meta-intel-ipu3
>  pixfmt-meta-d4xx
>  pixfmt-meta-uvc
>  pixfmt-meta-vsp1-hgo
> 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 000..23b945b
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> @@ -0,0 +1,181 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _intel-ipu3:

Instead, to avoid a warning from Sphinx, replace the line with these:

.. _v4l2-meta-fmt-ipu3-params:
.. _v4l2-meta-fmt-ipu3-stat-3a:

> +
> +**
> +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> +**
> +
> +.. c:type:: ipu3_uapi_stats_3a
> +
> +3A statistics
> +=
> +
> +For IPU3 ImgU, the 3A statistics accelerators collect different statistics 
> over
> +an input bayer frame. Those statistics, defined in data struct
> +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a 
> stat"
> +video node, which are then passed to user space for statistics analysis
> +using :c:type:`v4l2_meta_format` interface.
> +
> +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green, Blue 
> and 

Extra whitespace at the end of the line.

> +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter 
> response,
> +and AE (Auto-exposure) histogram.
> +
> +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all 
> above.
> +
> +
> +.. code-block:: c
> +
> +
> + struct ipu3_uapi_stats_3a {
> + struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> +  __attribute__((aligned(32)));
> + 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;

I think you could just unwrap these, even if it causes them to be over 80
characters per line. They display better in a web browser that way. Or
alternatively align the wrapped lines to the same column.

> + struct ipu3_uapi_ff_status stats_3a_status;
> + } __packed;
> +
> +
> +.. c:type:: ipu3_uapi_params
> +
> +Pipeline parameters
> +===
> +
> +IPU3 pipeline has a number of image processing stages, each of which takes a
> +set of parameters as input. The major stages of pipelines are shown here:
> +
> +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> +
> +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> +
> +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> +
> +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> +
> +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> +
> +Correction -> XNR3 -> TNR -> DDR
> +
> +The table below presents a description of the above algorithms.
> +
> + 
> ===
> +Name  Description
> + 
> ===
> +Optical Black Correction Optical Black Correction block subtracts a 
> pre-defined
> +  value from the respective pixel values to obtain better
> +  image quality.
> +  Defined in :c:type:`ipu3_uapi_obgrid_param`.
> +Linearization This algo block uses linearization 

[PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-10-29 Thread Yong Zhi
These meta formats are used on Intel IPU3 ImgU video queues
to carry 3A statistics and ISP pipeline parameters.

V4L2_META_FMT_IPU3_3A
V4L2_META_FMT_IPU3_PARAMS

Signed-off-by: Yong Zhi 
Signed-off-by: Chao C Li 
Signed-off-by: Rajmohan Mani 
---
 Documentation/media/uapi/v4l/meta-formats.rst  |1 +
 .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++
 include/uapi/linux/intel-ipu3.h| 2819 
 3 files changed, 3001 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
 create mode 100644 include/uapi/linux/intel-ipu3.h

diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
b/Documentation/media/uapi/v4l/meta-formats.rst
index cf971d5..eafc534 100644
--- a/Documentation/media/uapi/v4l/meta-formats.rst
+++ b/Documentation/media/uapi/v4l/meta-formats.rst
@@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface only.
 .. toctree::
 :maxdepth: 1
 
+pixfmt-meta-intel-ipu3
 pixfmt-meta-d4xx
 pixfmt-meta-uvc
 pixfmt-meta-vsp1-hgo
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 000..23b945b
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
@@ -0,0 +1,181 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _intel-ipu3:
+
+**
+V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
+**
+
+.. c:type:: ipu3_uapi_stats_3a
+
+3A statistics
+=
+
+For IPU3 ImgU, the 3A statistics accelerators collect different statistics over
+an input bayer frame. Those statistics, defined in data struct
+:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a stat"
+video node, which are then passed to user space for statistics analysis
+using :c:type:`v4l2_meta_format` interface.
+
+The statistics collected are AWB (Auto-white balance) RGBS (Red, Green, Blue 
and 
+Saturation measure) cells, AWB filter response, AF (Auto-focus) filter 
response,
+and AE (Auto-exposure) histogram.
+
+struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all 
above.
+
+
+.. code-block:: c
+
+
+ struct ipu3_uapi_stats_3a {
+   struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
+__attribute__((aligned(32)));
+   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;
+ } __packed;
+
+
+.. c:type:: ipu3_uapi_params
+
+Pipeline parameters
+===
+
+IPU3 pipeline has a number of image processing stages, each of which takes a
+set of parameters as input. The major stages of pipelines are shown here:
+
+Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
+
+Linearization -> Lens Shading Correction -> White Balance / Exposure /
+
+Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
+
+Correction Matrix -> Gamma correction -> Color Space Conversion ->
+
+Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
+
+Correction -> XNR3 -> TNR -> DDR
+
+The table below presents a description of the above algorithms.
+
+ 
===
+NameDescription
+ 
===
+Optical Black Correction Optical Black Correction block subtracts a pre-defined
+value from the respective pixel values to obtain better
+image quality.
+Defined in :c:type:`ipu3_uapi_obgrid_param`.
+Linearization   This algo block uses linearization parameters to
+address non-linearity sensor effects. The Lookup table
+table is defined in
+:c:type:`ipu3_uapi_isp_lin_vmem_params`.
+SHD Lens shading correction is used to correct spatial
+non-uniformity of the pixel response due to optical
+lens shading. This is done by applying a different gain
+for each pixel. The gain, black level etc are
+configured in :c:type:`ipu3_uapi_shd_config_static`.
+BNR Bayer noise reduction block removes image noise by
+applying a bilateral filter.
+See :c:type:`ipu3_uapi_bnr_static_config` for details.