| Message ID | 20221011122516.32135-1-aakarsh.jain@samsung.com |
|---|---|
| Headers | show |
| Series | Add MFC v12 support. | expand |
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzk@kernel.org] > Sent: 12 October 2022 18:27 > 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 01/15] dt-bindings: media: s5p-mfc: Add new DT > schema for MFC > > On 11/10/2022 08:25, aakarsh jain wrote: > > From: Smitha T Murthy <smitha.t@samsung.com> > > > > Convert DT schema for s5p-mfc in yaml format > > Convert S5P-MFC bindings to DT schema. > > You also missed full-stop. > Ok will address it in next series. > My second comment was ignored here. > I missed one review comment which was related to change in commit message. Will address it in next series. > > > > Cc: linux-fsd@tesla.com > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > > --- > > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > > .../bindings/media/samsung,s5p-mfc.yaml | 163 > ++++++++++++++++++ > > 2 files changed, 163 insertions(+), 75 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt > > b/Documentation/devicetree/bindings/media/s5p-mfc.txt > > index aa54c8159d9f..8b137891791f 100644 > > --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt > > +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all requested > changes or keep discussing them. > > Thank you. > Apart from your second review comment I guess we have addressed all your review comments in the current patch. Please review other changes as well and let us know if any changes required. Thanks for the review. > Best regards, > Krzysztof
> -----Original Message----- > From: Rob Herring [mailto:robh@kernel.org] > Sent: 12 October 2022 18:50 > To: aakarsh jain <aakarsh.jain@samsung.com> > Cc: linux-fsd@tesla.com; linux-media@vger.kernel.org; > pankaj.dubey@samsung.com; linux-arm-kernel@lists.infradead.org; > dillon.minfei@gmail.com; devicetree@vger.kernel.org; krzk+dt@kernel.org; > smitha.t@samsung.com; benjamin.gaignard@collabora.com; > stanimir.varbanov@linaro.org; jernej.skrabec@gmail.com; > robh+dt@kernel.org; aswani.reddy@samsung.com; mchehab@kernel.org; > hverkuil-cisco@xs4all.nl; mark.rutland@arm.com; > m.szyprowski@samsung.com; linux-kernel@vger.kernel.org; > alim.akhtar@samsung.com; andi@etezian.org; andrzej.hajda@intel.com; > ezequiel@vanguardiasur.com.ar; david.plowman@raspberrypi.com > Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT > schema for MFC > > On Tue, 11 Oct 2022 17:55:02 +0530, aakarsh jain wrote: > > From: Smitha T Murthy <smitha.t@samsung.com> > > > > Convert DT schema for s5p-mfc in yaml format > > > > Cc: linux-fsd@tesla.com > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > > --- > > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > > .../bindings/media/samsung,s5p-mfc.yaml | 163 > ++++++++++++++++++ > > 2 files changed, 163 insertions(+), 75 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > > > > Running 'make dtbs_check' with the schema in this patch gives the following > warnings. Consider if they are expected or the schema is incorrect. These > may not be new warnings. > > Note that it is not yet a requirement to have 0 warnings for dtbs_check. > This will change in the future. > > Full log is available here: https://protect2.fireeye.com/v1/url?k=b651482e- > e9ca7131-b650c361-000babdfecba-bb28230f82534754&q=1&e=25a7ee78- > 697f-4371-880e- > dcf4e3072c99&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F > > > codec@13400000: clock-names: ['mfc', 'sclk_mfc'] is too long > arch/arm/boot/dts/exynos3250-artik5-eval.dtb > arch/arm/boot/dts/exynos3250-monk.dtb > arch/arm/boot/dts/exynos3250-rinato.dtb > > codec@13400000: clocks: [[7, 178], [7, 228]] is too long > arch/arm/boot/dts/exynos3250-artik5-eval.dtb > arch/arm/boot/dts/exynos3250-monk.dtb > arch/arm/boot/dts/exynos3250-rinato.dtb > > codec@13400000: iommus: [[36]] is too short > arch/arm/boot/dts/exynos3250-monk.dtb > arch/arm/boot/dts/exynos3250-monk.dtb > > codec@13400000: iommus: [[40]] is too short > arch/arm/boot/dts/exynos3250-artik5-eval.dtb > arch/arm/boot/dts/exynos3250-artik5-eval.dtb > > codec@13400000: iommus: [[47]] is too short > arch/arm/boot/dts/exynos3250-rinato.dtb > arch/arm/boot/dts/exynos3250-rinato.dtb > > codec@13400000: memory-region: [[31], [32]] is too long > arch/arm/boot/dts/exynos4210-smdkv310.dtb > > codec@13400000: memory-region: [[37], [38]] is too long > arch/arm/boot/dts/exynos4210-origen.dtb > > codec@13400000: memory-region: [[41], [42]] is too long > arch/arm/boot/dts/exynos4412-smdk4412.dtb > > codec@13400000: memory-region: [[43], [44]] is too long > arch/arm/boot/dts/exynos4412-origen.dtb > > codec@13400000: memory-region: [[47], [48]] is too long > arch/arm/boot/dts/exynos4412-itop-elite.dtb > arch/arm/boot/dts/exynos4412-odroidx2.dtb > arch/arm/boot/dts/exynos4412-odroidx.dtb > > codec@13400000: memory-region: [[48], [49]] is too long > arch/arm/boot/dts/exynos4412-odroidu3.dtb > > codec@f1700000: clock-names:0: 'mfc' was expected > arch/arm/boot/dts/s5pv210-aquila.dtb > arch/arm/boot/dts/s5pv210-fascinate4g.dtb > arch/arm/boot/dts/s5pv210-galaxys.dtb > arch/arm/boot/dts/s5pv210-goni.dtb > arch/arm/boot/dts/s5pv210-smdkc110.dtb > arch/arm/boot/dts/s5pv210-smdkv210.dtb > arch/arm/boot/dts/s5pv210-torbreck.dtb > > codec@f1700000: clock-names:1: 'sclk_mfc' was expected > arch/arm/boot/dts/s5pv210-aquila.dtb > arch/arm/boot/dts/s5pv210-fascinate4g.dtb > arch/arm/boot/dts/s5pv210-galaxys.dtb > arch/arm/boot/dts/s5pv210-goni.dtb > arch/arm/boot/dts/s5pv210-smdkc110.dtb > arch/arm/boot/dts/s5pv210-smdkv210.dtb > arch/arm/boot/dts/s5pv210-torbreck.dtb > > codec@f1700000: memory-region: [[51], [52]] is too long > arch/arm/boot/dts/s5pv210-fascinate4g.dtb > > codec@f1700000: memory-region: [[55], [56]] is too long > arch/arm/boot/dts/s5pv210-galaxys.dtb Hi Rob, We tried reproducing warnings as reported above, but I am not able to see these warnings after running make dtbs_check & make DT_CHECKER_FLAGS=-m dt_binding_check. Packages used- yamllint 1.10.0 $ dt-mk-schema --version 2022.9 Am I missing anything? Do let me know. Thanks, Aakarsh
On 17/10/2022 10:16, Aakarsh Jain wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski [mailto:krzk@kernel.org] >> Sent: 12 October 2022 18:27 >> 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 01/15] dt-bindings: media: s5p-mfc: Add new DT >> schema for MFC >> >> On 11/10/2022 08:25, aakarsh jain wrote: >>> From: Smitha T Murthy <smitha.t@samsung.com> >>> >>> Convert DT schema for s5p-mfc in yaml format >> >> Convert S5P-MFC bindings to DT schema. >> >> You also missed full-stop. >> > Ok will address it in next series. > >> My second comment was ignored here. >> > I missed one review comment which was related to change in commit message. Will address it in next series. >>> >>> Cc: linux-fsd@tesla.com >>> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> >>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> >>> --- >>> .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- >>> .../bindings/media/samsung,s5p-mfc.yaml | 163 >> ++++++++++++++++++ >>> 2 files changed, 163 insertions(+), 75 deletions(-) create mode >>> 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt >>> b/Documentation/devicetree/bindings/media/s5p-mfc.txt >>> index aa54c8159d9f..8b137891791f 100644 >>> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt >>> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt >> >> This is a friendly reminder during the review process. >> >> It seems my previous comments were not fully addressed. Maybe my >> feedback got lost between the quotes, maybe you just forgot to apply it. >> Please go back to the previous discussion and either implement all requested >> changes or keep discussing them. >> >> Thank you. >> > Apart from your second review comment I guess we have addressed all your review comments in the current patch. > Please review other changes as well and let us know if any changes required. No, as I said you did not. Instead of bouncing back immediately, go through previous emails and through reviews. I already reviewed this patch and it did not apply all my comments. Asking me to review second time is not the solution here. The solution here is for you to go through the comments and implement them, or answer and keep discussion going. Best regards, Krzysztof
On 18/10/2022 06:48, Aakarsh Jain wrote: >> >> codec@f1700000: clock-names:1: 'sclk_mfc' was expected >> arch/arm/boot/dts/s5pv210-aquila.dtb >> arch/arm/boot/dts/s5pv210-fascinate4g.dtb >> arch/arm/boot/dts/s5pv210-galaxys.dtb >> arch/arm/boot/dts/s5pv210-goni.dtb >> arch/arm/boot/dts/s5pv210-smdkc110.dtb >> arch/arm/boot/dts/s5pv210-smdkv210.dtb >> arch/arm/boot/dts/s5pv210-torbreck.dtb >> >> codec@f1700000: memory-region: [[51], [52]] is too long >> arch/arm/boot/dts/s5pv210-fascinate4g.dtb >> >> codec@f1700000: memory-region: [[55], [56]] is too long >> arch/arm/boot/dts/s5pv210-galaxys.dtb > > > > Hi Rob, > > We tried reproducing warnings as reported above, but I am not able to see > these warnings after running make dtbs_check & make DT_CHECKER_FLAGS=-m > dt_binding_check. > Packages used- > yamllint 1.10.0 > $ dt-mk-schema --version > 2022.9 Are you sure you are running these commands on proper config and arch? Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzk@kernel.org] > Sent: 18 October 2022 18:40 > To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Rob Herring' > <robh@kernel.org> > Cc: linux-fsd@tesla.com; linux-media@vger.kernel.org; > pankaj.dubey@samsung.com; linux-arm-kernel@lists.infradead.org; > dillon.minfei@gmail.com; devicetree@vger.kernel.org; krzk+dt@kernel.org; > smitha.t@samsung.com; benjamin.gaignard@collabora.com; > stanimir.varbanov@linaro.org; jernej.skrabec@gmail.com; > robh+dt@kernel.org; aswani.reddy@samsung.com; mchehab@kernel.org; > hverkuil-cisco@xs4all.nl; mark.rutland@arm.com; > m.szyprowski@samsung.com; linux-kernel@vger.kernel.org; > alim.akhtar@samsung.com; andi@etezian.org; andrzej.hajda@intel.com; > ezequiel@vanguardiasur.com.ar; david.plowman@raspberrypi.com > Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT > schema for MFC > > On 18/10/2022 06:48, Aakarsh Jain wrote: > >> > >> codec@f1700000: clock-names:1: 'sclk_mfc' was expected > >> arch/arm/boot/dts/s5pv210-aquila.dtb > >> arch/arm/boot/dts/s5pv210-fascinate4g.dtb > >> arch/arm/boot/dts/s5pv210-galaxys.dtb > >> arch/arm/boot/dts/s5pv210-goni.dtb > >> arch/arm/boot/dts/s5pv210-smdkc110.dtb > >> arch/arm/boot/dts/s5pv210-smdkv210.dtb > >> arch/arm/boot/dts/s5pv210-torbreck.dtb > >> > >> codec@f1700000: memory-region: [[51], [52]] is too long > >> arch/arm/boot/dts/s5pv210-fascinate4g.dtb > >> > >> codec@f1700000: memory-region: [[55], [56]] is too long > >> arch/arm/boot/dts/s5pv210-galaxys.dtb > > > > > > > > Hi Rob, > > > > We tried reproducing warnings as reported above, but I am not able to > > see these warnings after running make dtbs_check & make > > DT_CHECKER_FLAGS=-m dt_binding_check. > > Packages used- > > yamllint 1.10.0 > > $ dt-mk-schema --version > > 2022.9 > > > Are you sure you are running these commands on proper config and arch? > yes. arm64/defconfig . > Best regards, > Krzysztof
On 18/10/2022 09:21, Aakarsh Jain wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski [mailto:krzk@kernel.org] >> Sent: 18 October 2022 18:40 >> To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Rob Herring' >> <robh@kernel.org> >> Cc: linux-fsd@tesla.com; linux-media@vger.kernel.org; >> pankaj.dubey@samsung.com; linux-arm-kernel@lists.infradead.org; >> dillon.minfei@gmail.com; devicetree@vger.kernel.org; krzk+dt@kernel.org; >> smitha.t@samsung.com; benjamin.gaignard@collabora.com; >> stanimir.varbanov@linaro.org; jernej.skrabec@gmail.com; >> robh+dt@kernel.org; aswani.reddy@samsung.com; mchehab@kernel.org; >> hverkuil-cisco@xs4all.nl; mark.rutland@arm.com; >> m.szyprowski@samsung.com; linux-kernel@vger.kernel.org; >> alim.akhtar@samsung.com; andi@etezian.org; andrzej.hajda@intel.com; >> ezequiel@vanguardiasur.com.ar; david.plowman@raspberrypi.com >> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT >> schema for MFC >> >> On 18/10/2022 06:48, Aakarsh Jain wrote: >>>> >>>> codec@f1700000: clock-names:1: 'sclk_mfc' was expected >>>> arch/arm/boot/dts/s5pv210-aquila.dtb >>>> arch/arm/boot/dts/s5pv210-fascinate4g.dtb >>>> arch/arm/boot/dts/s5pv210-galaxys.dtb >>>> arch/arm/boot/dts/s5pv210-goni.dtb >>>> arch/arm/boot/dts/s5pv210-smdkc110.dtb >>>> arch/arm/boot/dts/s5pv210-smdkv210.dtb >>>> arch/arm/boot/dts/s5pv210-torbreck.dtb >>>> >>>> codec@f1700000: memory-region: [[51], [52]] is too long >>>> arch/arm/boot/dts/s5pv210-fascinate4g.dtb >>>> >>>> codec@f1700000: memory-region: [[55], [56]] is too long >>>> arch/arm/boot/dts/s5pv210-galaxys.dtb >>> >>> >>> >>> Hi Rob, >>> >>> We tried reproducing warnings as reported above, but I am not able to >>> see these warnings after running make dtbs_check & make >>> DT_CHECKER_FLAGS=-m dt_binding_check. >>> Packages used- >>> yamllint 1.10.0 >>> $ dt-mk-schema --version >>> 2022.9 >> >> >> Are you sure you are running these commands on proper config and arch? >> > yes. arm64/defconfig . Which does not look like proper arch. Look at the warnings Rob robot provided. Best regards, Krzysztof
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 > ===========================================================
> -----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.
Hi Aakarsh Thanks for the patch. >-----Original Message----- >From: aakarsh jain [mailto:aakarsh.jain@samsung.com] >Sent: Tuesday, October 11, 2022 5:55 PM >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 01/15] dt-bindings: media: s5p-mfc: Add new DT schema for >MFC > >From: Smitha T Murthy <smitha.t@samsung.com> > >Convert DT schema for s5p-mfc in yaml format > >Cc: linux-fsd@tesla.com >Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> >Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> >--- > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > .../bindings/media/samsung,s5p-mfc.yaml | 163 ++++++++++++++++++ > 2 files changed, 163 insertions(+), 75 deletions(-) create mode 100644 >Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > >diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt >b/Documentation/devicetree/bindings/media/s5p-mfc.txt >index aa54c8159d9f..8b137891791f 100644 >--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt >+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt >@@ -1,76 +1 @@ >-* Samsung Multi Format Codec (MFC) > >-Multi Format Codec (MFC) is the IP present in Samsung SoCs which -supports >high resolution decoding and encoding functionalities. >-The MFC device driver is a v4l2 driver which can encode/decode -video >raw/elementary streams and has support for all popular -video codecs. >- >-Required properties: >- - compatible : value should be either one among the following >- (a) "samsung,mfc-v5" for MFC v5 present in Exynos4 SoCs >- (b) "samsung,mfc-v6" for MFC v6 present in Exynos5 SoCs >- (c) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC >- (d) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC >- (e) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 SoC >- (f) "samsung,mfc-v10" for MFC v10 present in Exynos7880 SoC >- >- - reg : Physical base address of the IP registers and length of memory >- mapped region. >- >- - interrupts : MFC interrupt number to the CPU. >- - clocks : from common clock binding: handle to mfc clock. >- - clock-names : from common clock binding: must contain "mfc", >- corresponding to entry in the clocks property. >- >-Optional properties: >- - power-domains : power-domain property defined with a phandle >- to respective power domain. >- - memory-region : from reserved memory binding: phandles to two >reserved >- memory regions, first is for "left" mfc memory bus interfaces, >- second if for the "right" mfc memory bus, used when no SYSMMU >- support is available; used only by MFC v5 present in Exynos4 SoCs >- >-Obsolete properties: >- - samsung,mfc-r, samsung,mfc-l : support removed, please use memory- >region >- property instead >- >- >-Example: >-SoC specific DT entry: >- >-mfc: codec@13400000 { >- compatible = "samsung,mfc-v5"; >- reg = <0x13400000 0x10000>; >- interrupts = <0 94 0>; >- power-domains = <&pd_mfc>; >- clocks = <&clock 273>; >- clock-names = "mfc"; >-}; >- >-Reserved memory specific DT entry for given board (see reserved memory >binding -for more information): >- >-reserved-memory { >- #address-cells = <1>; >- #size-cells = <1>; >- ranges; >- >- mfc_left: region@51000000 { >- compatible = "shared-dma-pool"; >- no-map; >- reg = <0x51000000 0x800000>; >- }; >- >- mfc_right: region@43000000 { >- compatible = "shared-dma-pool"; >- no-map; >- reg = <0x43000000 0x800000>; >- }; >-}; >- >-Board specific DT entry: >- >-codec@13400000 { >- memory-region = <&mfc_left>, <&mfc_right>; >-}; >diff --git a/Documentation/devicetree/bindings/media/samsung,s5p- >mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p- >mfc.yaml >new file mode 100644 >index 000000000000..ad61b509846f >--- /dev/null >+++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml >@@ -0,0 +1,163 @@ >+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2 >+--- >+$id: http://devicetree.org/schemas/media/samsung,s5p-mfc.yaml# >+$schema: http://devicetree.org/meta-schemas/core.yaml# >+ >+title: Samsung Exynos Multi Format Codec (MFC) >+ >+maintainers: >+ - Marek Szyprowski <m.szyprowski@samsung.com> >+ - Smitha T Murthy <smitha.t@samsung.com> >+ - Aakarsh Jain <aakarsh.jain@samsung.com> >+ >+description: >+ Multi Format Codec (MFC) is the IP present in Samsung SoCs which >+ supports high resolution decoding and encoding functionalities. >+ >+properties: >+ compatible: >+ enum: >+ - samsung,mfc-v5 # Exynos4 >+ - samsung,mfc-v6 # Exynos5 >+ - samsung,mfc-v7 # Exynos5420 >+ - samsung,mfc-v8 # Exynos5800 >+ - samsung,exynos5433-mfc # Exynos5433 >+ - samsung,mfc-v10 # Exynos7880 >+ >+ reg: >+ maxItems: 1 >+ >+ clocks: >+ minItems: 1 >+ maxItems: 3 >+ >+ clock-names: >+ minItems: 1 >+ maxItems: 3 >+ >+ interrupts: >+ maxItems: 1 >+ >+ iommus: >+ maxItems: 2 >+ >+ iommu-names: >+ maxItems: 2 >+ >+ power-domains: >+ maxItems: 1 >+ >+ memory-region: >+ maxItems: 1 >+ >+required: >+ - compatible >+ - reg >+ - clocks >+ - clock-names >+ - interrupts >+ >+allOf: >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,mfc-v6 >+ - samsung,mfc-v7 >+ - samsung,mfc-v8 >+ - tesla,fsd-mfc You have not introduce "tesla,fsd-mfc" compatible yet, so this should be part of the patch which adds fsd-mfc compatible >+ then: >+ properties: >+ clocks: >+ maxItems: 1 >+ clock-names: >+ items: >+ - const: mfc >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,mfc-v5 >+ then: >+ properties: >+ clocks: >+ minItems: 2 >+ maxItems: 2 >+ clock-names: >+ items: >+ - const: mfc >+ - const: sclk_mfc >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,exynos5433-mfc >+ then: >+ properties: >+ clocks: >+ minItems: 3 >+ maxItems: 3 >+ clock-names: >+ items: >+ - const: pclk >+ - const: aclk >+ - const: aclk_xiu >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,mfc-v5 >+ - samsung,mfc-v6 >+ - samsung,mfc-v7 >+ - samsung,mfc-v8 >+ - samsung,exynos5433-mfc >+ >+ then: >+ properties: >+ iommus: >+ minItems: 2 >+ maxItems: 2 >+ iommu-names: >+ items: >+ - const: left >+ - const: right >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - tesla,fsd-mfc Same comments as above. My suggestion, first just convert the current s5p-mfc.txt to yaml format and then add fsd-mfc support. That way it will be easy for reviewer to understand the changes. While reviewing and code walk through, I also noticed that exynos3250.dtsi and exynos5420.dtsi are using same compatible as "samsung,mfc-v7" but there "clocks" property is different. That doesn't look correct. This will cause issues when you convert s5p-mfc.txt to schema format. I don’t know why that was done that way, but surely this need to be _fixed_ first as h/w itself are different. So my suggestion is to fix exynos3250.dtsi and exynos5420.dtsi compatible and mfc driver for it first (as a separate patch series). You can use the same approach what was done for mfc-v8 and mfc-v8-5433. >+ then: >+ properties: >+ memory-region: >+ maxItems: 1 >+ >+additionalProperties: false >+ >+examples: >+ - | >+ #include <dt-bindings/clock/exynos4.h> >+ #include <dt-bindings/clock/exynos-audss-clk.h> >+ #include <dt-bindings/interrupt-controller/arm-gic.h> >+ #include <dt-bindings/interrupt-controller/irq.h> >+ >+ codec@13400000 { >+ compatible = "samsung,mfc-v5"; >+ reg = <0x13400000 0x10000>; >+ interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>; >+ power-domains = <&pd_mfc>; >+ clocks = <&clock CLK_MFC>, <&clock CLK_SCLK_MFC>; >+ clock-names = "mfc", "sclk_mfc"; >+ iommus = <&sysmmu_mfc_l>, <&sysmmu_mfc_r>; >+ iommu-names = "left", "right"; >+ >+ }; >-- >2.17.1
> -----Original Message----- > From: Alim Akhtar [mailto:alim.akhtar@samsung.com] > Sent: 27 October 2022 16:10 > 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; aswani.reddy@samsung.com; > pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com > Subject: RE: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT > schema for MFC > > Hi Aakarsh > Thanks for the patch. > > >-----Original Message----- > >From: aakarsh jain [mailto:aakarsh.jain@samsung.com] > >Sent: Tuesday, October 11, 2022 5:55 PM > >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 01/15] dt-bindings: media: s5p-mfc: Add new DT > >schema for MFC > > > >From: Smitha T Murthy <smitha.t@samsung.com> > > > >Convert DT schema for s5p-mfc in yaml format > > > >Cc: linux-fsd@tesla.com > >Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > >Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > >--- > > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > > .../bindings/media/samsung,s5p-mfc.yaml | 163 ++++++++++++++++++ > > 2 files changed, 163 insertions(+), 75 deletions(-) create mode > >100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > > > >diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt > >b/Documentation/devicetree/bindings/media/s5p-mfc.txt > >index aa54c8159d9f..8b137891791f 100644 > >--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt > >+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt > >@@ -1,76 +1 @@ > >-* Samsung Multi Format Codec (MFC) > > > >-Multi Format Codec (MFC) is the IP present in Samsung SoCs which > >-supports high resolution decoding and encoding functionalities. > >-The MFC device driver is a v4l2 driver which can encode/decode -video > >raw/elementary streams and has support for all popular -video codecs. > >- > >-Required properties: > >- - compatible : value should be either one among the following > >- (a) "samsung,mfc-v5" for MFC v5 present in Exynos4 SoCs > >- (b) "samsung,mfc-v6" for MFC v6 present in Exynos5 SoCs > >- (c) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC > >- (d) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC > >- (e) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 > SoC > >- (f) "samsung,mfc-v10" for MFC v10 present in Exynos7880 SoC > >- > >- - reg : Physical base address of the IP registers and length of memory > >- mapped region. > >- > >- - interrupts : MFC interrupt number to the CPU. > >- - clocks : from common clock binding: handle to mfc clock. > >- - clock-names : from common clock binding: must contain "mfc", > >- corresponding to entry in the clocks property. > >- > >-Optional properties: > >- - power-domains : power-domain property defined with a phandle > >- to respective power domain. > >- - memory-region : from reserved memory binding: phandles to two > >reserved > >- memory regions, first is for "left" mfc memory bus interfaces, > >- second if for the "right" mfc memory bus, used when no SYSMMU > >- support is available; used only by MFC v5 present in Exynos4 SoCs > >- > >-Obsolete properties: > >- - samsung,mfc-r, samsung,mfc-l : support removed, please use memory- > >region > >- property instead > >- > >- > >-Example: > >-SoC specific DT entry: > >- > >-mfc: codec@13400000 { > >- compatible = "samsung,mfc-v5"; > >- reg = <0x13400000 0x10000>; > >- interrupts = <0 94 0>; > >- power-domains = <&pd_mfc>; > >- clocks = <&clock 273>; > >- clock-names = "mfc"; > >-}; > >- > >-Reserved memory specific DT entry for given board (see reserved memory > >binding -for more information): > >- > >-reserved-memory { > >- #address-cells = <1>; > >- #size-cells = <1>; > >- ranges; > >- > >- mfc_left: region@51000000 { > >- compatible = "shared-dma-pool"; > >- no-map; > >- reg = <0x51000000 0x800000>; > >- }; > >- > >- mfc_right: region@43000000 { > >- compatible = "shared-dma-pool"; > >- no-map; > >- reg = <0x43000000 0x800000>; > >- }; > >-}; > >- > >-Board specific DT entry: > >- > >-codec@13400000 { > >- memory-region = <&mfc_left>, <&mfc_right>; > >-}; > >diff --git a/Documentation/devicetree/bindings/media/samsung,s5p- > >mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p- > >mfc.yaml > >new file mode 100644 > >index 000000000000..ad61b509846f > >--- /dev/null > >+++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > >@@ -0,0 +1,163 @@ > >+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2 > >+--- > >+$id: http://devicetree.org/schemas/media/samsung,s5p-mfc.yaml# > >+$schema: http://devicetree.org/meta-schemas/core.yaml# > >+ > >+title: Samsung Exynos Multi Format Codec (MFC) > >+ > >+maintainers: > >+ - Marek Szyprowski <m.szyprowski@samsung.com> > >+ - Smitha T Murthy <smitha.t@samsung.com> > >+ - Aakarsh Jain <aakarsh.jain@samsung.com> > >+ > >+description: > >+ Multi Format Codec (MFC) is the IP present in Samsung SoCs which > >+ supports high resolution decoding and encoding functionalities. > >+ > >+properties: > >+ compatible: > >+ enum: > >+ - samsung,mfc-v5 # Exynos4 > >+ - samsung,mfc-v6 # Exynos5 > >+ - samsung,mfc-v7 # Exynos5420 > >+ - samsung,mfc-v8 # Exynos5800 > >+ - samsung,exynos5433-mfc # Exynos5433 > >+ - samsung,mfc-v10 # Exynos7880 > >+ > >+ reg: > >+ maxItems: 1 > >+ > >+ clocks: > >+ minItems: 1 > >+ maxItems: 3 > >+ > >+ clock-names: > >+ minItems: 1 > >+ maxItems: 3 > >+ > >+ interrupts: > >+ maxItems: 1 > >+ > >+ iommus: > >+ maxItems: 2 > >+ > >+ iommu-names: > >+ maxItems: 2 > >+ > >+ power-domains: > >+ maxItems: 1 > >+ > >+ memory-region: > >+ maxItems: 1 > >+ > >+required: > >+ - compatible > >+ - reg > >+ - clocks > >+ - clock-names > >+ - interrupts > >+ > >+allOf: > >+ - if: > >+ properties: > >+ compatible: > >+ contains: > >+ enum: > >+ - samsung,mfc-v6 > >+ - samsung,mfc-v7 > >+ - samsung,mfc-v8 > >+ - tesla,fsd-mfc > You have not introduce "tesla,fsd-mfc" compatible yet, so this should be part > of the patch which adds fsd-mfc compatible > ok. > >+ then: > >+ properties: > >+ clocks: > >+ maxItems: 1 > >+ clock-names: > >+ items: > >+ - const: mfc > >+ > >+ - if: > >+ properties: > >+ compatible: > >+ contains: > >+ enum: > >+ - samsung,mfc-v5 > >+ then: > >+ properties: > >+ clocks: > >+ minItems: 2 > >+ maxItems: 2 > >+ clock-names: > >+ items: > >+ - const: mfc > >+ - const: sclk_mfc > >+ > >+ - if: > >+ properties: > >+ compatible: > >+ contains: > >+ enum: > >+ - samsung,exynos5433-mfc > >+ then: > >+ properties: > >+ clocks: > >+ minItems: 3 > >+ maxItems: 3 > >+ clock-names: > >+ items: > >+ - const: pclk > >+ - const: aclk > >+ - const: aclk_xiu > >+ > >+ - if: > >+ properties: > >+ compatible: > >+ contains: > >+ enum: > >+ - samsung,mfc-v5 > >+ - samsung,mfc-v6 > >+ - samsung,mfc-v7 > >+ - samsung,mfc-v8 > >+ - samsung,exynos5433-mfc > >+ > >+ then: > >+ properties: > >+ iommus: > >+ minItems: 2 > >+ maxItems: 2 > >+ iommu-names: > >+ items: > >+ - const: left > >+ - const: right > >+ > >+ - if: > >+ properties: > >+ compatible: > >+ contains: > >+ enum: > >+ - tesla,fsd-mfc > > Same comments as above. > > My suggestion, first just convert the current s5p-mfc.txt to yaml format and > then add fsd-mfc support. > That way it will be easy for reviewer to understand the changes. > > While reviewing and code walk through, I also noticed that exynos3250.dtsi > and exynos5420.dtsi are using same compatible as "samsung,mfc-v7" but > there "clocks" property is different. That doesn't look correct. > This will cause issues when you convert s5p-mfc.txt to schema format. > I don’t know why that was done that way, but surely this need to be _fixed_ > first as h/w itself are different. > > So my suggestion is to fix exynos3250.dtsi and exynos5420.dtsi compatible > and mfc driver for it first (as a separate patch series). > You can use the same approach what was done for mfc-v8 and mfc-v8-5433. > > ok will fix this issue . > >+ then: > >+ properties: > >+ memory-region: > >+ maxItems: 1 > >+ > >+additionalProperties: false > >+ > >+examples: > >+ - | > >+ #include <dt-bindings/clock/exynos4.h> > >+ #include <dt-bindings/clock/exynos-audss-clk.h> > >+ #include <dt-bindings/interrupt-controller/arm-gic.h> > >+ #include <dt-bindings/interrupt-controller/irq.h> > >+ > >+ codec@13400000 { > >+ compatible = "samsung,mfc-v5"; > >+ reg = <0x13400000 0x10000>; > >+ interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>; > >+ power-domains = <&pd_mfc>; > >+ clocks = <&clock CLK_MFC>, <&clock CLK_SCLK_MFC>; > >+ clock-names = "mfc", "sclk_mfc"; > >+ iommus = <&sysmmu_mfc_l>, <&sysmmu_mfc_r>; > >+ iommu-names = "left", "right"; > >+ > >+ }; > >-- > >2.17.1 > > Thanks for the review.
> -----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.
> -----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 > > > ========================================================== > =
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 > > > > > ========================================================== > > = > >
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 > > > > > > > > > > > ========================================================== > > > > = > > > > > > > > >
On 11/10/2022 14:25, aakarsh jain wrote: > From: Smitha T Murthy <smitha.t@samsung.com> > > Convert DT schema for s5p-mfc in yaml format > > Cc: linux-fsd@tesla.com > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > --- > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > .../bindings/media/samsung,s5p-mfc.yaml | 163 ++++++++++++++++++ Hi Aakarsh, Any plans for new version of this DT schema conversion? The Samsung MFC is one of last remaining bigger pieces for conversion. We already have 100% bindings compliance for ARM64 but the MFC is missing for ARMv7: https://pastebin.com/D8jXnjEe (that's on my pending branch with all patches from LKML collected). Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: 21 February 2023 21:56 > 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 01/15] dt-bindings: media: s5p-mfc: Add new DT > schema for MFC > > On 11/10/2022 14:25, aakarsh jain wrote: > > From: Smitha T Murthy <smitha.t@samsung.com> > > > > Convert DT schema for s5p-mfc in yaml format > > > > Cc: linux-fsd@tesla.com > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > > --- > > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > > .../bindings/media/samsung,s5p-mfc.yaml | 163 > ++++++++++++++++++ > > Hi Aakarsh, > > Any plans for new version of this DT schema conversion? The Samsung MFC > is one of last remaining bigger pieces for conversion. We already have 100% > bindings compliance for ARM64 but the MFC is missing for ARMv7: > > https://protect2.fireeye.com/v1/url?k=9661b377-f7eaa63c-96603838- > 74fe485fb305-bc4c89dbd0204557&q=1&e=1485ea1d-43e8-4541-8078- > 4d13ff6cc42f&u=https%3A%2F%2Fpastebin.com%2FD8jXnjEe > (that's on my pending branch with all patches from LKML collected). > > Best regards, > Krzysztof Hi Krzysztof, I was little occupied with other stuff. I will fix all warnings and errors. Will post the patch soon. Thanks, Aakarsh