Re: [PATCH 3/3] arm64: dts: mt8195: Add video decoder node

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 09/11/22 08:35, Yunfei Dong ha scritto:
> Add video decoder node to mt8195 device tree.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
> dtbs_check pass.
> ---
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 63 ++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 905d1a90b406..ffabf91d4273 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -1874,6 +1874,69 @@
>   			power-domains = <&spm MT8195_POWER_DOMAIN_CAM>;
>   		};
>   
> +		video-codec@18000000 {
> +			compatible = "mediatek,mt8195-vcodec-dec";
> +			mediatek,scp = <&scp>;
> +			iommus = <&iommu_vdo M4U_PORT_L21_VDEC_MC_EXT>;
> +			dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			reg = <0 0x18000000 0 0x1000>,      /* VDEC_SYS */
> +			      <0 0x18004000 0 0x1000>;      /* VDEC_RACING_CTRL */

Since we're adding register descriptions to the schema file, you don't need any
comments in front of the iospaces that you're declaring here... this means that
it also fits on one line:
			reg = <0 0x18000000 0 0x1000>, <0 0x18004000 0 0x1000>;

> +			ranges = <0 0 0 0x18000000 0 0x26000>;
> +			clocks = <&topckgen CLK_TOP_VDEC>,
> +			         <&topckgen CLK_TOP_UNIVPLL_D4>;
> +			clock-names = "vdec-sel", "top";
> +			assigned-clocks = <&topckgen CLK_TOP_VDEC>;
> +			assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D4>;
> +
> +			vcodec-lat-soc@2000 {
> +				compatible = "mediatek,mtk-vcodec-lat-soc";
> +				reg = <0 0x2000 0 0x800>;		/* VDEC_MISC */

We can perhaps add descriptions to the schema file for VDEC_MISC as well,
meaning that we don't need that comment as well.

> +				iommus = <&iommu_vpp M4U_PORT_L23_VDEC_UFO_ENC_EXT>,
> +					 <&iommu_vpp M4U_PORT_L23_VDEC_RDMA_EXT>;
> +				clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> +					 <&vdecsys_soc CLK_VDEC_SOC_LAT>;
> +				clock-names = "vdec-soc-vdec", "vdec-soc-lat";
> +				power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
> +			};
> +
> +			vcodec-lat@10000 {
> +				compatible = "mediatek,mtk-vcodec-lat";
> +				reg = <0 0x10000 0 0x800>;		/* VDEC_MISC */

same here

> +				interrupts = <GIC_SPI 708 IRQ_TYPE_LEVEL_HIGH 0>;
> +				iommus = <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_VLD_EXT>,
> +					 <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_VLD2_EXT>,
> +					 <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_AVC_MC_EXT>,
> +					 <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_PRED_RD_EXT>,
> +					 <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_TILE_EXT>,
> +					 <&iommu_vdo M4U_PORT_L24_VDEC_LAT0_WDMA_EXT>;
> +				clocks = <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> +					 <&vdecsys_soc CLK_VDEC_SOC_LAT>;
> +				clock-names = "vdec-soc-vdec", "vdec-soc-lat";
> +				power-domains = <&spm MT8195_POWER_DOMAIN_VDEC0>;
> +			};
> +
> +			vcodec-core@25000 {
> +				compatible = "mediatek,mtk-vcodec-core";
> +				reg = <0 0x25000 0 0x1000>;		/* VDEC_CORE_MISC */

...and same here too.

Regards,
Angelo