[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <98e154f7-a01d-43d0-bd0b-70122ad880c6@linaro.org>
Date: Mon, 4 Mar 2024 18:36:40 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Mike Tipton <quic_mdtipton@...cinc.com>
Cc: Bjorn Andersson <andersson@...nel.org>, Georgi Djakov
<djakov@...nel.org>, Abel Vesa <abel.vesa@...aro.org>,
Sibi Sankar <quic_sibis@...cinc.com>,
Rajendra Nayak <quic_rjendra@...cinc.com>,
Marijn Suijten <marijn.suijten@...ainline.org>,
linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] interconnect: qcom: x1e80100: Remove inexistent ACV_PERF
BCM
On 3/4/24 17:40, Mike Tipton wrote:
> On Sat, Mar 02, 2024 at 03:22:49AM +0100, Konrad Dybcio wrote:
>> Booting the kernel on X1E results in a message like:
>>
>> [ 2.561524] qnoc-x1e80100 interconnect-0: ACV_PERF could not find RPMh address
>>
>> And indeed, taking a look at cmd-db, no such BCM exists. Remove it.
>>
>> Fixes: 9f196772841e ("interconnect: qcom: Add X1E80100 interconnect provider driver")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
>
> Reviewed-by: Mike Tipton <quic_mdtipton@...cinc.com>
>
> For some background, ACV "perf mode" does exist, but not as a separate
> BCM. It's controlled by a separate bit in the ACV mask. By default, the
> ACV node only sets the bit indicating the HLOS voter. It doesn't assert
> the perf mode bit.
>
> Enabling perf mode toggles different trade-offs within the DDR subsystem
> for slightly improved performance at the expense of slightly higher
> power. There are limited use cases of this downstream, where we expose
> control over this bit to clients through icc_set_tag(). It primarily
> improves certain latency sensitive benchmarks, AFAIK. I don't think it
> has much impact on real world use cases.
>
> This is true for many other targets as well, not just x1e80100.
>
> Voting for perf-mode is entirely optional and in most cases also
> entirely unnecessary. So, removing this broken way to control it without
> adding the proper control is totally fine.
>
> I have a local series to add the proper support, but haven't posted it
> yet. There aren't any users for it upstream yet, nor am I aware of any
> near term plans to add them. So, it would be unused for a little while,
> at least. That said, anybody could use it to set that tag on their BW
> votes on the off-chance it improves performance and they don't care
> about the power trade-offs.
>
> I could post the series soon if there's interest.
I think adding a sysfs entry for toggling this could be very interesting.
Userspace could toggle this based on "power profile"-style settings.
Konrad
Powered by blists - more mailing lists