On Fri, 24 Nov 2023 at 14:30, Vikash Garodia <quic_vgarodia@xxxxxxxxxxx> wrote: > > On 11/24/2023 5:05 PM, Luca Weiss wrote: > > On Fri Nov 24, 2023 at 7:38 AM CET, Vikash Garodia wrote: > >> > >> On 11/22/2023 7:50 PM, Luca Weiss wrote: > >>> On Wed Nov 22, 2023 at 2:17 PM CET, Vikash Garodia wrote: > >>>> > >>>> On 10/2/2023 7:50 PM, Luca Weiss wrote: > >>>>> If the video-firmware node is present, the venus driver assumes we're on > >>>>> a system that doesn't use TZ for starting venus, like on ChromeOS > >>>>> devices. > >>>>> > >>>>> Move the video-firmware node to chrome-common.dtsi so we can use venus > >>>>> on a non-ChromeOS devices. > >>>>> > >>>>> At the same time also disable the venus node by default in the dtsi, > >>>>> like it's done on other SoCs. > >>>>> > >>>>> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > >>>>> Signed-off-by: Luca Weiss <luca.weiss@xxxxxxxxxxxxx> > >>>>> --- > >>>>> arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 8 ++++++++ > >>>>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 6 ++---- > >>>>> 2 files changed, 10 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi > >>>>> index 5d462ae14ba1..cd491e46666d 100644 > >>>>> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi > >>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi > >>>>> @@ -104,6 +104,14 @@ &scm { > >>>>> dma-coherent; > >>>>> }; > >>>>> > >>>>> +&venus { > >>>>> + status = "okay"; > >>>>> + > >>>>> + video-firmware { > >>>>> + iommus = <&apps_smmu 0x21a2 0x0>; > >>>>> + }; > >>>>> +}; > >>>>> + > >>>>> &watchdog { > >>>>> status = "okay"; > >>>>> }; > >>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>>>> index 66f1eb83cca7..fa53f54d4675 100644 > >>>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>>>> @@ -3740,6 +3740,8 @@ venus: video-codec@aa00000 { > >>>>> <&apps_smmu 0x2184 0x20>; > >> 0x2184 is a secure SID. I think qcm6490-fairphone-fp5.dts needs to override the > >> iommus property as well to retain only the non secure SID i.e 0x2180 ? I am > >> seeing below crash > >> > >> Call trace: > >> [ 47.663593] qcom_smmu_write_s2cr+0x64/0xa4 > >> [ 47.663616] arm_smmu_attach_dev+0x120/0x284 > >> [ 47.663647] __iommu_attach_device+0x24/0xf8 > >> [ 47.676845] __iommu_device_set_domain+0x70/0xd0 > >> [ 47.681632] __iommu_group_set_domain_internal+0x60/0x1b4 > >> [ 47.687218] iommu_setup_default_domain+0x358/0x418 > >> [ 47.692258] __iommu_probe_device+0x3e4/0x404 > >> > >> Could you please reconfirm if Video SID 0x2184 (and mask) is allowed by the > >> qcm6490-fairphone-fp5 hardware having TZ ? > > > > Hi, > > > > On FP5 it seems it's no problem to have both SIDs in there, probe and > > using venus appears to work fine. > > > > Are you using different firmware than QCM6490.LA.3.0 on the device where > > you tested this? > I was testing this on RB3 board which uses firmware [1]. There is something wrong here. RB3 board uses venus-5.2 RB5 board uses vpu-1.0 Only sc7280 uses vpu-2.0 > > Regards, > Vikash > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/qcom/vpu-2.0 > > >> > >>>>> memory-region = <&video_mem>; > >>>>> > >>>>> + status = "disabled"; > >>>>> + > >>>>> video-decoder { > >>>>> compatible = "venus-decoder"; > >>>>> }; > >>>>> @@ -3748,10 +3750,6 @@ video-encoder { > >>>>> compatible = "venus-encoder"; > >>>>> }; > >>>>> > >>>>> - video-firmware { > >>>>> - iommus = <&apps_smmu 0x21a2 0x0>; > >>>>> - }; > >>>>> - > >>>>> venus_opp_table: opp-table { > >>>>> compatible = "operating-points-v2"; > >>>>> > >>>>> > >>>> Changes look good. Is this tested on SC7280 ? > >>> > >>> Hi Vikash, > >>> > >>> I didn't test it myself on sc7280 (just qcm6490-fp5) but dtx_diff > >>> reports no differences except for status = okay property being added, so > >>> there should be no change on those boards. See below. > >>> > >>> Regards > >>> Luca > >> > >> I tested on SC7280 (herobrine) and all good. > > > > Great, thanks! > > > > Regards > > Luca > > > >> > >> Regards, > >> Vikash > > > -- With best wishes Dmitry