mbox series

[v3,00/15] Add MFC v12 support.

Message ID 20221011122516.32135-1-aakarsh.jain@samsung.com
Headers show
Series Add MFC v12 support. | expand

Message

Aakarsh Jain Oct. 11, 2022, 12:25 p.m. UTC
This patch series adds MFC v12 support. MFC v12 is used in
Tesla FSD SoC.

This adds support for following:

* Add support for VP9 encoder
* Add support for YV12 and I420 format (3-plane)
* Add support for Rate Control, UHD and DMABUF for encoder
* Add support for DPB buffers allocation based on MFC requirement
* Update Documentation for control id definitions

Changes since v2:
 - Addressed review comments by Rob Herring.
 - Addressed review comments by Krzysztof Kozlowski.
 - Addressed review comments by Andi Shyti.

Smitha T Murthy (15):
  dt-bindings: media: s5p-mfc: Add new DT schema for MFC
  dt-bindings: media: s5p-mfc: Add mfcv12 variant
  media: s5p-mfc: Rename IS_MFCV10 macro
  media: s5p-mfc: Add initial support for MFCv12
  Documention: v4l: Documentation for VP9 CIDs.
  media: v4l2: Add v4l2 control IDs for VP9 encoder.
  media: s5p-mfc: Add support for VP9 encoder.
  media: s5p-mfc: Add YV12 and I420 multiplanar format support
  media: s5p-mfc: Add support for rate controls in MFCv12
  media: s5p-mfc: Add support for UHD encoding.
  media: s5p-mfc: Add support for DMABUF for encoder
  media: s5p-mfc: Set context for valid case before calling try_run
  media: s5p-mfc: Load firmware for each run in MFCv12.
  media: s5p-mfc: DPB Count Independent of VIDIOC_REQBUF
  arm64: dts: fsd: Add MFC related DT enteries

 .../devicetree/bindings/media/s5p-mfc.txt     |  75 ---------
 .../bindings/media/samsung,s5p-mfc.yaml       | 164 ++++++++++++++++++
 .../media/v4l/ext-ctrls-codec.rst             | 167 +++++++
 arch/arm64/boot/dts/tesla/fsd.dtsi            |  21 +
 .../platform/samsung/s5p-mfc/regs-mfc-v12.h   |  60 +++
 .../platform/samsung/s5p-mfc/regs-mfc-v7.h    |   1 +
 .../platform/samsung/s5p-mfc/regs-mfc-v8.h    |   3 +
 .../media/platform/samsung/s5p-mfc/s5p_mfc.c  |  36 +-
 .../platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c |   3 +
 .../platform/samsung/s5p-mfc/s5p_mfc_common.h |  56 ++-
 .../platform/samsung/s5p-mfc/s5p_mfc_ctrl.c   |   9 +-
 .../platform/samsung/s5p-mfc/s5p_mfc_dec.c    |  51 ++-
 .../platform/samsung/s5p-mfc/s5p_mfc_enc.c    | 410 +++++++++++++++--
 .../platform/samsung/s5p-mfc/s5p_mfc_opr.h    |  16 +-
 .../platform/samsung/s5p-mfc/s5p_mfc_opr_v5.c |  12 +-
 .../platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c | 435 ++++++++++++++++--
 .../platform/samsung/s5p-mfc/s5p_mfc_opr_v6.h |   7 +-
 drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  44 ++
 include/uapi/linux/v4l2-controls.h            |  33 ++
 19 files changed, 1349 insertions(+), 203 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
 create mode 100644 drivers/media/platform/samsung/s5p-mfc/regs-mfc-v12.h

Comments

Aakarsh Jain Oct. 12, 2022, 4:01 a.m. UTC | #1
Hi All,

Please find the attached v4l2-compliance complete log(mfc encoder and decoder) along with this mail for reference.

Thanks,
Aakarsh

> -----Original Message-----
> From: aakarsh jain [mailto:aakarsh.jain@samsung.com]
> Sent: 11 October 2022 17:55
> To: linux-arm-kernel@lists.infradead.org; linux-media@vger.kernel.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> ezequiel@vanguardiasur.com.ar; jernej.skrabec@gmail.com;
> benjamin.gaignard@collabora.com; stanimir.varbanov@linaro.org;
> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
> mark.rutland@arm.com; robh+dt@kernel.org; krzk+dt@kernel.org;
> andi@etezian.org; alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com;
> smitha.t@samsung.com; aakarsh.jain@samsung.com
> Subject: [Patch v3 00/15] Add MFC v12 support.
> 
> This patch series adds MFC v12 support. MFC v12 is used in Tesla FSD SoC.
> 
> This adds support for following:
> 
> * Add support for VP9 encoder
> * Add support for YV12 and I420 format (3-plane)
> * Add support for Rate Control, UHD and DMABUF for encoder
> * Add support for DPB buffers allocation based on MFC requirement
> * Update Documentation for control id definitions
> 
> Changes since v2:
>  - Addressed review comments by Rob Herring.
>  - Addressed review comments by Krzysztof Kozlowski.
>  - Addressed review comments by Andi Shyti.
> 
> Smitha T Murthy (15):
>   dt-bindings: media: s5p-mfc: Add new DT schema for MFC
>   dt-bindings: media: s5p-mfc: Add mfcv12 variant
>   media: s5p-mfc: Rename IS_MFCV10 macro
>   media: s5p-mfc: Add initial support for MFCv12
>   Documention: v4l: Documentation for VP9 CIDs.
>   media: v4l2: Add v4l2 control IDs for VP9 encoder.
>   media: s5p-mfc: Add support for VP9 encoder.
>   media: s5p-mfc: Add YV12 and I420 multiplanar format support
>   media: s5p-mfc: Add support for rate controls in MFCv12
>   media: s5p-mfc: Add support for UHD encoding.
>   media: s5p-mfc: Add support for DMABUF for encoder
>   media: s5p-mfc: Set context for valid case before calling try_run
>   media: s5p-mfc: Load firmware for each run in MFCv12.
>   media: s5p-mfc: DPB Count Independent of VIDIOC_REQBUF
>   arm64: dts: fsd: Add MFC related DT enteries
> 
>  .../devicetree/bindings/media/s5p-mfc.txt     |  75 ---------
>  .../bindings/media/samsung,s5p-mfc.yaml       | 164 ++++++++++++++++++
>  .../media/v4l/ext-ctrls-codec.rst             | 167 +++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi            |  21 +
>  .../platform/samsung/s5p-mfc/regs-mfc-v12.h   |  60 +++
>  .../platform/samsung/s5p-mfc/regs-mfc-v7.h    |   1 +
>  .../platform/samsung/s5p-mfc/regs-mfc-v8.h    |   3 +
>  .../media/platform/samsung/s5p-mfc/s5p_mfc.c  |  36 +-
>  .../platform/samsung/s5p-mfc/s5p_mfc_cmd_v6.c |   3 +
>  .../platform/samsung/s5p-mfc/s5p_mfc_common.h |  56 ++-
>  .../platform/samsung/s5p-mfc/s5p_mfc_ctrl.c   |   9 +-
>  .../platform/samsung/s5p-mfc/s5p_mfc_dec.c    |  51 ++-
>  .../platform/samsung/s5p-mfc/s5p_mfc_enc.c    | 410 +++++++++++++++--
>  .../platform/samsung/s5p-mfc/s5p_mfc_opr.h    |  16 +-
>  .../platform/samsung/s5p-mfc/s5p_mfc_opr_v5.c |  12 +-
> .../platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c | 435
> ++++++++++++++++--
>  .../platform/samsung/s5p-mfc/s5p_mfc_opr_v6.h |   7 +-
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  44 ++
>  include/uapi/linux/v4l2-controls.h            |  33 ++
>  19 files changed, 1349 insertions(+), 203 deletions(-)  create mode 100644
> Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
>  create mode 100644 drivers/media/platform/samsung/s5p-mfc/regs-mfc-
> v12.h
> 
> --
> 2.17.1
# v4l2-compliance -d /dev/video1
v4l2-compliance 1.22.1, 64 bits, 64-bit time_t

Compliance test for s5p-mfc device /dev/video1:

Dr[   95.014797] vidioc_g_parm:2576: Setting FPS is only possible for the output queue
[   95.022670] s5p-mfc 12880000.mfc: Encoding not initialised
[   95.022728] s5p-mfc 12880000.mfc: Encoding not initialised
[   95.022812] vidioc_g_parm:2576: Setting FPS is only possible for the output queue
[   95.022871] vidioc_try_fmt:1607: failed to try output format
[   95.047169] s5p_mfc_queue_setup:2690: invalid state: 0
[   95.047181] vidioc_reqbufs:1725: error in vb2_reqbufs() for E(D)
iver Info:
        Driver name      : s5p-mfc
        Card type        : s5p-mfc-enc
        Bus info         : platform:12880000.mfc
        Driver version   : 5.19.0
        Capabilities     : 0x84204000
                Video Memory-to-Memory Multiplanar
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x04204000
                Video Memory-to-Memory Multiplanar
                Streaming
                Extended Pix Format
        Detected Stateful Encoder

Required ioctls:
        test VIDIOC_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/video1 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
                fail: v4l2-compliance.cpp(736): !ok
        test for unlimited opens: FAIL

Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
        test VIDIOC_QUERYCTRL: OK
                fail: v4l2-test-controls.cpp(473): g_ctrl returned an error (22)
        test VIDIOC_G/S_CTRL: FAIL
                fail: v4l2-test-controls.cpp(704): g_ext_ctrls returned an error (22)
        test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
                fail: v4l2-test-controls.cpp(872): subscribe event for control 'User Controls' failed
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 128 Private Controls: 11

Format ioctls:
                fail: v4l2-test-formats.cpp(282): node->codec_mask & STATEFUL_ENCODER
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
                fail: v4l2-test-formats.cpp(1310): is_stateful_enc && !out->capability
        test VIDIOC_G/S_PARM: FAIL
        test VIDIOC_G_FBUF: OK (Not Supported)
                fail: v4l2-test-formats.cpp(474): !pix_mp.width || !pix_mp.height
        test VIDIOC_G_FMT: FAIL
                fail: v4l2-test-formats.cpp(474): !pix_mp.width || !pix_mp.height
        test VIDIOC_TRY_FMT: FAIL
                warn: v4l2-test-formats.cpp(1147): S_FMT cannot handle an invalid pixelformat.
                warn: v4l2-test-formats.cpp(1148): This may or may not be a problem. For more information see:
                warn: v4l2-test-formats.cpp(1149): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html
                fail: v4l2-test-formats.cpp(478): pixelformat 34363248 (H264) for buftype 9 not reported by ENUM_FMT
        test VIDIOC_S_FMT: FAIL
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls:
                fail: v4l2-test-codecs.cpp(35): node->function != MEDIA_ENT_F_PROC_VIDEO_ENCODER
        test VIDIOC_(TRY_)ENCODER_CMD: FAIL
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK (Not Suppor[   95.403655] vidioc_g_parm:2576: Setting FPS is only possible for the output queue
ted)

Buffer ioctls:
                fail: v4l2-test-buffers.cpp(607): q.reqbufs(node, 1)
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
                fail: v4l2-test-buffers.cpp(783): VIDIOC_EXPBUF is supported, but the V4L2_MEMORY_MMAP support is missing or malfunctioning.
                fail: v4l2-test-buffers.cpp(784): VIDIOC_EXPBUF is supported, but the V4L2_MEMORY_MMAP support is missing, probably due to earlier failing format tests.
        test VIDIOC_EXPBUF: OK (Not Supported)
        test Requests: OK (Not Supported)

Total for s5p-mfc device /dev/video1: 45, Succeeded: 34, Failed: 11, Warnings: 3
#


# v4l2-compliance -d /dev/video0
v4l2-compliance 1.22.1, 64 bits, 64-bit time_t

Compliance test for s5p-mfc device /dev/video0:

Drive[  198.767611] vidioc_g_selection:816: Can not get compose information
[  198.768087] vidioc_g_selection:816: Can not get compose information
[  198.768175] vidioc_g_fmt:397: Format could not be read
[  198.768179] vidioc_g_selection:816: Can not get compose information
[  198.768182] vidioc_g_selection:816: Can not get compose information
[  198.768448] s5p-mfc 12880000.mfc: Decoding not initialised
[  198.768469] s5p-mfc 12880000.mfc: Decoding not initialised
[  198.768610] vidioc_g_fmt:397: Format could not be read
[  198.768640] vidioc_g_selection:816: Can not get compose information
[  198.768643] vidioc_g_selection:816: Can not get compose information
[  198.768646] vidioc_g_selection:816: Can not get compose information
[  198.768648] vidioc_g_selection:816: Can not get compose information
[  198.768650] vidioc_g_selection:816: Can not get compose information
[  198.768658] vidioc_g_selection:816: Can not get compose information
[  198.768731] vidioc_g_selection:816: Can not get compose information
[  198.768760] vidioc_g_selection:816: Can not get compose information
[  198.768837] vidioc_g_selection:816: Can not get compose information
[  198.768861] vidioc_g_selection:816: Can not get compose information
[  198.768866] vidioc_try_fmt:429: Unsupported format for destination.
[  198.768894] vidioc_g_selection:816: Can not get compose information
[  198.768915] vidioc_g_selection:816: Can not get compose information
[  198.768917] vidioc_try_fmt:429: Unsupported format for destination.
[  198.768939] vidioc_g_selection:816: Can not get compose information
r Info:
        Driver name      : s5p-mfc
        Card type        : s5p-mfc-dec
        Bus info         : platform:12880000.mfc
        Driver version   : 5.19.0
        Capabilities     : 0x84204000
                Video Memory-to-Memory Multiplanar
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x04204000
                Video Memory-to-Memory Multiplanar
                Streaming
                Extended Pix Format
        Detected Stateful Decoder

Required ioctls:
        test VIDIOC_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/video0 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
                fail: v4l2-compliance.cpp(736): !ok
        test for unlimited opens: FAIL

Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
        test VIDIOC_QUERYCTRL: OK
                fail: v4l2-test-controls.cpp(473): g_ctrl returned an error (22)
        test VIDIOC_G/S_CTRL: FAIL
                fail: v4l2-test-controls.cpp(704): g_ext_ctrls returned an error (22)
        test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
                fail: v4l2-test-controls.cpp(872): subscribe event for control 'User Controls' failed
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 7 Private Controls: 2

Format ioctls:
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
                fail: v4l2-test-formats.cpp(620): Video Capture Multiplanar cap set, but no Video Capture Multiplanar formats defined
        test VIDIOC_G_FMT: FAIL
        test VIDIOC_TRY_FMT: OK (Not Supported)
        test VIDIOC_S_FMT: OK (Not Supported)
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls:
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
                fail: v4l2-test-codecs.cpp(104): node->function != MEDIA_ENT_F_PROC_VIDEO_DECODER
        test VIDIOC_(TRY_)DECODER_CMD: FAIL

Buffer ioctls:
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK (Not Supported)
        test Requests: OK (Not Supported)

Total for s5p-mfc device /dev/video0: 45, Succeeded: 39, Failed: 6, Warnings: 0
#
Nicolas Dufresne Oct. 18, 2022, 3:05 p.m. UTC | #2
Hi,

thanks for your patch, very minor comment below.

Le mardi 11 octobre 2022 à 17:55 +0530, aakarsh jain a écrit :
> From: Smitha T Murthy <smitha.t@samsung.com>
> 
> Adds V4l2 controls for VP9 encoder documention.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
>  1 file changed, 167 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 2a165ae063fb..2277d83a7cf0 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
>      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
>        - Profile 3
>  
> +VP9 Control Reference
> +---------------------
> +
> +The VP9 controls include controls for encoding parameters of VP9 video
> +codec.
> +
> +.. _vp9-control-id:
> +
> +VP9 Control IDs
> +
>  .. _v4l2-mpeg-video-vp9-profile:
>  
>  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
>      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
>        - Level 6.2
>  
> +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> +    Quantization parameter for an I frame for VP9. Valid range: from 1 to 255.
> +
> +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> +    Quantization parameter for an P frame for VP9. Valid range: from 1 to 255.
> +
> +``V4L2_CID_CODEC_VP9_MAX_QP``
> +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> +    Recommended range for MFC is from 230 to 255.

We don't usually want every single HW to be documented in the generic part of
the documentation. The range supported by the HW should be found at run-time I
suppose, by querying the control. Would that work for you to remove the MFC
specifics here and in other controls ?

> +
> +``V4L2_CID_CODEC_VP9_MIN_QP``
> +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> +    Recommended range for MFC is from 1 to 24.
> +
> +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> +    Indicates the number of evenly spaced subintervals, called ticks, within
> +    one second. This is a 16 bit unsigned integer and has a maximum value up to
> +    0xffff and a minimum value of 1.
> +
> +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> +    Indicates the refresh period of the golden frame for VP9 encoder.
> +
> +.. _v4l2-vp9-golden-frame-sel:
> +
> +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> +    (enum)
> +
> +enum v4l2_mpeg_vp9_golden_framesel -
> +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> +    Possible values are:
> +
> +.. raw:: latex
> +
> +    \footnotesize
> +
> +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> +      - Use the (n-2)th frame as a golden frame, current frame index being
> +        'n'.
> +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> +      - Use the previous specific frame indicated by
> +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> +        golden frame.
> +
> +.. raw:: latex
> +
> +    \normalsize
> +
> +
> +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> +    Allows host to specify the quantization parameter values for each
> +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> +    value to 1 enables setting of the QP values for the layers.
> +
> +.. _v4l2-vp9-ref-number-of-pframes:
> +
> +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> +    (enum)
> +
> +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> +    Number of reference pictures for encoding P frames.
> +
> +.. raw:: latex
> +
> +    \footnotesize
> +
> +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> +      - Indicates one reference frame, last encoded frame will be searched.
> +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> +      - Indicates 2 reference frames, last encoded frame and golden frame
> +        will be searched.
> +
> +.. raw:: latex
> +
> +    \normalsize
> +
> +
> +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> +    Indicates the number of hierarchial coding layer.
> +    In normal encoding (non-hierarchial coding), it should be zero.
> +    VP9 has upto 3 layer of encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> +    Indicates quantization parameter for hierarchical coding layer 0.
> +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> +    V4L2_CID_CODEC_VP9_MAX_QP].
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> +    Indicates quantization parameter for hierarchical coding layer 1.
> +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> +    V4L2_CID_CODEC_VP9_MAX_QP].
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> +    Indicates quantization parameter for hierarchical coding layer 2.
> +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> +    V4L2_CID_CODEC_VP9_MAX_QP].
> +
> +.. _v4l2-vp9-max-partition-depth:
> +
> +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> +    (enum)
> +
> +enum v4l2_mpeg_vp9_num_partitions -
> +    Indicate maximum coding unit depth.
> +
> +.. raw:: latex
> +
> +    \footnotesize
> +
> +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> +      - No coding unit partition depth.
> +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> +      - Allows one coding unit partition depth.
> +
> +.. raw:: latex
> +
> +    \normalsize
> +
> +
> +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> +    Zero indicates enable intra NxN PU split.
> +    One indicates disable intra NxN PU split.
> +
> +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> +    Indicates IVF header generation. Zero indicates enable IVF format.
> +    One indicates disable IVF format.
> +
>  
>  High Efficiency Video Coding (HEVC/H.265) Control Reference
>  ===========================================================
Aakarsh Jain Oct. 21, 2022, 5:26 a.m. UTC | #3
> -----Original Message-----
> From: Nicolas Dufresne [mailto:nicolas@ndufresne.ca]
> Sent: 18 October 2022 20:36
> To: aakarsh jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> ezequiel@vanguardiasur.com.ar; jernej.skrabec@gmail.com;
> benjamin.gaignard@collabora.com; stanimir.varbanov@linaro.org;
> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
> mark.rutland@arm.com; robh+dt@kernel.org; krzk+dt@kernel.org;
> andi@etezian.org; alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> 
> Hi,
> 
> thanks for your patch, very minor comment below.
> 
> Le mardi 11 octobre 2022 à 17:55 +0530, aakarsh jain a écrit :
> > From: Smitha T Murthy <smitha.t@samsung.com>
> >
> > Adds V4l2 controls for VP9 encoder documention.
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> >  1 file changed, 167 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 2a165ae063fb..2277d83a7cf0 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> >        - Profile 3
> >
> > +VP9 Control Reference
> > +---------------------
> > +
> > +The VP9 controls include controls for encoding parameters of VP9
> > +video codec.
> > +
> > +.. _vp9-control-id:
> > +
> > +VP9 Control IDs
> > +
> >  .. _v4l2-mpeg-video-vp9-profile:
> >
> >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> >        - Level 6.2
> >
> > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> > +    Quantization parameter for an I frame for VP9. Valid range: from 1 to
> 255.
> > +
> > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > +    Quantization parameter for an P frame for VP9. Valid range: from 1 to
> 255.
> > +
> > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> > +    Recommended range for MFC is from 230 to 255.
> 
> We don't usually want every single HW to be documented in the generic part
> of the documentation. The range supported by the HW should be found at
> run-time I suppose, by querying the control. Would that work for you to
> remove the MFC specifics here and in other controls ?
> 
ok. will remove it in next series.
> > +
> > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> > +    Recommended range for MFC is from 1 to 24.
> > +
> > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > +    Indicates the number of evenly spaced subintervals, called ticks, within
> > +    one second. This is a 16 bit unsigned integer and has a maximum value
> up to
> > +    0xffff and a minimum value of 1.
> > +
> > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > +
> > +.. _v4l2-vp9-golden-frame-sel:
> > +
> > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_golden_framesel -
> > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> > +    Possible values are:
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > +      - Use the (n-2)th frame as a golden frame, current frame index being
> > +        'n'.
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > +      - Use the previous specific frame indicated by
> > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > +        golden frame.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > +    Allows host to specify the quantization parameter values for each
> > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> > +    value to 1 enables setting of the QP values for the layers.
> > +
> > +.. _v4l2-vp9-ref-number-of-pframes:
> > +
> > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > +    Number of reference pictures for encoding P frames.
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > +      - Indicates one reference frame, last encoded frame will be searched.
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > +      - Indicates 2 reference frames, last encoded frame and golden frame
> > +        will be searched.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > +    Indicates the number of hierarchial coding layer.
> > +    In normal encoding (non-hierarchial coding), it should be zero.
> > +    VP9 has upto 3 layer of encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 0.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 1.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 2.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +.. _v4l2-vp9-max-partition-depth:
> > +
> > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_num_partitions -
> > +    Indicate maximum coding unit depth.
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > +      - No coding unit partition depth.
> > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > +      - Allows one coding unit partition depth.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > +    Zero indicates enable intra NxN PU split.
> > +    One indicates disable intra NxN PU split.
> > +
> > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > +    One indicates disable IVF format.
> > +
> >
> >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> >
> ==========================================================
> =
Thanks for the review.
Hans Verkuil Nov. 24, 2022, 11:23 a.m. UTC | #4
On 11/10/2022 14:25, aakarsh jain wrote:
> From: Smitha T Murthy <smitha.t@samsung.com>
> 
> Adds V4l2 controls for VP9 encoder documention.
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
>  1 file changed, 167 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 2a165ae063fb..2277d83a7cf0 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
>      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
>        - Profile 3
>  
> +VP9 Control Reference

This is wrong. There is a VPX Control Reference section for both VP8 and VP9
controls. That's where this should be added. I suspect several of the controls
you are adding here already exist, e.g. V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The
documentation may have to be updated to specify that it is for both VP8 and VP9.

> +---------------------
> +
> +The VP9 controls include controls for encoding parameters of VP9 video
> +codec.
> +
> +.. _vp9-control-id:
> +
> +VP9 Control IDs
> +
>  .. _v4l2-mpeg-video-vp9-profile:
>  
>  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
>      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
>        - Level 6.2
>  
> +``V4L2_CID_CODEC_VP9_I_FRAME_QP``

If you do need to add new controls, then please use the same MPEG_VIDEO_ prefix.
It's a bit ugly and historical, but let's keep it consistent with the others.

Regards,

	Hans

> +    Quantization parameter for an I frame for VP9. Valid range: from 1 to 255.
> +
> +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> +    Quantization parameter for an P frame for VP9. Valid range: from 1 to 255.
> +
> +``V4L2_CID_CODEC_VP9_MAX_QP``
> +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> +    Recommended range for MFC is from 230 to 255.
> +
> +``V4L2_CID_CODEC_VP9_MIN_QP``
> +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> +    Recommended range for MFC is from 1 to 24.
> +
> +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> +    Indicates the number of evenly spaced subintervals, called ticks, within
> +    one second. This is a 16 bit unsigned integer and has a maximum value up to
> +    0xffff and a minimum value of 1.
> +
> +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> +    Indicates the refresh period of the golden frame for VP9 encoder.
> +
> +.. _v4l2-vp9-golden-frame-sel:
> +
> +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> +    (enum)
> +
> +enum v4l2_mpeg_vp9_golden_framesel -
> +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> +    Possible values are:
> +
> +.. raw:: latex
> +
> +    \footnotesize
> +
> +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> +      - Use the (n-2)th frame as a golden frame, current frame index being
> +        'n'.
> +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> +      - Use the previous specific frame indicated by
> +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> +        golden frame.
> +
> +.. raw:: latex
> +
> +    \normalsize
> +
> +
> +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> +    Allows host to specify the quantization parameter values for each
> +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> +    value to 1 enables setting of the QP values for the layers.
> +
> +.. _v4l2-vp9-ref-number-of-pframes:
> +
> +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> +    (enum)
> +
> +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> +    Number of reference pictures for encoding P frames.
> +
> +.. raw:: latex
> +
> +    \footnotesize
> +
> +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> +      - Indicates one reference frame, last encoded frame will be searched.
> +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> +      - Indicates 2 reference frames, last encoded frame and golden frame
> +        will be searched.
> +
> +.. raw:: latex
> +
> +    \normalsize
> +
> +
> +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> +    Indicates the number of hierarchial coding layer.
> +    In normal encoding (non-hierarchial coding), it should be zero.
> +    VP9 has upto 3 layer of encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> +    Indicates quantization parameter for hierarchical coding layer 0.
> +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> +    V4L2_CID_CODEC_VP9_MAX_QP].
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> +    Indicates quantization parameter for hierarchical coding layer 1.
> +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> +    V4L2_CID_CODEC_VP9_MAX_QP].
> +
> +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> +    Indicates quantization parameter for hierarchical coding layer 2.
> +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> +    V4L2_CID_CODEC_VP9_MAX_QP].
> +
> +.. _v4l2-vp9-max-partition-depth:
> +
> +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> +    (enum)
> +
> +enum v4l2_mpeg_vp9_num_partitions -
> +    Indicate maximum coding unit depth.
> +
> +.. raw:: latex
> +
> +    \footnotesize
> +
> +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> +      - No coding unit partition depth.
> +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> +      - Allows one coding unit partition depth.
> +
> +.. raw:: latex
> +
> +    \normalsize
> +
> +
> +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> +    Zero indicates enable intra NxN PU split.
> +    One indicates disable intra NxN PU split.
> +
> +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> +    Indicates IVF header generation. Zero indicates enable IVF format.
> +    One indicates disable IVF format.
> +
>  
>  High Efficiency Video Coding (HEVC/H.265) Control Reference
>  ===========================================================
Aakarsh Jain Dec. 9, 2022, 6:49 a.m. UTC | #5
> -----Original Message-----
> From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl]
> Sent: 24 November 2022 16:54
> To: aakarsh jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> david.plowman@raspberrypi.com; mark.rutland@arm.com;
> robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> 
> On 11/10/2022 14:25, aakarsh jain wrote:
> > From: Smitha T Murthy <smitha.t@samsung.com>
> >
> > Adds V4l2 controls for VP9 encoder documention.
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> >  1 file changed, 167 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 2a165ae063fb..2277d83a7cf0 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> >        - Profile 3
> >
> > +VP9 Control Reference
> 
> This is wrong. There is a VPX Control Reference section for both VP8 and VP9
> controls. That's where this should be added. I suspect several of the controls
> you are adding here already exist, e.g.
> V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The documentation may have to be
> updated to specify that it is for both VP8 and VP9.
> 
okay. I will add all VP9 Control IDs in VPX Control reference section.
> > +---------------------
> > +
> > +The VP9 controls include controls for encoding parameters of VP9
> > +video codec.
> > +
> > +.. _vp9-control-id:
> > +
> > +VP9 Control IDs
> > +
> >  .. _v4l2-mpeg-video-vp9-profile:
> >
> >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> >        - Level 6.2
> >
> > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> 
> If you do need to add new controls, then please use the same
> MPEG_VIDEO_ prefix.
> It's a bit ugly and historical, but let's keep it consistent with the others.
> 
But initially you recommended to use CODEC instead of MPEG_VIDEO.
https://patchwork.kernel.org/project/linux-media/patch/20220517125548.14746-7-smitha.t@samsung.com/

Anyway I will rename from CODEC to MPEG_VIDEO again.

> Regards,
> 
> 	Hans
> 
> > +    Quantization parameter for an I frame for VP9. Valid range: from 1 to
> 255.
> > +
> > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > +    Quantization parameter for an P frame for VP9. Valid range: from 1 to
> 255.
> > +
> > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> > +    Recommended range for MFC is from 230 to 255.
> > +
> > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> > +    Recommended range for MFC is from 1 to 24.
> > +
> > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > +    Indicates the number of evenly spaced subintervals, called ticks, within
> > +    one second. This is a 16 bit unsigned integer and has a maximum value
> up to
> > +    0xffff and a minimum value of 1.
> > +
> > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > +
> > +.. _v4l2-vp9-golden-frame-sel:
> > +
> > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_golden_framesel -
> > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> > +    Possible values are:
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > +      - Use the (n-2)th frame as a golden frame, current frame index being
> > +        'n'.
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > +      - Use the previous specific frame indicated by
> > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > +        golden frame.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > +    Allows host to specify the quantization parameter values for each
> > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> > +    value to 1 enables setting of the QP values for the layers.
> > +
> > +.. _v4l2-vp9-ref-number-of-pframes:
> > +
> > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > +    Number of reference pictures for encoding P frames.
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > +      - Indicates one reference frame, last encoded frame will be searched.
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > +      - Indicates 2 reference frames, last encoded frame and golden frame
> > +        will be searched.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > +    Indicates the number of hierarchial coding layer.
> > +    In normal encoding (non-hierarchial coding), it should be zero.
> > +    VP9 has upto 3 layer of encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 0.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 1.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 2.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +.. _v4l2-vp9-max-partition-depth:
> > +
> > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_num_partitions -
> > +    Indicate maximum coding unit depth.
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > +      - No coding unit partition depth.
> > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > +      - Allows one coding unit partition depth.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > +    Zero indicates enable intra NxN PU split.
> > +    One indicates disable intra NxN PU split.
> > +
> > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > +    One indicates disable IVF format.
> > +
> >
> >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> >
> ==========================================================
> =
Thanks for the review.
Aakarsh Jain Dec. 14, 2022, 10:22 a.m. UTC | #6
> -----Original Message-----
> From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl]
> Sent: 24 November 2022 16:54
> To: aakarsh jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> david.plowman@raspberrypi.com; mark.rutland@arm.com;
> robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> 
> On 11/10/2022 14:25, aakarsh jain wrote:
> > From: Smitha T Murthy <smitha.t@samsung.com>
> >
> > Adds V4l2 controls for VP9 encoder documention.
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> >  1 file changed, 167 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 2a165ae063fb..2277d83a7cf0 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> >        - Profile 3
> >
> > +VP9 Control Reference
> 
> This is wrong. There is a VPX Control Reference section for both VP8 and VP9
> controls. That's where this should be added. I suspect several of the controls
> you are adding here already exist, e.g.
> V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The documentation may have to be
> updated to specify that it is for both VP8 and VP9.
> 
Since MFC has different profiles, different quantization parameter ranges for both VP8 and VP9. So we can't use same control ID's for both.
So for example in VP8 with control ID (V4L2_CID_MPEG_VIDEO_VPX_MIN_QP), QP ranges from 0-11 and in VP9 with control ID  (V4L2_CID_CODEC_VP9_MIN_QP) QP ranges from 1-24. So we can't club together into single control. 

> > +---------------------
> > +
> > +The VP9 controls include controls for encoding parameters of VP9
> > +video codec.
> > +
> > +.. _vp9-control-id:
> > +
> > +VP9 Control IDs
> > +
> >  .. _v4l2-mpeg-video-vp9-profile:
> >
> >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> >        - Level 6.2
> >
> > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> 
> If you do need to add new controls, then please use the same
> MPEG_VIDEO_ prefix.
> It's a bit ugly and historical, but let's keep it consistent with the others.
> 
> Regards,
> 
> 	Hans
> 
> > +    Quantization parameter for an I frame for VP9. Valid range: from 1 to
> 255.
> > +
> > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > +    Quantization parameter for an P frame for VP9. Valid range: from 1 to
> 255.
> > +
> > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> > +    Recommended range for MFC is from 230 to 255.
> > +
> > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> > +    Recommended range for MFC is from 1 to 24.
> > +
> > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > +    Indicates the number of evenly spaced subintervals, called ticks, within
> > +    one second. This is a 16 bit unsigned integer and has a maximum value
> up to
> > +    0xffff and a minimum value of 1.
> > +
> > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > +
> > +.. _v4l2-vp9-golden-frame-sel:
> > +
> > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_golden_framesel -
> > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> > +    Possible values are:
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > +      - Use the (n-2)th frame as a golden frame, current frame index being
> > +        'n'.
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > +      - Use the previous specific frame indicated by
> > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > +        golden frame.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > +    Allows host to specify the quantization parameter values for each
> > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> > +    value to 1 enables setting of the QP values for the layers.
> > +
> > +.. _v4l2-vp9-ref-number-of-pframes:
> > +
> > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > +    Number of reference pictures for encoding P frames.
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > +      - Indicates one reference frame, last encoded frame will be searched.
> > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > +      - Indicates 2 reference frames, last encoded frame and golden frame
> > +        will be searched.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > +    Indicates the number of hierarchial coding layer.
> > +    In normal encoding (non-hierarchial coding), it should be zero.
> > +    VP9 has upto 3 layer of encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 0.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 1.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > +    Indicates quantization parameter for hierarchical coding layer 2.
> > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > +
> > +.. _v4l2-vp9-max-partition-depth:
> > +
> > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > +    (enum)
> > +
> > +enum v4l2_mpeg_vp9_num_partitions -
> > +    Indicate maximum coding unit depth.
> > +
> > +.. raw:: latex
> > +
> > +    \footnotesize
> > +
> > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > +
> > +.. flat-table::
> > +    :header-rows:  0
> > +    :stub-columns: 0
> > +
> > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > +      - No coding unit partition depth.
> > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > +      - Allows one coding unit partition depth.
> > +
> > +.. raw:: latex
> > +
> > +    \normalsize
> > +
> > +
> > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > +    Zero indicates enable intra NxN PU split.
> > +    One indicates disable intra NxN PU split.
> > +
> > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > +    One indicates disable IVF format.
> > +
> >
> >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> >
> ==========================================================
> =
Nicolas Dufresne Dec. 16, 2022, 5:21 p.m. UTC | #7
Le mercredi 14 décembre 2022 à 15:52 +0530, Aakarsh Jain a écrit :
> 
> > -----Original Message-----
> > From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl]
> > Sent: 24 November 2022 16:54
> > To: aakarsh jain <aakarsh.jain@samsung.com>; linux-arm-
> > kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> > kernel@vger.kernel.org; devicetree@vger.kernel.org
> > Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> > mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> > jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> > stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> > david.plowman@raspberrypi.com; mark.rutland@arm.com;
> > robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> > alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> > pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> > Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> > 
> > On 11/10/2022 14:25, aakarsh jain wrote:
> > > From: Smitha T Murthy <smitha.t@samsung.com>
> > > 
> > > Adds V4l2 controls for VP9 encoder documention.
> > > 
> > > Cc: linux-fsd@tesla.com
> > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > > ---
> > >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> > >  1 file changed, 167 insertions(+)
> > > 
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > index 2a165ae063fb..2277d83a7cf0 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> > >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> > >        - Profile 3
> > > 
> > > +VP9 Control Reference
> > 
> > This is wrong. There is a VPX Control Reference section for both VP8 and VP9
> > controls. That's where this should be added. I suspect several of the controls
> > you are adding here already exist, e.g.
> > V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The documentation may have to be
> > updated to specify that it is for both VP8 and VP9.
> > 
> Since MFC has different profiles, different quantization parameter ranges for both VP8 and VP9. So we can't use same control ID's for both.
> So for example in VP8 with control ID (V4L2_CID_MPEG_VIDEO_VPX_MIN_QP), QP ranges from 0-11 and in VP9 with control ID  (V4L2_CID_CODEC_VP9_MIN_QP) QP ranges from 1-24. So we can't club together into single control.
> 

V4L2_CID_MPEG_VIDEO_VPX_PROFILE has been deprecated, and replace with menu
controls. So we now have a V4L2_CID_MPEG_VIDEO_VP8_PROFILE and a
V4L2_CID_MPEG_VIDEO_VP9_PROFILE as menues. Newly written drivers should use
these. I see that GStreamer notably has never been ported, I'll fix it.

When you implement a driver, the generic uAPI will cover all possible items, as
menus (a integer was an API mistake made in 2011, hence the deprecation). You
driver can then select which menu items it support, and its server at telling
userspace what this HW supports. Though, this should be no problem if you want
to keep the old CID for backward compat, since the range is just totally
undefined there.

For V4L2_CID_MPEG_VIDEO_VPX_MIN_QP (and friends), the doc says "Minimum
quantization parameter for VP8.". A bit strange for a supposedly VPX parameter.
But its defines in the code as "VPX Minimum QP Value". Clearly something to be
fixed. There is no VP9 encoder drivers yet in mainline.

Though, the range for these controls is driver defined. In Venus, for VP8:


        v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
                V4L2_CID_MPEG_VIDEO_VPX_MIN_QP, 1, 128, 1, 1);

It seems to be 1 to 128. While in MFC, it oddly 1 to 11:


        {
                .id = V4L2_CID_MPEG_VIDEO_VPX_MIN_QP,
                .type = V4L2_CTRL_TYPE_INTEGER,
                .minimum = 0,
                .maximum = 11,
                .step = 1,
                .default_value = 0,
        },

While I'm not a huge fan of this, since we all know QP does not scale linearly,
this is how it is, and this is kind of part of the kernel API now. So userspace
must ask the driver what is the QP range, and adapt. And in your case, you
should have no issue adding VP9 encoder with a 1 to 24 range (even if this is a
bit odd and hw specific).

Nicolas


> > > +---------------------
> > > +
> > > +The VP9 controls include controls for encoding parameters of VP9
> > > +video codec.
> > > +
> > > +.. _vp9-control-id:
> > > +
> > > +VP9 Control IDs
> > > +
> > >  .. _v4l2-mpeg-video-vp9-profile:
> > > 
> > >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> > >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> > >        - Level 6.2
> > > 
> > > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> > 
> > If you do need to add new controls, then please use the same
> > MPEG_VIDEO_ prefix.
> > It's a bit ugly and historical, but let's keep it consistent with the others.
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > > +    Quantization parameter for an I frame for VP9. Valid range: from 1 to
> > 255.
> > > +
> > > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > > +    Quantization parameter for an P frame for VP9. Valid range: from 1 to
> > 255.
> > > +
> > > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > > +    Maximum quantization parameter for VP9. Valid range: from 1 to 255.
> > > +    Recommended range for MFC is from 230 to 255.
> > > +
> > > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > > +    Minimum quantization parameter for VP9. Valid range: from 1 to 255.
> > > +    Recommended range for MFC is from 1 to 24.
> > > +
> > > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > > +    Indicates the number of evenly spaced subintervals, called ticks, within
> > > +    one second. This is a 16 bit unsigned integer and has a maximum value
> > up to
> > > +    0xffff and a minimum value of 1.
> > > +
> > > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > > +
> > > +.. _v4l2-vp9-golden-frame-sel:
> > > +
> > > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > > +    (enum)
> > > +
> > > +enum v4l2_mpeg_vp9_golden_framesel -
> > > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is 2.
> > > +    Possible values are:
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \footnotesize
> > > +
> > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +
> > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > > +      - Use the (n-2)th frame as a golden frame, current frame index being
> > > +        'n'.
> > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > +      - Use the previous specific frame indicated by
> > > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > > +        golden frame.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \normalsize
> > > +
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > > +    Allows host to specify the quantization parameter values for each
> > > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the control
> > > +    value to 1 enables setting of the QP values for the layers.
> > > +
> > > +.. _v4l2-vp9-ref-number-of-pframes:
> > > +
> > > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > > +    (enum)
> > > +
> > > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > > +    Number of reference pictures for encoding P frames.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \footnotesize
> > > +
> > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +
> > > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > > +      - Indicates one reference frame, last encoded frame will be searched.
> > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > +      - Indicates 2 reference frames, last encoded frame and golden frame
> > > +        will be searched.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \normalsize
> > > +
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > > +    Indicates the number of hierarchial coding layer.
> > > +    In normal encoding (non-hierarchial coding), it should be zero.
> > > +    VP9 has upto 3 layer of encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > > +    Indicates enabling of bit rate for hierarchical coding layers VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > > +    Indicates quantization parameter for hierarchical coding layer 0.
> > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > > +    Indicates quantization parameter for hierarchical coding layer 1.
> > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > +
> > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > > +    Indicates quantization parameter for hierarchical coding layer 2.
> > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > +
> > > +.. _v4l2-vp9-max-partition-depth:
> > > +
> > > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > > +    (enum)
> > > +
> > > +enum v4l2_mpeg_vp9_num_partitions -
> > > +    Indicate maximum coding unit depth.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \footnotesize
> > > +
> > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > +
> > > +.. flat-table::
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +
> > > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > > +      - No coding unit partition depth.
> > > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > > +      - Allows one coding unit partition depth.
> > > +
> > > +.. raw:: latex
> > > +
> > > +    \normalsize
> > > +
> > > +
> > > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > > +    Zero indicates enable intra NxN PU split.
> > > +    One indicates disable intra NxN PU split.
> > > +
> > > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > > +    One indicates disable IVF format.
> > > +
> > > 
> > >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> > > 
> > ==========================================================
> > =
> 
>
Aakarsh Jain Dec. 21, 2022, 9:56 a.m. UTC | #8
> -----Original Message-----
> From: Nicolas Dufresne [mailto:nicolas@ndufresne.ca]
> Sent: 16 December 2022 22:51
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Hans Verkuil' <hverkuil-
> cisco@xs4all.nl>; linux-arm-kernel@lists.infradead.org; linux-
> media@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> david.plowman@raspberrypi.com; mark.rutland@arm.com;
> robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> 
> Le mercredi 14 décembre 2022 à 15:52 +0530, Aakarsh Jain a écrit :
> >
> > > -----Original Message-----
> > > From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl]
> > > Sent: 24 November 2022 16:54
> > > To: aakarsh jain <aakarsh.jain@samsung.com>; linux-arm-
> > > kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; devicetree@vger.kernel.org
> > > Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> > > mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> > > jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> > > stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> > > david.plowman@raspberrypi.com; mark.rutland@arm.com;
> > > robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> > > alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> > > pankaj.dubey@samsung.com; linux-fsd@tesla.com;
> smitha.t@samsung.com
> > > Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9
> CIDs.
> > >
> > > On 11/10/2022 14:25, aakarsh jain wrote:
> > > > From: Smitha T Murthy <smitha.t@samsung.com>
> > > >
> > > > Adds V4l2 controls for VP9 encoder documention.
> > > >
> > > > Cc: linux-fsd@tesla.com
> > > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > > > ---
> > > >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> > > >  1 file changed, 167 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > index 2a165ae063fb..2277d83a7cf0 100644
> > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> > > >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> > > >        - Profile 3
> > > >
> > > > +VP9 Control Reference
> > >
> > > This is wrong. There is a VPX Control Reference section for both VP8
> > > and VP9 controls. That's where this should be added. I suspect
> > > several of the controls you are adding here already exist, e.g.
> > > V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The documentation may have
> to be
> > > updated to specify that it is for both VP8 and VP9.
> > >
> > Since MFC has different profiles, different quantization parameter ranges
> for both VP8 and VP9. So we can't use same control ID's for both.
> > So for example in VP8 with control ID
> (V4L2_CID_MPEG_VIDEO_VPX_MIN_QP), QP ranges from 0-11 and in VP9
> with control ID  (V4L2_CID_CODEC_VP9_MIN_QP) QP ranges from 1-24. So
> we can't club together into single control.
> >
> 
> V4L2_CID_MPEG_VIDEO_VPX_PROFILE has been deprecated, and replace
> with menu controls. So we now have a
> V4L2_CID_MPEG_VIDEO_VP8_PROFILE and a
> V4L2_CID_MPEG_VIDEO_VP9_PROFILE as menues. Newly written drivers
> should use these. I see that GStreamer notably has never been ported, I'll fix
> it.
> 
> When you implement a driver, the generic uAPI will cover all possible items,
> as menus (a integer was an API mistake made in 2011, hence the
> deprecation). You driver can then select which menu items it support, and its
> server at telling userspace what this HW supports. Though, this should be no
> problem if you want to keep the old CID for backward compat, since the
> range is just totally undefined there.
> 
> For V4L2_CID_MPEG_VIDEO_VPX_MIN_QP (and friends), the doc says
> "Minimum quantization parameter for VP8.". A bit strange for a supposedly
> VPX parameter.
> But its defines in the code as "VPX Minimum QP Value". Clearly something to
> be fixed. There is no VP9 encoder drivers yet in mainline.
> 
> Though, the range for these controls is driver defined. In Venus, for VP8:
> 
> 
>         v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
>                 V4L2_CID_MPEG_VIDEO_VPX_MIN_QP, 1, 128, 1, 1);
> 
> It seems to be 1 to 128. While in MFC, it oddly 1 to 11:
> 
> 
>         {
>                 .id = V4L2_CID_MPEG_VIDEO_VPX_MIN_QP,
>                 .type = V4L2_CTRL_TYPE_INTEGER,
>                 .minimum = 0,
>                 .maximum = 11,
>                 .step = 1,
>                 .default_value = 0,
>         },
> 
> While I'm not a huge fan of this, since we all know QP does not scale linearly,
> this is how it is, and this is kind of part of the kernel API now. So userspace
> must ask the driver what is the QP range, and adapt. And in your case, you
> should have no issue adding VP9 encoder with a 1 to 24 range (even if this is a
> bit odd and hw specific).
> 
> Nicolas
> 

So all controls which I am using in VP9 similar to VPX will implement that as menu control. Will not touch VPX controls for backward compatibility.
Also will change all remaining VP9 controls implementation from Integer to menu control. 

Will this be fine ?

> 
> > > > +---------------------
> > > > +
> > > > +The VP9 controls include controls for encoding parameters of VP9
> > > > +video codec.
> > > > +
> > > > +.. _vp9-control-id:
> > > > +
> > > > +VP9 Control IDs
> > > > +
> > > >  .. _v4l2-mpeg-video-vp9-profile:
> > > >
> > > >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > > > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> > > >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> > > >        - Level 6.2
> > > >
> > > > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> > >
> > > If you do need to add new controls, then please use the same
> > > MPEG_VIDEO_ prefix.
> > > It's a bit ugly and historical, but let's keep it consistent with the others.
> > >
> > > Regards,
> > >
> > > 	Hans
> > >
> > > > +    Quantization parameter for an I frame for VP9. Valid range:
> > > > + from 1 to
> > > 255.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > > > +    Quantization parameter for an P frame for VP9. Valid range:
> > > > +from 1 to
> > > 255.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > > > +    Maximum quantization parameter for VP9. Valid range: from 1 to
> 255.
> > > > +    Recommended range for MFC is from 230 to 255.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > > > +    Minimum quantization parameter for VP9. Valid range: from 1 to
> 255.
> > > > +    Recommended range for MFC is from 1 to 24.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > > > +    Indicates the number of evenly spaced subintervals, called ticks,
> within
> > > > +    one second. This is a 16 bit unsigned integer and has a
> > > > +maximum value
> > > up to
> > > > +    0xffff and a minimum value of 1.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > > > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > > > +
> > > > +.. _v4l2-vp9-golden-frame-sel:
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > > > +    (enum)
> > > > +
> > > > +enum v4l2_mpeg_vp9_golden_framesel -
> > > > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is
> 2.
> > > > +    Possible values are:
> > > > +
> > > > +.. raw:: latex
> > > > +
> > > > +    \footnotesize
> > > > +
> > > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > > +
> > > > +.. flat-table::
> > > > +    :header-rows:  0
> > > > +    :stub-columns: 0
> > > > +
> > > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > > > +      - Use the (n-2)th frame as a golden frame, current frame index
> being
> > > > +        'n'.
> > > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > > +      - Use the previous specific frame indicated by
> > > > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > > > +        golden frame.
> > > > +
> > > > +.. raw:: latex
> > > > +
> > > > +    \normalsize
> > > > +
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > > > +    Allows host to specify the quantization parameter values for each
> > > > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > > > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the
> control
> > > > +    value to 1 enables setting of the QP values for the layers.
> > > > +
> > > > +.. _v4l2-vp9-ref-number-of-pframes:
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > > > +    (enum)
> > > > +
> > > > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > > > +    Number of reference pictures for encoding P frames.
> > > > +
> > > > +.. raw:: latex
> > > > +
> > > > +    \footnotesize
> > > > +
> > > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > > +
> > > > +.. flat-table::
> > > > +    :header-rows:  0
> > > > +    :stub-columns: 0
> > > > +
> > > > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > > > +      - Indicates one reference frame, last encoded frame will be
> searched.
> > > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > > +      - Indicates 2 reference frames, last encoded frame and golden
> frame
> > > > +        will be searched.
> > > > +
> > > > +.. raw:: latex
> > > > +
> > > > +    \normalsize
> > > > +
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > > > +    Indicates the number of hierarchial coding layer.
> > > > +    In normal encoding (non-hierarchial coding), it should be zero.
> > > > +    VP9 has upto 3 layer of encoder.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > > > +    Indicates enabling of bit rate for hierarchical coding layers VP9
> encoder.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > > > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > > > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > > > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > > > +    Indicates quantization parameter for hierarchical coding layer 0.
> > > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > > > +    Indicates quantization parameter for hierarchical coding layer 1.
> > > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > > > +    Indicates quantization parameter for hierarchical coding layer 2.
> > > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > > +
> > > > +.. _v4l2-vp9-max-partition-depth:
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > > > +    (enum)
> > > > +
> > > > +enum v4l2_mpeg_vp9_num_partitions -
> > > > +    Indicate maximum coding unit depth.
> > > > +
> > > > +.. raw:: latex
> > > > +
> > > > +    \footnotesize
> > > > +
> > > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > > +
> > > > +.. flat-table::
> > > > +    :header-rows:  0
> > > > +    :stub-columns: 0
> > > > +
> > > > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > > > +      - No coding unit partition depth.
> > > > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > > > +      - Allows one coding unit partition depth.
> > > > +
> > > > +.. raw:: latex
> > > > +
> > > > +    \normalsize
> > > > +
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > > > +    Zero indicates enable intra NxN PU split.
> > > > +    One indicates disable intra NxN PU split.
> > > > +
> > > > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > > > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > > > +    One indicates disable IVF format.
> > > > +
> > > >
> > > >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> > > >
> > >
> ==========================================================
> > > =
> >
> >
Nicolas Dufresne Dec. 22, 2022, 7:23 p.m. UTC | #9
Le mercredi 21 décembre 2022 à 15:26 +0530, Aakarsh Jain a écrit :
> 
> > -----Original Message-----
> > From: Nicolas Dufresne [mailto:nicolas@ndufresne.ca]
> > Sent: 16 December 2022 22:51
> > To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Hans Verkuil' <hverkuil-
> > cisco@xs4all.nl>; linux-arm-kernel@lists.infradead.org; linux-
> > media@vger.kernel.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org
> > Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> > mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> > jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> > stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> > david.plowman@raspberrypi.com; mark.rutland@arm.com;
> > robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> > alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> > pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> > Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9 CIDs.
> > 
> > Le mercredi 14 décembre 2022 à 15:52 +0530, Aakarsh Jain a écrit :
> > > 
> > > > -----Original Message-----
> > > > From: Hans Verkuil [mailto:hverkuil-cisco@xs4all.nl]
> > > > Sent: 24 November 2022 16:54
> > > > To: aakarsh jain <aakarsh.jain@samsung.com>; linux-arm-
> > > > kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; devicetree@vger.kernel.org
> > > > Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> > > > mchehab@kernel.org; ezequiel@vanguardiasur.com.ar;
> > > > jernej.skrabec@gmail.com; benjamin.gaignard@collabora.com;
> > > > stanimir.varbanov@linaro.org; dillon.minfei@gmail.com;
> > > > david.plowman@raspberrypi.com; mark.rutland@arm.com;
> > > > robh+dt@kernel.org; krzk+dt@kernel.org; andi@etezian.org;
> > > > alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> > > > pankaj.dubey@samsung.com; linux-fsd@tesla.com;
> > smitha.t@samsung.com
> > > > Subject: Re: [Patch v3 05/15] Documention: v4l: Documentation for VP9
> > CIDs.
> > > > 
> > > > On 11/10/2022 14:25, aakarsh jain wrote:
> > > > > From: Smitha T Murthy <smitha.t@samsung.com>
> > > > > 
> > > > > Adds V4l2 controls for VP9 encoder documention.
> > > > > 
> > > > > Cc: linux-fsd@tesla.com
> > > > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > > > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > > > > ---
> > > > >  .../media/v4l/ext-ctrls-codec.rst             | 167 ++++++++++++++++++
> > > > >  1 file changed, 167 insertions(+)
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > > b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > > index 2a165ae063fb..2277d83a7cf0 100644
> > > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > > > > @@ -2187,6 +2187,16 @@ enum v4l2_mpeg_video_vp8_profile -
> > > > >      * - ``V4L2_MPEG_VIDEO_VP8_PROFILE_3``
> > > > >        - Profile 3
> > > > > 
> > > > > +VP9 Control Reference
> > > > 
> > > > This is wrong. There is a VPX Control Reference section for both VP8
> > > > and VP9 controls. That's where this should be added. I suspect
> > > > several of the controls you are adding here already exist, e.g.
> > > > V4L2_CID_MPEG_VIDEO_VPX_MIN_QP. The documentation may have
> > to be
> > > > updated to specify that it is for both VP8 and VP9.
> > > > 
> > > Since MFC has different profiles, different quantization parameter ranges
> > for both VP8 and VP9. So we can't use same control ID's for both.
> > > So for example in VP8 with control ID
> > (V4L2_CID_MPEG_VIDEO_VPX_MIN_QP), QP ranges from 0-11 and in VP9
> > with control ID  (V4L2_CID_CODEC_VP9_MIN_QP) QP ranges from 1-24. So
> > we can't club together into single control.
> > > 
> > 
> > V4L2_CID_MPEG_VIDEO_VPX_PROFILE has been deprecated, and replace
> > with menu controls. So we now have a
> > V4L2_CID_MPEG_VIDEO_VP8_PROFILE and a
> > V4L2_CID_MPEG_VIDEO_VP9_PROFILE as menues. Newly written drivers
> > should use these. I see that GStreamer notably has never been ported, I'll fix
> > it.
> > 
> > When you implement a driver, the generic uAPI will cover all possible items,
> > as menus (a integer was an API mistake made in 2011, hence the
> > deprecation). You driver can then select which menu items it support, and its
> > server at telling userspace what this HW supports. Though, this should be no
> > problem if you want to keep the old CID for backward compat, since the
> > range is just totally undefined there.
> > 
> > For V4L2_CID_MPEG_VIDEO_VPX_MIN_QP (and friends), the doc says
> > "Minimum quantization parameter for VP8.". A bit strange for a supposedly
> > VPX parameter.
> > But its defines in the code as "VPX Minimum QP Value". Clearly something to
> > be fixed. There is no VP9 encoder drivers yet in mainline.
> > 
> > Though, the range for these controls is driver defined. In Venus, for VP8:
> > 
> > 
> >         v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> >                 V4L2_CID_MPEG_VIDEO_VPX_MIN_QP, 1, 128, 1, 1);
> > 
> > It seems to be 1 to 128. While in MFC, it oddly 1 to 11:
> > 
> > 
> >         {
> >                 .id = V4L2_CID_MPEG_VIDEO_VPX_MIN_QP,
> >                 .type = V4L2_CTRL_TYPE_INTEGER,
> >                 .minimum = 0,
> >                 .maximum = 11,
> >                 .step = 1,
> >                 .default_value = 0,
> >         },
> > 
> > While I'm not a huge fan of this, since we all know QP does not scale linearly,
> > this is how it is, and this is kind of part of the kernel API now. So userspace
> > must ask the driver what is the QP range, and adapt. And in your case, you
> > should have no issue adding VP9 encoder with a 1 to 24 range (even if this is a
> > bit odd and hw specific).
> > 
> > Nicolas
> > 
> 
> So all controls which I am using in VP9 similar to VPX will implement that as menu control. Will not touch VPX controls for backward compatibility.
> Also will change all remaining VP9 controls implementation from Integer to menu control. 
> 
> Will this be fine ?

I think so, I'd add menu control to the existing VP8 decoder, so that both are
supported, but I'd only implement menu controls for newly added formats. This is
to maintain backward compatibility indeed.

> 
> > 
> > > > > +---------------------
> > > > > +
> > > > > +The VP9 controls include controls for encoding parameters of VP9
> > > > > +video codec.
> > > > > +
> > > > > +.. _vp9-control-id:
> > > > > +
> > > > > +VP9 Control IDs
> > > > > +
> > > > >  .. _v4l2-mpeg-video-vp9-profile:
> > > > > 
> > > > >  ``V4L2_CID_MPEG_VIDEO_VP9_PROFILE``
> > > > > @@ -2253,6 +2263,163 @@ enum v4l2_mpeg_video_vp9_level -
> > > > >      * - ``V4L2_MPEG_VIDEO_VP9_LEVEL_6_2``
> > > > >        - Level 6.2
> > > > > 
> > > > > +``V4L2_CID_CODEC_VP9_I_FRAME_QP``
> > > > 
> > > > If you do need to add new controls, then please use the same
> > > > MPEG_VIDEO_ prefix.
> > > > It's a bit ugly and historical, but let's keep it consistent with the others.
> > > > 
> > > > Regards,
> > > > 
> > > > 	Hans
> > > > 
> > > > > +    Quantization parameter for an I frame for VP9. Valid range:
> > > > > + from 1 to
> > > > 255.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_P_FRAME_QP``
> > > > > +    Quantization parameter for an P frame for VP9. Valid range:
> > > > > +from 1 to
> > > > 255.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_MAX_QP``
> > > > > +    Maximum quantization parameter for VP9. Valid range: from 1 to
> > 255.
> > > > > +    Recommended range for MFC is from 230 to 255.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_MIN_QP``
> > > > > +    Minimum quantization parameter for VP9. Valid range: from 1 to
> > 255.
> > > > > +    Recommended range for MFC is from 1 to 24.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_RC_FRAME_RATE``
> > > > > +    Indicates the number of evenly spaced subintervals, called ticks,
> > within
> > > > > +    one second. This is a 16 bit unsigned integer and has a
> > > > > +maximum value
> > > > up to
> > > > > +    0xffff and a minimum value of 1.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD``
> > > > > +    Indicates the refresh period of the golden frame for VP9 encoder.
> > > > > +
> > > > > +.. _v4l2-vp9-golden-frame-sel:
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_GOLDEN_FRAMESEL``
> > > > > +    (enum)
> > > > > +
> > > > > +enum v4l2_mpeg_vp9_golden_framesel -
> > > > > +    Selects the golden frame for encoding. Valid when NUM_OF_REF is
> > 2.
> > > > > +    Possible values are:
> > > > > +
> > > > > +.. raw:: latex
> > > > > +
> > > > > +    \footnotesize
> > > > > +
> > > > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > > > +
> > > > > +.. flat-table::
> > > > > +    :header-rows:  0
> > > > > +    :stub-columns: 0
> > > > > +
> > > > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_PREV``
> > > > > +      - Use the (n-2)th frame as a golden frame, current frame index
> > being
> > > > > +        'n'.
> > > > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > > > +      - Use the previous specific frame indicated by
> > > > > +        ``V4L2_CID_CODEC_VP9_GF_REFRESH_PERIOD`` as a
> > > > > +        golden frame.
> > > > > +
> > > > > +.. raw:: latex
> > > > > +
> > > > > +    \normalsize
> > > > > +
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIERARCHY_QP_ENABLE``
> > > > > +    Allows host to specify the quantization parameter values for each
> > > > > +    temporal layer through HIERARCHICAL_QP_LAYER. This is valid only
> > > > > +    if HIERARCHICAL_CODING_LAYER is greater than 1. Setting the
> > control
> > > > > +    value to 1 enables setting of the QP values for the layers.
> > > > > +
> > > > > +.. _v4l2-vp9-ref-number-of-pframes:
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_REF_NUMBER_FOR_PFRAMES``
> > > > > +    (enum)
> > > > > +
> > > > > +enum v4l2_mpeg_vp9_ref_num_for_pframes -
> > > > > +    Number of reference pictures for encoding P frames.
> > > > > +
> > > > > +.. raw:: latex
> > > > > +
> > > > > +    \footnotesize
> > > > > +
> > > > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > > > +
> > > > > +.. flat-table::
> > > > > +    :header-rows:  0
> > > > > +    :stub-columns: 0
> > > > > +
> > > > > +    * - ``V4L2_CID_CODEC_VP9_1_REF_PFRAME``
> > > > > +      - Indicates one reference frame, last encoded frame will be
> > searched.
> > > > > +    * - ``V4L2_CID_CODEC_VP9_GOLDEN_FRAME_USE_REF_PERIOD``
> > > > > +      - Indicates 2 reference frames, last encoded frame and golden
> > frame
> > > > > +        will be searched.
> > > > > +
> > > > > +.. raw:: latex
> > > > > +
> > > > > +    \normalsize
> > > > > +
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIERARCHICAL_CODING_LAYER``
> > > > > +    Indicates the number of hierarchial coding layer.
> > > > > +    In normal encoding (non-hierarchial coding), it should be zero.
> > > > > +    VP9 has upto 3 layer of encoder.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIERARCHY_RC_ENABLE``
> > > > > +    Indicates enabling of bit rate for hierarchical coding layers VP9
> > encoder.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_BR``
> > > > > +    Indicates bit rate for hierarchical coding layer 0 for VP9 encoder.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_BR``
> > > > > +    Indicates bit rate for hierarchical coding layer 1 for VP9 encoder.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_BR``
> > > > > +    Indicates bit rate for hierarchical coding layer 2 for VP9 encoder.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L0_QP``
> > > > > +    Indicates quantization parameter for hierarchical coding layer 0.
> > > > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L1_QP``
> > > > > +    Indicates quantization parameter for hierarchical coding layer 1.
> > > > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_HIER_CODING_L2_QP``
> > > > > +    Indicates quantization parameter for hierarchical coding layer 2.
> > > > > +    Valid range: [V4L2_CID_CODEC_VP9_MIN_QP,
> > > > > +    V4L2_CID_CODEC_VP9_MAX_QP].
> > > > > +
> > > > > +.. _v4l2-vp9-max-partition-depth:
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_MAX_PARTITION_DEPTH``
> > > > > +    (enum)
> > > > > +
> > > > > +enum v4l2_mpeg_vp9_num_partitions -
> > > > > +    Indicate maximum coding unit depth.
> > > > > +
> > > > > +.. raw:: latex
> > > > > +
> > > > > +    \footnotesize
> > > > > +
> > > > > +.. tabularcolumns:: |p{9.0cm}|p{8.0cm}|
> > > > > +
> > > > > +.. flat-table::
> > > > > +    :header-rows:  0
> > > > > +    :stub-columns: 0
> > > > > +
> > > > > +    * - ``V4L2_CID_CODEC_VP9_0_PARTITION``
> > > > > +      - No coding unit partition depth.
> > > > > +    * - ``V4L2_CID_CODEC_VP9_1_PARTITION``
> > > > > +      - Allows one coding unit partition depth.
> > > > > +
> > > > > +.. raw:: latex
> > > > > +
> > > > > +    \normalsize
> > > > > +
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_DISABLE_INTRA_PU_SPLIT``
> > > > > +    Zero indicates enable intra NxN PU split.
> > > > > +    One indicates disable intra NxN PU split.
> > > > > +
> > > > > +``V4L2_CID_CODEC_VP9_DISABLE_IVF_HEADER``
> > > > > +    Indicates IVF header generation. Zero indicates enable IVF format.
> > > > > +    One indicates disable IVF format.
> > > > > +
> > > > > 
> > > > >  High Efficiency Video Coding (HEVC/H.265) Control Reference
> > > > > 
> > > > 
> > ==========================================================
> > > > =
> > > 
> > > 
> 
> 
>