| Message ID | 20231213122017.102100-1-eugen.hristev@collabora.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/4] dt-bindings: media: mtk-vcodec-encoder: add dma-ranges | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Il 13/12/23 13:20, Eugen Hristev ha scritto: > From: Kyrie Wu <kyrie.wu@mediatek.com> > > Add video encoder node. > > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com> > Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> > Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> > [eugen.hristev@collabora.com: minor cleanup] > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> > --- > arch/arm64/boot/dts/mediatek/mt8186.dtsi | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi > index 66ead3f23336..8535ff2b44e9 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi > @@ -1993,6 +1993,30 @@ larb7: smi@17010000 { > power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; > }; > > + venc: venc@17020000 { > + compatible = "mediatek,mt8183-vcodec-enc"; > + #address-cells = <2>; > + #size-cells = <2>; > + reg = <0 0x17020000 0 0x2000>; > + interrupts = <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH 0>; > + iommus = <&iommu_mm IOMMU_PORT_L7_VENC_RCPU>, > + <&iommu_mm IOMMU_PORT_L7_VENC_REC>, > + <&iommu_mm IOMMU_PORT_L7_VENC_BSDMA>, > + <&iommu_mm IOMMU_PORT_L7_VENC_SV_COMV>, > + <&iommu_mm IOMMU_PORT_L7_VENC_RD_COMV>, > + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_LUMA>, > + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_CHROMA>, > + <&iommu_mm IOMMU_PORT_L7_VENC_REF_LUMA>, > + <&iommu_mm IOMMU_PORT_L7_VENC_REF_CHROMA>; > + dma-ranges = <0x1 0x0 0x1 0x0 0x1 0x0>; > + mediatek,scp = <&scp>; > + clocks = <&vencsys CLK_VENC_CKE1_VENC>; > + clock-names = "MT_CG_VENC"; clock-names = "venc"; (please no underscores and please lower case) > + assigned-clocks = <&topckgen CLK_TOP_VENC>; > + assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D3>; > + power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; > + }; ....also: The following order of properties in device nodes is preferred: 1. "compatible" 2. "reg" 3. "ranges" 4. Standard/common properties (defined by common bindings, e.g. without vendor-prefixes) 5. Vendor-specific properties 6. "status" (if applicable) 7. Child nodes, where each node is preceded with a blank line Documentation/devicetree/bindings/dts-coding-style.rst Please reorder as per the DTS coding style document, and also please rename the venc node to use a generic name, such as "video-encoder@xxxx" Cheers, Angelo
Il 13/12/23 13:20, Eugen Hristev ha scritto: > It is possible that mtk_vcodec_enc_pw_on fails, and in that scenario > the PM counter is not incremented, and subsequent call to > mtk_vcodec_enc_pw_off decrements the counter, leading to a PM imbalance. > Fix by bailing out of venc_if_encode in the case when mtk_vcodec_enc_pw_on > fails. > > Fixes: 4e855a6efa54 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver") > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregnoòcollabora.com>
Il 14/12/23 11:44, AngeloGioacchino Del Regno ha scritto: > Il 13/12/23 13:20, Eugen Hristev ha scritto: >> From: Kyrie Wu <kyrie.wu@mediatek.com> >> >> Add video encoder node. >> >> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com> >> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> >> Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> >> [eugen.hristev@collabora.com: minor cleanup] >> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> >> --- >> arch/arm64/boot/dts/mediatek/mt8186.dtsi | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> b/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> index 66ead3f23336..8535ff2b44e9 100644 >> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> @@ -1993,6 +1993,30 @@ larb7: smi@17010000 { >> power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; >> }; >> + venc: venc@17020000 { >> + compatible = "mediatek,mt8183-vcodec-enc"; Sorry for the double email; I've just noticed: where's mediatek,mt8186-vcodec-enc? :-) >> + #address-cells = <2>; >> + #size-cells = <2>; >> + reg = <0 0x17020000 0 0x2000>; >> + interrupts = <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH 0>; >> + iommus = <&iommu_mm IOMMU_PORT_L7_VENC_RCPU>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_REC>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_BSDMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_SV_COMV>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_RD_COMV>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_LUMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_CHROMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_REF_LUMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_REF_CHROMA>; >> + dma-ranges = <0x1 0x0 0x1 0x0 0x1 0x0>; >> + mediatek,scp = <&scp>; >> + clocks = <&vencsys CLK_VENC_CKE1_VENC>; >> + clock-names = "MT_CG_VENC"; > > clock-names = "venc"; (please no underscores and please lower case) > >> + assigned-clocks = <&topckgen CLK_TOP_VENC>; >> + assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D3>; >> + power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; >> + }; > > > ....also: > > The following order of properties in device nodes is preferred: > > 1. "compatible" > 2. "reg" > 3. "ranges" > 4. Standard/common properties (defined by common bindings, e.g. without > vendor-prefixes) > 5. Vendor-specific properties > 6. "status" (if applicable) > 7. Child nodes, where each node is preceded with a blank line > > Documentation/devicetree/bindings/dts-coding-style.rst > > Please reorder as per the DTS coding style document, and also please rename the > venc node to use a generic name, such as "video-encoder@xxxx" > > Cheers, > Angelo
On 12/14/23 12:50, AngeloGioacchino Del Regno wrote: > Il 14/12/23 11:44, AngeloGioacchino Del Regno ha scritto: >> Il 13/12/23 13:20, Eugen Hristev ha scritto: >>> From: Kyrie Wu <kyrie.wu@mediatek.com> >>> >>> Add video encoder node. >>> >>> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com> >>> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> >>> Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> >>> [eugen.hristev@collabora.com: minor cleanup] >>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> >>> --- >>> arch/arm64/boot/dts/mediatek/mt8186.dtsi | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi >>> b/arch/arm64/boot/dts/mediatek/mt8186.dtsi >>> index 66ead3f23336..8535ff2b44e9 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi >>> @@ -1993,6 +1993,30 @@ larb7: smi@17010000 { >>> power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; >>> }; >>> + venc: venc@17020000 { >>> + compatible = "mediatek,mt8183-vcodec-enc"; > > Sorry for the double email; > > I've just noticed: where's mediatek,mt8186-vcodec-enc? :-) Hi, There is none. This just works exactly as mt8183, thus reusing the same compatible. Do you want a new dedicated mt8186 compatible as well for the situation *just in case* some specific difference showing up later ? Eugen > >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + reg = <0 0x17020000 0 0x2000>; >>> + interrupts = <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH 0>; >>> + iommus = <&iommu_mm IOMMU_PORT_L7_VENC_RCPU>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_REC>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_BSDMA>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_SV_COMV>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_RD_COMV>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_LUMA>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_CHROMA>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_REF_LUMA>, >>> + <&iommu_mm IOMMU_PORT_L7_VENC_REF_CHROMA>; >>> + dma-ranges = <0x1 0x0 0x1 0x0 0x1 0x0>; >>> + mediatek,scp = <&scp>; >>> + clocks = <&vencsys CLK_VENC_CKE1_VENC>; >>> + clock-names = "MT_CG_VENC"; >> >> clock-names = "venc"; (please no underscores and please lower case) >> >>> + assigned-clocks = <&topckgen CLK_TOP_VENC>; >>> + assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D3>; >>> + power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; >>> + }; >> >> >> ....also: >> >> The following order of properties in device nodes is preferred: >> >> 1. "compatible" >> 2. "reg" >> 3. "ranges" >> 4. Standard/common properties (defined by common bindings, e.g. without >> vendor-prefixes) >> 5. Vendor-specific properties >> 6. "status" (if applicable) >> 7. Child nodes, where each node is preceded with a blank line >> >> Documentation/devicetree/bindings/dts-coding-style.rst >> >> Please reorder as per the DTS coding style document, and also please rename the >> venc node to use a generic name, such as "video-encoder@xxxx" >> >> Cheers, >> Angelo > > > _______________________________________________ > Kernel mailing list -- kernel@mailman.collabora.com > To unsubscribe send an email to kernel-leave@mailman.collabora.com
On 12/14/23 12:44, AngeloGioacchino Del Regno wrote: > Il 13/12/23 13:20, Eugen Hristev ha scritto: >> From: Kyrie Wu <kyrie.wu@mediatek.com> >> >> Add video encoder node. >> >> Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com> >> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> >> Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> >> [eugen.hristev@collabora.com: minor cleanup] >> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> >> --- >> arch/arm64/boot/dts/mediatek/mt8186.dtsi | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> index 66ead3f23336..8535ff2b44e9 100644 >> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi >> @@ -1993,6 +1993,30 @@ larb7: smi@17010000 { >> power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; >> }; >> >> + venc: venc@17020000 { >> + compatible = "mediatek,mt8183-vcodec-enc"; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + reg = <0 0x17020000 0 0x2000>; >> + interrupts = <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH 0>; >> + iommus = <&iommu_mm IOMMU_PORT_L7_VENC_RCPU>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_REC>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_BSDMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_SV_COMV>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_RD_COMV>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_LUMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_CUR_CHROMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_REF_LUMA>, >> + <&iommu_mm IOMMU_PORT_L7_VENC_REF_CHROMA>; >> + dma-ranges = <0x1 0x0 0x1 0x0 0x1 0x0>; >> + mediatek,scp = <&scp>; >> + clocks = <&vencsys CLK_VENC_CKE1_VENC>; >> + clock-names = "MT_CG_VENC"; > > clock-names = "venc"; (please no underscores and please lower case) The clock name must be `venc_sel` (*with* underscores) and it's ABI as defined in Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml so I will it change to that. > >> + assigned-clocks = <&topckgen CLK_TOP_VENC>; >> + assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D3>; >> + power-domains = <&spm MT8186_POWER_DOMAIN_VENC>; >> + }; > > > ....also: > > The following order of properties in device nodes is preferred: > > 1. "compatible" > 2. "reg" > 3. "ranges" > 4. Standard/common properties (defined by common bindings, e.g. without > vendor-prefixes) > 5. Vendor-specific properties > 6. "status" (if applicable) > 7. Child nodes, where each node is preceded with a blank line > > Documentation/devicetree/bindings/dts-coding-style.rst > > Please reorder as per the DTS coding style document, and also please rename the > venc node to use a generic name, such as "video-encoder@xxxx" > > Cheers, > Angelo > _______________________________________________ > Kernel mailing list -- kernel@mailman.collabora.com > To unsubscribe send an email to kernel-leave@mailman.collabora.com
diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml index a2051b31fa29..403530de5624 100644 --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml @@ -38,6 +38,8 @@ properties: minItems: 1 maxItems: 5 + dma-ranges: true + assigned-clocks: true assigned-clock-parents: true
As IOMMUs are supported, dma-ranges is not mentioned but additionalProperties=false, thus we have an error when adding dma-ranges. Add dma-ranges as a possible property because this may be present. Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> --- .../devicetree/bindings/media/mediatek,vcodec-encoder.yaml | 2 ++ 1 file changed, 2 insertions(+)