[PATCH 3/3] remoteproc: qcom_q6v5_pas: Unload lite firmware on ADSP

Abel Vesa posted 3 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH 3/3] remoteproc: qcom_q6v5_pas: Unload lite firmware on ADSP
Posted by Abel Vesa 1 year, 10 months ago
From: Sibi Sankar <quic_sibis@quicinc.com>

The UEFI loads a lite variant of the ADSP firmware to support charging
use cases. The kernel needs to unload and reload it with the firmware
that has full feature support for audio. This patch arbitarily shutsdown
the lite firmware before loading the full firmware.

Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 083d71f80e5c..4f6940368eb4 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -39,6 +39,7 @@ struct adsp_data {
 	const char *dtb_firmware_name;
 	int pas_id;
 	int dtb_pas_id;
+	int lite_pas_id;
 	unsigned int minidump_id;
 	bool auto_boot;
 	bool decrypt_shutdown;
@@ -72,6 +73,7 @@ struct qcom_adsp {
 	const char *dtb_firmware_name;
 	int pas_id;
 	int dtb_pas_id;
+	int lite_pas_id;
 	unsigned int minidump_id;
 	int crash_reason_smem;
 	bool decrypt_shutdown;
@@ -210,6 +212,10 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
 	/* Store firmware handle to be used in adsp_start() */
 	adsp->firmware = fw;
 
+	/* WIP: Shutdown the ADSP if it's running a lite version of the firmware*/
+	if (adsp->lite_pas_id)
+		ret = qcom_scm_pas_shutdown(adsp->lite_pas_id);
+
 	if (adsp->dtb_pas_id) {
 		ret = request_firmware(&adsp->dtb_firmware, adsp->dtb_firmware_name, adsp->dev);
 		if (ret) {
@@ -693,6 +699,7 @@ static int adsp_probe(struct platform_device *pdev)
 	adsp->rproc = rproc;
 	adsp->minidump_id = desc->minidump_id;
 	adsp->pas_id = desc->pas_id;
+	adsp->lite_pas_id = desc->lite_pas_id;
 	adsp->info_name = desc->sysmon_name;
 	adsp->decrypt_shutdown = desc->decrypt_shutdown;
 	adsp->region_assign_idx = desc->region_assign_idx;
@@ -990,6 +997,7 @@ static const struct adsp_data x1e80100_adsp_resource = {
 	.dtb_firmware_name = "adsp_dtb.mdt",
 	.pas_id = 1,
 	.dtb_pas_id = 0x24,
+	.lite_pas_id = 0x1f,
 	.minidump_id = 5,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){

-- 
2.34.1
Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Unload lite firmware on ADSP
Posted by Dmitry Baryshkov 1 year, 10 months ago
On Mon, 29 Jan 2024 at 15:35, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> From: Sibi Sankar <quic_sibis@quicinc.com>
>
> The UEFI loads a lite variant of the ADSP firmware to support charging
> use cases. The kernel needs to unload and reload it with the firmware
> that has full feature support for audio. This patch arbitarily shutsdown
> the lite firmware before loading the full firmware.
>
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 083d71f80e5c..4f6940368eb4 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -39,6 +39,7 @@ struct adsp_data {
>         const char *dtb_firmware_name;
>         int pas_id;
>         int dtb_pas_id;
> +       int lite_pas_id;
>         unsigned int minidump_id;
>         bool auto_boot;
>         bool decrypt_shutdown;
> @@ -72,6 +73,7 @@ struct qcom_adsp {
>         const char *dtb_firmware_name;
>         int pas_id;
>         int dtb_pas_id;
> +       int lite_pas_id;
>         unsigned int minidump_id;
>         int crash_reason_smem;
>         bool decrypt_shutdown;
> @@ -210,6 +212,10 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
>         /* Store firmware handle to be used in adsp_start() */
>         adsp->firmware = fw;
>
> +       /* WIP: Shutdown the ADSP if it's running a lite version of the firmware*/

Why is it still marked as WIP?

> +       if (adsp->lite_pas_id)
> +               ret = qcom_scm_pas_shutdown(adsp->lite_pas_id);
> +
>         if (adsp->dtb_pas_id) {
>                 ret = request_firmware(&adsp->dtb_firmware, adsp->dtb_firmware_name, adsp->dev);
>                 if (ret) {
> @@ -693,6 +699,7 @@ static int adsp_probe(struct platform_device *pdev)
>         adsp->rproc = rproc;
>         adsp->minidump_id = desc->minidump_id;
>         adsp->pas_id = desc->pas_id;
> +       adsp->lite_pas_id = desc->lite_pas_id;
>         adsp->info_name = desc->sysmon_name;
>         adsp->decrypt_shutdown = desc->decrypt_shutdown;
>         adsp->region_assign_idx = desc->region_assign_idx;
> @@ -990,6 +997,7 @@ static const struct adsp_data x1e80100_adsp_resource = {
>         .dtb_firmware_name = "adsp_dtb.mdt",
>         .pas_id = 1,
>         .dtb_pas_id = 0x24,
> +       .lite_pas_id = 0x1f,
>         .minidump_id = 5,
>         .auto_boot = true,
>         .proxy_pd_names = (char*[]){
>
> --
> 2.34.1
>
>


-- 
With best wishes
Dmitry
Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Unload lite firmware on ADSP
Posted by Abel Vesa 1 year, 10 months ago
On 24-01-29 17:17:28, Dmitry Baryshkov wrote:
> On Mon, 29 Jan 2024 at 15:35, Abel Vesa <abel.vesa@linaro.org> wrote:
> >
> > From: Sibi Sankar <quic_sibis@quicinc.com>
> >
> > The UEFI loads a lite variant of the ADSP firmware to support charging
> > use cases. The kernel needs to unload and reload it with the firmware
> > that has full feature support for audio. This patch arbitarily shutsdown
> > the lite firmware before loading the full firmware.
> >
> > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >  drivers/remoteproc/qcom_q6v5_pas.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index 083d71f80e5c..4f6940368eb4 100644
> > --- a/drivers/remoteproc/qcom_q6v5_pas.c
> > +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> > @@ -39,6 +39,7 @@ struct adsp_data {
> >         const char *dtb_firmware_name;
> >         int pas_id;
> >         int dtb_pas_id;
> > +       int lite_pas_id;
> >         unsigned int minidump_id;
> >         bool auto_boot;
> >         bool decrypt_shutdown;
> > @@ -72,6 +73,7 @@ struct qcom_adsp {
> >         const char *dtb_firmware_name;
> >         int pas_id;
> >         int dtb_pas_id;
> > +       int lite_pas_id;
> >         unsigned int minidump_id;
> >         int crash_reason_smem;
> >         bool decrypt_shutdown;
> > @@ -210,6 +212,10 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
> >         /* Store firmware handle to be used in adsp_start() */
> >         adsp->firmware = fw;
> >
> > +       /* WIP: Shutdown the ADSP if it's running a lite version of the firmware*/
> 
> Why is it still marked as WIP?

AFAIU, there was more to be done here w.r.t. preloaded lite version
firmware.

Later, was agreed that that is not case.

So maybe I just need to drop the comment.

Sibi, can you confirm?

> 
> > +       if (adsp->lite_pas_id)
> > +               ret = qcom_scm_pas_shutdown(adsp->lite_pas_id);
> > +
> >         if (adsp->dtb_pas_id) {
> >                 ret = request_firmware(&adsp->dtb_firmware, adsp->dtb_firmware_name, adsp->dev);
> >                 if (ret) {
> > @@ -693,6 +699,7 @@ static int adsp_probe(struct platform_device *pdev)
> >         adsp->rproc = rproc;
> >         adsp->minidump_id = desc->minidump_id;
> >         adsp->pas_id = desc->pas_id;
> > +       adsp->lite_pas_id = desc->lite_pas_id;
> >         adsp->info_name = desc->sysmon_name;
> >         adsp->decrypt_shutdown = desc->decrypt_shutdown;
> >         adsp->region_assign_idx = desc->region_assign_idx;
> > @@ -990,6 +997,7 @@ static const struct adsp_data x1e80100_adsp_resource = {
> >         .dtb_firmware_name = "adsp_dtb.mdt",
> >         .pas_id = 1,
> >         .dtb_pas_id = 0x24,
> > +       .lite_pas_id = 0x1f,
> >         .minidump_id = 5,
> >         .auto_boot = true,
> >         .proxy_pd_names = (char*[]){
> >
> > --
> > 2.34.1
> >
> >
> 
> 
> -- 
> With best wishes
> Dmitry
Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Unload lite firmware on ADSP
Posted by Sibi Sankar 1 year, 10 months ago

On 1/31/24 15:00, Abel Vesa wrote:
> On 24-01-29 17:17:28, Dmitry Baryshkov wrote:
>> On Mon, 29 Jan 2024 at 15:35, Abel Vesa <abel.vesa@linaro.org> wrote:
>>>
>>> From: Sibi Sankar <quic_sibis@quicinc.com>
>>>
>>> The UEFI loads a lite variant of the ADSP firmware to support charging
>>> use cases. The kernel needs to unload and reload it with the firmware
>>> that has full feature support for audio. This patch arbitarily shutsdown
>>> the lite firmware before loading the full firmware.
>>>
>>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> ---
>>>   drivers/remoteproc/qcom_q6v5_pas.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
>>> index 083d71f80e5c..4f6940368eb4 100644
>>> --- a/drivers/remoteproc/qcom_q6v5_pas.c
>>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
>>> @@ -39,6 +39,7 @@ struct adsp_data {
>>>          const char *dtb_firmware_name;
>>>          int pas_id;
>>>          int dtb_pas_id;
>>> +       int lite_pas_id;
>>>          unsigned int minidump_id;
>>>          bool auto_boot;
>>>          bool decrypt_shutdown;
>>> @@ -72,6 +73,7 @@ struct qcom_adsp {
>>>          const char *dtb_firmware_name;
>>>          int pas_id;
>>>          int dtb_pas_id;
>>> +       int lite_pas_id;
>>>          unsigned int minidump_id;
>>>          int crash_reason_smem;
>>>          bool decrypt_shutdown;
>>> @@ -210,6 +212,10 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
>>>          /* Store firmware handle to be used in adsp_start() */
>>>          adsp->firmware = fw;
>>>
>>> +       /* WIP: Shutdown the ADSP if it's running a lite version of the firmware*/
>>
>> Why is it still marked as WIP?
> 
> AFAIU, there was more to be done here w.r.t. preloaded lite version
> firmware.
> 
> Later, was agreed that that is not case.
> 
> So maybe I just need to drop the comment.
> 
> Sibi, can you confirm?

ack, this is the best we can currently do. Please drop the comment when
you re-spin the series. Thanks for sending this out.

-Sibi

> 
>>
>>> +       if (adsp->lite_pas_id)
>>> +               ret = qcom_scm_pas_shutdown(adsp->lite_pas_id);
>>> +
>>>          if (adsp->dtb_pas_id) {
>>>                  ret = request_firmware(&adsp->dtb_firmware, adsp->dtb_firmware_name, adsp->dev);
>>>                  if (ret) {
>>> @@ -693,6 +699,7 @@ static int adsp_probe(struct platform_device *pdev)
>>>          adsp->rproc = rproc;
>>>          adsp->minidump_id = desc->minidump_id;
>>>          adsp->pas_id = desc->pas_id;
>>> +       adsp->lite_pas_id = desc->lite_pas_id;
>>>          adsp->info_name = desc->sysmon_name;
>>>          adsp->decrypt_shutdown = desc->decrypt_shutdown;
>>>          adsp->region_assign_idx = desc->region_assign_idx;
>>> @@ -990,6 +997,7 @@ static const struct adsp_data x1e80100_adsp_resource = {
>>>          .dtb_firmware_name = "adsp_dtb.mdt",
>>>          .pas_id = 1,
>>>          .dtb_pas_id = 0x24,
>>> +       .lite_pas_id = 0x1f,
>>>          .minidump_id = 5,
>>>          .auto_boot = true,
>>>          .proxy_pd_names = (char*[]){
>>>
>>> --
>>> 2.34.1
>>>
>>>
>>
>>
>> -- 
>> With best wishes
>> Dmitry