linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Fix up icc clock rate calculation on some platforms
@ 2023-07-26 16:25 Konrad Dybcio
  2023-07-26 16:25 ` [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients Konrad Dybcio
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 16:25 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Georgi Djakov
  Cc: Marijn Suijten, linux-arm-msm, linux-pm, linux-kernel,
	Konrad Dybcio

Certain platforms require that some buses (or individual nodes) make
some additional changes to the clock rate formula, throwing in some
magic, Qualcomm-defined coefficients, to account for "inefficiencies".

Add the framework for it and utilize it on a couple SoCs.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (4):
      interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients
      interconnect: qcom: qcm2290: Set AB coefficients
      interconnect: qcom: sdm660: Set AB/IB coefficients
      interconnect: qcom: msm8996: Set AB/IB coefficients

 drivers/interconnect/qcom/icc-rpm.c | 10 +++++++++-
 drivers/interconnect/qcom/icc-rpm.h |  5 +++++
 drivers/interconnect/qcom/msm8996.c |  8 ++++++--
 drivers/interconnect/qcom/qcm2290.c |  3 +++
 drivers/interconnect/qcom/sdm660.c  |  4 ++++
 5 files changed, 27 insertions(+), 3 deletions(-)
---
base-commit: 1e25dd7772483f477f79986d956028e9f47f990a
change-id: 20230726-topic-icc_coeff-b053d5409b9f

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients
  2023-07-26 16:25 [PATCH 0/4] Fix up icc clock rate calculation on some platforms Konrad Dybcio
@ 2023-07-26 16:25 ` Konrad Dybcio
  2023-07-26 17:16   ` Stephan Gerhold
  2023-07-29  9:25   ` kernel test robot
  2023-07-26 16:25 ` [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients Konrad Dybcio
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 16:25 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Georgi Djakov
  Cc: Marijn Suijten, linux-arm-msm, linux-pm, linux-kernel,
	Konrad Dybcio

Presumably due to the hardware being so complex, some nodes (or busses)
have different (usually higher) requirements for bandwidth than what
the usual calculations would suggest.

Looking at the available downstream files, it seems like AB values are
adjusted per-bus and IB values are adjusted per-node.
With that in mind, introduce percentage-based coefficient struct members
and use them in the calculations.

One thing to note is that downstream does (X%)*AB and IB/(Y%) which
feels a bit backwards, especially given that the divisors for IB turn
out to always be 25, 50, 200 making this a convenient conversion to 4x,
2x, 0.5x.. This commit uses the more sane, non-inverse approach.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/interconnect/qcom/icc-rpm.c | 10 +++++++++-
 drivers/interconnect/qcom/icc-rpm.h |  5 +++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
index 2c16917ba1fd..2de0e1dfe225 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -298,9 +298,11 @@ static int qcom_icc_bw_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
  */
 static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_rate)
 {
-	u64 agg_avg_rate, agg_rate;
+	struct qcom_icc_provider *qp = to_qcom_provider(provider);
+	u64 agg_avg_rate, agg_peak_rate, agg_rate;
 	struct qcom_icc_node *qn;
 	struct icc_node *node;
+	u16 percent;
 	int i;
 
 	/*
@@ -315,6 +317,12 @@ static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_r
 			else
 				agg_avg_rate = qn->sum_avg[i];
 
+			percent = qp->ab_percent ? qp->ab_percent : 100;
+			agg_avg_rate = mult_frac(percent, agg_avg_rate, 100);
+
+			percent = qn->ib_percent ? qn->ib_percent : 100;
+			agg_peak_rate = mult_frac(percent, qn->max_peak[i], 100);
+
 			agg_rate = max_t(u64, agg_avg_rate, qn->max_peak[i]);
 			do_div(agg_rate, qn->buswidth);
 
diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
index eed3451af3e6..dbb3146a81c4 100644
--- a/drivers/interconnect/qcom/icc-rpm.h
+++ b/drivers/interconnect/qcom/icc-rpm.h
@@ -45,6 +45,7 @@ struct rpm_clk_resource {
  * @regmap: regmap for QoS registers read/write access
  * @qos_offset: offset to QoS registers
  * @bus_clk_rate: bus clock rate in Hz
+ * @ab_percent: a percentage-based coefficient for compensating the AB calculations
  * @bus_clk_desc: a pointer to a rpm_clk_resource description of bus clocks
  * @bus_clk: a pointer to a HLOS-owned bus clock
  * @intf_clks: a clk_bulk_data array of interface clocks
@@ -58,6 +59,7 @@ struct qcom_icc_provider {
 	struct regmap *regmap;
 	unsigned int qos_offset;
 	u32 bus_clk_rate[QCOM_SMD_RPM_STATE_NUM];
+	u16 ab_percent;
 	const struct rpm_clk_resource *bus_clk_desc;
 	struct clk *bus_clk;
 	struct clk_bulk_data *intf_clks;
@@ -93,6 +95,7 @@ struct qcom_icc_qos {
  * @num_links: the total number of @links
  * @channels: number of channels at this node (e.g. DDR channels)
  * @buswidth: width of the interconnect between a node and the bus (bytes)
+ * @ib_percent: a percentage-based coefficient for compensating the IB calculations
  * @sum_avg: current sum aggregate value of all avg bw requests
  * @max_peak: current max aggregate value of all peak bw requests
  * @mas_rpm_id:	RPM id for devices that are bus masters
@@ -106,6 +109,7 @@ struct qcom_icc_node {
 	u16 num_links;
 	u16 channels;
 	u16 buswidth;
+	u16 ib_percent;
 	u64 sum_avg[QCOM_SMD_RPM_STATE_NUM];
 	u64 max_peak[QCOM_SMD_RPM_STATE_NUM];
 	int mas_rpm_id;
@@ -123,6 +127,7 @@ struct qcom_icc_desc {
 	enum qcom_icc_type type;
 	const struct regmap_config *regmap_cfg;
 	unsigned int qos_offset;
+	u16 ab_percent;
 };
 
 /* Valid for all bus types */

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients
  2023-07-26 16:25 [PATCH 0/4] Fix up icc clock rate calculation on some platforms Konrad Dybcio
  2023-07-26 16:25 ` [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients Konrad Dybcio
@ 2023-07-26 16:25 ` Konrad Dybcio
  2023-07-26 17:18   ` Stephan Gerhold
  2023-07-26 16:25 ` [PATCH 3/4] interconnect: qcom: sdm660: Set AB/IB coefficients Konrad Dybcio
  2023-07-26 16:25 ` [PATCH 4/4] interconnect: qcom: msm8996: " Konrad Dybcio
  3 siblings, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 16:25 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Georgi Djakov
  Cc: Marijn Suijten, linux-arm-msm, linux-pm, linux-kernel,
	Konrad Dybcio

Some buses need additional manual adjustments atop the usual
calculations. Fill in the missing coefficients.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/interconnect/qcom/qcm2290.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/interconnect/qcom/qcm2290.c b/drivers/interconnect/qcom/qcm2290.c
index 3c3b24264a5b..457e5713ae43 100644
--- a/drivers/interconnect/qcom/qcm2290.c
+++ b/drivers/interconnect/qcom/qcm2290.c
@@ -1198,6 +1198,7 @@ static const struct qcom_icc_desc qcm2290_bimc = {
 	.regmap_cfg = &qcm2290_bimc_regmap_config,
 	/* M_REG_BASE() in vendor msm_bus_bimc_adhoc driver */
 	.qos_offset = 0x8000,
+	.ab_percent = 153,
 };
 
 static struct qcom_icc_node * const qcm2290_cnoc_nodes[] = {
@@ -1324,6 +1325,7 @@ static const struct qcom_icc_desc qcm2290_mmnrt_virt = {
 	.bus_clk_desc = &mmaxi_0_clk,
 	.regmap_cfg = &qcm2290_snoc_regmap_config,
 	.qos_offset = 0x15000,
+	.ab_percent = 142,
 };
 
 static struct qcom_icc_node * const qcm2290_mmrt_virt_nodes[] = {
@@ -1339,6 +1341,7 @@ static const struct qcom_icc_desc qcm2290_mmrt_virt = {
 	.bus_clk_desc = &mmaxi_1_clk,
 	.regmap_cfg = &qcm2290_snoc_regmap_config,
 	.qos_offset = 0x15000,
+	.ab_percent = 139,
 };
 
 static const struct of_device_id qcm2290_noc_of_match[] = {

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] interconnect: qcom: sdm660: Set AB/IB coefficients
  2023-07-26 16:25 [PATCH 0/4] Fix up icc clock rate calculation on some platforms Konrad Dybcio
  2023-07-26 16:25 ` [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients Konrad Dybcio
  2023-07-26 16:25 ` [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients Konrad Dybcio
@ 2023-07-26 16:25 ` Konrad Dybcio
  2023-07-26 16:25 ` [PATCH 4/4] interconnect: qcom: msm8996: " Konrad Dybcio
  3 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 16:25 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Georgi Djakov
  Cc: Marijn Suijten, linux-arm-msm, linux-pm, linux-kernel,
	Konrad Dybcio

Some buses and nodes need additional manual adjustments atop the usual
calculations. Fill in the missing coefficients.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/interconnect/qcom/sdm660.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/interconnect/qcom/sdm660.c b/drivers/interconnect/qcom/sdm660.c
index 36962f7bd7bb..cb93e2f2c2f4 100644
--- a/drivers/interconnect/qcom/sdm660.c
+++ b/drivers/interconnect/qcom/sdm660.c
@@ -602,6 +602,7 @@ static struct qcom_icc_node mas_mdp_p0 = {
 	.name = "mas_mdp_p0",
 	.id = SDM660_MASTER_MDP_P0,
 	.buswidth = 16,
+	.ib_percent = 200,
 	.mas_rpm_id = 8,
 	.slv_rpm_id = -1,
 	.qos.ap_owned = true,
@@ -621,6 +622,7 @@ static struct qcom_icc_node mas_mdp_p1 = {
 	.name = "mas_mdp_p1",
 	.id = SDM660_MASTER_MDP_P1,
 	.buswidth = 16,
+	.ib_percent = 200,
 	.mas_rpm_id = 61,
 	.slv_rpm_id = -1,
 	.qos.ap_owned = true,
@@ -1540,6 +1542,7 @@ static const struct qcom_icc_desc sdm660_bimc = {
 	.num_nodes = ARRAY_SIZE(sdm660_bimc_nodes),
 	.bus_clk_desc = &bimc_clk,
 	.regmap_cfg = &sdm660_bimc_regmap_config,
+	.ab_percent = 153,
 };
 
 static struct qcom_icc_node * const sdm660_cnoc_nodes[] = {
@@ -1659,6 +1662,7 @@ static const struct qcom_icc_desc sdm660_mnoc = {
 	.intf_clocks = mm_intf_clocks,
 	.num_intf_clocks = ARRAY_SIZE(mm_intf_clocks),
 	.regmap_cfg = &sdm660_mnoc_regmap_config,
+	.ab_percent = 153,
 };
 
 static struct qcom_icc_node * const sdm660_snoc_nodes[] = {

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] interconnect: qcom: msm8996: Set AB/IB coefficients
  2023-07-26 16:25 [PATCH 0/4] Fix up icc clock rate calculation on some platforms Konrad Dybcio
                   ` (2 preceding siblings ...)
  2023-07-26 16:25 ` [PATCH 3/4] interconnect: qcom: sdm660: Set AB/IB coefficients Konrad Dybcio
@ 2023-07-26 16:25 ` Konrad Dybcio
  3 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 16:25 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Georgi Djakov
  Cc: Marijn Suijten, linux-arm-msm, linux-pm, linux-kernel,
	Konrad Dybcio

Some buses and nodes need additional manual adjustments atop the usual
calculations. Fill in the missing coefficients.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/interconnect/qcom/msm8996.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/interconnect/qcom/msm8996.c b/drivers/interconnect/qcom/msm8996.c
index 88683dfa468f..dec38cd42df4 100644
--- a/drivers/interconnect/qcom/msm8996.c
+++ b/drivers/interconnect/qcom/msm8996.c
@@ -448,6 +448,7 @@ static struct qcom_icc_node mas_mdp_p0 = {
 	.name = "mas_mdp_p0",
 	.id = MSM8996_MASTER_MDP_PORT0,
 	.buswidth = 32,
+	.ib_percent = 400,
 	.mas_rpm_id = 8,
 	.slv_rpm_id = -1,
 	.qos.ap_owned = true,
@@ -463,6 +464,7 @@ static struct qcom_icc_node mas_mdp_p1 = {
 	.name = "mas_mdp_p1",
 	.id = MSM8996_MASTER_MDP_PORT1,
 	.buswidth = 32,
+	.ib_percent = 400,
 	.mas_rpm_id = 61,
 	.slv_rpm_id = -1,
 	.qos.ap_owned = true,
@@ -1889,7 +1891,8 @@ static const struct qcom_icc_desc msm8996_bimc = {
 	.nodes = bimc_nodes,
 	.num_nodes = ARRAY_SIZE(bimc_nodes),
 	.bus_clk_desc = &bimc_clk,
-	.regmap_cfg = &msm8996_bimc_regmap_config
+	.regmap_cfg = &msm8996_bimc_regmap_config,
+	.ab_percent = 154,
 };
 
 static struct qcom_icc_node * const cnoc_nodes[] = {
@@ -2004,7 +2007,8 @@ static const struct qcom_icc_desc msm8996_mnoc = {
 	.bus_clk_desc = &mmaxi_0_clk,
 	.intf_clocks = mm_intf_clocks,
 	.num_intf_clocks = ARRAY_SIZE(mm_intf_clocks),
-	.regmap_cfg = &msm8996_mnoc_regmap_config
+	.regmap_cfg = &msm8996_mnoc_regmap_config,
+	.ab_percent = 154,
 };
 
 static struct qcom_icc_node * const pnoc_nodes[] = {

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients
  2023-07-26 16:25 ` [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients Konrad Dybcio
@ 2023-07-26 17:16   ` Stephan Gerhold
  2023-07-26 17:19     ` Konrad Dybcio
  2023-07-29  9:25   ` kernel test robot
  1 sibling, 1 reply; 13+ messages in thread
From: Stephan Gerhold @ 2023-07-26 17:16 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On Wed, Jul 26, 2023 at 06:25:43PM +0200, Konrad Dybcio wrote:
> Presumably due to the hardware being so complex, some nodes (or busses)
> have different (usually higher) requirements for bandwidth than what
> the usual calculations would suggest.
> 

Weird. I just hope this was never abused to workaround other broken
configuration. A nice round ib_percent = 200 has mostly the same effect as

  - Doubling the requested peek bandwidth in the consumer driver (perhaps
    they were too lazy to fix the driver in downstream at some point)
  - Halving the node buswidth

It's probably hard to say for sure...

> Looking at the available downstream files, it seems like AB values are
> adjusted per-bus and IB values are adjusted per-node.
> With that in mind, introduce percentage-based coefficient struct members
> and use them in the calculations.
> 
> One thing to note is that downstream does (X%)*AB and IB/(Y%) which
> feels a bit backwards, especially given that the divisors for IB turn
> out to always be 25, 50, 200 making this a convenient conversion to 4x,
> 2x, 0.5x.. This commit uses the more sane, non-inverse approach.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/interconnect/qcom/icc-rpm.c | 10 +++++++++-
>  drivers/interconnect/qcom/icc-rpm.h |  5 +++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
> index 2c16917ba1fd..2de0e1dfe225 100644
> --- a/drivers/interconnect/qcom/icc-rpm.c
> +++ b/drivers/interconnect/qcom/icc-rpm.c
> @@ -298,9 +298,11 @@ static int qcom_icc_bw_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
>   */
>  static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_rate)
>  {
> -	u64 agg_avg_rate, agg_rate;
> +	struct qcom_icc_provider *qp = to_qcom_provider(provider);
> +	u64 agg_avg_rate, agg_peak_rate, agg_rate;
>  	struct qcom_icc_node *qn;
>  	struct icc_node *node;
> +	u16 percent;
>  	int i;
>  
>  	/*
> @@ -315,6 +317,12 @@ static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_r
>  			else
>  				agg_avg_rate = qn->sum_avg[i];
>  
> +			percent = qp->ab_percent ? qp->ab_percent : 100;
> +			agg_avg_rate = mult_frac(percent, agg_avg_rate, 100);

			if (qp->ab_percent)
				agg_avg_rate = mult_frac(qp->ab_percent, agg_avg_rate, 100);

Would be likely more efficient (no calculation if unspecified) and not
much harder to read.

> +
> +			percent = qn->ib_percent ? qn->ib_percent : 100;
> +			agg_peak_rate = mult_frac(percent, qn->max_peak[i], 100);
> +

agg_peak_rate doesn't seem to be used anywhere else? 🤔

Thanks,
Stephan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients
  2023-07-26 16:25 ` [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients Konrad Dybcio
@ 2023-07-26 17:18   ` Stephan Gerhold
  2023-07-26 17:20     ` Konrad Dybcio
  0 siblings, 1 reply; 13+ messages in thread
From: Stephan Gerhold @ 2023-07-26 17:18 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On Wed, Jul 26, 2023 at 06:25:44PM +0200, Konrad Dybcio wrote:
> Some buses need additional manual adjustments atop the usual
> calculations. Fill in the missing coefficients.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>

What about the funny util-fact/vrail-comp on the mas-apps-proc node
downstream?

Thanks,
Stephan

> ---
>  drivers/interconnect/qcom/qcm2290.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/interconnect/qcom/qcm2290.c b/drivers/interconnect/qcom/qcm2290.c
> index 3c3b24264a5b..457e5713ae43 100644
> --- a/drivers/interconnect/qcom/qcm2290.c
> +++ b/drivers/interconnect/qcom/qcm2290.c
> @@ -1198,6 +1198,7 @@ static const struct qcom_icc_desc qcm2290_bimc = {
>  	.regmap_cfg = &qcm2290_bimc_regmap_config,
>  	/* M_REG_BASE() in vendor msm_bus_bimc_adhoc driver */
>  	.qos_offset = 0x8000,
> +	.ab_percent = 153,
>  };
>  
>  static struct qcom_icc_node * const qcm2290_cnoc_nodes[] = {
> @@ -1324,6 +1325,7 @@ static const struct qcom_icc_desc qcm2290_mmnrt_virt = {
>  	.bus_clk_desc = &mmaxi_0_clk,
>  	.regmap_cfg = &qcm2290_snoc_regmap_config,
>  	.qos_offset = 0x15000,
> +	.ab_percent = 142,
>  };
>  
>  static struct qcom_icc_node * const qcm2290_mmrt_virt_nodes[] = {
> @@ -1339,6 +1341,7 @@ static const struct qcom_icc_desc qcm2290_mmrt_virt = {
>  	.bus_clk_desc = &mmaxi_1_clk,
>  	.regmap_cfg = &qcm2290_snoc_regmap_config,
>  	.qos_offset = 0x15000,
> +	.ab_percent = 139,
>  };
>  
>  static const struct of_device_id qcm2290_noc_of_match[] = {
> 
> -- 
> 2.41.0
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients
  2023-07-26 17:16   ` Stephan Gerhold
@ 2023-07-26 17:19     ` Konrad Dybcio
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 17:19 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On 26.07.2023 19:16, Stephan Gerhold wrote:
> On Wed, Jul 26, 2023 at 06:25:43PM +0200, Konrad Dybcio wrote:
>> Presumably due to the hardware being so complex, some nodes (or busses)
>> have different (usually higher) requirements for bandwidth than what
>> the usual calculations would suggest.
>>
> 
> Weird. I just hope this was never abused to workaround other broken
> configuration. A nice round ib_percent = 200 has mostly the same effect as
> 
>   - Doubling the requested peek bandwidth in the consumer driver (perhaps
>     they were too lazy to fix the driver in downstream at some point)
>   - Halving the node buswidth
> 
> It's probably hard to say for sure...
As per usual..

[...]

>>  
>>  	/*
>> @@ -315,6 +317,12 @@ static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_r
>>  			else
>>  				agg_avg_rate = qn->sum_avg[i];
>>  
>> +			percent = qp->ab_percent ? qp->ab_percent : 100;
>> +			agg_avg_rate = mult_frac(percent, agg_avg_rate, 100);
> 
> 			if (qp->ab_percent)
> 				agg_avg_rate = mult_frac(qp->ab_percent, agg_avg_rate, 100);
> 
> Would be likely more efficient (no calculation if unspecified) and not
> much harder to read.
Oh right!

> 
>> +
>> +			percent = qn->ib_percent ? qn->ib_percent : 100;
>> +			agg_peak_rate = mult_frac(percent, qn->max_peak[i], 100);
>> +
> 
> agg_peak_rate doesn't seem to be used anywhere else? 🤔
Whoooooops....

Konrad

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients
  2023-07-26 17:18   ` Stephan Gerhold
@ 2023-07-26 17:20     ` Konrad Dybcio
  2023-07-26 17:26       ` Stephan Gerhold
  0 siblings, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 17:20 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On 26.07.2023 19:18, Stephan Gerhold wrote:
> On Wed, Jul 26, 2023 at 06:25:44PM +0200, Konrad Dybcio wrote:
>> Some buses need additional manual adjustments atop the usual
>> calculations. Fill in the missing coefficients.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> What about the funny util-fact/vrail-comp on the mas-apps-proc node
> downstream?
Can't see it neither on msm-5.4 (with the icc API) nor in the 4.19 (msmbus)
device tree.

Konrad

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients
  2023-07-26 17:20     ` Konrad Dybcio
@ 2023-07-26 17:26       ` Stephan Gerhold
  2023-07-26 17:46         ` Konrad Dybcio
  0 siblings, 1 reply; 13+ messages in thread
From: Stephan Gerhold @ 2023-07-26 17:26 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On Wed, Jul 26, 2023 at 07:20:27PM +0200, Konrad Dybcio wrote:
> On 26.07.2023 19:18, Stephan Gerhold wrote:
> > On Wed, Jul 26, 2023 at 06:25:44PM +0200, Konrad Dybcio wrote:
> >> Some buses need additional manual adjustments atop the usual
> >> calculations. Fill in the missing coefficients.
> >>
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > 
> > What about the funny util-fact/vrail-comp on the mas-apps-proc node
> > downstream?
> Can't see it neither on msm-5.4 (with the icc API) nor in the 4.19 (msmbus)
> device tree.
> 

Not sure where to get up-to-date device trees nowadays. The AOSP repo
I was looking at has this commit where it was added:
https://android.googlesource.com/kernel/msm-extra/devicetree/+/02f8c342b23c20a5cf967df649814be37a08227c%5E%21/#F0

Stephan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients
  2023-07-26 17:26       ` Stephan Gerhold
@ 2023-07-26 17:46         ` Konrad Dybcio
  2023-07-26 17:52           ` Konrad Dybcio
  0 siblings, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 17:46 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On 26.07.2023 19:26, Stephan Gerhold wrote:
> On Wed, Jul 26, 2023 at 07:20:27PM +0200, Konrad Dybcio wrote:
>> On 26.07.2023 19:18, Stephan Gerhold wrote:
>>> On Wed, Jul 26, 2023 at 06:25:44PM +0200, Konrad Dybcio wrote:
>>>> Some buses need additional manual adjustments atop the usual
>>>> calculations. Fill in the missing coefficients.
>>>>
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>
>>> What about the funny util-fact/vrail-comp on the mas-apps-proc node
>>> downstream?
>> Can't see it neither on msm-5.4 (with the icc API) nor in the 4.19 (msmbus)
>> device tree.
>>
> 
> Not sure where to get up-to-date device trees nowadays. The AOSP repo
> I was looking at has this commit where it was added:
> https://android.googlesource.com/kernel/msm-extra/devicetree/+/02f8c342b23c20a5cf967df649814be37a08227c%5E%21/#F0
Oh right, take a look at this one:

https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/commit/201df022706e100cef8d28983c6a7b883fcaec5a

I guess I'll need to update the icc driver then.

Konrad

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients
  2023-07-26 17:46         ` Konrad Dybcio
@ 2023-07-26 17:52           ` Konrad Dybcio
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2023-07-26 17:52 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Andy Gross, Bjorn Andersson, Georgi Djakov, Marijn Suijten,
	linux-arm-msm, linux-pm, linux-kernel

On 26.07.2023 19:46, Konrad Dybcio wrote:
> On 26.07.2023 19:26, Stephan Gerhold wrote:
>> On Wed, Jul 26, 2023 at 07:20:27PM +0200, Konrad Dybcio wrote:
>>> On 26.07.2023 19:18, Stephan Gerhold wrote:
>>>> On Wed, Jul 26, 2023 at 06:25:44PM +0200, Konrad Dybcio wrote:
>>>>> Some buses need additional manual adjustments atop the usual
>>>>> calculations. Fill in the missing coefficients.
>>>>>
>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>>
>>>> What about the funny util-fact/vrail-comp on the mas-apps-proc node
>>>> downstream?
>>> Can't see it neither on msm-5.4 (with the icc API) nor in the 4.19 (msmbus)
>>> device tree.
>>>
>>
>> Not sure where to get up-to-date device trees nowadays. The AOSP repo
>> I was looking at has this commit where it was added:
>> https://android.googlesource.com/kernel/msm-extra/devicetree/+/02f8c342b23c20a5cf967df649814be37a08227c%5E%21/#F0
> Oh right, take a look at this one:
> 
> https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/commit/201df022706e100cef8d28983c6a7b883fcaec5a
> 
> I guess I'll need to update the icc driver then.
Moreover, this having vrail-comp = 96, means I'll have to go with
the wrecked downstream way of *(100/(percent)) instead of
*(percent/100)...

I also noticed that sm6125 makes very heavy use of per-node clocks..

Konrad

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients
  2023-07-26 16:25 ` [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients Konrad Dybcio
  2023-07-26 17:16   ` Stephan Gerhold
@ 2023-07-29  9:25   ` kernel test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-07-29  9:25 UTC (permalink / raw)
  To: Konrad Dybcio, Andy Gross, Bjorn Andersson, Georgi Djakov
  Cc: oe-kbuild-all, Marijn Suijten, linux-arm-msm, linux-pm,
	linux-kernel, Konrad Dybcio

Hi Konrad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1e25dd7772483f477f79986d956028e9f47f990a]

url:    https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/interconnect-qcom-icc-rpm-Add-AB-IB-calculations-coefficients/20230727-002710
base:   1e25dd7772483f477f79986d956028e9f47f990a
patch link:    https://lore.kernel.org/r/20230726-topic-icc_coeff-v1-1-31616960818c%40linaro.org
patch subject: [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20230729/202307291745.0JcdYvBz-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230729/202307291745.0JcdYvBz-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307291745.0JcdYvBz-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/interconnect/qcom/icc-rpm.c: In function 'qcom_icc_bus_aggregate':
>> drivers/interconnect/qcom/icc-rpm.c:302:27: warning: variable 'agg_peak_rate' set but not used [-Wunused-but-set-variable]
     302 |         u64 agg_avg_rate, agg_peak_rate, agg_rate;
         |                           ^~~~~~~~~~~~~


vim +/agg_peak_rate +302 drivers/interconnect/qcom/icc-rpm.c

   293	
   294	/**
   295	 * qcom_icc_bus_aggregate - calculate bus clock rates by traversing all nodes
   296	 * @provider: generic interconnect provider
   297	 * @agg_clk_rate: array containing the aggregated clock rates in kHz
   298	 */
   299	static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_rate)
   300	{
   301		struct qcom_icc_provider *qp = to_qcom_provider(provider);
 > 302		u64 agg_avg_rate, agg_peak_rate, agg_rate;
   303		struct qcom_icc_node *qn;
   304		struct icc_node *node;
   305		u16 percent;
   306		int i;
   307	
   308		/*
   309		 * Iterate nodes on the provider, aggregate bandwidth requests for
   310		 * every bucket and convert them into bus clock rates.
   311		 */
   312		list_for_each_entry(node, &provider->nodes, node_list) {
   313			qn = node->data;
   314			for (i = 0; i < QCOM_SMD_RPM_STATE_NUM; i++) {
   315				if (qn->channels)
   316					agg_avg_rate = div_u64(qn->sum_avg[i], qn->channels);
   317				else
   318					agg_avg_rate = qn->sum_avg[i];
   319	
   320				percent = qp->ab_percent ? qp->ab_percent : 100;
   321				agg_avg_rate = mult_frac(percent, agg_avg_rate, 100);
   322	
   323				percent = qn->ib_percent ? qn->ib_percent : 100;
   324				agg_peak_rate = mult_frac(percent, qn->max_peak[i], 100);
   325	
   326				agg_rate = max_t(u64, agg_avg_rate, qn->max_peak[i]);
   327				do_div(agg_rate, qn->buswidth);
   328	
   329				agg_clk_rate[i] = max_t(u64, agg_clk_rate[i], agg_rate);
   330			}
   331		}
   332	}
   333	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-07-29  9:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-26 16:25 [PATCH 0/4] Fix up icc clock rate calculation on some platforms Konrad Dybcio
2023-07-26 16:25 ` [PATCH 1/4] interconnect: qcom: icc-rpm: Add AB/IB calculations coefficients Konrad Dybcio
2023-07-26 17:16   ` Stephan Gerhold
2023-07-26 17:19     ` Konrad Dybcio
2023-07-29  9:25   ` kernel test robot
2023-07-26 16:25 ` [PATCH 2/4] interconnect: qcom: qcm2290: Set AB coefficients Konrad Dybcio
2023-07-26 17:18   ` Stephan Gerhold
2023-07-26 17:20     ` Konrad Dybcio
2023-07-26 17:26       ` Stephan Gerhold
2023-07-26 17:46         ` Konrad Dybcio
2023-07-26 17:52           ` Konrad Dybcio
2023-07-26 16:25 ` [PATCH 3/4] interconnect: qcom: sdm660: Set AB/IB coefficients Konrad Dybcio
2023-07-26 16:25 ` [PATCH 4/4] interconnect: qcom: msm8996: " Konrad Dybcio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).