| Message ID | 1576828760-13176-2-git-send-email-dikshita@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable video on sc7180 | expand |
Hi Dikshita, Thanks for the patch. On 12/20/19 9:59 AM, Dikshita Agarwal wrote: > This adds Venus video codec DT node for sc7180. > > Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 6876aae2..42c70f5 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -10,6 +10,7 @@ > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/phy/phy-qcom-qusb2.h> > #include <dt-bindings/soc/qcom,rpmh-rsc.h> > +#include <dt-bindings/clock/qcom,videocc-sc7180.h> > > / { > interrupt-parent = <&intc>; > @@ -66,6 +67,11 @@ > compatible = "qcom,cmd-db"; > no-map; > }; > + > + venus_mem: memory@8F600000 { > + reg = <0 0x8F600000 0 0x500000>; Please use lower-case for hex numbers. > + no-map; > + }; > }; > > cpus { > @@ -1042,6 +1048,36 @@ > }; > }; > > + venus: video-codec@aa00000 { > + compatible = "qcom,sc7180-venus"; > + reg = <0 0x0aa00000 0 0xff000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&videocc VENUS_GDSC>, > + <&videocc VCODEC0_GDSC>; > + power-domain-names = "venus", "vcodec0"; > + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, > + <&videocc VIDEO_CC_VENUS_AHB_CLK>, > + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, > + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, > + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; could you align those entries to the first one (you can use tabs and after that spaces to align) > + clock-names = "core", "iface", "bus", > + "vcodec0_core", "vcodec0_bus"; > + iommus = <&apps_smmu 0x0C00 0x60>; lower-case please > + memory-region = <&venus_mem>; > + > + video-core0 { > + compatible = "venus-decoder"; something is wrong with the indentation? Please run checkpatch with --strict > + }; > + > + video-core1 { > + compatible = "venus-encoder"; > + }; > + > + video-firmware { > + iommus = <&apps_smmu 0x0C42 0x0>; lower-case > + }; This subnode should be in sc7180-idp.dts, because we assume that by default the qcom platforms have TZ. > + }; > + > pdc: interrupt-controller@b220000 { > compatible = "qcom,sc7180-pdc", "qcom,pdc"; > reg = <0 0x0b220000 0 0x30000>; >
Hi Stan, Thanks for the review! I will address all the comments in the next version. On 2019-12-20 15:04, Stanimir Varbanov wrote: > Hi Dikshita, > > Thanks for the patch. > > On 12/20/19 9:59 AM, Dikshita Agarwal wrote: >> This adds Venus video codec DT node for sc7180. >> >> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> >> --- >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 >> ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi >> b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> index 6876aae2..42c70f5 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> @@ -10,6 +10,7 @@ >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> #include <dt-bindings/phy/phy-qcom-qusb2.h> >> #include <dt-bindings/soc/qcom,rpmh-rsc.h> >> +#include <dt-bindings/clock/qcom,videocc-sc7180.h> >> >> / { >> interrupt-parent = <&intc>; >> @@ -66,6 +67,11 @@ >> compatible = "qcom,cmd-db"; >> no-map; >> }; >> + >> + venus_mem: memory@8F600000 { >> + reg = <0 0x8F600000 0 0x500000>; > > Please use lower-case for hex numbers. > >> + no-map; >> + }; >> }; >> >> cpus { >> @@ -1042,6 +1048,36 @@ >> }; >> }; >> >> + venus: video-codec@aa00000 { >> + compatible = "qcom,sc7180-venus"; >> + reg = <0 0x0aa00000 0 0xff000>; >> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >> + power-domains = <&videocc VENUS_GDSC>, >> + <&videocc VCODEC0_GDSC>; >> + power-domain-names = "venus", "vcodec0"; >> + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, >> + <&videocc VIDEO_CC_VENUS_AHB_CLK>, >> + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; > > could you align those entries to the first one (you can use tabs and > after that spaces to align) > >> + clock-names = "core", "iface", "bus", >> + "vcodec0_core", "vcodec0_bus"; >> + iommus = <&apps_smmu 0x0C00 0x60>; > > lower-case please > >> + memory-region = <&venus_mem>; >> + >> + video-core0 { >> + compatible = "venus-decoder"; > > something is wrong with the indentation? > > Please run checkpatch with --strict > >> + }; >> + >> + video-core1 { >> + compatible = "venus-encoder"; >> + }; >> + >> + video-firmware { >> + iommus = <&apps_smmu 0x0C42 0x0>; > > lower-case > >> + }; > > This subnode should be in sc7180-idp.dts, because we assume that by > default the qcom platforms have TZ. > >> + }; >> + >> pdc: interrupt-controller@b220000 { >> compatible = "qcom,sc7180-pdc", "qcom,pdc"; >> reg = <0 0x0b220000 0 0x30000>; >>
Hi Stan, On 2019-12-20 15:04, Stanimir Varbanov wrote: > > This subnode should be in sc7180-idp.dts, because we assume that by > default the qcom platforms have TZ. > sc7180.dtsi will not be used on TZ based platforms which was the case for SDM845 where sdm845.dtsi was common for TZ (dragonboards) and non TZ(cheza boards) based platforms. So in order to avoid duplicating this node in other board specific dts files, it would be better to have it here itself. Thanks, Sai
Hi Stan, On 2019-12-24 17:42, Sai Prakash Ranjan wrote: > Hi Stan, > > On 2019-12-20 15:04, Stanimir Varbanov wrote: >> >> This subnode should be in sc7180-idp.dts, because we assume that by >> default the qcom platforms have TZ. >> > > sc7180.dtsi will not be used on TZ based platforms which was the case > for SDM845 > where sdm845.dtsi was common for TZ (dragonboards) and non TZ(cheza > boards) based platforms. > So in order to avoid duplicating this node in other board specific dts > files, it > would be better to have it here itself. > Sorry, please ignore my previous comment. sc7180 will be used for compute platforms and some would be TZ based. Thanks, Sai
On Thu 19 Dec 23:59 PST 2019, Dikshita Agarwal wrote: > This adds Venus video codec DT node for sc7180. > > Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 6876aae2..42c70f5 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -10,6 +10,7 @@ > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/phy/phy-qcom-qusb2.h> > #include <dt-bindings/soc/qcom,rpmh-rsc.h> > +#include <dt-bindings/clock/qcom,videocc-sc7180.h> > > / { > interrupt-parent = <&intc>; > @@ -66,6 +67,11 @@ > compatible = "qcom,cmd-db"; > no-map; > }; > + > + venus_mem: memory@8F600000 { > + reg = <0 0x8F600000 0 0x500000>; > + no-map; > + }; > }; > > cpus { > @@ -1042,6 +1048,36 @@ > }; > }; > > + venus: video-codec@aa00000 { > + compatible = "qcom,sc7180-venus"; > + reg = <0 0x0aa00000 0 0xff000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&videocc VENUS_GDSC>, Should this be aligned with the DT refactoring done for sdm845, where the GDSC is moved into the *-core subnodes etc? Regards, Bjorn > + <&videocc VCODEC0_GDSC>; > + power-domain-names = "venus", "vcodec0"; > + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, > + <&videocc VIDEO_CC_VENUS_AHB_CLK>, > + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, > + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, > + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; > + clock-names = "core", "iface", "bus", > + "vcodec0_core", "vcodec0_bus"; > + iommus = <&apps_smmu 0x0C00 0x60>; > + memory-region = <&venus_mem>; > + > + video-core0 { > + compatible = "venus-decoder"; > + }; > + > + video-core1 { > + compatible = "venus-encoder"; > + }; > + > + video-firmware { > + iommus = <&apps_smmu 0x0C42 0x0>; > + }; > + }; > + > pdc: interrupt-controller@b220000 { > compatible = "qcom,sc7180-pdc", "qcom,pdc"; > reg = <0 0x0b220000 0 0x30000>; > -- > 1.9.1 >
Hi Bjorn, Thanks for your review comments. On 2019-12-29 08:48, Bjorn Andersson wrote: > On Thu 19 Dec 23:59 PST 2019, Dikshita Agarwal wrote: > >> This adds Venus video codec DT node for sc7180. >> >> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> >> --- >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 >> ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi >> b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> index 6876aae2..42c70f5 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi >> @@ -10,6 +10,7 @@ >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> #include <dt-bindings/phy/phy-qcom-qusb2.h> >> #include <dt-bindings/soc/qcom,rpmh-rsc.h> >> +#include <dt-bindings/clock/qcom,videocc-sc7180.h> >> >> / { >> interrupt-parent = <&intc>; >> @@ -66,6 +67,11 @@ >> compatible = "qcom,cmd-db"; >> no-map; >> }; >> + >> + venus_mem: memory@8F600000 { >> + reg = <0 0x8F600000 0 0x500000>; >> + no-map; >> + }; >> }; >> >> cpus { >> @@ -1042,6 +1048,36 @@ >> }; >> }; >> >> + venus: video-codec@aa00000 { >> + compatible = "qcom,sc7180-venus"; >> + reg = <0 0x0aa00000 0 0xff000>; >> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; >> + power-domains = <&videocc VENUS_GDSC>, > > Should this be aligned with the DT refactoring done for sdm845, where > the GDSC is moved into the *-core subnodes etc? This is already aligned to new refactored design i.e clocks/GDSCs are no more core specific. > Regards, > Bjorn > >> + <&videocc VCODEC0_GDSC>; >> + power-domain-names = "venus", "vcodec0"; >> + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, >> + <&videocc VIDEO_CC_VENUS_AHB_CLK>, >> + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, >> + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; >> + clock-names = "core", "iface", "bus", >> + "vcodec0_core", "vcodec0_bus"; >> + iommus = <&apps_smmu 0x0C00 0x60>; >> + memory-region = <&venus_mem>; >> + >> + video-core0 { >> + compatible = "venus-decoder"; >> + }; >> + >> + video-core1 { >> + compatible = "venus-encoder"; >> + }; >> + >> + video-firmware { >> + iommus = <&apps_smmu 0x0C42 0x0>; >> + }; >> + }; >> + >> pdc: interrupt-controller@b220000 { >> compatible = "qcom,sc7180-pdc", "qcom,pdc"; >> reg = <0 0x0b220000 0 0x30000>; >> -- >> 1.9.1 >>
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 6876aae2..42c70f5 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -10,6 +10,7 @@ #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/phy/phy-qcom-qusb2.h> #include <dt-bindings/soc/qcom,rpmh-rsc.h> +#include <dt-bindings/clock/qcom,videocc-sc7180.h> / { interrupt-parent = <&intc>; @@ -66,6 +67,11 @@ compatible = "qcom,cmd-db"; no-map; }; + + venus_mem: memory@8F600000 { + reg = <0 0x8F600000 0 0x500000>; + no-map; + }; }; cpus { @@ -1042,6 +1048,36 @@ }; }; + venus: video-codec@aa00000 { + compatible = "qcom,sc7180-venus"; + reg = <0 0x0aa00000 0 0xff000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&videocc VENUS_GDSC>, + <&videocc VCODEC0_GDSC>; + power-domain-names = "venus", "vcodec0"; + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, + <&videocc VIDEO_CC_VENUS_AHB_CLK>, + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; + clock-names = "core", "iface", "bus", + "vcodec0_core", "vcodec0_bus"; + iommus = <&apps_smmu 0x0C00 0x60>; + memory-region = <&venus_mem>; + + video-core0 { + compatible = "venus-decoder"; + }; + + video-core1 { + compatible = "venus-encoder"; + }; + + video-firmware { + iommus = <&apps_smmu 0x0C42 0x0>; + }; + }; + pdc: interrupt-controller@b220000 { compatible = "qcom,sc7180-pdc", "qcom,pdc"; reg = <0 0x0b220000 0 0x30000>;
This adds Venus video codec DT node for sc7180. Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)