| Message ID | 20241217-add-venus-for-qcs615-v5-3-747395d9e630@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: venus: enable venus on qcs615 | expand |
On 17/12/2024 09:17, Renjiang Han wrote: > + > + video-decoder { > + compatible = "venus-decoder"; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + }; I gave you feedback on this in v4. Could you please provide some commentary on why you're persisting with this ? - Driver configuration should not live in dts - A patchset exists to mitigate this - If you don't want to use that series, what do you propose to resolve this ? Please don't just ignore feedback, either act on it or add to your commit log _why_ you didn't act on it. --- bod
On 12/17/2024 5:38 PM, Bryan O'Donoghue wrote: > On 17/12/2024 09:17, Renjiang Han wrote: >> + >> + video-decoder { >> + compatible = "venus-decoder"; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + }; > > I gave you feedback on this in v4. > > Could you please provide some commentary on why you're persisting with > this ? > > - Driver configuration should not live in dts > - A patchset exists to mitigate this > - If you don't want to use that series, what do you propose > to resolve this ? > > Please don't just ignore feedback, either act on it or add to your > commit log _why_ you didn't act on it. > > --- > bod Thanks for your review. You pointed it out correctly. As replied in v4, I also think your change is a good change, but your change involves many platforms. I am not sure if other guys have comments and when it will pass the review. Currently, it only refers to SC7180 and falls back QCS615 to SC7180. I have verified it on both SC7180 and QCS615 platforms. I think when your change review passes, we only need to remove the video-decoder and video-encoder nodes in the device tree.
On 17/12/2024 09:54, Renjiang Han wrote: > > I think when your change review passes, we only need to remove the > > video-decoder and video-encoder nodes in the device tree. Yes but the _point_ is we shouldn't be adding in driver configuration to dts. Which means I don't think your patch can be applied until we resolve in impasse in venus. --- bod
On Tue, Dec 17, 2024 at 05:54:57PM +0800, Renjiang Han wrote: > > On 12/17/2024 5:38 PM, Bryan O'Donoghue wrote: > > On 17/12/2024 09:17, Renjiang Han wrote: > > > + > > > + video-decoder { > > > + compatible = "venus-decoder"; > > > + }; > > > + > > > + video-encoder { > > > + compatible = "venus-encoder"; > > > + }; > > > > I gave you feedback on this in v4. > > > > Could you please provide some commentary on why you're persisting with > > this ? > > > > - Driver configuration should not live in dts > > - A patchset exists to mitigate this > > - If you don't want to use that series, what do you propose > > to resolve this ? > > > > Please don't just ignore feedback, either act on it or add to your > > commit log _why_ you didn't act on it. > > > > --- > > bod > > Thanks for your review. You pointed it out correctly. As replied in v4, > > I also think your change is a good change, but your change involves many > > platforms. You can help it by reviewing it and then providing a Tested-by tag for it. P.S. Something is wrong with your emails, I see a lot of lines separated by empty lines. It makes it harder to read.
On 12/17/2024 7:01 PM, Bryan O'Donoghue wrote: > On 17/12/2024 09:54, Renjiang Han wrote: >> >> I think when your change review passes, we only need to remove the >> >> video-decoder and video-encoder nodes in the device tree. > > Yes but the _point_ is we shouldn't be adding in driver configuration > to dts. > > Which means I don't think your patch can be applied until we resolve > in impasse in venus. > > --- > bod OK! Thanks. I'll try this patch with your patch and update it with next patch version.
On 12/17/2024 8:09 PM, Dmitry Baryshkov wrote: > On Tue, Dec 17, 2024 at 05:54:57PM +0800, Renjiang Han wrote: >> On 12/17/2024 5:38 PM, Bryan O'Donoghue wrote: >>> On 17/12/2024 09:17, Renjiang Han wrote: >>>> + >>>> + video-decoder { >>>> + compatible = "venus-decoder"; >>>> + }; >>>> + >>>> + video-encoder { >>>> + compatible = "venus-encoder"; >>>> + }; >>> I gave you feedback on this in v4. >>> >>> Could you please provide some commentary on why you're persisting with >>> this ? >>> >>> - Driver configuration should not live in dts >>> - A patchset exists to mitigate this >>> - If you don't want to use that series, what do you propose >>> to resolve this ? >>> >>> Please don't just ignore feedback, either act on it or add to your >>> commit log _why_ you didn't act on it. >>> >>> --- >>> bod >> Thanks for your review. You pointed it out correctly. As replied in v4, >> >> I also think your change is a good change, but your change involves many >> >> platforms. > You can help it by reviewing it and then providing a Tested-by tag for > it. OK. I'll try it. > > P.S. Something is wrong with your emails, I see a lot of lines separated > by empty lines. It makes it harder to read. > Thanks for your reminder. I'll check my environment.
diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi index 37a189e0834d2f4b75ed9deb6fff73da163cb3a3..e92a0d90a4606a450f160c1ed9ee84c16b9c22cf 100644 --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi @@ -427,6 +427,11 @@ smem_region: smem@86000000 { no-map; hwlocks = <&tcsr_mutex 3>; }; + + pil_video_mem: pil-video@93400000 { + reg = <0x0 0x93400000 0x0 0x500000>; + no-map; + }; }; soc: soc@0 { @@ -2806,6 +2811,87 @@ gem_noc: interconnect@9680000 { qcom,bcm-voters = <&apps_bcm_voter>; }; + venus: video-codec@aa00000 { + compatible = "qcom,qcs615-venus", "qcom,sc7180-venus"; + reg = <0x0 0x0aa00000 0x0 0x100000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + + 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"; + + power-domains = <&videocc VENUS_GDSC>, + <&videocc VCODEC0_GDSC>, + <&rpmhpd RPMHPD_CX>; + power-domain-names = "venus", + "vcodec0", + "cx"; + + operating-points-v2 = <&venus_opp_table>; + + interconnects = <&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS + &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>; + interconnect-names = "video-mem", + "cpu-cfg"; + + iommus = <&apps_smmu 0xe40 0x20>; + + memory-region = <&pil_video_mem>; + + status = "disabled"; + + venus_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-133330000 { + opp-hz = /bits/ 64 <133330000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-240000000 { + opp-hz = /bits/ 64 <240000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + }; + + opp-380000000 { + opp-hz = /bits/ 64 <380000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + + opp-410000000 { + opp-hz = /bits/ 64 <410000000>; + required-opps = <&rpmhpd_opp_turbo>; + }; + + opp-460000000 { + opp-hz = /bits/ 64 <460000000>; + required-opps = <&rpmhpd_opp_turbo_l1>; + }; + }; + + video-decoder { + compatible = "venus-decoder"; + }; + + video-encoder { + compatible = "venus-encoder"; + }; + }; + videocc: clock-controller@ab00000 { compatible = "qcom,qcs615-videocc"; reg = <0 0xab00000 0 0x10000>;
Add venus node into devicetree for the qcs615 video and fallback qcs615 to sc7180 due to the same video core. Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> --- arch/arm64/boot/dts/qcom/qcs615.dtsi | 86 ++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)