[1/2] arm64: dts: qcom: qcm6490-idp: Enable various remoteprocs

Message ID 20231220114225.26567-2-quic_kbajaj@quicinc.com
State New
Headers
Series Enable various remoteprocs for qcm6490-idp and qcs6490-rb3gen2 |

Commit Message

Komal Bajaj Dec. 20, 2023, 11:42 a.m. UTC
  Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

--
2.42.0
  

Comments

Krzysztof Kozlowski Dec. 20, 2023, 11:45 a.m. UTC | #1
On 20/12/2023 12:42, Komal Bajaj wrote:
> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> index 03e97e27d16d..ad78efa9197d 100644
> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> @@ -419,6 +419,26 @@ &qupv3_id_0 {
>  	status = "okay";
>  };
> 
> +&remoteproc_adsp {
> +	firmware-name = "qcom/qcm6490/adsp.mdt";

Why MDT not MBN?

I don't see these files in linux-firmware and your cover letter did not
explain anything around their submission. What's the status on that part?

Best regards,
Krzysztof
  
Dmitry Baryshkov Dec. 20, 2023, 12:18 p.m. UTC | #2
On Wed, 20 Dec 2023 at 13:46, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/12/2023 12:42, Komal Bajaj wrote:
> > Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
> >
> > Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > index 03e97e27d16d..ad78efa9197d 100644
> > --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> > @@ -419,6 +419,26 @@ &qupv3_id_0 {
> >       status = "okay";
> >  };
> >
> > +&remoteproc_adsp {
> > +     firmware-name = "qcom/qcm6490/adsp.mdt";
>
> Why MDT not MBN?

I agree here. NAK until this is .mbn. Please follow the example of
other boards when you write patches.

>
> I don't see these files in linux-firmware and your cover letter did not
> explain anything around their submission. What's the status on that part?

This isn't usually required, is it? I mean, the firmware can come from
linux-firmware, from the device partition or in any other way. With
the FW_LOADER_USER_HELPER this becomes just the key string used to
identify firmware to be loaded.
  
Krzysztof Kozlowski Dec. 20, 2023, 12:27 p.m. UTC | #3
On 20/12/2023 13:18, Dmitry Baryshkov wrote:
>> I don't see these files in linux-firmware and your cover letter did not
>> explain anything around their submission. What's the status on that part?
> 
> This isn't usually required, is it? I mean, the firmware can come from
> linux-firmware, from the device partition or in any other way. With
> the FW_LOADER_USER_HELPER this becomes just the key string used to
> identify firmware to be loaded.

No, it is not required, but anyway it is a good time to ask that question.

Best regards,
Krzysztof
  
Konrad Dybcio Dec. 20, 2023, 12:29 p.m. UTC | #4
On 20.12.2023 13:18, Dmitry Baryshkov wrote:
> On Wed, 20 Dec 2023 at 13:46, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 20/12/2023 12:42, Komal Bajaj wrote:
>>> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
>>>
>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>> index 03e97e27d16d..ad78efa9197d 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>> @@ -419,6 +419,26 @@ &qupv3_id_0 {
>>>       status = "okay";
>>>  };
>>>
>>> +&remoteproc_adsp {
>>> +     firmware-name = "qcom/qcm6490/adsp.mdt";
>>
>> Why MDT not MBN?
> 
> I agree here. NAK until this is .mbn. Please follow the example of
> other boards when you write patches.
> 
>>
>> I don't see these files in linux-firmware and your cover letter did not
>> explain anything around their submission. What's the status on that part?
> 
> This isn't usually required, is it? I mean, the firmware can come from
> linux-firmware, from the device partition or in any other way. With
> the FW_LOADER_USER_HELPER this becomes just the key string used to
> identify firmware to be loaded.
I think Krzysztof referenced the fact that the Qualcomm-made boards
usually came with redistributable firmware.

As far as my 5 cents go, not submitting the files to linux-firmware.git
only harms the user experience, so I'd always advocate for it, whenever
that is actually possible.

Konrad
  
Dmitry Baryshkov Dec. 20, 2023, 12:34 p.m. UTC | #5
On Wed, 20 Dec 2023 at 14:29, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 20.12.2023 13:18, Dmitry Baryshkov wrote:
> > On Wed, 20 Dec 2023 at 13:46, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 20/12/2023 12:42, Komal Bajaj wrote:
> >>> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
> >>>
> >>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
> >>>  1 file changed, 20 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >>> index 03e97e27d16d..ad78efa9197d 100644
> >>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >>> @@ -419,6 +419,26 @@ &qupv3_id_0 {
> >>>       status = "okay";
> >>>  };
> >>>
> >>> +&remoteproc_adsp {
> >>> +     firmware-name = "qcom/qcm6490/adsp.mdt";
> >>
> >> Why MDT not MBN?
> >
> > I agree here. NAK until this is .mbn. Please follow the example of
> > other boards when you write patches.
> >
> >>
> >> I don't see these files in linux-firmware and your cover letter did not
> >> explain anything around their submission. What's the status on that part?
> >
> > This isn't usually required, is it? I mean, the firmware can come from
> > linux-firmware, from the device partition or in any other way. With
> > the FW_LOADER_USER_HELPER this becomes just the key string used to
> > identify firmware to be loaded.
> I think Krzysztof referenced the fact that the Qualcomm-made boards
> usually came with redistributable firmware.
>
> As far as my 5 cents go, not submitting the files to linux-firmware.git
> only harms the user experience, so I'd always advocate for it, whenever
> that is actually possible.

Me too. I think this is work in progress on the Qualcomm side, see the
discussion at https://github.com/Linaro/meta-qcom/pull/551 .
  
Komal Bajaj Dec. 22, 2023, 1:24 p.m. UTC | #6
On 12/20/2023 6:04 PM, Dmitry Baryshkov wrote:
> On Wed, 20 Dec 2023 at 14:29, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 20.12.2023 13:18, Dmitry Baryshkov wrote:
>>> On Wed, 20 Dec 2023 at 13:46, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 20/12/2023 12:42, Komal Bajaj wrote:
>>>>> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
>>>>>
>>>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
>>>>>   1 file changed, 20 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>>>> index 03e97e27d16d..ad78efa9197d 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
>>>>> @@ -419,6 +419,26 @@ &qupv3_id_0 {
>>>>>        status = "okay";
>>>>>   };
>>>>>
>>>>> +&remoteproc_adsp {
>>>>> +     firmware-name = "qcom/qcm6490/adsp.mdt";
>>>>
>>>> Why MDT not MBN?
>>>
>>> I agree here. NAK until this is .mbn. Please follow the example of
>>> other boards when you write patches.
>>>
>>>>
>>>> I don't see these files in linux-firmware and your cover letter did not
>>>> explain anything around their submission. What's the status on that part?
>>>
>>> This isn't usually required, is it? I mean, the firmware can come from
>>> linux-firmware, from the device partition or in any other way. With
>>> the FW_LOADER_USER_HELPER this becomes just the key string used to
>>> identify firmware to be loaded.
>> I think Krzysztof referenced the fact that the Qualcomm-made boards
>> usually came with redistributable firmware.
>>
>> As far as my 5 cents go, not submitting the files to linux-firmware.git
>> only harms the user experience, so I'd always advocate for it, whenever
>> that is actually possible.
> 
> Me too. I think this is work in progress on the Qualcomm side, see the
> discussion at https://github.com/Linaro/meta-qcom/pull/551 .
> 

I was searching for MBN files pushed for SM8550 at 
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/, 
but could not find it. Am I missing something?

Are we maintaining it somewhere else asking just out of curiosity and 
learning and do similar for QCM/QCS6490.

Thanks
Komal
  
Dmitry Baryshkov Dec. 22, 2023, 2:26 p.m. UTC | #7
On Fri, 22 Dec 2023 at 15:25, Komal Bajaj <quic_kbajaj@quicinc.com> wrote:
>
>
>
> On 12/20/2023 6:04 PM, Dmitry Baryshkov wrote:
> > On Wed, 20 Dec 2023 at 14:29, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >> On 20.12.2023 13:18, Dmitry Baryshkov wrote:
> >>> On Wed, 20 Dec 2023 at 13:46, Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 20/12/2023 12:42, Komal Bajaj wrote:
> >>>>> Enable the ADSP, CDSP, MPSS and WPSS that are found on the SoC.
> >>>>>
> >>>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> >>>>> ---
> >>>>>   arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 20 ++++++++++++++++++++
> >>>>>   1 file changed, 20 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >>>>> index 03e97e27d16d..ad78efa9197d 100644
> >>>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >>>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
> >>>>> @@ -419,6 +419,26 @@ &qupv3_id_0 {
> >>>>>        status = "okay";
> >>>>>   };
> >>>>>
> >>>>> +&remoteproc_adsp {
> >>>>> +     firmware-name = "qcom/qcm6490/adsp.mdt";
> >>>>
> >>>> Why MDT not MBN?
> >>>
> >>> I agree here. NAK until this is .mbn. Please follow the example of
> >>> other boards when you write patches.
> >>>
> >>>>
> >>>> I don't see these files in linux-firmware and your cover letter did not
> >>>> explain anything around their submission. What's the status on that part?
> >>>
> >>> This isn't usually required, is it? I mean, the firmware can come from
> >>> linux-firmware, from the device partition or in any other way. With
> >>> the FW_LOADER_USER_HELPER this becomes just the key string used to
> >>> identify firmware to be loaded.
> >> I think Krzysztof referenced the fact that the Qualcomm-made boards
> >> usually came with redistributable firmware.
> >>
> >> As far as my 5 cents go, not submitting the files to linux-firmware.git
> >> only harms the user experience, so I'd always advocate for it, whenever
> >> that is actually possible.
> >
> > Me too. I think this is work in progress on the Qualcomm side, see the
> > discussion at https://github.com/Linaro/meta-qcom/pull/551 .
> >
>
> I was searching for MBN files pushed for SM8550 at
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/,
> but could not find it. Am I missing something?

Nobody handed out the files for sm8550 yet. So we could push only the
audio topology files.

> Are we maintaining it somewhere else asking just out of curiosity and
> learning and do similar for QCM/QCS6490.

That repo contains existing firmware for older platforms.
The firmware signed with the test keys goes to the SoC directory,
vendor-signed firmware goes to the subdir.

See qcom/sm8250 (RB5), qcom/sdm845 (RB3 aka db845c), qcom/qrb4210
(RB2), qcom/qcm2290 (RB1), qcom/apq8096 (db820c) and qcom/apq8016
(db410c).

For X13s there is a vendor-signed firmware at qcom/sc8280xp/LENOVO/21BX/

Venus / VPU firmware, being chip agnostic, goes to qcom/venus-* and
qcom/vpu-* Vendor-signed venus firmware should go to the same subdir
as all other device-specific files.

Generic Adreno firmware (SQE, GMU, GPMU, etc.) goes to qcom/ directly.
Signed ZAP shaders go to the qcom/SOC or device subdirs.

WiFI firmware goes to ath10k, ath11k or ath12k dirs. But please
coordinate with Kalle Valo, he maintains those subdirs.

BT firmware is piled up in qca subdir.

All mentioned files except the WiFi have migrated to .mbn format.

All licences are documented in the WHENCE file.
  

Patch

diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index 03e97e27d16d..ad78efa9197d 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -419,6 +419,26 @@  &qupv3_id_0 {
 	status = "okay";
 };

+&remoteproc_adsp {
+	firmware-name = "qcom/qcm6490/adsp.mdt";
+	status = "okay";
+};
+
+&remoteproc_cdsp {
+	firmware-name = "qcom/qcm6490/cdsp.mdt";
+	status = "okay";
+};
+
+&remoteproc_mpss {
+	firmware-name = "qcom/qcm6490/modem.mdt";
+	status = "okay";
+};
+
+&remoteproc_wpss {
+	firmware-name = "qcom/qcm6490/wpss.mdt";
+	status = "okay";
+};
+
 &sdhc_1 {
 	non-removable;
 	no-sd;