All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm
@ 2022-01-28 16:09 Bryan O'Donoghue
  2022-01-28 16:10 ` [PATCH 1/3] dt-bindings: interconnect: Create modified msm8939-snoc description Bryan O'Donoghue
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 16:09 UTC (permalink / raw)
  To: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm
  Cc: jun.nie, shawn.guo, benl, dmitry.baryshkov, bryan.odonoghue

Booting msm8939 on tip-of-tree I encountered the following error.

[    1.212340] qnoc-msm8939 580000.interconnect_mm: can't request region for resource [mem 0x00580000-0x0059407f]
[    1.212391] qnoc-msm8939 580000.interconnect_mm: Cannot ioremap interconnect bus resource
[    1.221524] qnoc-msm8939: probe of 580000.interconnect_mm failed with error -16

Initially I thought this was a bug with the interconnect driver but,
examining it a bit more I realized the DTS I was working with based on
downstream, declares snoc and snoc_mm as existing at the same address
range.

When we were developing the DTS for 8939 we weren't using the common rpm
interconnect driver so we never saw the ioremap collision.

Taking a hard look at the qcom documentation as well as the downstream code
we see that yes downstream declares snoc and snoc_mm separately but, also
at the same overlapping address.

The qcom documentation for performance points for msm8936/msm8939 snoc,
deliniates snoc_mm as simply two new performance-points i.e. a faster GPLL0
vote associated with new multi-media devices attached to the snoc.

In other words the snoc had two new RPM vote indices added to it, to
represent the higher performance clocks, should one of the multi-media IP
blocks call for it.

We can fix the ioremap collision and still represent the two higher
performance point clock votes by coalsecing snoc and snoc_mm into snoc. The
DTS clock references will take care of the appropriate votes.

Bryan O'Donoghue (3):
  dt-bindings: interconnect: Create modified msm8939-snoc description
  interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  interconnect: qcom: msm8939: Deliniate bus, bus_a, bus_mm and bus_a_mm
    clocks

 .../bindings/interconnect/qcom,rpm.yaml       | 25 +++++++++++-
 drivers/interconnect/qcom/msm8939.c           | 39 +++++++------------
 2 files changed, 37 insertions(+), 27 deletions(-)

-- 
2.33.0


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

* [PATCH 1/3] dt-bindings: interconnect: Create modified msm8939-snoc description
  2022-01-28 16:09 [PATCH 0/3] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm Bryan O'Donoghue
@ 2022-01-28 16:10 ` Bryan O'Donoghue
  2022-01-28 16:10 ` [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one Bryan O'Donoghue
  2022-01-28 16:10 ` [PATCH 3/3] interconnect: qcom: msm8939: Deliniate bus, bus_a, bus_mm and bus_a_mm clocks Bryan O'Donoghue
  2 siblings, 0 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 16:10 UTC (permalink / raw)
  To: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm
  Cc: jun.nie, shawn.guo, benl, dmitry.baryshkov, bryan.odonoghue,
	Rob Herring, devicetree

Subsume msm8939-snoc-mm and msm8939-snoc into the one device.
Looking at the DTS description for this downstream we see that snoc and
snoc_mm share the same address space, the same clocks, indeed the only hint
at all in qcom documentation these to are separate in anyway is boxes drawn
on a diagram.

snoc_mm is in fact simply two higher performance points for multimedia
devices tacked onto the msm8916 snoc which was reused for msm8936/msm8939.

The various client IP blocks make requests to vote for either X0 or GPLL0
clock rates with the multi-media votes indicating a higher operating point.

We don't need to model snoc and snoc_mm separately since the datasheet for
this part shows snoc_mm is not a separate device.

Breaking up snoc into two pieces is a mistake that was made downstream
which we carried over into upstream.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../bindings/interconnect/qcom,rpm.yaml       | 25 +++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml b/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml
index e4c3c2818119e..1110a242b2132 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml
@@ -79,8 +79,6 @@ allOf:
               - qcom,msm8916-snoc
               - qcom,msm8939-bimc
               - qcom,msm8939-pcnoc
-              - qcom,msm8939-snoc
-              - qcom,msm8939-snoc-mm
               - qcom,msm8996-a1noc
               - qcom,msm8996-a2noc
               - qcom,msm8996-bimc
@@ -182,6 +180,29 @@ allOf:
             - description: Aggregate2 USB3 AXI Clock.
             - description: Config NoC USB2 AXI Clock.
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,msm8939-snoc
+
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: bus
+            - const: bus_a
+            - const: bus_mm
+            - const: bus_a_mm
+
+        clocks:
+          items:
+            - description: Bus Clock.
+            - description: Bus A Clock.
+            - description: Bus Clock MultiMedia.
+            - description: Bus A Clock MultiMedia.
+
 examples:
   - |
       #include <dt-bindings/clock/qcom,rpmcc.h>
-- 
2.33.0


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

* [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  2022-01-28 16:09 [PATCH 0/3] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm Bryan O'Donoghue
  2022-01-28 16:10 ` [PATCH 1/3] dt-bindings: interconnect: Create modified msm8939-snoc description Bryan O'Donoghue
@ 2022-01-28 16:10 ` Bryan O'Donoghue
  2022-01-28 22:24   ` Dmitry Baryshkov
  2022-01-28 16:10 ` [PATCH 3/3] interconnect: qcom: msm8939: Deliniate bus, bus_a, bus_mm and bus_a_mm clocks Bryan O'Donoghue
  2 siblings, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 16:10 UTC (permalink / raw)
  To: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm
  Cc: jun.nie, shawn.guo, benl, dmitry.baryshkov, bryan.odonoghue

The current msm8939 snoc and snoc_mm definitions are represented as
separate entities based on downstream definition which declares two
identical and therefore overlapping mmio regions.

Downstream:
reg = <0x580000 0x14080>,
      <0x580000 0x14080>;
reg-names = "snoc-base", "snoc-mm-base";

Upstream:
snoc: interconnect@580000 {
        compatible = "qcom,msm8939-snoc";
        #interconnect-cells = <1>;
        reg = <0x580000 0x14080>;
        clock-names = "bus", "bus_a";
        clocks = <&rpmcc RPM_SMD_SNOC_CLK>,
                 <&rpmcc RPM_SMD_SNOC_A_CLK>;
        status = "okay";
};

snoc: interconnect@580000 {
        compatible = "qcom,msm8939-snoc_mm";
        #interconnect-cells = <1>;
        reg = <0x580000 0x14080>;
        clock-names = "bus", "bus_a",
        clocks = <&rpmcc RPM_SMD_SYSMMNOC_CLK>,
                 <&rpmcc RPM_SMD_SYSMMNOC_A_CLK>;
        status = "okay";
};

This overlapping declaration leads to the following failure on boot.

[    1.212340] qnoc-msm8939 580000.interconnect_mm: can't request region for resource [mem 0x00580000-0x0059407f]
[    1.212391] qnoc-msm8939 580000.interconnect_mm: Cannot ioremap interconnect bus resource
[    1.221524] qnoc-msm8939: probe of 580000.interconnect_mm failed with error -16

snoc_mm is a complete misnomer though, as there is no distinct register
space, simply an additional clock to drive higher frequences on snoc for
new multi-media 'mm' devices tacked on to the old msm8916 snoc.

The difference can be captured with

- A new clock
- Performance points/clock settings in the relevant multi-media devices.

Fix the above bug by representing snoc_mm as two new clocks to the existing
snoc, not a separate interconnect bus.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/interconnect/qcom/msm8939.c | 30 +++++------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c
index d188f3636e4c3..7030911e25adc 100644
--- a/drivers/interconnect/qcom/msm8939.c
+++ b/drivers/interconnect/qcom/msm8939.c
@@ -1271,25 +1271,6 @@ static struct qcom_icc_node *msm8939_snoc_nodes[] = {
 	[SNOC_INT_BIMC] = &snoc_int_bimc,
 	[SNOC_PCNOC_MAS] = &snoc_pcnoc_mas,
 	[SNOC_QDSS_INT] = &qdss_int,
-};
-
-static const struct regmap_config msm8939_snoc_regmap_config = {
-	.reg_bits	= 32,
-	.reg_stride	= 4,
-	.val_bits	= 32,
-	.max_register	= 0x14080,
-	.fast_io	= true,
-};
-
-static struct qcom_icc_desc msm8939_snoc = {
-	.type = QCOM_ICC_NOC,
-	.nodes = msm8939_snoc_nodes,
-	.num_nodes = ARRAY_SIZE(msm8939_snoc_nodes),
-	.regmap_cfg = &msm8939_snoc_regmap_config,
-	.qos_offset = 0x7000,
-};
-
-static struct qcom_icc_node *msm8939_snoc_mm_nodes[] = {
 	[MASTER_VIDEO_P0] = &mas_video,
 	[MASTER_JPEG] = &mas_jpeg,
 	[MASTER_VFE] = &mas_vfe,
@@ -1301,7 +1282,7 @@ static struct qcom_icc_node *msm8939_snoc_mm_nodes[] = {
 	[SNOC_MM_INT_2] = &mm_int_2,
 };
 
-static const struct regmap_config msm8939_snoc_mm_regmap_config = {
+static const struct regmap_config msm8939_snoc_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
 	.val_bits	= 32,
@@ -1309,11 +1290,11 @@ static const struct regmap_config msm8939_snoc_mm_regmap_config = {
 	.fast_io	= true,
 };
 
-static struct qcom_icc_desc msm8939_snoc_mm = {
+static struct qcom_icc_desc msm8939_snoc = {
 	.type = QCOM_ICC_NOC,
-	.nodes = msm8939_snoc_mm_nodes,
-	.num_nodes = ARRAY_SIZE(msm8939_snoc_mm_nodes),
-	.regmap_cfg = &msm8939_snoc_mm_regmap_config,
+	.nodes = msm8939_snoc_nodes,
+	.num_nodes = ARRAY_SIZE(msm8939_snoc_nodes),
+	.regmap_cfg = &msm8939_snoc_regmap_config,
 	.qos_offset = 0x7000,
 };
 
@@ -1420,7 +1401,6 @@ static const struct of_device_id msm8939_noc_of_match[] = {
 	{ .compatible = "qcom,msm8939-bimc", .data = &msm8939_bimc },
 	{ .compatible = "qcom,msm8939-pcnoc", .data = &msm8939_pcnoc },
 	{ .compatible = "qcom,msm8939-snoc", .data = &msm8939_snoc },
-	{ .compatible = "qcom,msm8939-snoc-mm", .data = &msm8939_snoc_mm },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, msm8939_noc_of_match);
-- 
2.33.0


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

* [PATCH 3/3] interconnect: qcom: msm8939: Deliniate bus, bus_a, bus_mm and bus_a_mm clocks
  2022-01-28 16:09 [PATCH 0/3] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm Bryan O'Donoghue
  2022-01-28 16:10 ` [PATCH 1/3] dt-bindings: interconnect: Create modified msm8939-snoc description Bryan O'Donoghue
  2022-01-28 16:10 ` [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one Bryan O'Donoghue
@ 2022-01-28 16:10 ` Bryan O'Donoghue
  2 siblings, 0 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 16:10 UTC (permalink / raw)
  To: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm
  Cc: jun.nie, shawn.guo, benl, dmitry.baryshkov, bryan.odonoghue

System NoC (snoc) and System MM NoC (snoc_mm) clocks can be defined
together and passed as part of a single snoc interconnect. rpmcc indexes
take care of sending the right RPM messages.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/interconnect/qcom/msm8939.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c
index 7030911e25adc..431e77d63df46 100644
--- a/drivers/interconnect/qcom/msm8939.c
+++ b/drivers/interconnect/qcom/msm8939.c
@@ -1290,10 +1290,19 @@ static const struct regmap_config msm8939_snoc_regmap_config = {
 	.fast_io	= true,
 };
 
+static const char * const snoc_clocks[] = {
+	"bus",
+	"bus_a",
+	"bus_mm",
+	"bus_a_mm",
+};
+
 static struct qcom_icc_desc msm8939_snoc = {
 	.type = QCOM_ICC_NOC,
 	.nodes = msm8939_snoc_nodes,
 	.num_nodes = ARRAY_SIZE(msm8939_snoc_nodes),
+	.clocks = snoc_clocks,
+	.num_clocks = ARRAY_SIZE(snoc_clocks),
 	.regmap_cfg = &msm8939_snoc_regmap_config,
 	.qos_offset = 0x7000,
 };
-- 
2.33.0


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

* Re: [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  2022-01-28 16:10 ` [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one Bryan O'Donoghue
@ 2022-01-28 22:24   ` Dmitry Baryshkov
  2022-01-28 23:23     ` Bryan O'Donoghue
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Baryshkov @ 2022-01-28 22:24 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm,
	jun.nie, shawn.guo, benl

On Fri, 28 Jan 2022 at 19:10, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> The current msm8939 snoc and snoc_mm definitions are represented as
> separate entities based on downstream definition which declares two
> identical and therefore overlapping mmio regions.
>
> Downstream:
> reg = <0x580000 0x14080>,
>       <0x580000 0x14080>;
> reg-names = "snoc-base", "snoc-mm-base";
>
> Upstream:
> snoc: interconnect@580000 {
>         compatible = "qcom,msm8939-snoc";
>         #interconnect-cells = <1>;
>         reg = <0x580000 0x14080>;
>         clock-names = "bus", "bus_a";
>         clocks = <&rpmcc RPM_SMD_SNOC_CLK>,
>                  <&rpmcc RPM_SMD_SNOC_A_CLK>;
>         status = "okay";
> };
>
> snoc: interconnect@580000 {
>         compatible = "qcom,msm8939-snoc_mm";
>         #interconnect-cells = <1>;
>         reg = <0x580000 0x14080>;
>         clock-names = "bus", "bus_a",
>         clocks = <&rpmcc RPM_SMD_SYSMMNOC_CLK>,
>                  <&rpmcc RPM_SMD_SYSMMNOC_A_CLK>;
>         status = "okay";
> };
>
> This overlapping declaration leads to the following failure on boot.
>
> [    1.212340] qnoc-msm8939 580000.interconnect_mm: can't request region for resource [mem 0x00580000-0x0059407f]
> [    1.212391] qnoc-msm8939 580000.interconnect_mm: Cannot ioremap interconnect bus resource
> [    1.221524] qnoc-msm8939: probe of 580000.interconnect_mm failed with error -16
>
> snoc_mm is a complete misnomer though, as there is no distinct register
> space, simply an additional clock to drive higher frequences on snoc for
> new multi-media 'mm' devices tacked on to the old msm8916 snoc.
>
> The difference can be captured with
>
> - A new clock
> - Performance points/clock settings in the relevant multi-media devices.
>
> Fix the above bug by representing snoc_mm as two new clocks to the existing
> snoc, not a separate interconnect bus.

This would lead to higher frequencies being set on both 'normal' and
mm snoc clocks, thus (possibly) increasing power consumption.

The proper fix should be implemented following patches 4 and 5 from
https://lore.kernel.org/all/20211215002324.1727-1-shawn.guo@linaro.org/

>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/interconnect/qcom/msm8939.c | 30 +++++------------------------
>  1 file changed, 5 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c
> index d188f3636e4c3..7030911e25adc 100644
> --- a/drivers/interconnect/qcom/msm8939.c
> +++ b/drivers/interconnect/qcom/msm8939.c
> @@ -1271,25 +1271,6 @@ static struct qcom_icc_node *msm8939_snoc_nodes[] = {
>         [SNOC_INT_BIMC] = &snoc_int_bimc,
>         [SNOC_PCNOC_MAS] = &snoc_pcnoc_mas,
>         [SNOC_QDSS_INT] = &qdss_int,
> -};
> -
> -static const struct regmap_config msm8939_snoc_regmap_config = {
> -       .reg_bits       = 32,
> -       .reg_stride     = 4,
> -       .val_bits       = 32,
> -       .max_register   = 0x14080,
> -       .fast_io        = true,
> -};
> -
> -static struct qcom_icc_desc msm8939_snoc = {
> -       .type = QCOM_ICC_NOC,
> -       .nodes = msm8939_snoc_nodes,
> -       .num_nodes = ARRAY_SIZE(msm8939_snoc_nodes),
> -       .regmap_cfg = &msm8939_snoc_regmap_config,
> -       .qos_offset = 0x7000,
> -};
> -
> -static struct qcom_icc_node *msm8939_snoc_mm_nodes[] = {
>         [MASTER_VIDEO_P0] = &mas_video,
>         [MASTER_JPEG] = &mas_jpeg,
>         [MASTER_VFE] = &mas_vfe,
> @@ -1301,7 +1282,7 @@ static struct qcom_icc_node *msm8939_snoc_mm_nodes[] = {
>         [SNOC_MM_INT_2] = &mm_int_2,
>  };
>
> -static const struct regmap_config msm8939_snoc_mm_regmap_config = {
> +static const struct regmap_config msm8939_snoc_regmap_config = {
>         .reg_bits       = 32,
>         .reg_stride     = 4,
>         .val_bits       = 32,
> @@ -1309,11 +1290,11 @@ static const struct regmap_config msm8939_snoc_mm_regmap_config = {
>         .fast_io        = true,
>  };
>
> -static struct qcom_icc_desc msm8939_snoc_mm = {
> +static struct qcom_icc_desc msm8939_snoc = {
>         .type = QCOM_ICC_NOC,
> -       .nodes = msm8939_snoc_mm_nodes,
> -       .num_nodes = ARRAY_SIZE(msm8939_snoc_mm_nodes),
> -       .regmap_cfg = &msm8939_snoc_mm_regmap_config,
> +       .nodes = msm8939_snoc_nodes,
> +       .num_nodes = ARRAY_SIZE(msm8939_snoc_nodes),
> +       .regmap_cfg = &msm8939_snoc_regmap_config,
>         .qos_offset = 0x7000,
>  };
>
> @@ -1420,7 +1401,6 @@ static const struct of_device_id msm8939_noc_of_match[] = {
>         { .compatible = "qcom,msm8939-bimc", .data = &msm8939_bimc },
>         { .compatible = "qcom,msm8939-pcnoc", .data = &msm8939_pcnoc },
>         { .compatible = "qcom,msm8939-snoc", .data = &msm8939_snoc },
> -       { .compatible = "qcom,msm8939-snoc-mm", .data = &msm8939_snoc_mm },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, msm8939_noc_of_match);
> --
> 2.33.0
>


-- 
With best wishes
Dmitry

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

* Re: [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  2022-01-28 22:24   ` Dmitry Baryshkov
@ 2022-01-28 23:23     ` Bryan O'Donoghue
  2022-01-28 23:30       ` Bryan O'Donoghue
  2022-01-29  4:10       ` Dmitry Baryshkov
  0 siblings, 2 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 23:23 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm,
	jun.nie, shawn.guo, benl

On 28/01/2022 22:24, Dmitry Baryshkov wrote:
> This would lead to higher frequencies being set on both 'normal' and
> mm snoc clocks, thus (possibly) increasing power consumption.
> 
How so ?

There are four clocks

bus
bus_a
bus_mm
bus_a_mm

The last two clocks

SNOC performance points are
0 | 19.2  | XO
1 | 50    | GPLL0
2 | 100   | GPLL0
3 | 133.3 | GPLL0
4 | 160   | GPLL0
5 | 200   | GPLL0
6 | 266.6 | GPLL0

SNOC_MM performance points are
0 | 19.2  | XO
1 | 50    | GPLL0
2 | 100   | GPLL0
3 | 133.3 | GPLL0
4 | 160   | GPLL0
5 | 200   | GPLL0
6 | 266.6 | GPLL0
7 | 320   | GPLL0
8 | 400   | GPLL0

Its GPLL0 being set, the snoc_mm clocks really just map back to GPLL0

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

* Re: [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  2022-01-28 23:23     ` Bryan O'Donoghue
@ 2022-01-28 23:30       ` Bryan O'Donoghue
  2022-01-28 23:38         ` Bryan O'Donoghue
  2022-01-29  4:10       ` Dmitry Baryshkov
  1 sibling, 1 reply; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 23:30 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm,
	jun.nie, shawn.guo, benl

On 28/01/2022 23:23, Bryan O'Donoghue wrote:
> On 28/01/2022 22:24, Dmitry Baryshkov wrote:
>> This would lead to higher frequencies being set on both 'normal' and
>> mm snoc clocks, thus (possibly) increasing power consumption.
>>
> How so ?
> 
> There are four clocks
> 
> bus
> bus_a
> bus_mm
> bus_a_mm
> 
> The last two clocks
> 
> SNOC performance points are
> 0 | 19.2  | XO
> 1 | 50    | GPLL0
> 2 | 100   | GPLL0
> 3 | 133.3 | GPLL0
> 4 | 160   | GPLL0
> 5 | 200   | GPLL0
> 6 | 266.6 | GPLL0
> 
> SNOC_MM performance points are
> 0 | 19.2  | XO
> 1 | 50    | GPLL0
> 2 | 100   | GPLL0
> 3 | 133.3 | GPLL0
> 4 | 160   | GPLL0
> 5 | 200   | GPLL0
> 6 | 266.6 | GPLL0
> 7 | 320   | GPLL0
> 8 | 400   | GPLL0
> 
> Its GPLL0 being set, the snoc_mm clocks really just map back to GPLL0

So taking an example

If venus
interconnects = <&snoc_mm MASTER_VIDEO_P0 &bimc SLAVE_EBI_CH0>;

or mdp
interconnects = <&snoc_mm MASTER_MDP_PORT0 &bimc SLAVE_EBI_CH0>,
                 <&snoc_mm MASTER_MDP_PORT1 &bimc SLAVE_EBI_CH0>;

wants performance point #6 GPLL0 runs at 266.6 but for performance point 
#7 it runs at 320 MHz

Since the points all map back to a GPLL0 frequency, how does aggregating 
snoc and snoc_mm - result in higher power consumption ?

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

* Re: [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  2022-01-28 23:30       ` Bryan O'Donoghue
@ 2022-01-28 23:38         ` Bryan O'Donoghue
  0 siblings, 0 replies; 9+ messages in thread
From: Bryan O'Donoghue @ 2022-01-28 23:38 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm,
	jun.nie, shawn.guo, benl

On 28/01/2022 23:30, Bryan O'Donoghue wrote:
> On 28/01/2022 23:23, Bryan O'Donoghue wrote:
>> On 28/01/2022 22:24, Dmitry Baryshkov wrote:
>>> This would lead to higher frequencies being set on both 'normal' and
>>> mm snoc clocks, thus (possibly) increasing power consumption.
>>>
>> How so ?
>>
>> There are four clocks
>>
>> bus
>> bus_a
>> bus_mm
>> bus_a_mm
>>
>> The last two clocks
>>
>> SNOC performance points are
>> 0 | 19.2  | XO
>> 1 | 50    | GPLL0
>> 2 | 100   | GPLL0
>> 3 | 133.3 | GPLL0
>> 4 | 160   | GPLL0
>> 5 | 200   | GPLL0
>> 6 | 266.6 | GPLL0
>>
>> SNOC_MM performance points are
>> 0 | 19.2  | XO
>> 1 | 50    | GPLL0
>> 2 | 100   | GPLL0
>> 3 | 133.3 | GPLL0
>> 4 | 160   | GPLL0
>> 5 | 200   | GPLL0
>> 6 | 266.6 | GPLL0
>> 7 | 320   | GPLL0
>> 8 | 400   | GPLL0
>>
>> Its GPLL0 being set, the snoc_mm clocks really just map back to GPLL0
> 
> So taking an example
> 
> If venus
> interconnects = <&snoc_mm MASTER_VIDEO_P0 &bimc SLAVE_EBI_CH0>;
> 
> or mdp
> interconnects = <&snoc_mm MASTER_MDP_PORT0 &bimc SLAVE_EBI_CH0>,
>                  <&snoc_mm MASTER_MDP_PORT1 &bimc SLAVE_EBI_CH0>;
> 
> wants performance point #6 GPLL0 runs at 266.6 but for performance point 
> #7 it runs at 320 MHz
> 
> Since the points all map back to a GPLL0 frequency, how does aggregating 
> snoc and snoc_mm - result in higher power consumption ?

Anyway - thanks for the pointer to the virt clock.

I don't mind re-implementing the fix this way given there's an 
established upstream canon.

---
bod

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

* Re: [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one
  2022-01-28 23:23     ` Bryan O'Donoghue
  2022-01-28 23:30       ` Bryan O'Donoghue
@ 2022-01-29  4:10       ` Dmitry Baryshkov
  1 sibling, 0 replies; 9+ messages in thread
From: Dmitry Baryshkov @ 2022-01-29  4:10 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: djakov, bjorn.andersson, agross, linux-arm-msm, linux-pm,
	jun.nie, shawn.guo, benl

On Sat, 29 Jan 2022 at 02:23, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 28/01/2022 22:24, Dmitry Baryshkov wrote:
> > This would lead to higher frequencies being set on both 'normal' and
> > mm snoc clocks, thus (possibly) increasing power consumption.
> >
> How so ?

If I remember correctly, bus clocks are set to max(sum(avg_bw),
max(peak_bw)) calculated over all bandwidth paths (nodes).
If you merge snoc and snoc_mm, the resulting sum(avg_bw) would be a
sum of (older) snoc's and snoc_mm's sums.
Thus the bus clocks (both bus and bus_mm) would be set to higher frequencies.

>
> There are four clocks
>
> bus
> bus_a
> bus_mm
> bus_a_mm
>
> The last two clocks
>
> SNOC performance points are
> 0 | 19.2  | XO
> 1 | 50    | GPLL0
> 2 | 100   | GPLL0
> 3 | 133.3 | GPLL0
> 4 | 160   | GPLL0
> 5 | 200   | GPLL0
> 6 | 266.6 | GPLL0
>
> SNOC_MM performance points are
> 0 | 19.2  | XO
> 1 | 50    | GPLL0
> 2 | 100   | GPLL0
> 3 | 133.3 | GPLL0
> 4 | 160   | GPLL0
> 5 | 200   | GPLL0
> 6 | 266.6 | GPLL0
> 7 | 320   | GPLL0
> 8 | 400   | GPLL0
>
> Its GPLL0 being set, the snoc_mm clocks really just map back to GPLL0



-- 
With best wishes
Dmitry

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

end of thread, other threads:[~2022-01-29  4:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 16:09 [PATCH 0/3] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm Bryan O'Donoghue
2022-01-28 16:10 ` [PATCH 1/3] dt-bindings: interconnect: Create modified msm8939-snoc description Bryan O'Donoghue
2022-01-28 16:10 ` [PATCH 2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one Bryan O'Donoghue
2022-01-28 22:24   ` Dmitry Baryshkov
2022-01-28 23:23     ` Bryan O'Donoghue
2022-01-28 23:30       ` Bryan O'Donoghue
2022-01-28 23:38         ` Bryan O'Donoghue
2022-01-29  4:10       ` Dmitry Baryshkov
2022-01-28 16:10 ` [PATCH 3/3] interconnect: qcom: msm8939: Deliniate bus, bus_a, bus_mm and bus_a_mm clocks Bryan O'Donoghue

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.