| Message ID | 20230605162030.274395-1-nfraprado@collabora.com |
|---|---|
| Headers | show |
| Series | Enable decoder for mt8183 | expand |
Il 05/06/23 18:20, Nícolas F. R. A. Prado ha scritto: > MT8173 and MT8183 have different clocks, and consequently clock-names. > Relax the number of clocks and set clock-names based on compatible. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Il 05/06/23 18:20, Nícolas F. R. A. Prado ha scritto: > From: Yunfei Dong <yunfei.dong@mediatek.com> > > Add node for the hardware decoder present on the MT8183 SoC. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > Signed-off-by: Qianqian Yan <qianqian.yan@mediatek.com> > Signed-off-by: Frederic Chen <frederic.chen@mediatek.com> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 39 ++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi > index 5169779d01df..8bb10ed67e87 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi > @@ -2019,6 +2019,45 @@ vdecsys: syscon@16000000 { > #clock-cells = <1>; > }; > > + vcodec_dec: video-codec@16020000 { > + compatible = "mediatek,mt8183-vcodec-dec"; > + reg = <0 0x16020000 0 0x1000>, /* VDEC_MISC */ > + <0 0x16021000 0 0x800>, /* VDEC_VLD */ > + <0 0x16021800 0 0x800>, /* VDEC_TOP */ > + <0 0x16022000 0 0x1000>, /* VDEC_MC */ > + <0 0x16023000 0 0x1000>, /* VDEC_AVCVLD */ > + <0 0x16024000 0 0x1000>, /* VDEC_AVCMV */ > + <0 0x16025000 0 0x1000>, /* VDEC_PP */ > + <0 0x16026800 0 0x800>, /* VP8_VD */ > + <0 0x16027000 0 0x800>, /* VP6_VD */ > + <0 0x16027800 0 0x800>, /* VP8_VL */ > + <0 0x16028400 0 0x400>; /* VP9_VD */ > + reg-names = "misc", > + "ld", > + "top", > + "cm", > + "ad", > + "av", > + "pp", > + "hwd", > + "hwq", > + "hwb", > + "hwg"; Do we really need one line for each 2/3 characters reg name? :-P Regards, Angelo
On 05/06/2023 18:20, Nícolas F. R. A. Prado wrote: > MT8173 and MT8183 have different clocks, and consequently clock-names. > Relax the number of clocks and set clock-names based on compatible. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > --- > > .../media/mediatek,vcodec-decoder.yaml | 29 +++++++++++++------ > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml > index fad59b486d5d..57d5ca776df0 100644 > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml > @@ -27,18 +27,12 @@ properties: > maxItems: 1 > > clocks: > + minItems: 1 > maxItems: 8 > > clock-names: > - items: > - - const: vcodecpll > - - const: univpll_d2 > - - const: clk_cci400_sel > - - const: vdec_sel > - - const: vdecpll > - - const: vencpll > - - const: venc_lt_sel > - - const: vdec_bus_clk_src > + minItems: 1 > + maxItems: 8 > > assigned-clocks: true > > @@ -88,6 +82,11 @@ allOf: > required: > - mediatek,scp > > + properties: > + clock-names: > + items: > + - const: vdec You should constrain also clocks as they must be in sync with names. > + > - if: > properties: > compatible: > @@ -99,6 +98,18 @@ allOf: > required: > - mediatek,vpu > > + properties: > + clock-names: > + items: > + - const: vcodecpll > + - const: univpll_d2 > + - const: clk_cci400_sel > + - const: vdec_sel > + - const: vdecpll > + - const: vencpll > + - const: venc_lt_sel > + - const: vdec_bus_clk_src Ditto. Best regards, Krzysztof
On 05/06/2023 18:20, Nícolas F. R. A. Prado wrote: > The binding expects the first register space to be VDEC_SYS. But on > mt8183, which uses the stateless decoders, this space is used only for > controlling clocks and resets, which are better described as separate > clock-controller and reset-controller nodes. > > In fact, in mt8173's devicetree there are already such separate > clock-controller nodes, which cause duplicate addresses between the > vdecsys node and the vcodec node. But for this SoC, since the stateful > decoder code makes other uses of the VDEC_SYS register space, it's not > straightforward to remove it. > > In order to avoid the same address conflict to happen on mt8183, > since the only current use of the VDEC_SYS register space in > the driver is to read the status of a clock that indicates the hardware > is active, remove the VDEC_SYS register space from the binding and > describe an extra clock that will be used to directly check the hardware > status. > > Also add reg-names to be able to tell that this new register schema is > used, so the driver can keep backward compatibility. > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > --- > > .../media/mediatek,vcodec-decoder.yaml | 29 +++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml > index 6447e6c86f29..36a53b2484d6 100644 > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml > @@ -21,17 +21,21 @@ properties: > - mediatek,mt8183-vcodec-dec > > reg: > + minItems: 11 > maxItems: 12 > > + reg-names: > + minItems: 11 maxItems > + > interrupts: > maxItems: 1 > > clocks: > - minItems: 1 > + minItems: 2 It does not make any sense. Just two patches ago you made it 1! Don't add incorrect values which are immediately changed in the same patchset. > maxItems: 8 > > clock-names: > - minItems: 1 > + minItems: 2 > maxItems: 8 > > assigned-clocks: true > @@ -84,6 +88,24 @@ allOf: > clock-names: > items: > - const: vdec > + - const: active > + > + reg: > + maxItems: 11 > + > + reg-names: > + items: > + - const: misc > + - const: ld > + - const: top > + - const: cm > + - const: ad > + - const: av > + - const: pp > + - const: hwd > + - const: hwq > + - const: hwb > + - const: hwg > > - if: > properties: > @@ -108,6 +130,9 @@ allOf: > - const: venc_lt_sel > - const: vdec_bus_clk_src > > + reg: > + minItems: 12 so max can be 1000? Best regards, Krzysztof