diff mbox series

[2/3] interconnect: qcom: msm8939: Merge snoc and snoc_mm into one

Message ID 20220128161002.2308563-3-bryan.odonoghue@linaro.org
State New
Headers show
Series [1/3] dt-bindings: interconnect: Create modified msm8939-snoc description | expand

Commit Message

Bryan O'Donoghue Jan. 28, 2022, 4:10 p.m. UTC
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(-)

Comments

Bryan O'Donoghue Jan. 28, 2022, 11:30 p.m. UTC | #1
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 ?
Dmitry Baryshkov Jan. 29, 2022, 4:10 a.m. UTC | #2
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
diff mbox series

Patch

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);