Message-ID: <TYAPR01MB620180FF82B8993B45FF6E0A9286A@TYAPR01MB6201.jpnprd01.prod.outlook.com>
Date:   Mon, 4 Dec 2023 04:32:40 +0000
From:   <yuji2.ishikawa@...hiba.co.jp>
To:     <sakari.ailus@....fi>
CC:     <hverkuil@...all.nl>, <laurent.pinchart@...asonboard.com>,
        <mchehab@...nel.org>, <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
        <nobuhiro1.iwamatsu@...hiba.co.jp>, <linux-media@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v9 4/5] documentation: media: add documentation for
 Toshiba Visconti Video Input Interface driver

Hello Sakari,
Thank you for your review.

> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@....fi>
> Sent: Wednesday, November 22, 2023 8:40 PM
> To: ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開)
> <yuji2.ishikawa@...hiba.co.jp>
> Cc: Hans Verkuil <hverkuil@...all.nl>; Laurent Pinchart
> <laurent.pinchart@...asonboard.com>; Mauro Carvalho Chehab
> <mchehab@...nel.org>; Rob Herring <robh+dt@...nel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@...aro.org>; Conor Dooley
> <conor+dt@...nel.org>; iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○
> ＯＳＴ) <nobuhiro1.iwamatsu@...hiba.co.jp>; linux-media@...r.kernel.org;
> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v9 4/5] documentation: media: add documentation for
> Toshiba Visconti Video Input Interface driver
> 
> Hi Yuji,
> 
> Thank you for the update and my apologies for not reviewing this properly
> earlier. Please see my commens below.
> 
> On Thu, Oct 12, 2023 at 04:13:28PM +0900, Yuji Ishikawa wrote:
> > Added basic description of Video Input Interface driver of Toshiba
> > Visconti architecture.
> > It includes hardware organization, structure of the driver and
> > description of vendor specific V4L2 controls to configure the embedded
> > image signal processor.
> >
> > Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@...hiba.co.jp>
> > ---
> > Changelog v3:
> > - Newly add documentation to describe SW and HW
> >
> > Changelog v4:
> > - no change
> >
> > Changelog v5:
> > - no change
> >
> > Changelog v6:
> > - add description of CSI2RX subdevice
> > - add ordering of ioctl(S_FMT) and ioctl(S_EXT_CTRLS)
> >
> > Changelog v7:
> > - no change
> >
> > Changelog v8:
> > - add usage of V4L2_CTRL_TYPE_VISCONTI_ISP
> >
> > Changelog v9:
> > - fix warning: set reference target for keyword
> > V4L2_CTRL_TYPE_VISCONTI_ISP
> >
> >  .../driver-api/media/drivers/index.rst        |   1 +
> >  .../media/drivers/visconti-viif.rst           | 462
> ++++++++++++++++++
> >  .../media/v4l/vidioc-g-ext-ctrls.rst          |   4 +
> >  .../media/v4l/vidioc-queryctrl.rst            |   5 +
> >  .../media/videodev2.h.rst.exceptions          |   1 +
> >  5 files changed, 473 insertions(+)
> >  create mode 100644
> > Documentation/driver-api/media/drivers/visconti-viif.rst
> >
> > diff --git a/Documentation/driver-api/media/drivers/index.rst
> > b/Documentation/driver-api/media/drivers/index.rst
> > index c4123a16b5f9..5592bd99ae5c 100644
> > --- a/Documentation/driver-api/media/drivers/index.rst
> > +++ b/Documentation/driver-api/media/drivers/index.rst
> > @@ -24,6 +24,7 @@ Video4Linux (V4L) drivers
> >  	sh_mobile_ceu_camera
> >  	tuners
> >  	vimc-devel
> > +	visconti-viif
> >  	zoran
> >  	ccs/ccs
> >
> > diff --git a/Documentation/driver-api/media/drivers/visconti-viif.rst
> > b/Documentation/driver-api/media/drivers/visconti-viif.rst
> > new file mode 100644
> > index 000000000000..fd2480cbde46
> > --- /dev/null
> > +++ b/Documentation/driver-api/media/drivers/visconti-viif.rst
> > @@ -0,0 +1,462 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +============================================
> > +Visconti Video Input Interface (VIIF) Driver
> > +============================================
> > +
> > +Overview
> > +========
> > +
> > +The Visconti VIIF Hardware
> > +--------------------------
> > +
> > +The Visconti Video Input Interface (VIIF) hardware is  a proprietary
> videocapture device of Toshiba.
> > +Following function modules are integrated:
> > +
> > +* MIPI CSI2 receiver (CSI2RX)
> > +* L1 Image Signal Processor (L1ISP)
> > +
> > +  * Correction, enhancement, adjustment on RAW pictures.
> > +
> > +* L2 Image Signal Processor (L2ISP)
> > +
> > +  * Lens distortion correction
> > +  * Scaling
> > +  * Cropping
> > +
> > +* Video DMAC
> > +
> > +  * format picture (RGB, YUV, Grayscale, ...)
> > +  * write picture into DRAM
> > +
> > +Visconti5 SoC has two VIIF hardware instances.
> > +
> > +software architecture
> > +---------------------
> > +
> > +The Visconti VIIF driver is composed of following components:
> > +
> > +* (image sensor driver)
> > +* MIPI CSI2 receiver subdevice driver
> > +
> > +  * corresponding to CSI2RX
> > +
> > +* Visconti ISP subdevice driver
> > +
> > +  * corresponding to L1ISP, L2ISP (Lens distortion correction,
> > + Scaling)
> > +
> > +* Visconti Capture V4L2 device driver
> > +
> > +  * corresponding to L2ISP (Cropping) and Video DMAC
> > +  * multiple output videobuf queues
> > +
> > +    * main path0 (RGB, YUV, Grayscale, ...)
> > +    * main path1 (RGB, YUV, Grayscale, ...)
> > +    * sub path (RAW picture)
> > +
> > +::
> > +
> > +  +-----------+      +-----------+     +----------------+
> +-------------------------+
> > +  | Sensor    |      | CSI2RX    |     | ISP            |       |
> Capture MAIN PATH0      |
> > +  | subdevice | ---- | subdevice | --- | subdevice      | --+-- | V4L2 device
> |
> > +  | (IMX219)  |      | (CSI2RX)  |     | (L1ISP, L2ISP) |   |   | (L2ISP
> crop, VideoDMAC) |
> > +  +-----------+      +-----------+     +----------------+   |
> +-------------------------+
> > +                                                            |
> > +                                                            |
> +-------------------------+
> > +                                                            |   |
> Capture MAIN PATH1      |
> > +                                                            +-- |
> V4L2 device             |
> > +                                                            |   |
> (L2ISP crop, VideoDMAC) |
> > +                                                            |
> +-------------------------+
> > +                                                            |
> > +                                                            |
> +-------------------------+
> > +                                                            |   |
> Capture SUB PATH        |
> > +                                                            +-- |
> V4L2 device             |
> > +                                                                |
> (VideoDMAC)             |
> > +
> > + +-------------------------+
> > +
> > +
> > +The VIIF driver provides following device nodes for Visconti5 SoC:
> > +
> > +* VIIF0
> > +
> > +  * /dev/media0
> > +  * /dev/video0 (main path0)
> > +  * /dev/video1 (main path1)
> > +  * /dev/video2 (sub path)
> > +
> > +* VIIF1
> > +
> > +  * /dev/media1
> > +  * /dev/video3 (main path0)
> > +  * /dev/video4 (main path1)
> > +  * /dev/video5 (sub path)
> > +
> > +Use of coherent memory
> > +----------------------
> > +
> > +Visconti5 SoC has two independent DDR SDRAM controllers.
> > +Each controller is mapped to 36bit address space.
> > +
> > +Accelerator bus masters have two paths to access memory; one is
> > +directly connected to SDRAM controller, the another is connected via
> > +a cache coherency bus which keeps coherency among CPUs.
> > +
> > +From acclerators and CPUs, the address map is following:
> > +
> > +* 0x0_8000_0000 DDR0 direct access
> > +* 0x4_8000_0000 DDR0 coherency bus
> > +* 0x8_8000_0000 DDR1 direct access
> > +* 0xC_8000_0000 DDR1 coherency bus
> > +
> > +The base address can be specified with "memory" and "reserved-memory"
> > +elements in a device tree description.
> > +It's not recommended to mix direct address and coherent address.
> > +
> > +The Visconti5 VIIF driver always use only direct address to configure Video
> DMACs of the hardware.
> > +This design is to avoid great performance loss at coherency bus caused by
> massive memory access.
> > +You should not put the dma_coherent attribute to viif element in device
> tree.
> > +Cache operations are done automatically by videobuf2 driver.
> > +
> > +Tested environment
> > +------------------
> > +
> > +The Visconti VIIF driver was tested with following items:
> > +
> > +* IMX219 image sensor
> > +* IMX335 image sensor
> > +
> > +IOCTLs
> > +======
> > +
> > +Following public IOCTLs are supported
> > +
> > +* VIDIOC_QUERYCAP
> > +* VIDIOC_ENUM_FMT
> > +* VIDIOC_TRY_FMT
> > +* VIDIOC_S_FMT
> > +* VIDIOC_G_FMT
> > +* VIDIOC_ENUM_FRAMESIZES
> > +* VIDIOC_G_EXT_CTRLS
> > +* VIDIOC_S_EXT_CTRLS
> > +* VIDIOC_REQBUFS
> > +* VIDIOC_QUERYBUF
> > +* VIDIOC_QBUF
> > +* VIDIOC_EXPBUF
> > +* VIDIOC_DQBUF
> > +* VIDIOC_CREATE_BUFS
> > +* VIDIOC_PREPARE_BUF
> > +* VIDIOC_STREAMON
> > +* VIDIOC_STREAMOFF
> > +
> > +Vendor specific v4l2 controls
> > +(except for V4L2_CID_VISCONTI_VIIF_MAIN_SET_RAWPACK_MODE and
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_INPUT_MODE) should be called
> after
> > +ioctl(S_FMT) because setting the frame format may affect valid range
> > +of parameters of the controls.
> > +
> > +Vendor specific v4l2 controls
> > +=============================
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_MAIN_SET_RAWPACK_MODE:
> > +
> > +V4L2_CID_VISCONTI_VIIF_MAIN_SET_RAWPACK_MODE
> > +--------------------------------------------
> > +
> > +This control sets the format to pack multiple RAW pixel values into a word.
> > +
> > +This control accepts a __u32 value defined as `enum viif_rawpack_mode`.
> > +
> > +This control should be set before ioctl(S_FMT) and should not be changed
> after that.
> 
> Why do you need this? Doesn't the V4L2 pixelformat already explicitly define
> the format, including packing?
> 

This control value affects the unpacker between CSI2 receiver and ISP.
The rawpack mode is an option to receive from Wide-Dynamic-Range image sensors,
whose output bit width is larger than standard format (RAW20, RAW24).
For example, some WDR image sensors split 20 bit value into upper/lower 10bit values,
then put them into consective pixels to make a double-width RAW10 format image.

This option is only related to the interpretation of CSI2 input data.
Therefore, I think V4L2 pixel format (for resulting image) is not related to it.
The media bus format seems more relevant, but there is no corresponding definition to this case.

> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_INPUT_MODE:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_INPUT_MODE
> > +--------------------------------------------
> > +
> > +This control sets L1ISP preprocessing mode for RAW input images.
> > +
> > +This control accepts a `struct viif_l1_input_mode_config` instance.
> > +
> > +This control should be set before ioctl(S_FMT) and should not be changed
> after that.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_RGB_TO_Y_COEF:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_RGB_TO_Y_COEF
> > +-----------------------------------------------
> > +
> > +This control sets parameters to yield Y value from RGB pixel values.
> > +
> > +This control accepts a `struct viif_l1_rgb_to_y_coef_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AG_MODE:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AG_MODE
> > +-----------------------------------------
> > +
> > +This control sets rules of generating analog gains for each feature in L1ISP.
> > +Related features are:
> > +
> > +* Optical Black Clamp Correction (OBCC)
> > +* Defect Pixel Correction (DPC)
> > +* RAW Color Noise Reduction (RCNR)
> > +* Lens Shading Correction (LSC)
> > +* Color matrix correction (MPRO)
> > +* Image quality adjustment (VPRO)
> > +
> > +The base gain is set with V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AG
> control.
> > +
> > +This control accepts a `struct viif_l1_ag_mode_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AG:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AG
> > +------------------------------------
> > +
> > +This control sets base analog gain commonly used in L1ISP features.
> > +Analog gain for each L1ISP feature is generated from the base analog
> > +gain and a configuration via
> V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AG_MODE control.
> > +
> > +This control accepts a `struct viif_l1_ag_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRE:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRE
> > +--------------------------------------
> > +
> > +This controls sets parameters for HDR Expansion feature.
> > +
> > +This control accepts a `struct viif_l1_hdre_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_IMG_EXTRACTION:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_IMG_EXTRACTION
> > +------------------------------------------------
> > +
> > +This control sets black level parameters for L1ISP inputs.
> > +
> > +This control accepts a `struct viif_l1_img_extraction_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_DPC:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_DPC
> > +-------------------------------------
> > +
> > +This control sets parameters for Defect Pixel Correction.
> > +
> > +This control accepts a `struct viif_l1_dpc_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_PRESET_WHITE_BALANCE:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_PRESET_WHITE_BALANCE
> > +------------------------------------------------------
> > +
> > +This control sets parameters for white balance.
> > +
> > +This control accepts a `struct viif_l1_preset_white_balance_config`
> instance.
> > +
> > +..
> _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_RAW_COLOR_NOISE_REDUCTION:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_RAW_COLOR_NOISE_REDUCTION
> > +-----------------------------------------------------------
> > +
> > +This control sets parameters for RAW color noise reduction (RCNR) feature.
> > +
> > +This control accepts a `struct viif_l1_raw_color_noise_reduction_config`
> instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRS:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRS
> > +--------------------------------------
> > +
> > +This control sets parameters for HDR synthesis.
> > +
> > +This control accepts a `struct viif_l1_hdrs_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_BLACK_LEVEL_CORRECTION:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_BLACK_LEVEL_CORRECTION
> > +--------------------------------------------------------
> > +
> > +This control sets parameters for black level correction feature.
> > +
> > +This control accepts a `struct viif_l1_black_level_correction_config`
> instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_LSC:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_LSC
> > +-------------------------------------
> > +
> > +This control sets parameters for Lens Shading Correction feature.
> > +L1ISP supports 2 correction methods:
> > +
> > +* parabola shading
> > +* grid shading
> > +
> > +This control accepts a `struct viif_l1_lsc_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_MAIN_PROCESS:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_MAIN_PROCESS
> > +----------------------------------------------
> > +
> > +This controls sets parameter for the MAIN PROCESS feature which is
> composed of:
> > +
> > +* demosaic
> > +* color matrix correction
> > +
> > +This control accepts a `struct viif_l1_main_process_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AWB:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AWB
> > +-------------------------------------
> > +
> > +This control sets parameter for auto white balance feature.
> > +
> > +This control accepts a `struct viif_l1_awb_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_LOCK_AWB_GAIN:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_LOCK_AWB_GAIN
> > +-------------------------------------------
> > +
> > +This control requests enable/disable of lock for AWB gain.
> > +
> > +This control accepts a u32 value; 0 for disable lock, 1 for enable lock.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRC:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRC
> > +--------------------------------------
> > +
> > +This control sets parameter for HDR Compression feature.
> > +
> > +This control accepts a `struct viif_l1_hdrc_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRC_LTM:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_HDRC_LTM
> > +------------------------------------------
> > +
> > +This control sets parameter for HDR Compression Local Tone Mapping
> feature.
> > +
> > +This control accepts a `struct viif_l1_hdrc_ltm_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_GAMMA:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_GAMMA
> > +---------------------------------------
> > +
> > +This control sets parameter for gamma correction at L1ISP.
> > +
> > +This control accepts a `struct viif_l1_gamma_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_IMG_QUALITY_ADJUSTMENT:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_IMG_QUALITY_ADJUSTMENT
> > +--------------------------------------------------------
> > +
> > +This control sets parameter for VPRO feature which is composed of:
> > +
> > +* luminance adjustment:
> > +
> > + * brightness adjustment
> > + * linear contrast adjusment
> > + * nonlinear contrast adjustment
> > + * luminance noise reduction
> > + * edge enhancement
> > +
> > +* chroma adjustment:
> > +
> > + * chroma suppression
> > + * color level adjustment
> > + * chroma noise reduction
> > + * coring suppression
> > + * edge chroma suppression
> > + * color noise reduction
> > +
> > +This control accepts a `struct viif_l1_img_quality_adjustment_config`
> instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AVG_LUM_GENERATION:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L1_SET_AVG_LUM_GENERATION
> > +----------------------------------------------------
> > +
> > +This control sets parameter for average luminance statistics feature.
> > +
> > +This control accepts a `struct viif_l1_avg_lum_generation_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L2_SET_UNDIST:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L2_SET_UNDIST
> > +----------------------------------------
> > +
> > +This control sets parameter for the lens undistortion feature of L2ISP.
> > +Lens undistortion parameters are defined as either or combination of
> polinomial parameter and grid table.
> > +
> > +This control accepts a `struct viif_l2_undist_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L2_SET_ROI:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L2_SET_ROI
> > +-------------------------------------
> > +
> > +This control sets dimensions of intermediate images and scaling parameter
> of L2ISP.
> > +If you want to crop the output image, you should set crop parameter
> > +to the corresponding source pad of the ISP subdevice with media-ctl tool.
> > +
> > +This control accepts a `struct viif_l2_roi_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_ISP_L2_SET_GAMMA:
> > +
> > +V4L2_CID_VISCONTI_VIIF_ISP_L2_SET_GAMMA
> > +---------------------------------------
> > +
> > +This control sets gamma parameter for L2ISP.
> > +
> > +This control accepts a `struct viif_l2_gamma_config` instance.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_CSI2RX_GET_CALIBRATION_STATUS:
> > +
> > +V4L2_CID_VISCONTI_VIIF_CSI2RX_GET_CALIBRATION_STATUS
> > +----------------------------------------------------
> > +
> > +This control provides CSI2 receiver calibration status.
> > +
> > +This control fills a `struct viif_csi2rx_cal_status` instance with current
> status.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_CSI2RX_GET_ERR_STATUS:
> > +
> > +V4L2_CID_VISCONTI_VIIF_CSI2RX_GET_ERR_STATUS
> > +--------------------------------------------
> > +
> > +This control provides CSI2 receiver error description.
> > +
> > +This control fills a `struct viif_csi2rx_err_status` instance with accumerated
> error status.
> > +Note that internal accumerated status is cleared after reading.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_GET_LAST_CAPTURE_STATUS:
> > +
> > +V4L2_CID_VISCONTI_VIIF_GET_LAST_CAPTURE_STATUS
> > +----------------------------------------------
> > +
> > +This control provides status information for the last captured frame.
> > +
> > +This control fills a `struct viif_l1_info` instance with current status.
> > +
> > +.. _V4L2_CID_VISCONTI_VIIF_GET_REPORTED_ERRORS:
> > +
> > +V4L2_CID_VISCONTI_VIIF_GET_REPORTED_ERRORS
> > +------------------------------------------
> > +
> > +This control provides error information since the last read of this control.
> > +
> > +This control fills a `struct viif_reported_errors` instance with accumerated
> error status.
> > +Note that internal accumerated status is cleared after reading.
> > +
> > +Structures
> > +==========
> > +
> > +.. kernel-doc:: include/uapi/linux/visconti_viif.h
> 
> Instead of using controls, all ISP drivers merged within five or so years use a
> parameter buffer. This could be said as being the best known approach to pass
> the parameters to the ISP. Could you do the same here?
> 

I'll try implement parameter buffer.

> > +
> > diff --git
> > a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > index f9f73530a6be..2f0e4f955167 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > @@ -295,6 +295,10 @@ still cause this situation.
> >        - ``p_av1_film_grain``
> >        - A pointer to a struct :c:type:`v4l2_ctrl_av1_film_grain`. Valid if this
> control is
> >          of type ``V4L2_CTRL_TYPE_AV1_FILM_GRAIN``.
> > +    * - void *
> > +      - ``ptr``
> > +      - A pointer to Visconti ISP control specific structure. Valid if this
> control is
> > +        of type ``V4L2_CTRL_TYPE_VISCONTI_ISP``
> >      * - void *
> >        - ``ptr``
> >        - A pointer to a compound type which can be an N-dimensional
> > array diff --git
> > a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > index 4d38acafe8e1..cd57fac8bbf9 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> > @@ -549,6 +549,11 @@ See also the examples in :ref:`control`.
> >        - n/a
> >        - A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film
> Grain
> >          parameters for stateless video decoders.
> > +    * - ``V4L2_CTRL_TYPE_VISCONTI_ISP``
> > +      - n/a
> > +      - n/a
> > +      - n/a
> > +      - Structs specific for Visconti ISP configuration controls.
> >
> >  .. raw:: latex
> >
> > diff --git
> > a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > index 3e58aac4ef0b..dbab7527445e 100644
> > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > @@ -165,6 +165,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE
> > :c:type:`v4l2_ctrl_type`  replace symbol
> > V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type`
> replace
> > symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type`  replace
> > symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type`
> > +replace symbol V4L2_CTRL_TYPE_VISCONTI_ISP :c:type:`v4l2_ctrl_type`
> >
> >  # V4L2 capability defines
> >  replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities
> 
> --
> Kind regards,
> 
> Sakari Ailus

Regards,
Yuji

Powered by blists - more mailing lists