Hello Yong,
Thank you for the patch.
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
>
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> b/Documentation/media/uapi/v4l/meta-formats.rst index
> 438bd244bd2f..5f956fa784b7 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -19,6 +19,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
Please keep this list alphabetically sorted.
> 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
> @@ -0,0 +1,178 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _v4l2-meta-fmt-params:
> +.. _v4l2-meta-fmt-stat-3a:
> +
> +******************************************************************
> +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> +******************************************************************
> +
> +.. c:type:: ipu3_uapi_stats_3a
No need for c:type:: here, the structure is already properly defined in
drivers/staging/media/ipu3/include/intel-ipu3.h
> +3A statistics
> +=============
> +
> +For IPU3 ImgU, the 3A statistics accelerators collect different statistics
I'd write "The IPU3 ImgU 3A statistics accelerators collect" or "The IPU3 ImgU
includes 3A statistics accelerators that collect"
> over +an input bayer frame. Those statistics, defined in data struct
bayer should be spelled Bayer (here and below).
> :c:type:`ipu3_uapi_stats_3a`, +are obtained from "ipu3-imgu 3a stat"
> metadata capture video node, which are then +passed to user space for
> statistics analysis using :c:type:`v4l2_meta_format` interface.
How about simply
"Those statistics are obtained from the "ipu3-imgu [01] 3a stat" metadata
capture video nodes, using the :c:type:`v4l2_meta_format` interface. They are
formatted as described by the :c:type:`ipu3_uapi_stats_3a` structure."
> +
> +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.
Could you please wrap lines at the 80 columns boundary ?
> +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 ?
> +
> +.. 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
No need for c:type:: here either.
> +Pipeline parameters
> +===================
> +
> +IPU3 pipeline has a number of image processing stages, each of which takes
s/IPU3/The IPU3/
> 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
You can replace this list with
.. kernel-render:: DOT
:alt: IPU3 ImgU Pipeline
:caption: IPU3 ImgU Pipeline Diagram
digraph "IPU3 ImgU" {
node [shape=box]
splines="ortho"
rankdir="LR"
a [label="Raw pixels"]
b [label="Bayer Downscaling"]
c [label="Optical Black Correction"]
d [label="Linearization"]
e [label="Lens Shading Correction"]
f [label="White Balance / Exposure / Focus Apply"]
g [label="Bayer Noise Reduction"]
h [label="ANR"]
i [label="Demosaicing"]
j [label="Color Correction Matrix"]
k [label="Gamma correction"]
l [label="Color Space Conversion"]
m [label="Chroma Down Scaling"]
n [label="Chromatic Noise Reduction"]
o [label="Total Color Correction"]
p [label="XNR3"]
q [label="TNR"]
r [label="DDR"]
{ rank=same; a -> b -> c -> d -> e -> f }
{ rank=same; g -> h -> i -> j -> k -> l }
{ rank=same; m -> n -> o -> p -> q -> r }
a -> g -> m [style=invis, weight=10]
f -> g
l -> m
}
to get a nicer diagram.
> +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
> + 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.
> +ANR Advanced Noise Reduction is a block based algorithm
> + that performs noise reduction in the Bayer domain. The
> + convolution matrix etc can be found in
> + :c:type:`ipu3_uapi_anr_config`.
> +Demosaicing Demosaicing converts raw sensor data in Bayer format
> + into RGB (Red, Green, Blue) presentation. Then add
> + outputs of estimation of Y channel for following stream
> + processing by Firmware. The struct is defined as
> + :c:type:`ipu3_uapi_dm_config`. (TODO)
> +Color Correction Color Correction algo transforms sensor specific color
> + space to the standard "sRGB" color space. This is done
> + by applying 3x3 matrix defined in
> + :c:type:`ipu3_uapi_ccm_mat_config`.
> +Gamma correction Gamma correction :c:type:`ipu3_uapi_gamma_config` is a
> + basic non-linear tone mapping correction that is
> + applied per pixel for each pixel component.
> +CSC Color space conversion transforms each pixel from the
> + RGB primary presentation to YUV (Y: brightness,
> + UV: Luminance) presentation. This is done by applying
> + a 3x3 matrix defined in
> + :c:type:`ipu3_uapi_csc_mat_config`
> +CDS Chroma down sampling
> + After the CSC is performed, the Chroma Down Sampling
> + is applied for a UV plane down sampling by a factor
> + of 2 in each direction for YUV 4:2:0 using a 4x2
> + configurable filter :c:type:`ipu3_uapi_cds_params`.
> +CHNR Chroma noise reduction
> + This block processes only the chrominance pixels and
> + performs noise reduction by cleaning the high
> + frequency noise.
> + See struct :c:type:`ipu3_uapi_yuvp1_chnr_config`.
> +TCC Total color correction as defined in struct
> + :c:type:`ipu3_uapi_yuvp2_tcc_static_config`.
> +XNR3 eXtreme Noise Reduction V3 is the third revision of
> + noise reduction algorithm used to improve image
> + quality. This removes the low frequency noise in the
> + captured image. Two related structs are being defined,
> + :c:type:`ipu3_uapi_isp_xnr3_params` for ISP data memory
> + and :c:type:`ipu3_uapi_isp_xnr3_vmem_params` for vector
> + memory.
> +TNR Temporal Noise Reduction block compares successive
> + frames in time to remove anomalies / noise in pixel
> + values. :c:type:`ipu3_uapi_isp_tnr3_vmem_params` and
> + :c:type:`ipu3_uapi_isp_tnr3_params` are defined for ISP
> + vector and data memory respectively.
> +========================
> =======================================================
> +
> +A few stages of the pipeline will be executed by firmware running on the
> ISP +processor, while many others will use a set of fixed hardware blocks
> also +called accelerator cluster (ACC) to crunch pixel data and produce
> statistics.
> +
> +ACC parameters of individual algorithms, as defined by
> +:c:type:`ipu3_uapi_acc_param`, can be chosen to be applied by the user
> +space through struct :c:type:`ipu3_uapi_flags` embedded in
> +:c:type:`ipu3_uapi_params` structure. For parameters that are configured as
> +not enabled by the user space, the corresponding structs are ignored by
> the +driver, in which case the existing configuration of the algorithm will
> be +preserved.
> +
> +Both 3A statistics and pipeline parameters described here are closely tied
> to +the underlying camera sub-system (CSS) APIs. They are usually consumed
> and +produced by dedicated user space libraries that comprise the important
> tuning +tools, thus freeing the developers from being bothered with the low
> level +hardware and algorithm details.
> +
> +It should be noted that IPU3 DMA operations require the addresses of all
> data +structures (that includes both input and output) to be aligned on 32
> byte +boundaries.
I think most of the above (from the diagram to here) belongs to Documentation/
media/v4l-drivers/ipu3.rst. It can be referenced here, but this file should
focus on the description of the metadata formats, not on the description of
the IPU3 ImgU internals.
> +The meta data :c:type:`ipu3_uapi_params` will be sent to "ipu3-imgu
> parameters" +video node in ``V4L2_BUF_TYPE_META_CAPTURE`` format.
To be consistent with the statistics documentation, how about the following ?
"The pipeline parameters are passed to the "ipu3-imgu [01] parameters"
metadata output video nodes, using the :c:type:`v4l2_meta_format` interface.
They are formatted as described by the :c:type:`ipu3_uapi_params` structure."
> +.. code-block:: c
> +
> + struct ipu3_uapi_params {
> + /* Flags which of the settings below are to be applied */
> + struct ipu3_uapi_flags use;
> +
> + /* Accelerator cluster parameters */
> + struct ipu3_uapi_acc_param acc_param;
> +
> + /* ISP vector address space parameters */
> + struct ipu3_uapi_isp_lin_vmem_params lin_vmem_params;
> + struct ipu3_uapi_isp_tnr3_vmem_params tnr3_vmem_params;
> + struct ipu3_uapi_isp_xnr3_vmem_params xnr3_vmem_params;
> +
> + /* ISP data memory (DMEM) parameters */
> + struct ipu3_uapi_isp_tnr3_params tnr3_dmem_params;
> + struct ipu3_uapi_isp_xnr3_params xnr3_dmem_params;
> +
> + /* Optical black level compensation */
> + struct ipu3_uapi_obgrid_param obgrid_param;
> + };
> +
> +Intel IPU3 ImgU uAPI data types
> +===============================
> +
> +.. kernel-doc:: include/uapi/linux/intel-ipu3.h
This file has moved to drivers/staging/media/ipu3/include/intel-ipu3.h.
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c index a1806d3a1c41..0701cb8a03ef
> 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1300,6 +1300,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> case V4L2_META_FMT_VSP1_HGO: descr = "R-Car VSP1 1-D Histogram"; break;
> case V4L2_META_FMT_VSP1_HGT: descr = "R-Car VSP1 2-D Histogram"; break;
> case V4L2_META_FMT_UVC: descr = "UVC payload header metadata"; break;
> + case V4L2_META_FMT_IPU3_PARAMS: descr = "IPU3 processing parameters";
> break; + case V4L2_META_FMT_IPU3_STAT_3A: descr = "IPU3 3A statistics";
> break;
>
> default:
> /* Compressed formats */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index a9d47b1b9437..f2b973b36e29 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -721,6 +721,10 @@ struct v4l2_pix_format {
> #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC
> Payload Header metadata */ #define V4L2_META_FMT_D4XX
> v4l2_fourcc('D', '4', 'X', 'X') /* D4XX Payload Header metadata */
>
> +/* Vendor specific - used for IPU3 camera sub-system */
> +#define V4L2_META_FMT_IPU3_PARAMS v4l2_fourcc('i', 'p', '3', 'p') /* IPU3
> processing parameters */ +#define
> V4L2_META_FMT_IPU3_STAT_3A v4l2_fourcc('i', 'p', '3', 's') /* IPU3 3A
> statistics */ +
> /* priv field value to indicates that subsequent fields are valid. */
> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
--
Regards,
Laurent Pinchart
