| Message ID | 20231002-sc7280-venus-pas-v2-2-bd2408891317@fairphone.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers |
Received: from vger.kernel.org ([23.128.96.18])
by www.linuxtv.org with esmtp (Exim 4.92)
(envelope-from <linux-media-owner@vger.kernel.org>)
id 1qnJms-003hAD-OA; Mon, 02 Oct 2023 14:20:43 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S237756AbjJBOUm (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other);
Mon, 2 Oct 2023 10:20:42 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38952 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S237740AbjJBOUh (ORCPT
<rfc822;linux-media@vger.kernel.org>); Mon, 2 Oct 2023 10:20:37 -0400
Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com
[IPv6:2a00:1450:4864:20::52d])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7CF27EA
for <linux-media@vger.kernel.org>;
Mon, 2 Oct 2023 07:20:33 -0700 (PDT)
Received: by mail-ed1-x52d.google.com with SMTP id
4fb4d7f45d1cf-5344d996bedso15220161a12.3
for <linux-media@vger.kernel.org>;
Mon, 02 Oct 2023 07:20:33 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=fairphone.com; s=fair; t=1696256432; x=1696861232;
darn=vger.kernel.org;
h=cc:to:in-reply-to:references:message-id:content-transfer-encoding
:mime-version:subject:date:from:from:to:cc:subject:date:message-id
:reply-to;
bh=W3IqRDQO3YMqUzu0c5SK0CfZMleyMRbL6H+Fh9GL8Gk=;
b=30g4eMrWlMyBhabHisXWqEIatsSjDwWwPdceCWbL8z0f+GQwKFnRdUNXrQSNrPEqGg
SK1+1BYenMY+AIEQnQpIw6jYLBaxOZ/VITRhXOxc+EQbV9mzHrc3ZBHAC8XX3v5wzdul
OISuWWMWnY4SKbOZhlaAaKzPlIa/9BWO8t0emwJlqVe0/8K+XU8YQ4ZR0pDQMPLiuEc7
qS4OfkMvlmj0kPdQUnorTU4HlWg/j/gLneP2fSg4ubTY222Jpn3YmZd/5XIWCDai+s0i
NiwZLyFVvJwgh/bo8r8ca1Z6mhaquRHT5mwnYCQwGbrijny5NIBuAKhCj2SGv4soqxFX
jV0w==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1696256432; x=1696861232;
h=cc:to:in-reply-to:references:message-id:content-transfer-encoding
:mime-version:subject:date:from:x-gm-message-state:from:to:cc
:subject:date:message-id:reply-to;
bh=W3IqRDQO3YMqUzu0c5SK0CfZMleyMRbL6H+Fh9GL8Gk=;
b=tpN6t9ZsPOPfLgd7o9yO9ayk3UW1WP8mVagO46/49PyapSnesfmgG3FAPaOjm3EAAA
P8rs0Dj6u/c+s4QpD3DsQ+W0cs3NAimj4pNcvDOiZ8L5pKkVf9qqHbJNGJDV1Egim4lh
0CP3jofX2aUrLpVMS4WVI3MmV+9Fi7zoAl2fCcUDHKStKnUjCVPhKSSxYfiqIX4T6EC7
sPfAsueeRZSyCYcPeFgCkMhz+esk5evFlcGalmu0/I+tbDFfq+8WMC2HWIaIVjHUnPHq
srS+WWB3vnxnVVIZOD7C++XUt+iuYCLF2juhlm4l0u62oIX0HWNmK9h6ZE8J2Q1PfNzu
KAww==
X-Gm-Message-State: AOJu0Yz//Z6DetqnLUgHP+E3ZWivudh2wuAHqoAIwyP0LY1kDhFrC2AR
Zw/mhsXWVrGokeFEWgWKOwRHBA==
X-Google-Smtp-Source:
AGHT+IGOmdaZkFNtJWoBrZfjARmVpapUvSwzEv/rUm1T3TRWBoQTDESQR1ggJr7My1p3UmTBpg+BeQ==
X-Received: by 2002:aa7:c584:0:b0:530:a226:1f25 with SMTP id
g4-20020aa7c584000000b00530a2261f25mr9383353edq.17.1696256431920;
Mon, 02 Oct 2023 07:20:31 -0700 (PDT)
Received: from otso.luca.vpn.lucaweiss.eu
(144-178-202-138.static.ef-service.nl. [144.178.202.138])
by smtp.gmail.com with ESMTPSA id
w10-20020aa7dcca000000b005309eb7544fsm15583356edu.45.2023.10.02.07.20.31
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Mon, 02 Oct 2023 07:20:31 -0700 (PDT)
From: Luca Weiss <luca.weiss@fairphone.com>
Date: Mon, 02 Oct 2023 16:20:30 +0200
Subject: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to
chrome-common
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Message-Id: <20231002-sc7280-venus-pas-v2-2-bd2408891317@fairphone.com>
References: <20231002-sc7280-venus-pas-v2-0-bd2408891317@fairphone.com>
In-Reply-To: <20231002-sc7280-venus-pas-v2-0-bd2408891317@fairphone.com>
To: Stanimir Varbanov <stanimir.k.varbanov@gmail.com>,
Vikash Garodia <quic_vgarodia@quicinc.com>,
Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
cros-qcom-dts-watchers@chromium.org,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>
Cc: ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Luca Weiss <luca.weiss@fairphone.com>
X-Mailer: b4 0.12.3
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-media.vger.kernel.org>
X-Mailing-List: linux-media@vger.kernel.org
X-LSpam-Score: -2.5 (--)
X-LSpam-Report: No,
score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1
autolearn=ham autolearn_force=no
|
| Series |
Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support
|
|
Commit Message
Luca Weiss
Oct. 2, 2023, 2:20 p.m. UTC
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@linaro.org> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- 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(-)
Comments
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@linaro.org> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > 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>; > 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 ? Regards, Vikash
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@linaro.org> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > --- > > 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>; > > 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 --- test-pre/sc7280-crd-r3.dtb +++ test-post/sc7280-crd-r3.dtb @@ -5744,6 +5744,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x13b 0x01 0x13b 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-crd.dtb +++ test-post/sc7280-herobrine-crd.dtb @@ -6117,6 +6117,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x147 0x01 0x147 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-crd-pro.dtb +++ test-post/sc7280-herobrine-crd-pro.dtb @@ -6112,6 +6112,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x147 0x01 0x147 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-evoker.dtb +++ test-post/sc7280-herobrine-evoker.dtb @@ -6058,6 +6058,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x14b 0x01 0x14b 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-evoker-lte.dtb +++ test-post/sc7280-herobrine-evoker-lte.dtb @@ -6121,6 +6121,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x151 0x01 0x151 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-herobrine-r1.dtb +++ test-post/sc7280-herobrine-herobrine-r1.dtb @@ -6108,6 +6108,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x14f 0x01 0x14f 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-villager-r0.dtb +++ test-post/sc7280-herobrine-villager-r0.dtb @@ -6049,6 +6049,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x145 0x01 0x145 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-villager-r1.dtb +++ test-post/sc7280-herobrine-villager-r1.dtb @@ -6037,6 +6037,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x142 0x01 0x142 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-villager-r1-lte.dtb +++ test-post/sc7280-herobrine-villager-r1-lte.dtb @@ -6100,6 +6100,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x148 0x01 0x148 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-zombie.dtb +++ test-post/sc7280-herobrine-zombie.dtb @@ -6031,6 +6031,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x146 0x01 0x146 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-zombie-lte.dtb +++ test-post/sc7280-herobrine-zombie-lte.dtb @@ -6094,6 +6094,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x14c 0x01 0x14c 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-zombie-nvme.dtb +++ test-post/sc7280-herobrine-zombie-nvme.dtb @@ -6031,6 +6031,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x146 0x01 0x146 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-herobrine-zombie-nvme-lte.dtb +++ test-post/sc7280-herobrine-zombie-nvme-lte.dtb @@ -6094,6 +6094,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x14c 0x01 0x14c 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-idp2.dtb +++ test-post/sc7280-idp2.dtb @@ -5677,6 +5677,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x138 0x01 0x138 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; --- test-pre/sc7280-idp.dtb +++ test-post/sc7280-idp.dtb @@ -5642,6 +5642,7 @@ power-domain-names = "venus\0vcodec0\0cx"; power-domains = <0x133 0x01 0x133 0x00 0x34 0x00>; reg = <0x00 0xaa00000 0x00 0xd0600>; + status = "okay"; opp-table { compatible = "operating-points-v2"; > > Regards, > Vikash
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@linaro.org> >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>> --- >>> 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 ? >>> 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. Regards, Vikash
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@linaro.org> > >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >>> --- > >>> 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? > > >>> 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
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@linaro.org> >>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>> --- >>>>> 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]. 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 >
On Fri, 24 Nov 2023 at 14:30, Vikash Garodia <quic_vgarodia@quicinc.com> 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@linaro.org> > >>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >>>>> --- > >>>>> 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 > > >
On 11/24/2023 6:23 PM, Dmitry Baryshkov wrote: > On Fri, 24 Nov 2023 at 14:30, Vikash Garodia <quic_vgarodia@quicinc.com> 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@linaro.org> >>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>>>> --- >>>>>>> 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 Tested on QCM6490 IDP board, which is QCOM internal board similar to RB3 gen2. >> >> 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 >>> >> > >
On Fri Nov 24, 2023 at 2:35 PM CET, Vikash Garodia wrote: > > > On 11/24/2023 6:23 PM, Dmitry Baryshkov wrote: > > On Fri, 24 Nov 2023 at 14:30, Vikash Garodia <quic_vgarodia@quicinc.com> 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@linaro.org> > >>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >>>>>>> --- > >>>>>>> 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 > > Tested on QCM6490 IDP board, which is QCOM internal board similar to RB3 gen2. In any case, I don't know much about the venus & iommu setup here. I can try removing the 0x2184 SID and test if venus still works on FP5. Also should the chromebooks keep that iommu entry or not? Regards Luca > > >> > >> 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 > >>> > >> > > > >
On 11/24/2023 9:26 PM, Luca Weiss wrote: > On Fri Nov 24, 2023 at 2:35 PM CET, Vikash Garodia wrote: >> >> >> On 11/24/2023 6:23 PM, Dmitry Baryshkov wrote: >>> On Fri, 24 Nov 2023 at 14:30, Vikash Garodia <quic_vgarodia@quicinc.com> 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@linaro.org> >>>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>>>>>> --- >>>>>>>>> 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 >> >> Tested on QCM6490 IDP board, which is QCOM internal board similar to RB3 gen2. > > In any case, I don't know much about the venus & iommu setup here. I can > try removing the 0x2184 SID and test if venus still works on FP5. Please remove 0x2184 SID and confirm specifically encoder works. This SID is for encoder. > Also should the chromebooks keep that iommu entry or not? Chrome-common can have 0x2184 since its no-TZ based solution. So in sc7280.dtsi, you can keep the default SID i.e 0x2180 (with respective mask) and in chrome-common, we can override the iommus property with 0x2180 and 0x2184. Regards, Vikash > Regards > Luca > >> >>>> >>>> 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 >>>>> >>>> >>> >>> >
On Tue Nov 28, 2023 at 9:14 AM CET, Vikash Garodia wrote: > > On 11/24/2023 9:26 PM, Luca Weiss wrote: > > On Fri Nov 24, 2023 at 2:35 PM CET, Vikash Garodia wrote: > >> > >> > >> On 11/24/2023 6:23 PM, Dmitry Baryshkov wrote: > >>> On Fri, 24 Nov 2023 at 14:30, Vikash Garodia <quic_vgarodia@quicinc.com> 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@linaro.org> > >>>>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >>>>>>>>> --- > >>>>>>>>> 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 > >> > >> Tested on QCM6490 IDP board, which is QCOM internal board similar to RB3 gen2. > > > > In any case, I don't know much about the venus & iommu setup here. I can > > try removing the 0x2184 SID and test if venus still works on FP5. > > Please remove 0x2184 SID and confirm specifically encoder works. This SID is for > encoder. > > > Also should the chromebooks keep that iommu entry or not? > Chrome-common can have 0x2184 since its no-TZ based solution. So in sc7280.dtsi, > you can keep the default SID i.e 0x2180 (with respective mask) and in > chrome-common, we can override the iommus property with 0x2180 and 0x2184. Hi Vikash, I'm moving 0x2184 to chrome-common in v3 but I couldn't test venus encoding myself since I just don't know *how* to test it. Would be nice if you could share how you test venus (decoding & encoding) since seemingly nobody (at least in the postmarketOS community) seems to know how to test/use it properly. See also https://wiki.postmarketos.org/wiki/Hardware_video_acceleration Regards Luca > > Regards, > Vikash > > > Regards > > Luca > > > >> > >>>> > >>>> 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 > >>>>> > >>>> > >>> > >>> > >
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>; 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";