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