Make it possible to scale performance states of the power domain and
interconnect of the I2C QUP controller.
This is necessary to guarantee performance with power management
enabled. Otherwise these resources might run at minimal performance
state which is not sufficient for certain workloads.
Signed-off-by: Stephan Gerhold <[email protected]>
---
Stephan Gerhold (3):
dt-bindings: i2c: qcom,i2c-qup: Document power-domains
dt-bindings: i2c: qup: Document interconnects
i2c: qup: Vote for interconnect bandwidth to DRAM
.../devicetree/bindings/i2c/qcom,i2c-qup.yaml | 14 +++++++++
drivers/i2c/busses/i2c-qup.c | 36 ++++++++++++++++++++++
2 files changed, 50 insertions(+)
---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231106-i2c-qup-dvfs-bc60e2998dd8
Best regards,
--
Stephan Gerhold <[email protected]>
Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth
Similar to qcom,geni-i2c, for i2c-qup we need to vote for performance
states on the VDDCX power domain to ensure that required clock rates
can be generated correctly.
I2C is typically used with a fixed clock rate, so a single required-opp
is sufficient without a full OPP table (unlike spi-qup for example).
Signed-off-by: Stephan Gerhold <[email protected]>
---
Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
index f43947514d48..fc3077a7af0d 100644
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
@@ -52,9 +52,15 @@ properties:
- const: default
- const: sleep
+ power-domains:
+ maxItems: 1
+
reg:
maxItems: 1
+ required-opps:
+ maxItems: 1
+
required:
- compatible
- clock-names
@@ -68,6 +74,7 @@ examples:
- |
#include <dt-bindings/clock/qcom,gcc-msm8998.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
i2c@c175000 {
compatible = "qcom,i2c-qup-v2.2.1";
@@ -82,6 +89,8 @@ examples:
pinctrl-names = "default", "sleep";
pinctrl-0 = <&blsp1_i2c1_default>;
pinctrl-1 = <&blsp1_i2c1_sleep>;
+ power-domains = <&rpmpd MSM8909_VDDCX>;
+ required-opps = <&rpmpd_opp_svs_krait>;
clock-frequency = <400000>;
#address-cells = <1>;
--
2.39.2
When the I2C QUP controller is used together with a DMA engine it needs
to vote for the interconnect path to the DRAM. Otherwise it may be
unable to access the memory quickly enough.
The requested peak bandwidth is dependent on the I2C core clock.
To avoid sending votes too often the bandwidth is always requested when
a DMA transfer starts, but dropped only on runtime suspend. Runtime
suspend should only happen if no transfer is active. After resumption we
can defer the next vote until the first DMA transfer actually happens.
The implementation is largely identical to the one introduced for
spi-qup in commit ecdaa9473019 ("spi: qup: Vote for interconnect
bandwidth to DRAM") since both drivers represent the same hardware
block.
Signed-off-by: Stephan Gerhold <[email protected]>
---
The bandwidth calculation is taken over from Qualcomm's
downstream/vendor driver [1]. Due to lack of documentation about the
interconnect setup/behavior I cannot say exactly if this is right.
Unfortunately, this is not implemented very consistently downstream...
[1]: https://git.codelinaro.org/clo/la/kernel/msm-3.10/-/commit/67174e2624ea64814231e7e1e4af83fd882302c6
---
drivers/i2c/busses/i2c-qup.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 598102d16677..ee92a315f074 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -14,6 +14,7 @@
#include <linux/dma-mapping.h>
#include <linux/err.h>
#include <linux/i2c.h>
+#include <linux/interconnect.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -150,6 +151,8 @@
/* TAG length for DATA READ in RX FIFO */
#define READ_RX_TAGS_LEN 2
+#define QUP_BUS_WIDTH 8
+
static unsigned int scl_freq;
module_param_named(scl_freq, scl_freq, uint, 0444);
MODULE_PARM_DESC(scl_freq, "SCL frequency override");
@@ -227,6 +230,7 @@ struct qup_i2c_dev {
int irq;
struct clk *clk;
struct clk *pclk;
+ struct icc_path *icc_path;
struct i2c_adapter adap;
int clk_ctl;
@@ -255,6 +259,10 @@ struct qup_i2c_dev {
/* To configure when bus is in run state */
u32 config_run;
+ /* bandwidth votes */
+ u32 src_clk_freq;
+ u32 cur_bw_clk_freq;
+
/* dma parameters */
bool is_dma;
/* To check if the current transfer is using DMA */
@@ -453,6 +461,23 @@ static int qup_i2c_bus_active(struct qup_i2c_dev *qup, int len)
return ret;
}
+static int qup_i2c_vote_bw(struct qup_i2c_dev *qup, u32 clk_freq)
+{
+ u32 needed_peak_bw;
+ int ret;
+
+ if (qup->cur_bw_clk_freq == clk_freq)
+ return 0;
+
+ needed_peak_bw = Bps_to_icc(clk_freq * QUP_BUS_WIDTH);
+ ret = icc_set_bw(qup->icc_path, 0, needed_peak_bw);
+ if (ret)
+ return ret;
+
+ qup->cur_bw_clk_freq = clk_freq;
+ return 0;
+}
+
static void qup_i2c_write_tx_fifo_v1(struct qup_i2c_dev *qup)
{
struct qup_i2c_block *blk = &qup->blk;
@@ -840,6 +865,10 @@ static int qup_i2c_bam_xfer(struct i2c_adapter *adap, struct i2c_msg *msg,
int ret = 0;
int idx = 0;
+ ret = qup_i2c_vote_bw(qup, qup->src_clk_freq);
+ if (ret)
+ return ret;
+
enable_irq(qup->irq);
ret = qup_i2c_req_dma(qup);
@@ -1645,6 +1674,7 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup)
config = readl(qup->base + QUP_CONFIG);
config |= QUP_CLOCK_AUTO_GATE;
writel(config, qup->base + QUP_CONFIG);
+ qup_i2c_vote_bw(qup, 0);
clk_disable_unprepare(qup->pclk);
}
@@ -1745,6 +1775,11 @@ static int qup_i2c_probe(struct platform_device *pdev)
goto fail_dma;
}
qup->is_dma = true;
+
+ qup->icc_path = devm_of_icc_get(&pdev->dev, NULL);
+ if (IS_ERR(qup->icc_path))
+ return dev_err_probe(&pdev->dev, PTR_ERR(qup->icc_path),
+ "failed to get interconnect path\n");
}
nodma:
@@ -1793,6 +1828,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
qup_i2c_enable_clocks(qup);
src_clk_freq = clk_get_rate(qup->clk);
}
+ qup->src_clk_freq = src_clk_freq;
/*
* Bootloaders might leave a pending interrupt on certain QUP's,
--
2.39.2
When the I2C QUP controller is used together with a DMA engine it needs
to vote for the interconnect path to the DRAM. Otherwise it may be
unable to access the memory quickly enough.
Signed-off-by: Stephan Gerhold <[email protected]>
---
Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
index fc3077a7af0d..758d8f6321e1 100644
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
@@ -40,6 +40,9 @@ properties:
- const: tx
- const: rx
+ interconnects:
+ maxItems: 1
+
interrupts:
maxItems: 1
@@ -73,6 +76,7 @@ unevaluatedProperties: false
examples:
- |
#include <dt-bindings/clock/qcom,gcc-msm8998.h>
+ #include <dt-bindings/interconnect/qcom,msm8996.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/power/qcom-rpmpd.h>
@@ -91,6 +95,7 @@ examples:
pinctrl-1 = <&blsp1_i2c1_sleep>;
power-domains = <&rpmpd MSM8909_VDDCX>;
required-opps = <&rpmpd_opp_svs_krait>;
+ interconnects = <&pnoc MASTER_BLSP_1 &bimc SLAVE_EBI_CH0>;
clock-frequency = <400000>;
#address-cells = <1>;
--
2.39.2
On Tue, 28 Nov 2023 10:48:36 +0100, Stephan Gerhold wrote:
> When the I2C QUP controller is used together with a DMA engine it needs
> to vote for the interconnect path to the DRAM. Otherwise it may be
> unable to access the memory quickly enough.
>
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
> Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
On Tue, 28 Nov 2023 10:48:35 +0100, Stephan Gerhold wrote:
> Similar to qcom,geni-i2c, for i2c-qup we need to vote for performance
> states on the VDDCX power domain to ensure that required clock rates
> can be generated correctly.
>
> I2C is typically used with a fixed clock rate, so a single required-opp
> is sufficient without a full OPP table (unlike spi-qup for example).
>
> Signed-off-by: Stephan Gerhold <[email protected]>
> ---
> Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
Hi Stephan,
On Tue, Nov 28, 2023 at 10:48:37AM +0100, Stephan Gerhold wrote:
> When the I2C QUP controller is used together with a DMA engine it needs
> to vote for the interconnect path to the DRAM. Otherwise it may be
> unable to access the memory quickly enough.
>
> The requested peak bandwidth is dependent on the I2C core clock.
>
> To avoid sending votes too often the bandwidth is always requested when
> a DMA transfer starts, but dropped only on runtime suspend. Runtime
> suspend should only happen if no transfer is active. After resumption we
> can defer the next vote until the first DMA transfer actually happens.
>
> The implementation is largely identical to the one introduced for
> spi-qup in commit ecdaa9473019 ("spi: qup: Vote for interconnect
> bandwidth to DRAM") since both drivers represent the same hardware
> block.
>
> Signed-off-by: Stephan Gerhold <[email protected]>
the patch looks good to me.
> ---
> The bandwidth calculation is taken over from Qualcomm's
> downstream/vendor driver [1]. Due to lack of documentation about the
> interconnect setup/behavior I cannot say exactly if this is right.
> Unfortunately, this is not implemented very consistently downstream...
>
> [1]: https://git.codelinaro.org/clo/la/kernel/msm-3.10/-/commit/67174e2624ea64814231e7e1e4af83fd882302c6
Krzysztof, any chance you can help here?
Thanks,
Andi