[PATCH RFC 0/2] interconnect: qcom: sdm845: Drop the bootloader vote on ALC

yoxcu posted 2 patches 1 week, 3 days ago
drivers/interconnect/qcom/sdm845.c             | 17 +++++++++++++++++
include/dt-bindings/interconnect/qcom,sdm845.h |  1 +
2 files changed, 18 insertions(+)
[PATCH RFC 0/2] interconnect: qcom: sdm845: Drop the bootloader vote on ALC
Posted by yoxcu 1 week, 3 days ago
From: Michael Auer <mick@yoxcu.de>

On sdm845 the bootloader can leave a vote on the ALC BCM. No icc node
uses this BCM, so Linux never writes it and the vote stays in force
through sleep: the SoC never reaches CX collapse or AOSS sleep. Found
on a OnePlus 6 (rpmh_read() of ALC on the APPS DRV returns 0x2000000f).

This RFC adds a node without consumers on mem_noc that owns the ALC
BCM, so the provider drops the vote at sync_state like for any other
BCM. The node needs a new ID in the dt-bindings header. Android's
sdm845 bus DT has ALC as a node on mc_virt. Mainline has no mc_virt,
so the node goes on mem_noc next to ACV and MC0.

On icc-next the provider reads the boot vote back
(qcom_icc_bcm_get_bw()). The vote seen here has y = 0xf, so with
enable_mask BIT(0) the node keeps an initial vote until sync_state and
then votes 0. A boot vote with bit 0 clear would not be seen and would
stay. GENMASK(3, 0) would match the vote seen here; I do not know
which value ALC expects.

Questions:
- Is a node without consumers and a new binding ID acceptable for
  this? No other qcom icc driver models ALC.
- Or should the node use a driver-local index past the binding IDs,
  so the header is not touched?
- Or should the bcm-voter clear bootloader votes on BCMs that no node
  owns? It has no list of such BCMs, and cmd-db has no API to list
  them.

Tested on a OnePlus 6 with an sdm845-mainline 7.1-rc1 based kernel.
That kernel has no vote read-back, so the node started at INT_MAX:
ALC was voted with y = 1 until sync_state and 0 after it. ALC reads 0
after boot and suspend reaches CX collapse (qcom_stats cxsd counts).
The average current in an idle push standby test (Wi-Fi on) was
35.6 mA, against 43.7 mA with the ALC vote kept in the sleep set. With
ALC off in all sets the phone has been in daily use for two weeks with
no side effects seen. On icc-next both patches were only build tested
(W=1).

This is my first kernel submission, feedback on the process is welcome.

Michael Auer (2):
  dt-bindings: interconnect: qcom,sdm845: Add MASTER_ALC
  interconnect: qcom: sdm845: Own the ALC BCM to drop its boot vote

 drivers/interconnect/qcom/sdm845.c             | 17 +++++++++++++++++
 include/dt-bindings/interconnect/qcom,sdm845.h |  1 +
 2 files changed, 18 insertions(+)


base-commit: 3fd56a2833c22f62eb84297d0dbd47739632f1a0
-- 
2.43.0
Re: [PATCH RFC 0/2] interconnect: qcom: sdm845: Drop the bootloader vote on ALC
Posted by Konrad Dybcio 5 days ago
On 10/13/26 10:26 PM, yoxcu wrote:
> From: Michael Auer <mick@yoxcu.de>
> 
> On sdm845 the bootloader can leave a vote on the ALC BCM. No icc node
> uses this BCM, so Linux never writes it and the vote stays in force
> through sleep: the SoC never reaches CX collapse or AOSS sleep. Found
> on a OnePlus 6 (rpmh_read() of ALC on the APPS DRV returns 0x2000000f).

ALC essentially translates into "prevent DDR from sleeping to reduce
latency", I don't know what a good framework for that func would be,
but interconnect (even though it has the plumbing for BCMs) doesn't
seem like it.

> This RFC adds a node without consumers on mem_noc that owns the ALC
> BCM, so the provider drops the vote at sync_state like for any other
> BCM. The node needs a new ID in the dt-bindings header. Android's
> sdm845 bus DT has ALC as a node on mc_virt. Mainline has no mc_virt,
> so the node goes on mem_noc next to ACV and MC0.
> 
> On icc-next the provider reads the boot vote back
> (qcom_icc_bcm_get_bw()). The vote seen here has y = 0xf, so with
> enable_mask BIT(0) the node keeps an initial vote until sync_state and
> then votes 0. A boot vote with bit 0 clear would not be seen and would
> stay. GENMASK(3, 0) would match the vote seen here; I do not know
> which value ALC expects.

On msm-4.9, the downstream kernel for sdm845, the driver unconditionally
writes a (x, y) = (12, 0) on the active-only context and (0, 0) on the
remaining ones and that seems to be the right set of values. I don't
know how secret their meaning is, maybe +Mike can help.

Konrad

> Questions:
> - Is a node without consumers and a new binding ID acceptable for
>   this? No other qcom icc driver models ALC.
> - Or should the node use a driver-local index past the binding IDs,
>   so the header is not touched?
> - Or should the bcm-voter clear bootloader votes on BCMs that no node
>   owns? It has no list of such BCMs, and cmd-db has no API to list
>   them.
> 
> Tested on a OnePlus 6 with an sdm845-mainline 7.1-rc1 based kernel.
> That kernel has no vote read-back, so the node started at INT_MAX:
> ALC was voted with y = 1 until sync_state and 0 after it. ALC reads 0
> after boot and suspend reaches CX collapse (qcom_stats cxsd counts).
> The average current in an idle push standby test (Wi-Fi on) was
> 35.6 mA, against 43.7 mA with the ALC vote kept in the sleep set. With
> ALC off in all sets the phone has been in daily use for two weeks with
> no side effects seen. On icc-next both patches were only build tested
> (W=1).
> 
> This is my first kernel submission, feedback on the process is welcome.
> 
> Michael Auer (2):
>   dt-bindings: interconnect: qcom,sdm845: Add MASTER_ALC
>   interconnect: qcom: sdm845: Own the ALC BCM to drop its boot vote
> 
>  drivers/interconnect/qcom/sdm845.c             | 17 +++++++++++++++++
>  include/dt-bindings/interconnect/qcom,sdm845.h |  1 +
>  2 files changed, 18 insertions(+)
> 
> 
> base-commit: 3fd56a2833c22f62eb84297d0dbd47739632f1a0
Re: [PATCH RFC 0/2] interconnect: qcom: sdm845: Drop the bootloader vote on ALC
Posted by Michael Auer 16 hours ago
On 10/19/26 2:22 PM, Konrad Dybcio wrote:
 > ALC essentially translates into "prevent DDR from sleeping to reduce
 > latency", I don't know what a good framework for that func would be,
 > but interconnect (even though it has the plumbing for BCMs) doesn't
 > seem like it.

Thanks, that explains why the vote is so expensive in suspend.

I don't really want to add a consumer API for that.
I only want to get rid of the vote the bootloader leaves behind.

 > On msm-4.9, the downstream kernel for sdm845, the driver unconditionally
 > writes a (x, y) = (12, 0) on the active-only context and (0, 0) on the
 > remaining ones and that seems to be the right set of values. I don't
 > know how secret their meaning is, maybe +Mike can help.

That fits what I measured: the cost is the vote in the sleep set. With
the boot value kept in the wake set and 0 only in the sleep set, the
SoC also reached CX collapse.

I could only find the (12, 0) write for clients marked with
qcom,msm-bus,alc-voter (update_client_alc()), and I found no such client
in the sdm845 DTs (CAF LA.UM.7.3 and 10.3, and the OnePlus tree). Did I
miss something? If not, the only ALC write I see is the handoff removal
at late_initcall_sync, which writes 0, so downstream would end up with
ALC off, like this RFC.

Would it be acceptable for v2 to drop the dummy node and the binding
change, and instead let sdm845 list ALC as a BCM without nodes, which
icc-rpmh votes off once at probe? Or is there a better place for this?

Mike, is off the right state for ALC when nothing asks for low latency?
Or should Linux vote (12, 0) on the active-only set, the way msm_bus
does for an ALC client?

Thanks,
Michael