linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support
@ 2023-10-02 14:20 Luca Weiss
  2023-10-02 14:20 ` [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280 Luca Weiss
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Luca Weiss @ 2023-10-02 14:20 UTC (permalink / raw)
  To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
	Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Mauro Carvalho Chehab, cros-qcom-dts-watchers, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree, Luca Weiss

Devices with Qualcomm firmware (compared to ChromeOS firmware) need some
changes in the venus driver and dts layout so that venus can initialize.

Do these changes, similar to sc7180.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Changes in v2:
- Reword commit message 2/3 to be clearer (Konrad)
- Link to v1: https://lore.kernel.org/r/20230929-sc7280-venus-pas-v1-0-9c6738cf157a@fairphone.com

---
Luca Weiss (3):
      media: venus: core: Set up secure memory ranges for SC7280
      arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
      arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node

 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 5 +++++
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 8 ++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi               | 6 ++----
 drivers/media/platform/qcom/venus/core.c           | 4 ++++
 4 files changed, 19 insertions(+), 4 deletions(-)
---
base-commit: df964ce9ef9fea10cf131bf6bad8658fde7956f6
change-id: 20230929-sc7280-venus-pas-ea9630525753

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280
  2023-10-02 14:20 [PATCH v2 0/3] Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support Luca Weiss
@ 2023-10-02 14:20 ` Luca Weiss
  2023-11-22 13:15   ` Vikash Garodia
  2023-10-02 14:20 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common Luca Weiss
  2023-10-02 14:20 ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node Luca Weiss
  2 siblings, 1 reply; 16+ messages in thread
From: Luca Weiss @ 2023-10-02 14:20 UTC (permalink / raw)
  To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
	Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Mauro Carvalho Chehab, cros-qcom-dts-watchers, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree, Luca Weiss

Not all SC7280 devices ship with ChromeOS firmware. Other devices need
PAS for image authentication. That requires the predefined virtual
address ranges to be passed via scm calls. Define them to enable Venus
on non-CrOS SC7280 devices.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/media/platform/qcom/venus/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index 054b8e74ba4f..5c6baa0f4d45 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -881,6 +881,10 @@ static const struct venus_resources sc7280_res = {
 	.vmem_size = 0,
 	.vmem_addr = 0,
 	.dma_mask = 0xe0000000 - 1,
+	.cp_start = 0,
+	.cp_size = 0x25800000,
+	.cp_nonpixel_start = 0x1000000,
+	.cp_nonpixel_size = 0x24800000,
 	.fwname = "qcom/vpu-2.0/venus.mbn",
 };
 

-- 
2.42.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-10-02 14:20 [PATCH v2 0/3] Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support Luca Weiss
  2023-10-02 14:20 ` [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280 Luca Weiss
@ 2023-10-02 14:20 ` Luca Weiss
  2023-11-22 13:17   ` Vikash Garodia
  2023-10-02 14:20 ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node Luca Weiss
  2 siblings, 1 reply; 16+ messages in thread
From: Luca Weiss @ 2023-10-02 14:20 UTC (permalink / raw)
  To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
	Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Mauro Carvalho Chehab, cros-qcom-dts-watchers, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree, Luca Weiss

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";
 

-- 
2.42.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node
  2023-10-02 14:20 [PATCH v2 0/3] Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support Luca Weiss
  2023-10-02 14:20 ` [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280 Luca Weiss
  2023-10-02 14:20 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common Luca Weiss
@ 2023-10-02 14:20 ` Luca Weiss
  2023-11-22 13:18   ` Vikash Garodia
  2 siblings, 1 reply; 16+ messages in thread
From: Luca Weiss @ 2023-10-02 14:20 UTC (permalink / raw)
  To: Stanimir Varbanov, Vikash Garodia, Bryan O'Donoghue,
	Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Mauro Carvalho Chehab, cros-qcom-dts-watchers, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree, Luca Weiss

Enable the venus node so that the video encoder/decoder will start
working.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index 2de0b8c26c35..d29f10f822c9 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -665,3 +665,8 @@ &usb_1_qmpphy {
 
 	status = "okay";
 };
+
+&venus {
+	firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
+	status = "okay";
+};

-- 
2.42.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280
  2023-10-02 14:20 ` [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280 Luca Weiss
@ 2023-11-22 13:15   ` Vikash Garodia
  0 siblings, 0 replies; 16+ messages in thread
From: Vikash Garodia @ 2023-11-22 13:15 UTC (permalink / raw)
  To: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree

On 10/2/2023 7:50 PM, Luca Weiss wrote:
> Not all SC7280 devices ship with ChromeOS firmware. Other devices need
> PAS for image authentication. That requires the predefined virtual
> address ranges to be passed via scm calls. Define them to enable Venus
> on non-CrOS SC7280 devices.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  drivers/media/platform/qcom/venus/core.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 054b8e74ba4f..5c6baa0f4d45 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -881,6 +881,10 @@ static const struct venus_resources sc7280_res = {
>  	.vmem_size = 0,
>  	.vmem_addr = 0,
>  	.dma_mask = 0xe0000000 - 1,
> +	.cp_start = 0,
> +	.cp_size = 0x25800000,
> +	.cp_nonpixel_start = 0x1000000,
> +	.cp_nonpixel_size = 0x24800000,
>  	.fwname = "qcom/vpu-2.0/venus.mbn",
>  };
>  
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-10-02 14:20 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common Luca Weiss
@ 2023-11-22 13:17   ` Vikash Garodia
  2023-11-22 14:20     ` Luca Weiss
  0 siblings, 1 reply; 16+ messages in thread
From: Vikash Garodia @ 2023-11-22 13:17 UTC (permalink / raw)
  To: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node
  2023-10-02 14:20 ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node Luca Weiss
@ 2023-11-22 13:18   ` Vikash Garodia
  0 siblings, 0 replies; 16+ messages in thread
From: Vikash Garodia @ 2023-11-22 13:18 UTC (permalink / raw)
  To: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree

On 10/2/2023 7:50 PM, Luca Weiss wrote:
> Enable the venus node so that the video encoder/decoder will start
> working.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> index 2de0b8c26c35..d29f10f822c9 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
> @@ -665,3 +665,8 @@ &usb_1_qmpphy {
>  
>  	status = "okay";
>  };
> +
> +&venus {
> +	firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
> +	status = "okay";
> +};
> 
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

Regards,
Vikash

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-22 13:17   ` Vikash Garodia
@ 2023-11-22 14:20     ` Luca Weiss
  2023-11-24  6:38       ` Vikash Garodia
  0 siblings, 1 reply; 16+ messages in thread
From: Luca Weiss @ 2023-11-22 14:20 UTC (permalink / raw)
  To: Vikash Garodia, Stanimir Varbanov, Bryan O'Donoghue,
	Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Mauro Carvalho Chehab, cros-qcom-dts-watchers, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-22 14:20     ` Luca Weiss
@ 2023-11-24  6:38       ` Vikash Garodia
  2023-11-24 11:35         ` Luca Weiss
  0 siblings, 1 reply; 16+ messages in thread
From: Vikash Garodia @ 2023-11-24  6:38 UTC (permalink / raw)
  To: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-24  6:38       ` Vikash Garodia
@ 2023-11-24 11:35         ` Luca Weiss
  2023-11-24 12:29           ` Vikash Garodia
  0 siblings, 1 reply; 16+ messages in thread
From: Luca Weiss @ 2023-11-24 11:35 UTC (permalink / raw)
  To: Vikash Garodia, Stanimir Varbanov, Bryan O'Donoghue,
	Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Mauro Carvalho Chehab, cros-qcom-dts-watchers, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-24 11:35         ` Luca Weiss
@ 2023-11-24 12:29           ` Vikash Garodia
  2023-11-24 12:53             ` Dmitry Baryshkov
  0 siblings, 1 reply; 16+ messages in thread
From: Vikash Garodia @ 2023-11-24 12:29 UTC (permalink / raw)
  To: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: ~postmarketos/upstreaming, phone-devel, linux-media,
	linux-arm-msm, linux-kernel, devicetree

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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-24 12:29           ` Vikash Garodia
@ 2023-11-24 12:53             ` Dmitry Baryshkov
  2023-11-24 13:35               ` Vikash Garodia
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Baryshkov @ 2023-11-24 12:53 UTC (permalink / raw)
  To: Vikash Garodia
  Cc: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, ~postmarketos/upstreaming, phone-devel,
	linux-media, linux-arm-msm, linux-kernel, devicetree

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
> >
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-24 12:53             ` Dmitry Baryshkov
@ 2023-11-24 13:35               ` Vikash Garodia
  2023-11-24 15:56                 ` Luca Weiss
  0 siblings, 1 reply; 16+ messages in thread
From: Vikash Garodia @ 2023-11-24 13:35 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Luca Weiss, Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, ~postmarketos/upstreaming, phone-devel,
	linux-media, linux-arm-msm, linux-kernel, devicetree



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
>>>
>>
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-24 13:35               ` Vikash Garodia
@ 2023-11-24 15:56                 ` Luca Weiss
  2023-11-28  8:14                   ` Vikash Garodia
  0 siblings, 1 reply; 16+ messages in thread
From: Luca Weiss @ 2023-11-24 15:56 UTC (permalink / raw)
  To: Vikash Garodia, Dmitry Baryshkov
  Cc: Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, ~postmarketos/upstreaming, phone-devel,
	linux-media, linux-arm-msm, linux-kernel, devicetree

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
> >>>
> >>
> > 
> > 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-24 15:56                 ` Luca Weiss
@ 2023-11-28  8:14                   ` Vikash Garodia
  2023-12-01  9:30                     ` Luca Weiss
  0 siblings, 1 reply; 16+ messages in thread
From: Vikash Garodia @ 2023-11-28  8:14 UTC (permalink / raw)
  To: Luca Weiss, Dmitry Baryshkov
  Cc: Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, ~postmarketos/upstreaming, phone-devel,
	linux-media, linux-arm-msm, linux-kernel, devicetree


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
>>>>>
>>>>
>>>
>>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
  2023-11-28  8:14                   ` Vikash Garodia
@ 2023-12-01  9:30                     ` Luca Weiss
  0 siblings, 0 replies; 16+ messages in thread
From: Luca Weiss @ 2023-12-01  9:30 UTC (permalink / raw)
  To: Vikash Garodia, Dmitry Baryshkov
  Cc: Stanimir Varbanov, Bryan O'Donoghue, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Mauro Carvalho Chehab,
	cros-qcom-dts-watchers, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, ~postmarketos/upstreaming, phone-devel,
	linux-media, linux-arm-msm, linux-kernel, devicetree

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
> >>>>>
> >>>>
> >>>
> >>>
> > 


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-12-01  9:30 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-02 14:20 [PATCH v2 0/3] Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support Luca Weiss
2023-10-02 14:20 ` [PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280 Luca Weiss
2023-11-22 13:15   ` Vikash Garodia
2023-10-02 14:20 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common Luca Weiss
2023-11-22 13:17   ` Vikash Garodia
2023-11-22 14:20     ` Luca Weiss
2023-11-24  6:38       ` Vikash Garodia
2023-11-24 11:35         ` Luca Weiss
2023-11-24 12:29           ` Vikash Garodia
2023-11-24 12:53             ` Dmitry Baryshkov
2023-11-24 13:35               ` Vikash Garodia
2023-11-24 15:56                 ` Luca Weiss
2023-11-28  8:14                   ` Vikash Garodia
2023-12-01  9:30                     ` Luca Weiss
2023-10-02 14:20 ` [PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node Luca Weiss
2023-11-22 13:18   ` Vikash Garodia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).