| Message ID | 20231025102216.50480-1-aakarsh.jain@samsung.com |
|---|---|
| Headers | show |
| Series | Add MFC V12 support | expand |
On 25/10/2023 12:22, Aakarsh Jain wrote: > Add Tesla FSD MFC(MFC v12) compatible. > > Cc: linux-fsd@tesla.com > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > --- No changelog and your cover letter does not explain what happened here. Specifically, why did you decide to ignore received tag. > .../bindings/media/samsung,s5p-mfc.yaml | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > index 084b44582a43..c30eb309f670 100644 > --- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > @@ -24,6 +24,7 @@ properties: > - samsung,mfc-v7 # Exynos5420 > - samsung,mfc-v8 # Exynos5800 > - samsung,mfc-v10 # Exynos7880 > + - tesla,fsd-mfc # Tesla FSD > - items: > - enum: > - samsung,exynos3250-mfc # Exynos3250 > @@ -165,6 +166,21 @@ allOf: > minItems: 1 > maxItems: 2 > > + - if: > + properties: > + compatible: > + contains: > + enum: > + - tesla,fsd-mfc > + then: > + properties: > + clocks: > + maxItems: 1 > + clock-names: > + items: > + - const: mfc > + iommus: false That's odd. How so? MFC v12 does not support IOMMU? Best regards, Krzysztof
Hello Krzysztof > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: 25 October 2023 18:30 > 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; > krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com; > david.plowman@raspberrypi.com; mark.rutland@arm.com; > robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung- > soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com; > alim.akhtar@samsung.com; aswani.reddy@samsung.com; > pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux- > fsd@tesla.com > Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12 > variant > > On 25/10/2023 12:22, Aakarsh Jain wrote: > > Add Tesla FSD MFC(MFC v12) compatible. > > > > Cc: linux-fsd@tesla.com > > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > > --- > > No changelog and your cover letter does not explain what happened here. > Specifically, why did you decide to ignore received tag. > Last patch series we had two different patches for schema which was one for adding MFCv12 compatible string and other for adding its HW properties. In one of the patches you gave reviewed-by tag. Since mfc dt_schema got merged already, and this is relatively new patch so thought of getting reviewed again. Link to those patches: https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/ https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-3-aakarsh.jain@samsung.com/ if you are ok, I will add your reviewed-by in next patch series. > > .../bindings/media/samsung,s5p-mfc.yaml | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/samsung,s5p- > mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p- > mfc.yaml > > index 084b44582a43..c30eb309f670 100644 > > --- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > > +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > > @@ -24,6 +24,7 @@ properties: > > - samsung,mfc-v7 # Exynos5420 > > - samsung,mfc-v8 # Exynos5800 > > - samsung,mfc-v10 # Exynos7880 > > + - tesla,fsd-mfc # Tesla FSD > > - items: > > - enum: > > - samsung,exynos3250-mfc # Exynos3250 > > @@ -165,6 +166,21 @@ allOf: > > minItems: 1 > > maxItems: 2 > > > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - tesla,fsd-mfc > > + then: > > + properties: > > + clocks: > > + maxItems: 1 > > + clock-names: > > + items: > > + - const: mfc > > + iommus: false > > That's odd. How so? MFC v12 does not support IOMMU? > MFC v12 do support IOMMU. But currently it is not enabled in SW (has dependencies on some of the floating dma-mapping patches) and not tested on upstream kernel. Current patch sets intend to add support for MFCv12 using reserve memory and later patches related to enable iommu will be posted (after resolving the dependencies). So I marked iommu property as false. Now what is your suggestion here? Should I keep iommu as false or add memory-region as below? Ex- - if: properties: compatible: contains: enum: - tesla,fsd-mfc then: properties: clocks: maxItems: 1 clock-names: items: - const: mfc memory-region: maxItems: 1 > Best regards, > Krzysztof Thanks for review.
On 26/10/2023 15:31, Aakarsh Jain wrote: > Hello Krzysztof > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Sent: 25 October 2023 18:30 >> 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; >> krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com; >> david.plowman@raspberrypi.com; mark.rutland@arm.com; >> robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung- >> soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com; >> alim.akhtar@samsung.com; aswani.reddy@samsung.com; >> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux- >> fsd@tesla.com >> Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12 >> variant >> >> On 25/10/2023 12:22, Aakarsh Jain wrote: >>> Add Tesla FSD MFC(MFC v12) compatible. >>> >>> Cc: linux-fsd@tesla.com >>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> >>> --- >> >> No changelog and your cover letter does not explain what happened here. >> Specifically, why did you decide to ignore received tag. >> > Last patch series we had two different patches for schema which was one for adding MFCv12 compatible string and other for adding its HW properties. > In one of the patches you gave reviewed-by tag. Since mfc dt_schema got merged already, and this is relatively new patch so thought of getting reviewed again. > > Link to those patches: > https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/ > https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-3-aakarsh.jain@samsung.com/ > > if you are ok, I will add your reviewed-by in next patch series. It is okay to drop Reviewed-by tag, but this should be explicitly mentioned in the changelog with a reason. > >>> .../bindings/media/samsung,s5p-mfc.yaml | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/media/samsung,s5p- >> mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p- >> mfc.yaml >>> index 084b44582a43..c30eb309f670 100644 >>> --- a/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml >>> +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml >>> @@ -24,6 +24,7 @@ properties: >>> - samsung,mfc-v7 # Exynos5420 >>> - samsung,mfc-v8 # Exynos5800 >>> - samsung,mfc-v10 # Exynos7880 >>> + - tesla,fsd-mfc # Tesla FSD >>> - items: >>> - enum: >>> - samsung,exynos3250-mfc # Exynos3250 >>> @@ -165,6 +166,21 @@ allOf: >>> minItems: 1 >>> maxItems: 2 >>> >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - tesla,fsd-mfc >>> + then: >>> + properties: >>> + clocks: >>> + maxItems: 1 >>> + clock-names: >>> + items: >>> + - const: mfc >>> + iommus: false >> >> That's odd. How so? MFC v12 does not support IOMMU? >> > MFC v12 do support IOMMU. But currently it is not enabled in SW (has dependencies on some of the floating dma-mapping patches) and not tested on upstream kernel. Bindings describe hardware, not software. > Current patch sets intend to add support for MFCv12 using reserve memory and later patches related to enable iommu will be posted (after resolving the dependencies). So I marked iommu property as false. > Now what is your suggestion here? Should I keep iommu as false or add memory-region as below? I expect complete picture of the hardware, not something limited to current driver, so for sure iommus must be there. Please wrap your emails according to mailing lists rules. Best regards, Krzysztof
Hi Aakarsh > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, November 1, 2023 1:54 PM > 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; > krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com; > david.plowman@raspberrypi.com; mark.rutland@arm.com; > robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung- > soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com; > alim.akhtar@samsung.com; aswani.reddy@samsung.com; > pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux- > fsd@tesla.com > Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12 > variant > > On 26/10/2023 15:31, Aakarsh Jain wrote: > > Hello Krzysztof > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Sent: 25 October 2023 18:30 > >> 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; > >> krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com; > >> david.plowman@raspberrypi.com; mark.rutland@arm.com; > >> robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung- > >> soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com; > >> alim.akhtar@samsung.com; aswani.reddy@samsung.com; > >> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux- > >> fsd@tesla.com > >> Subject: Re: [Patch v4 01/11] dt-bindings: media: s5p-mfc: Add mfcv12 > >> variant > >> > >> On 25/10/2023 12:22, Aakarsh Jain wrote: > >>> Add Tesla FSD MFC(MFC v12) compatible. > >>> > >>> Cc: linux-fsd@tesla.com > >>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com> > >>> --- > >> > >> No changelog and your cover letter does not explain what happened > here. > >> Specifically, why did you decide to ignore received tag. > >> > > Last patch series we had two different patches for schema which was one > for adding MFCv12 compatible string and other for adding its HW properties. > > In one of the patches you gave reviewed-by tag. Since mfc dt_schema got > merged already, and this is relatively new patch so thought of getting > reviewed again. > > > > Link to those patches: > > https://patchwork.kernel.org/project/linux- > media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/ > > https://patchwork.kernel.org/project/linux- > media/patch/20221011122516.32135-3-aakarsh.jain@samsung.com/ > > > > if you are ok, I will add your reviewed-by in next patch series. > > It is okay to drop Reviewed-by tag, but this should be explicitly mentioned in > the changelog with a reason. > > > > >>> .../bindings/media/samsung,s5p-mfc.yaml | 16 > ++++++++++++++++ > >>> 1 file changed, 16 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/samsung,s5p- > >> mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p- > >> mfc.yaml > >>> index 084b44582a43..c30eb309f670 100644 > >>> --- a/Documentation/devicetree/bindings/media/samsung,s5p- > mfc.yaml > >>> +++ b/Documentation/devicetree/bindings/media/samsung,s5p- > mfc.yaml > >>> @@ -24,6 +24,7 @@ properties: > >>> - samsung,mfc-v7 # Exynos5420 > >>> - samsung,mfc-v8 # Exynos5800 > >>> - samsung,mfc-v10 # Exynos7880 > >>> + - tesla,fsd-mfc # Tesla FSD > >>> - items: > >>> - enum: > >>> - samsung,exynos3250-mfc # Exynos3250 > >>> @@ -165,6 +166,21 @@ allOf: > >>> minItems: 1 > >>> maxItems: 2 > >>> > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - tesla,fsd-mfc > >>> + then: > >>> + properties: > >>> + clocks: > >>> + maxItems: 1 > >>> + clock-names: > >>> + items: > >>> + - const: mfc > >>> + iommus: false > >> > >> That's odd. How so? MFC v12 does not support IOMMU? > >> > > MFC v12 do support IOMMU. But currently it is not enabled in SW (has > dependencies on some of the floating dma-mapping patches) and not tested > on upstream kernel. > > Bindings describe hardware, not software. > > > Current patch sets intend to add support for MFCv12 using reserve > memory and later patches related to enable iommu will be posted (after > resolving the dependencies). So I marked iommu property as false. > > Now what is your suggestion here? Should I keep iommu as false or add > memory-region as below? > > I expect complete picture of the hardware, not something limited to current > driver, so for sure iommus must be there. > As Krzysztof mentioned, DT binding should explain all the hardware features supported by SoC / IPs. Incase a feature is not enabled for some reason, that need to be handled in the dts file. > Please wrap your emails according to mailing lists rules. > > Best regards, > Krzysztof
Hello, > -----Original Message----- > From: Aakarsh Jain <aakarsh.jain@samsung.com> > Sent: 25 October 2023 15:52 > 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; > krzysztof.kozlowski+dt@linaro.org; dillon.minfei@gmail.com; > david.plowman@raspberrypi.com; mark.rutland@arm.com; > robh+dt@kernel.org; conor+dt@kernel.org; linux-samsung- > soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com; > alim.akhtar@samsung.com; aswani.reddy@samsung.com; > pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; > aakarsh.jain@samsung.com > Subject: [Patch v4 00/11] 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 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 > > Changes since v3: > -Removed vp9 codec support for now and just keeping MFC v12 base > patches with necessary hardware controls, decoder, encoder and structural > changes. Also covers luma dbp, chroma dpb and mv sizes for each codec as > per the UM for MFCv12, along with appropriate alignment. > v3 link: https://patchwork.kernel.org/project/linux- > media/cover/20221011122516.32135-1-aakarsh.jain@samsung.com/ > > Changes since v2: > -Addressed review comments by Rob Herring. > This was regarding the errors found by Rob bot in yaml file. File > 'samsung,s5p-mfc.yaml' is already converted into json schema and is > merged. > > -Addressed review comments by Krzysztof Kozlowski. > This was regarding depricated properties mentioned in s5p-mfc.txt file. > Review comment was addressed and 'samsung,s5p-mfc.yaml' is already > merged now. > > -Addressed review comments by Andi Shyti. > This was regarding addition of 'MFC_V10PLUS_BITS' macro in > 's5p_mfc_common.h file. > v2 link: https://patchwork.kernel.org/project/linux- > media/cover/20220907064715.55778-1-smitha.t@samsung.com/ > > Changes since v1: > -Addressed review comments by Krzysztof Kozlowski. > Separated bug fixes patches, resent again with fix tag and those are merged > now. > -Added SoC based compatible string. > > -Addressed review comments by Andrzej Hajda Assigned width64 and > height32 variable with ALIGN(ctx->img_..) used in the code in > 's5p_mfc_opr_v6.c' file. > v1 link: https://patchwork.kernel.org/project/linux- > media/patch/20220517125548.14746-2-smitha.t@samsung.com/ > > Aakarsh Jain (11): > dt-bindings: media: s5p-mfc: Add mfcv12 variant > media: s5p-mfc: Rename IS_MFCV10 macro > media: s5p-mfc: Add initial support for MFCv12 > 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 > > .../bindings/media/samsung,s5p-mfc.yaml | 16 + > arch/arm64/boot/dts/tesla/fsd.dtsi | 21 ++ > .../platform/samsung/s5p-mfc/regs-mfc-v12.h | 52 +++ > .../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_common.h | 29 +- > .../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 | 138 +++++--- > .../platform/samsung/s5p-mfc/s5p_mfc_opr.h | 14 +- > .../platform/samsung/s5p-mfc/s5p_mfc_opr_v5.c | 12 +- > .../platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c | 312 +++++++++++++++- > -- > .../platform/samsung/s5p-mfc/s5p_mfc_opr_v6.h | 7 +- > 14 files changed, 574 insertions(+), 127 deletions(-) create mode 100644 > drivers/media/platform/samsung/s5p-mfc/regs-mfc-v12.h > > -- > 2.17.1 Gentle reminder to review MFC v12 patches. I see Krzysztof have some comments on dt-schema patch. Waiting for reviewers to review driver changes as well. Post which I can send v5 changes. Thanks