mbox series

[v4,00/13] Use qmp_send to update co-processor load state

Message ID 1626755807-11865-1-git-send-email-sibis@codeaurora.org
Headers show
Series Use qmp_send to update co-processor load state | expand

Message

Sibi Sankar July 20, 2021, 4:36 a.m. UTC
The power domains exposed by the AOSS QMP driver control the load state
resources linked to modem, adsp, cdsp remoteprocs. These are used to
notify the Always on Subsystem (AOSS) that a particular co-processor is
up/down. AOSS uses this information to wait for the co-processors to
suspend before starting its sleep sequence. These co-processors enter
low-power modes independent to that of the application processor and
the load state resources linked to them are expected to remain unaltered
across system suspend/resume cycles. To achieve this behavior let's stop
modeling them as power-domains and replace them with generic qmp_send
interface instead.

https://lore.kernel.org/lkml/20200913034603.GV3715@yoga/
Previous discussion on dropping power-domain support from AOSS QMP driver

Depends on:
aoss yaml: https://patchwork.kernel.org/project/linux-arm-msm/cover/20210709174142.1274554-1-bjorn.andersson@linaro.org/
qmp_send: https://patchwork.kernel.org/project/linux-arm-msm/cover/1623237532-20829-1-git-send-email-sibis@codeaurora.org/

V4:
 * Rebase patch 1 due to the aoss-qmp yaml conversion (Dropping Rb).
 * Commit message change and sc8180x co-processor addition
   to patch 2. [Rob/Bjorn]
 * Drop unused pdev and kfree the load state string in q6v5_deinit
   /probe path for patch 4. [Matthias]
 * Replaced "binding" with "property" across the series. [Matthias]
 * Commit message change and drop incorrect cleanup on cooling
   device probe failures. [Matthias]

V3:
 * Misc. documentation fixes [patch 2]:
  - Reduce power-domain maxItems due to load_state pd removal
  - Combine compatibles where possible with the load_state pd removal
  - Fixup the qcom,qmp ref to phandle type

V2:
 * load_state is currently broken on mainline so be safely dropped
   without side-effects.
 * Rebased on top of qmp_send v3 series.
 * Dropped R-b from Stephen and Rob on patch 3 due to the yaml
   conversion.
 * New patch [12] to drop unused aoss-qmp header.
 * Commit message update [patch 1] [Rob]
 * Reorder the series [Stephen]

Sibi Sankar (13):
  dt-bindings: soc: qcom: aoss: Drop the load state power-domain
  dt-bindings: remoteproc: qcom: pas: Add QMP property
  dt-bindings: remoteproc: qcom: Add QMP property
  remoteproc: qcom: q6v5: Use qmp_send to update co-processor load state
  arm64: dts: qcom: sc7180: Use QMP property to control load state
  arm64: dts: qcom: sc7280: Use QMP property to control load state
  arm64: dts: qcom: sdm845: Use QMP property to control load state
  arm64: dts: qcom: sm8150: Use QMP property to control load state
  arm64: dts: qcom: sm8250: Use QMP property to control load state
  arm64: dts: qcom: sm8350: Use QMP property to control load state
  soc: qcom: aoss: Drop power domain support
  dt-bindings: msm/dp: Remove aoss-qmp header
  dt-bindings: soc: qcom: aoss: Delete unused power-domain definitions

 .../bindings/display/msm/dp-controller.yaml        |   1 -
 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  |  65 +++++++------
 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   7 +-
 .../bindings/soc/qcom/qcom,aoss-qmp.yaml           |  11 +--
 arch/arm64/boot/dts/qcom/sc7180.dtsi               |   9 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |   2 -
 arch/arm64/boot/dts/qcom/sdm845.dtsi               |   8 +-
 arch/arm64/boot/dts/qcom/sm8150.dtsi               |  28 +++---
 arch/arm64/boot/dts/qcom/sm8250.dtsi               |  22 ++---
 arch/arm64/boot/dts/qcom/sm8350.dtsi               |  30 +++---
 drivers/remoteproc/qcom_q6v5.c                     |  57 ++++++++++-
 drivers/remoteproc/qcom_q6v5.h                     |   7 +-
 drivers/remoteproc/qcom_q6v5_adsp.c                |   7 +-
 drivers/remoteproc/qcom_q6v5_mss.c                 |  44 ++-------
 drivers/remoteproc/qcom_q6v5_pas.c                 |  85 ++++------------
 drivers/remoteproc/qcom_q6v5_wcss.c                |   4 +-
 drivers/soc/qcom/qcom_aoss.c                       | 107 ---------------------
 include/dt-bindings/power/qcom-aoss-qmp.h          |  14 ---
 18 files changed, 185 insertions(+), 323 deletions(-)
 delete mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h

Comments

Matthias Kaehlcke July 20, 2021, 11:10 p.m. UTC | #1
On Tue, Jul 20, 2021 at 10:06:36AM +0530, Sibi Sankar wrote:
> The load state power-domain, used by the co-processors to notify the
> Always on Subsystem (AOSS) that a particular co-processor is up/down,
> suffers from the side-effect of changing states during suspend/resume.
> However the co-processors enter low-power modes independent to that of
> the application processor and their states are expected to remain
> unaltered across system suspend/resume cycles. To achieve this behavior
> let's drop the load state power-domain and replace them with the qmp
> property for all SoCs supporting low power mode signalling.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> 
> v4:
>  * Commit message change and sc8180x co-processor addition. [Rob/Bjorn]
> 
>  .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 65 +++++++++++-----------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> index c597ccced623..1182afb5f593 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
> @@ -78,11 +78,11 @@ properties:
>  
>    power-domains:
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 2
>  
>    power-domain-names:
>      minItems: 1
> -    maxItems: 3
> +    maxItems: 2

It seems maxItems should have been 4 in the first place and should remain
unchanged after removing the load state power domain. With this patch:

  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,sc7180-mpss-pas
    then:
      properties:
        power-domains:
          items:
            - description: CX power domain
            - description: MX power domain
            - description: MSS power domain
        power-domain-names:
          items:
            - const: cx
            - const: mx
            - const: mss
Stephen Boyd July 21, 2021, 5:16 a.m. UTC | #2
Quoting Sibi Sankar (2021-07-19 21:36:46)
> Remove the unused aoss-qmp header from the list of includes.

>

> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>

> Acked-by: Rob Herring <robh@kernel.org>

> ---


Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Stephen Boyd July 21, 2021, 5:35 a.m. UTC | #3
Quoting Sibi Sankar (2021-07-19 21:36:45)
> Strip out the load state power-domain support from the driver since the
> low power mode signalling for the co-processors is now accessible through
> the direct qmp message send interface.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Sibi Sankar July 21, 2021, 4:58 p.m. UTC | #4
Hey Matthias,

Thanks for taking time to review
the series.

On 2021-07-21 04:40, Matthias Kaehlcke wrote:
> On Tue, Jul 20, 2021 at 10:06:36AM +0530, Sibi Sankar wrote:
>> The load state power-domain, used by the co-processors to notify the
>> Always on Subsystem (AOSS) that a particular co-processor is up/down,
>> suffers from the side-effect of changing states during suspend/resume.
>> However the co-processors enter low-power modes independent to that of
>> the application processor and their states are expected to remain
>> unaltered across system suspend/resume cycles. To achieve this 
>> behavior
>> let's drop the load state power-domain and replace them with the qmp
>> property for all SoCs supporting low power mode signalling.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> 
>> v4:
>>  * Commit message change and sc8180x co-processor addition. 
>> [Rob/Bjorn]
>> 
>>  .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 65 
>> +++++++++++-----------
>>  1 file changed, 33 insertions(+), 32 deletions(-)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml 
>> b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> index c597ccced623..1182afb5f593 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml
>> @@ -78,11 +78,11 @@ properties:
>> 
>>    power-domains:
>>      minItems: 1
>> -    maxItems: 3
>> +    maxItems: 2
>> 
>>    power-domain-names:
>>      minItems: 1
>> -    maxItems: 3
>> +    maxItems: 2
> 
> It seems maxItems should have been 4 in the first place and should 
> remain
> unchanged after removing the load state power domain. With this patch:

sc7180-mpss-pas actually uses only
cx and mss. The mpss-pas compatible
is overridden by the mss-pil compatible
for all the platforms present upstream
for sc7180, that's the reason we probably
haven't run into any binding check failures.
I'll keep the max-items to 2 and fix-up
the sc7180 power-domain requirements
instead.

> 
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - qcom,sc7180-mpss-pas
>     then:
>       properties:
>         power-domains:
>           items:
>             - description: CX power domain
>             - description: MX power domain
>             - description: MSS power domain
>         power-domain-names:
>           items:
>             - const: cx
>             - const: mx
>             - const: mss