[PATCH] arm64: dts: qcom: sc7280: enable venus node

Vedang Nagar posted 1 patch 1 month, 3 weeks ago
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
1 file changed, 2 deletions(-)
[PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Vedang Nagar 1 month, 3 weeks ago
Enable the venus node on Qualcomm sc7280. It was made disabled
earlier to avoid bootup crash, which is fixed now with [1].

[1]
https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/

Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 3d8410683402fd4c03c5c2951721938fff20fc77..59dafbeeab1dfd6e1b021335ba1b04767d6c24e5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -4288,8 +4288,6 @@ venus: video-codec@aa00000 {
 			iommus = <&apps_smmu 0x2180 0x20>;
 			memory-region = <&video_mem>;
 
-			status = "disabled";
-
 			video-decoder {
 				compatible = "venus-decoder";
 			};

---
base-commit: 81ee62e8d09ee3c7107d11c8bbfd64073ab601ad
change-id: 20241003-venus_sc7280-642a6b81afe1
prerequisite-change-id: 20240913-qcm6490-clock-configs-0239f30babb5:v1
prerequisite-patch-id: faac726ebdf08240ab0913132beb2c620e52a98a

Best regards,
-- 
Vedang Nagar <quic_vnagar@quicinc.com>
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Dmitry Baryshkov 1 month, 3 weeks ago
On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
> Enable the venus node on Qualcomm sc7280. It was made disabled
> earlier to avoid bootup crash, which is fixed now with [1].

NAK, there might be other reasons to keep venus disabled, like the lack
of the vendor-signed firmware for the particular device.

> 
> [1]
> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
> 
> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
>  1 file changed, 2 deletions(-)

-- 
With best wishes
Dmitry
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Vedang Nagar 2 weeks, 1 day ago

On 10/7/2024 1:20 AM, Dmitry Baryshkov wrote:
> On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
>> Enable the venus node on Qualcomm sc7280. It was made disabled
>> earlier to avoid bootup crash, which is fixed now with [1].
> 
> NAK, there might be other reasons to keep venus disabled, like the lack
> of the vendor-signed firmware for the particular device.
Can you pls elaborate more on this? Any device with sc7280 SOC can use
venus.mbn which is already present in linux-firmware git.

Regards,
Vedang Nagar
> 
>>
>> [1]
>> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
>>
>> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
>>  1 file changed, 2 deletions(-)
>
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Dmitry Baryshkov 2 weeks, 1 day ago
On Tue, 12 Nov 2024 at 08:17, Vedang Nagar <quic_vnagar@quicinc.com> wrote:
>
>
>
> On 10/7/2024 1:20 AM, Dmitry Baryshkov wrote:
> > On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
> >> Enable the venus node on Qualcomm sc7280. It was made disabled
> >> earlier to avoid bootup crash, which is fixed now with [1].
> >
> > NAK, there might be other reasons to keep venus disabled, like the lack
> > of the vendor-signed firmware for the particular device.
> Can you pls elaborate more on this? Any device with sc7280 SOC can use
> venus.mbn which is already present in linux-firmware git.

Can it though if the device is fused to use vendor keys and to check
the trust chain?

>
> Regards,
> Vedang Nagar
> >
> >>
> >> [1]
> >> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
> >>
> >> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
> >> ---
> >>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
> >>  1 file changed, 2 deletions(-)
> >



-- 
With best wishes
Dmitry
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Vedang Nagar 2 weeks, 1 day ago

On 11/12/2024 6:43 PM, Dmitry Baryshkov wrote:
> On Tue, 12 Nov 2024 at 08:17, Vedang Nagar <quic_vnagar@quicinc.com> wrote:
>>
>>
>>
>> On 10/7/2024 1:20 AM, Dmitry Baryshkov wrote:
>>> On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
>>>> Enable the venus node on Qualcomm sc7280. It was made disabled
>>>> earlier to avoid bootup crash, which is fixed now with [1].
>>>
>>> NAK, there might be other reasons to keep venus disabled, like the lack
>>> of the vendor-signed firmware for the particular device.
>> Can you pls elaborate more on this? Any device with sc7280 SOC can use
>> venus.mbn which is already present in linux-firmware git.
> 
> Can it though if the device is fused to use vendor keys and to check
> the trust chain?
Yes, infact the existing ones are signed and works with trustzone authentication.
> 
>>
>> Regards,
>> Vedang Nagar
>>>
>>>>
>>>> [1]
>>>> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
>>>>
>>>> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
>>>>  1 file changed, 2 deletions(-)
>>>
> 
> 
>
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Luca Weiss 2 weeks, 1 day ago
Hi Vedang,

On Tue Nov 12, 2024 at 3:39 PM CET, Vedang Nagar wrote:
>
>
> On 11/12/2024 6:43 PM, Dmitry Baryshkov wrote:
> > On Tue, 12 Nov 2024 at 08:17, Vedang Nagar <quic_vnagar@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 10/7/2024 1:20 AM, Dmitry Baryshkov wrote:
> >>> On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
> >>>> Enable the venus node on Qualcomm sc7280. It was made disabled
> >>>> earlier to avoid bootup crash, which is fixed now with [1].
> >>>
> >>> NAK, there might be other reasons to keep venus disabled, like the lack
> >>> of the vendor-signed firmware for the particular device.
> >> Can you pls elaborate more on this? Any device with sc7280 SOC can use
> >> venus.mbn which is already present in linux-firmware git.
> > 
> > Can it though if the device is fused to use vendor keys and to check
> > the trust chain?
> Yes, infact the existing ones are signed and works with trustzone authentication.

No, the venus firmware from linux-firmware does not work on a device
with secure boot on, like the (QCM6490) Fairphone 5 smartphone.

$ rm /lib/firmware/qcom/qcm6490/fairphone5/venus.mbn
$ cp /lib/firmware/qcom/vpu-2.0/venus.mbn.zst /lib/firmware/qcom/qcm6490/fairphone5/venus.mbn.zst

leads to

[   10.848191] qcom-venus aa00000.video-codec: Adding to iommu group 13
[   10.863062] qcom-venus aa00000.video-codec: non legacy binding
[   10.909555] qcom-venus aa00000.video-codec: error -22 initializing firmware qcom/qcm6490/fairphone5/venus.mbn
[   10.910099] qcom-venus aa00000.video-codec: fail to load video firmware
[   10.910849] qcom-venus aa00000.video-codec: probe with driver qcom-venus failed with error -22

It's the same with e.g. adsp firmware, modem firmware, etc.

With secure boot off, yes, the hardware will load any firmware
regardless of the signature.

Regards
Luca

> > 
> >>
> >> Regards,
> >> Vedang Nagar
> >>>
> >>>>
> >>>> [1]
> >>>> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
> >>>>
> >>>> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
> >>>>  1 file changed, 2 deletions(-)
> >>>
> > 
> > 
> > 
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Vedang Nagar 2 weeks ago
Hi Luca,
On 11/12/2024 8:49 PM, Luca Weiss wrote:
> Hi Vedang,
> 
> On Tue Nov 12, 2024 at 3:39 PM CET, Vedang Nagar wrote:
>>
>>
>> On 11/12/2024 6:43 PM, Dmitry Baryshkov wrote:
>>> On Tue, 12 Nov 2024 at 08:17, Vedang Nagar <quic_vnagar@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 10/7/2024 1:20 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
>>>>>> Enable the venus node on Qualcomm sc7280. It was made disabled
>>>>>> earlier to avoid bootup crash, which is fixed now with [1].
>>>>>
>>>>> NAK, there might be other reasons to keep venus disabled, like the lack
>>>>> of the vendor-signed firmware for the particular device.
>>>> Can you pls elaborate more on this? Any device with sc7280 SOC can use
>>>> venus.mbn which is already present in linux-firmware git.
>>>
>>> Can it though if the device is fused to use vendor keys and to check
>>> the trust chain?
>> Yes, infact the existing ones are signed and works with trustzone authentication.
> 
> No, the venus firmware from linux-firmware does not work on a device
> with secure boot on, like the (QCM6490) Fairphone 5 smartphone.
Are you saying even after applying this [1] you are seeing the same ?

[1]
https://patchwork.kernel.org/project/linux-media/patch/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
> 
> $ rm /lib/firmware/qcom/qcm6490/fairphone5/venus.mbn
> $ cp /lib/firmware/qcom/vpu-2.0/venus.mbn.zst /lib/firmware/qcom/qcm6490/fairphone5/venus.mbn.zst
> 
> leads to
> 
> [   10.848191] qcom-venus aa00000.video-codec: Adding to iommu group 13
> [   10.863062] qcom-venus aa00000.video-codec: non legacy binding
> [   10.909555] qcom-venus aa00000.video-codec: error -22 initializing firmware qcom/qcm6490/fairphone5/venus.mbn
> [   10.910099] qcom-venus aa00000.video-codec: fail to load video firmware
> [   10.910849] qcom-venus aa00000.video-codec: probe with driver qcom-venus failed with error -22
> 
> It's the same with e.g. adsp firmware, modem firmware, etc.
> 
> With secure boot off, yes, the hardware will load any firmware
> regardless of the signature.
> 
> Regards
> Luca
> 
>>>
>>>>
>>>> Regards,
>>>> Vedang Nagar
>>>>>
>>>>>>
>>>>>> [1]
>>>>>> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
>>>>>>
>>>>>> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
>>>>>> ---
>>>>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
>>>>>>  1 file changed, 2 deletions(-)
>>>>>
>>>
>>>
>>>
>
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Luca Weiss 2 weeks ago
Hi Vedang,

On Wed Nov 13, 2024 at 8:01 AM CET, Vedang Nagar wrote:
> Hi Luca,
> On 11/12/2024 8:49 PM, Luca Weiss wrote:
> > Hi Vedang,
> > 
> > On Tue Nov 12, 2024 at 3:39 PM CET, Vedang Nagar wrote:
> >>
> >>
> >> On 11/12/2024 6:43 PM, Dmitry Baryshkov wrote:
> >>> On Tue, 12 Nov 2024 at 08:17, Vedang Nagar <quic_vnagar@quicinc.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 10/7/2024 1:20 AM, Dmitry Baryshkov wrote:
> >>>>> On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
> >>>>>> Enable the venus node on Qualcomm sc7280. It was made disabled
> >>>>>> earlier to avoid bootup crash, which is fixed now with [1].
> >>>>>
> >>>>> NAK, there might be other reasons to keep venus disabled, like the lack
> >>>>> of the vendor-signed firmware for the particular device.
> >>>> Can you pls elaborate more on this? Any device with sc7280 SOC can use
> >>>> venus.mbn which is already present in linux-firmware git.
> >>>
> >>> Can it though if the device is fused to use vendor keys and to check
> >>> the trust chain?
> >> Yes, infact the existing ones are signed and works with trustzone authentication.
> > 
> > No, the venus firmware from linux-firmware does not work on a device
> > with secure boot on, like the (QCM6490) Fairphone 5 smartphone.
> Are you saying even after applying this [1] you are seeing the same ?
>
> [1]
> https://patchwork.kernel.org/project/linux-media/patch/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/

That patch has been in mainline since v6.9 and my tree is newer, so yes.

See e.g. Qualcomm doc KBA-161204232438 for some details.

Regards
Luca

> > 
> > $ rm /lib/firmware/qcom/qcm6490/fairphone5/venus.mbn
> > $ cp /lib/firmware/qcom/vpu-2.0/venus.mbn.zst /lib/firmware/qcom/qcm6490/fairphone5/venus.mbn.zst
> > 
> > leads to
> > 
> > [   10.848191] qcom-venus aa00000.video-codec: Adding to iommu group 13
> > [   10.863062] qcom-venus aa00000.video-codec: non legacy binding
> > [   10.909555] qcom-venus aa00000.video-codec: error -22 initializing firmware qcom/qcm6490/fairphone5/venus.mbn
> > [   10.910099] qcom-venus aa00000.video-codec: fail to load video firmware
> > [   10.910849] qcom-venus aa00000.video-codec: probe with driver qcom-venus failed with error -22
> > 
> > It's the same with e.g. adsp firmware, modem firmware, etc.
> > 
> > With secure boot off, yes, the hardware will load any firmware
> > regardless of the signature.
> > 
> > Regards
> > Luca
> > 
> >>>
> >>>>
> >>>> Regards,
> >>>> Vedang Nagar
> >>>>>
> >>>>>>
> >>>>>> [1]
> >>>>>> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
> >>>>>>
> >>>>>> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
> >>>>>> ---
> >>>>>>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
> >>>>>>  1 file changed, 2 deletions(-)
> >>>>>
> >>>
> >>>
> >>>
> > 
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Krzysztof Kozlowski 1 month, 3 weeks ago
On 04/10/2024 12:52, Vedang Nagar wrote:
> Enable the venus node on Qualcomm sc7280. It was made disabled
> earlier to avoid bootup crash, which is fixed now with [1].
> 
> [1]
> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/
> 
> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 3d8410683402fd4c03c5c2951721938fff20fc77..59dafbeeab1dfd6e1b021335ba1b04767d6c24e5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -4288,8 +4288,6 @@ venus: video-codec@aa00000 {
>  			iommus = <&apps_smmu 0x2180 0x20>;
>  			memory-region = <&video_mem>;
>  
> -			status = "disabled";

That's just wrong. It's already enabled in other places which makes them
redundant, so patch is incomplete.

But what is more important: this is just not correct, IMO. The node
looks incomplete. You cannot enable incomplete nodes. Please carefully
read how DTS files are being constructed/used.

Best regards,
Krzysztof
Re: [PATCH] arm64: dts: qcom: sc7280: enable venus node
Posted by Bjorn Andersson 1 month, 3 weeks ago
On Fri, Oct 04, 2024 at 04:22:31PM GMT, Vedang Nagar wrote:
> Enable the venus node on Qualcomm sc7280. It was made disabled
> earlier to avoid bootup crash, which is fixed now with [1].
> 
> [1]
> https://lore.kernel.org/linux-media/20231201-sc7280-venus-pas-v3-2-bc132dc5fc30@fairphone.com/

Please refer to commits by sha1 and subject, not by links to the patch.


Please also clarify in your commit message why venus should be enabled
by default at platform level.

Regards,
Bjorn

> 
> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 3d8410683402fd4c03c5c2951721938fff20fc77..59dafbeeab1dfd6e1b021335ba1b04767d6c24e5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -4288,8 +4288,6 @@ venus: video-codec@aa00000 {
>  			iommus = <&apps_smmu 0x2180 0x20>;
>  			memory-region = <&video_mem>;
>  
> -			status = "disabled";
> -
>  			video-decoder {
>  				compatible = "venus-decoder";
>  			};
> 
> ---
> base-commit: 81ee62e8d09ee3c7107d11c8bbfd64073ab601ad
> change-id: 20241003-venus_sc7280-642a6b81afe1
> prerequisite-change-id: 20240913-qcm6490-clock-configs-0239f30babb5:v1
> prerequisite-patch-id: faac726ebdf08240ab0913132beb2c620e52a98a
> 
> Best regards,
> -- 
> Vedang Nagar <quic_vnagar@quicinc.com>
>