linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 00/11] DVFS in the OPP core
@ 2019-03-20  9:49 Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 01/11] OPP: Don't overwrite rounded clk rate Rajendra Nayak
                   ` (13 more replies)
  0 siblings, 14 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, linux-pm, linux-serial, linux-spi, dri-devel,
	linux-scsi, swboyd, ulf.hansson, viresh.kumar, dianders, rafael,
	Rajendra Nayak

This is a v2 of the RFC posted earlier by Stephen Boyd [1]

As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
API using clk framework to round the frequency passed and making it
accept 0 as a valid frequency indicating the frequency isn't required
anymore. It just has a few more drivers converted to use this approach
like dsi/dpu and ufs.
ufs demonstrates the case of having to handle multiple power domains, one
of which is scalable.

The patches are based on 5.1-rc1 and depend on some ufs fixes I posted
earlier [2] and a DT patch to include the rpmpd header [3]

[1] https://lkml.org/lkml/2019/1/28/2086
[2] https://lkml.org/lkml/2019/3/8/70
[3] https://lkml.org/lkml/2019/3/20/120

Rajendra Nayak (10):
  OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
  tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  spi: spi-geni-qcom: Use OPP API to set clk/perf state
  arm64: dts: sdm845: Add OPP table for all qup devices
  scsi: ufs: Add support to manage multiple power domains in
    ufshcd-pltfrm
  scsi: ufs: Add support for specifying OPP tables in DT
  arm64: dts: sdm845: Add ufs opps and power-domains
  drm/msm/dpu: Use OPP API to set clk/perf state
  drm/msm: dsi: Use OPP API to set clk/perf state
  arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains

Stephen Boyd (1):
  OPP: Don't overwrite rounded clk rate

 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 194 +++++++++++++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |   7 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |   9 +
 drivers/gpu/drm/msm/dsi/dsi.h                 |   2 +
 drivers/gpu/drm/msm/dsi/dsi_cfg.c             |   4 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c            |  88 +++++++-
 drivers/opp/core.c                            |  26 ++-
 drivers/scsi/ufs/ufshcd-pltfrm.c              |  52 ++++-
 drivers/scsi/ufs/ufshcd.c                     |  21 +-
 drivers/scsi/ufs/ufshcd.h                     |   3 +
 drivers/spi/spi-geni-qcom.c                   |  14 +-
 drivers/tty/serial/qcom_geni_serial.c         |  15 +-
 12 files changed, 406 insertions(+), 29 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-06-11 10:54   ` Viresh Kumar
  2019-03-20  9:49 ` [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid Rajendra Nayak
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, linux-pm, linux-serial, linux-spi, dri-devel,
	linux-scsi, swboyd, ulf.hansson, viresh.kumar, dianders, rafael,
	Rajendra Nayak

From: Stephen Boyd <swboyd@chromium.org>

Doing this allows us to call this API with any rate requested and have
it not need to match in the OPP table. Instead, we'll round the rate up
to the nearest OPP that we see so that we can get the voltage or level
that's required for that OPP. This supports users of OPP that want to
specify the 'fmax' tables of a device instead of every single frequency
that they need. And for devices that required the exact frequency, we
can rely on the clk framework to round the rate to the nearest supported
frequency instead of the OPP framework to do so.

Note that this may affect drivers that don't want the clk framework to
do rounding, but instead want the OPP table to do the rounding for them.
Do we have that case? Should we add some flag to the OPP table to
indicate this and then not have that flag set when there isn't an OPP
table for the device and also introduce a property like 'opp-use-clk' to
tell the table that it should use the clk APIs to round rates instead of
OPP?

Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/opp/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0420f7e8ad5b..bc9a7762dd4c 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -703,7 +703,7 @@ static int _set_required_opps(struct device *dev,
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
 	struct opp_table *opp_table;
-	unsigned long freq, old_freq;
+	unsigned long freq, opp_freq, old_freq, old_opp_freq;
 	struct dev_pm_opp *old_opp, *opp;
 	struct clk *clk;
 	int ret;
@@ -742,13 +742,15 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		goto put_opp_table;
 	}
 
-	old_opp = _find_freq_ceil(opp_table, &old_freq);
+	old_opp_freq = old_freq;
+	old_opp = _find_freq_ceil(opp_table, &old_opp_freq);
 	if (IS_ERR(old_opp)) {
 		dev_err(dev, "%s: failed to find current OPP for freq %lu (%ld)\n",
 			__func__, old_freq, PTR_ERR(old_opp));
 	}
 
-	opp = _find_freq_ceil(opp_table, &freq);
+	opp_freq = freq;
+	opp = _find_freq_ceil(opp_table, &opp_freq);
 	if (IS_ERR(opp)) {
 		ret = PTR_ERR(opp);
 		dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n",
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 01/11] OPP: Don't overwrite rounded clk rate Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-06-14  6:32   ` Viresh Kumar
  2019-03-20  9:49 ` [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state Rajendra Nayak
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, linux-pm, linux-serial, linux-spi, dri-devel,
	linux-scsi, swboyd, ulf.hansson, viresh.kumar, dianders, rafael,
	Rajendra Nayak

For devices with performance state, we use dev_pm_opp_set_rate()
to set the appropriate clk rate and the performance state.
We do need a way to *remove* the performance state vote when
we idle the device and turn the clocks off. Use dev_pm_opp_set_rate()
with freq=0 to achieve this.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/opp/core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index bc9a7762dd4c..d6acc880676e 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -708,18 +708,24 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 	struct clk *clk;
 	int ret;
 
-	if (unlikely(!target_freq)) {
-		dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
-			target_freq);
-		return -EINVAL;
-	}
-
 	opp_table = _find_opp_table(dev);
 	if (IS_ERR(opp_table)) {
 		dev_err(dev, "%s: device opp doesn't exist\n", __func__);
 		return PTR_ERR(opp_table);
 	}
 
+	if (unlikely(!target_freq)) {
+		if (opp_table->required_opp_tables) {
+			/* drop the performance state vote */
+			dev_pm_genpd_set_performance_state(dev, 0);
+			return 0;
+		} else {
+			dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
+				target_freq);
+			return -EINVAL;
+		}
+	}
+
 	clk = opp_table->clk;
 	if (IS_ERR(clk)) {
 		dev_err(dev, "%s: No clock available for the device\n",
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 01/11] OPP: Don't overwrite rounded clk rate Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2020-08-11 23:11   ` John Stultz
  2019-03-20  9:49 ` [RFC v2 04/11] spi: spi-geni-qcom: " Rajendra Nayak
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

geni serial needs to express a perforamnce state requirement on CX
depending on the frequency of the clock rates. Use OPP table from
DT to register with OPP framework and use dev_pm_opp_set_rate() to
set the clk/perf state.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3bcec1c20219..422852911141 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pm_opp.h>
 #include <linux/platform_device.h>
 #include <linux/qcom-geni-se.h>
 #include <linux/serial.h>
@@ -115,6 +116,7 @@ struct qcom_geni_serial_port {
 	bool brk;
 
 	unsigned int tx_remaining;
+	struct device *dev;
 };
 
 static const struct uart_ops qcom_geni_console_pops;
@@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
 		goto out_restart_rx;
 
 	uport->uartclk = clk_rate;
-	clk_set_rate(port->se.clk, clk_rate);
+	dev_pm_opp_set_rate(port->dev, clk_rate);
 	ser_clk_cfg = SER_CLK_EN;
 	ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
 
@@ -1198,8 +1200,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
 	if (new_state == UART_PM_STATE_ON && old_state == UART_PM_STATE_OFF)
 		geni_se_resources_on(&port->se);
 	else if (new_state == UART_PM_STATE_OFF &&
-			old_state == UART_PM_STATE_ON)
+			old_state == UART_PM_STATE_ON) {
+		dev_pm_opp_set_rate(port->dev, 0);
 		geni_se_resources_off(&port->se);
+	}
 }
 
 static const struct uart_ops qcom_geni_console_pops = {
@@ -1265,6 +1269,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Invalid line %d\n", line);
 		return PTR_ERR(port);
 	}
+	port->dev = &pdev->dev;
 
 	uport = &port->uport;
 	/* Don't allow 2 drivers to access the same port */
@@ -1286,6 +1291,12 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 		return -EINVAL;
 	uport->mapbase = res->start;
 
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to init OPP table: %d\n", ret);
+		return ret;
+	}
+
 	port->tx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
 	port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
 	port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 04/11] spi: spi-geni-qcom: Use OPP API to set clk/perf state
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (2 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 05/11] arm64: dts: sdm845: Add OPP table for all qup devices Rajendra Nayak
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

geni spi needs to express a perforamnce state requirement on CX
depending on the frequency of the clock rates. Use OPP table from
DT to register with OPP framework and use dev_pm_opp_set_rate() to
set the clk/perf state.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/spi/spi-geni-qcom.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 5f0b0d5bfef4..c251e6df1bc0 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
 #include <linux/qcom-geni-se.h>
 #include <linux/spi/spi.h>
@@ -96,7 +97,6 @@ static int get_spi_clk_cfg(unsigned int speed_hz,
 {
 	unsigned long sclk_freq;
 	unsigned int actual_hz;
-	struct geni_se *se = &mas->se;
 	int ret;
 
 	ret = geni_se_clk_freq_match(&mas->se,
@@ -113,9 +113,9 @@ static int get_spi_clk_cfg(unsigned int speed_hz,
 
 	dev_dbg(mas->dev, "req %u=>%u sclk %lu, idx %d, div %d\n", speed_hz,
 				actual_hz, sclk_freq, *clk_idx, *clk_div);
-	ret = clk_set_rate(se->clk, sclk_freq);
+	ret = dev_pm_opp_set_rate(mas->dev, sclk_freq);
 	if (ret)
-		dev_err(mas->dev, "clk_set_rate failed %d\n", ret);
+		dev_err(mas->dev, "dev_pm_opp_set_rate failed %d\n", ret);
 	return ret;
 }
 
@@ -560,6 +560,12 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (!spi)
 		return -ENOMEM;
 
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to init OPP table: %d\n", ret);
+		return ret;
+	}
+
 	platform_set_drvdata(pdev, spi);
 	mas = spi_master_get_devdata(spi);
 	mas->irq = irq;
@@ -625,6 +631,8 @@ static int __maybe_unused spi_geni_runtime_suspend(struct device *dev)
 	struct spi_master *spi = dev_get_drvdata(dev);
 	struct spi_geni_master *mas = spi_master_get_devdata(spi);
 
+	/* Drop the performance state vote */
+	dev_pm_opp_set_rate(dev, 0);
 	return geni_se_resources_off(&mas->se);
 }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 05/11] arm64: dts: sdm845: Add OPP table for all qup devices
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (3 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 04/11] spi: spi-geni-qcom: " Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 06/11] scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm Rajendra Nayak
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, linux-pm, linux-serial, linux-spi, dri-devel,
	linux-scsi, swboyd, ulf.hansson, viresh.kumar, dianders, rafael,
	Rajendra Nayak

qup has a requirement to vote on the performance state of the CX domain
in sdm845 devices. Add OPP tables for these and also add power-domains
property for all qup instances.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 115 +++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index e4b69c74fe07..027ffe6e93e8 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -409,6 +409,25 @@
 			clock-names = "core";
 		};
 
+		qup_opp_table: qup-opp-table {
+			compatible = "operating-points-v2";
+
+			opp-19200000 {
+				opp-hz = /bits/ 64 <19200000>;
+				required-opps = <&rpmhpd_opp_min_svs>;
+			};
+
+			opp-75000000 {
+				opp-hz = /bits/ 64 <75000000>;
+				required-opps = <&rpmhpd_opp_low_svs>;
+			};
+
+			opp-100000000 {
+				opp-hz = /bits/ 64 <100000000>;
+				required-opps = <&rpmhpd_opp_svs>;
+			};
+		};
+
 		qupv3_id_0: geniqup@8c0000 {
 			compatible = "qcom,geni-se-qup";
 			reg = <0 0x008c0000 0 0x6000>;
@@ -430,6 +449,8 @@
 				interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -443,6 +464,8 @@
 				interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -454,6 +477,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart0_default>;
 				interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -467,6 +492,8 @@
 				interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -480,6 +507,8 @@
 				interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -491,6 +520,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart1_default>;
 				interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -504,6 +535,8 @@
 				interrupts = <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -517,6 +550,8 @@
 				interrupts = <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -528,6 +563,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart2_default>;
 				interrupts = <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -541,6 +578,8 @@
 				interrupts = <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -554,6 +593,8 @@
 				interrupts = <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -565,6 +606,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart3_default>;
 				interrupts = <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -578,6 +621,8 @@
 				interrupts = <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -591,6 +636,8 @@
 				interrupts = <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -602,6 +649,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart4_default>;
 				interrupts = <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -615,6 +664,8 @@
 				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -628,6 +679,8 @@
 				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -639,6 +692,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart5_default>;
 				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -652,6 +707,8 @@
 				interrupts = <GIC_SPI 607 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -665,6 +722,8 @@
 				interrupts = <GIC_SPI 607 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -676,6 +735,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart6_default>;
 				interrupts = <GIC_SPI 607 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -689,6 +750,8 @@
 				interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -702,6 +765,8 @@
 				interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -713,6 +778,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart7_default>;
 				interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 		};
@@ -738,6 +805,8 @@
 				interrupts = <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -751,6 +820,8 @@
 				interrupts = <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -762,6 +833,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart8_default>;
 				interrupts = <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -775,6 +848,8 @@
 				interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -788,6 +863,8 @@
 				interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -799,6 +876,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart9_default>;
 				interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -812,6 +891,8 @@
 				interrupts = <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -825,6 +906,8 @@
 				interrupts = <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -836,6 +919,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart10_default>;
 				interrupts = <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -849,6 +934,8 @@
 				interrupts = <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -862,6 +949,8 @@
 				interrupts = <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -873,6 +962,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart11_default>;
 				interrupts = <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -886,6 +977,8 @@
 				interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -899,6 +992,8 @@
 				interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -910,6 +1005,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart12_default>;
 				interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -923,6 +1020,8 @@
 				interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -936,6 +1035,8 @@
 				interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -947,6 +1048,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart13_default>;
 				interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -960,6 +1063,8 @@
 				interrupts = <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -973,6 +1078,8 @@
 				interrupts = <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -984,6 +1091,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart14_default>;
 				interrupts = <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -997,6 +1106,8 @@
 				interrupts = <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -1010,6 +1121,8 @@
 				interrupts = <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 
@@ -1021,6 +1134,8 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&qup_uart15_default>;
 				interrupts = <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>;
+				power-domains = <&rpmhpd SDM845_CX>;
+				operating-points-v2 = <&qup_opp_table>;
 				status = "disabled";
 			};
 		};
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* [RFC v2 06/11] scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (4 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 05/11] arm64: dts: sdm845: Add OPP table for all qup devices Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 07/11] scsi: ufs: Add support for specifying OPP tables in DT Rajendra Nayak
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

Some UFS devices need to manage multiple powerdomains. Add support for
it as part of the ufshcd-pltfrm driver.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 52 +++++++++++++++++++++++++++++++-
 drivers/scsi/ufs/ufshcd.h        |  3 ++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 238a79f21e74..ce33eb8b7510 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -34,6 +34,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 
@@ -289,6 +290,43 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
 	}
 }
 
+static int ufshcd_attach_pds(struct device *dev, struct ufs_hba *hba, int num_pds)
+{
+	int i, ret;
+
+	hba->virt_devs = devm_kzalloc(dev, sizeof(struct device *) * num_pds,
+				      GFP_KERNEL);
+	if (!hba->virt_devs)
+		return -ENOMEM;
+
+	hba->num_virt_devs = num_pds;
+	for (i = 0; i < num_pds; i++) {
+		hba->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i);
+		if (IS_ERR(hba->virt_devs[i])) {
+			ret = PTR_ERR(hba->virt_devs[i]);
+			goto unroll_attach;
+		}
+		device_link_add(dev,hba->virt_devs[i], DL_FLAG_RPM_ACTIVE |
+				DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
+	}
+
+	return ret;
+
+unroll_attach:
+	for (i--; i >= 0; i--)
+		dev_pm_domain_detach(hba->virt_devs[i], false);
+
+	return ret;
+}
+
+static void ufshcd_detach_pds(struct ufs_hba *hba)
+{
+	int i;
+
+	for (i = 0; i < hba->num_virt_devs; i++)
+		dev_pm_domain_detach(hba->virt_devs[i], false);
+}
+
 /**
  * ufshcd_pltfrm_init - probe routine of the driver
  * @pdev: pointer to Platform device handle
@@ -302,7 +340,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 	struct ufs_hba *hba;
 	void __iomem *mmio_base;
 	struct resource *mem_res;
-	int irq, err;
+	int irq, err, num_pds;
 	struct device *dev = &pdev->dev;
 
 	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -340,6 +378,16 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 		goto dealloc_host;
 	}
 
+	num_pds = of_count_phandle_with_args(dev->of_node, "power-domains",
+					     "#power-domain-cells");
+	if (num_pds > 1) {
+		err = ufshcd_attach_pds(&pdev->dev, hba, num_pds);
+		if (err) {
+			dev_err(&pdev->dev, "%s: attach of power domains failed %d\n",
+				__func__, err);
+			goto dealloc_host;
+		}
+	}
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
@@ -358,6 +406,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 	return 0;
 
 out_disable_rpm:
+	if (num_pds > 1)
+		ufshcd_detach_pds(hba);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index ecfa898b9ccc..bca1e008f506 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -517,6 +517,9 @@ struct ufs_hba {
 
 	struct Scsi_Host *host;
 	struct device *dev;
+	struct device **virt_devs;
+	u8 num_virt_devs;
+
 	/*
 	 * This field is to keep a reference to "scsi_device" corresponding to
 	 * "UFS device" W-LU.
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 07/11] scsi: ufs: Add support for specifying OPP tables in DT
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (5 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 06/11] scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains Rajendra Nayak
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

Some platforms like qualcomms sdm845 SoC have a need to set
a performance state of a power domain for UFS along with
setting the clock rate. Add support for passing this freq/perf state
tuple from DT as an OPP table. Modify the driver to read the OPP
table and register with OPP layer.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ffa9e58680b4..2b260e83874a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -913,6 +913,16 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
 	if (ret)
 		return ret;
 
+	if (hba->virt_devs) {
+		struct dev_pm_opp *opp;
+		unsigned long freq = scale_up ? INT_MAX: 0;
+		if (scale_up)
+			opp = dev_pm_opp_find_freq_floor(hba->dev, &freq);
+		else
+			opp = dev_pm_opp_find_freq_ceil(hba->dev, &freq);
+		dev_pm_opp_set_rate(hba->dev, dev_pm_opp_get_freq(opp));
+	}
+
 	list_for_each_entry(clki, head, list) {
 		if (!IS_ERR_OR_NULL(clki->clk)) {
 			if (scale_up && clki->max_freq) {
@@ -1318,6 +1328,7 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
 	struct list_head *clk_list = &hba->clk_list_head;
 	struct ufs_clk_info *clki;
 	struct devfreq *devfreq;
+	struct device *virt_dev;
 	int ret;
 
 	/* Skip devfreq if we don't have any clocks in the list */
@@ -1325,8 +1336,14 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
 		return 0;
 
 	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
-	dev_pm_opp_add(hba->dev, clki->min_freq, 0);
-	dev_pm_opp_add(hba->dev, clki->max_freq, 0);
+
+	if (dev_pm_opp_of_add_table(hba->dev)) {
+		dev_pm_opp_add(hba->dev, clki->min_freq, 0);
+		dev_pm_opp_add(hba->dev, clki->max_freq, 0);
+	} else {
+		virt_dev = hba->virt_devs[hba->num_virt_devs -1];
+		dev_pm_opp_set_genpd_virt_dev(hba->dev, virt_dev, 0);
+	}
 
 	devfreq = devfreq_add_device(hba->dev,
 			&ufs_devfreq_profile,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (6 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 07/11] scsi: ufs: Add support for specifying OPP tables in DT Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-05-14  7:53   ` Ulf Hansson
  2019-03-20  9:49 ` [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state Rajendra Nayak
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

Add the additional power domain and the OPP table for ufs on sdm845
so the driver can set the appropriate performance state of the
power domain while setting the clock rate.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 027ffe6e93e8..a3af4a1757b4 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1140,6 +1140,21 @@
 			};
 		};
 
+		ufs_opp_table: ufs-opp-table {
+			compatible = "operating-points-v2";
+
+			opp-50000000 {
+				opp-hz = /bits/ 64 <50000000>;
+				required-opps = <&rpmhpd_opp_min_svs>;
+			};
+
+			opp-200000000 {
+				opp-hz = /bits/ 64 <200000000>;
+				required-opps = <&rpmhpd_opp_nom>;
+
+			};
+		};
+
 		ufs_mem_hc: ufshc@1d84000 {
 			compatible = "qcom,sdm845-ufshc", "qcom,ufshc",
 				     "jedec,ufs-2.0";
@@ -1148,7 +1163,7 @@
 			phys = <&ufs_mem_phy_lanes>;
 			phy-names = "ufsphy";
 			lanes-per-direction = <2>;
-			power-domains = <&gcc UFS_PHY_GDSC>;
+			power-domains = <&gcc UFS_PHY_GDSC>, <&rpmhpd SDM845_CX>;
 
 			iommus = <&apps_smmu 0x100 0xf>;
 
@@ -1170,6 +1185,9 @@
 				<&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
 				<&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
 				<&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
+
+			operating-points-v2 = <&ufs_opp_table>;
+
 			freq-table-hz =
 				<50000000 200000000>,
 				<0 0>,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (7 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-04-10  3:49   ` Viresh Kumar
  2019-03-20  9:49 ` [RFC v2 10/11] drm/msm: dsi: " Rajendra Nayak
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

On some qualcomm platforms DPU needs to express a perforamnce state
requirement on a power domain depennding on the clock rates.
Use OPP table from DT to register with OPP framework and use
dev_pm_opp_set_rate() to set the clk/perf state.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 7 ++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 9 +++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 9f20f397f77d..db21a86b242b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -15,6 +15,7 @@
 #include <linux/debugfs.h>
 #include <linux/errno.h>
 #include <linux/mutex.h>
+#include <linux/pm_opp.h>
 #include <linux/sort.h>
 #include <linux/clk.h>
 #include <linux/bitmap.h>
@@ -298,7 +299,11 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate)
 		rate = core_clk->max_rate;
 
 	core_clk->rate = rate;
-	return msm_dss_clk_set_rate(core_clk, 1);
+
+	if (dev_pm_opp_get_opp_table(&kms->pdev->dev))
+		return dev_pm_opp_set_rate(&kms->pdev->dev, core_clk->rate);
+	else
+		return msm_dss_clk_set_rate(core_clk, 1);
 }
 
 static u64 _dpu_core_perf_get_core_clk_rate(struct dpu_kms *kms)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88afa3e..684bd6982aaf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -22,6 +22,7 @@
 #include <linux/debugfs.h>
 #include <linux/of_irq.h>
 #include <linux/dma-buf.h>
+#include <linux/pm_opp.h>
 
 #include "msm_drv.h"
 #include "msm_mmu.h"
@@ -1014,6 +1015,12 @@ static int dpu_bind(struct device *dev, struct device *master, void *data)
 	if (!dpu_kms)
 		return -ENOMEM;
 
+	dev_pm_opp_set_clkname(dev, "core");
+
+	ret = dev_pm_opp_of_add_table(dev);
+	if (ret)
+		dev_err(dev, "failed to init OPP table: %d\n", ret);
+
 	mp = &dpu_kms->mp;
 	ret = msm_dss_parse_clock(pdev, mp);
 	if (ret) {
@@ -1040,6 +1047,7 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data)
 	struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
 	struct dss_module_power *mp = &dpu_kms->mp;
 
+	dev_pm_opp_of_remove_table(dev);
 	msm_dss_put_clk(mp->clk_config, mp->num_clk);
 	devm_kfree(&pdev->dev, mp->clk_config);
 	mp->num_clk = 0;
@@ -1078,6 +1086,7 @@ static int __maybe_unused dpu_runtime_suspend(struct device *dev)
 		return rc;
 	}
 
+	dev_pm_opp_set_rate(dev, 0);
 	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
 	if (rc)
 		DPU_ERROR("clock disable failed rc:%d\n", rc);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 10/11] drm/msm: dsi: Use OPP API to set clk/perf state
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (8 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-03-20  9:49 ` [RFC v2 11/11] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains Rajendra Nayak
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

On SDM845 DSI needs to express a perforamnce state
requirement on a power domain depending on the clock rates.
Use OPP table from DT to register with OPP framework and use
dev_pm_opp_set_rate() to set the clk/perf state.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/gpu/drm/msm/dsi/dsi.h      |  2 +
 drivers/gpu/drm/msm/dsi/dsi_cfg.c  |  4 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c | 88 +++++++++++++++++++++++++++---
 3 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 9c6b31c2d79f..b4398a798370 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -188,8 +188,10 @@ int msm_dsi_runtime_suspend(struct device *dev);
 int msm_dsi_runtime_resume(struct device *dev);
 int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host);
 int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host);
+int dsi_link_clk_enable_6g_v2(struct msm_dsi_host *msm_host);
 void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host);
 void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host);
+void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host);
 int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size);
 int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size);
 void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index dcdfb1bb54f9..c18532f92e4a 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -159,8 +159,8 @@ const static struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = {
 };
 
 const static struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = {
-	.link_clk_enable = dsi_link_clk_enable_6g,
-	.link_clk_disable = dsi_link_clk_disable_6g,
+	.link_clk_enable = dsi_link_clk_enable_6g_v2,
+	.link_clk_disable = dsi_link_clk_disable_6g_v2,
 	.clk_init_ver = dsi_clk_init_6g_v2,
 	.tx_buf_alloc = dsi_tx_buf_alloc_6g,
 	.tx_buf_get = dsi_tx_buf_get_6g,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 610183db1daf..6ed9e6a0520c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -21,6 +21,7 @@
 #include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/pm_opp.h>
 #include <linux/of_graph.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spinlock.h>
@@ -511,7 +512,7 @@ int msm_dsi_runtime_resume(struct device *dev)
 	return dsi_bus_clk_enable(msm_host);
 }
 
-int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
+static int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host)
 {
 	int ret;
 
@@ -521,29 +522,65 @@ int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
 	ret = clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate);
 	if (ret) {
 		pr_err("%s: Failed to set rate byte clk, %d\n", __func__, ret);
-		goto error;
+		return ret;
 	}
 
 	ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate);
 	if (ret) {
 		pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret);
-		goto error;
+		return ret;
 	}
 
 	if (msm_host->byte_intf_clk) {
 		ret = clk_set_rate(msm_host->byte_intf_clk,
 				   msm_host->byte_clk_rate / 2);
-		if (ret) {
+		if (ret)
+			pr_err("%s: Failed to set rate byte intf clk, %d\n",
+			       __func__, ret);
+	}
+
+	return ret;
+}
+
+static int dsi_link_clk_set_rate_6g_v2(struct msm_dsi_host *msm_host)
+{
+	int ret;
+	struct device *dev = &msm_host->pdev->dev;
+
+	DBG("Set clk rates: pclk=%d, byteclk=%d",
+		msm_host->mode->clock, msm_host->byte_clk_rate);
+
+	ret = dev_pm_opp_set_rate(dev, msm_host->byte_clk_rate);
+	if (ret) {
+		pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate);
+	if (ret) {
+		pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret);
+		return ret;
+	}
+
+	if (msm_host->byte_intf_clk) {
+		ret = clk_set_rate(msm_host->byte_intf_clk,
+				msm_host->byte_clk_rate / 2);
+		if (ret)
 			pr_err("%s: Failed to set rate byte intf clk, %d\n",
 			       __func__, ret);
-			goto error;
-		}
 	}
 
+	return ret;
+}
+
+static int dsi_link_clk_prepare_enable_6g(struct msm_dsi_host *msm_host)
+{
+	int ret;
+
 	ret = clk_prepare_enable(msm_host->esc_clk);
 	if (ret) {
 		pr_err("%s: Failed to enable dsi esc clk\n", __func__);
-		goto error;
+		return ret;
 	}
 
 	ret = clk_prepare_enable(msm_host->byte_clk);
@@ -575,10 +612,31 @@ int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
 	clk_disable_unprepare(msm_host->byte_clk);
 byte_clk_err:
 	clk_disable_unprepare(msm_host->esc_clk);
-error:
 	return ret;
 }
 
+int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
+{
+	int ret;
+
+	ret = dsi_link_clk_set_rate_6g(msm_host);
+	if (ret)
+		return ret;
+
+	return dsi_link_clk_prepare_enable_6g(msm_host);
+}
+
+int dsi_link_clk_enable_6g_v2(struct msm_dsi_host *msm_host)
+{
+	int ret;
+
+	ret = dsi_link_clk_set_rate_6g_v2(msm_host);
+	if (ret)
+		return ret;
+
+	return dsi_link_clk_prepare_enable_6g(msm_host);
+}
+
 int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host)
 {
 	int ret;
@@ -656,6 +714,13 @@ void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host)
 	clk_disable_unprepare(msm_host->byte_clk);
 }
 
+void dsi_link_clk_disable_6g_v2(struct msm_dsi_host *msm_host)
+{
+	/* Drop the performance state vote */
+	dev_pm_opp_set_rate(&msm_host->pdev->dev, 0);
+	dsi_link_clk_disable_6g(msm_host);
+}
+
 void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host)
 {
 	clk_disable_unprepare(msm_host->pixel_clk);
@@ -1864,6 +1929,12 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
 		goto fail;
 	}
 
+	dev_pm_opp_set_clkname(&pdev->dev, "byte");
+
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (ret < 0)
+		dev_err(&pdev->dev, "failed to init OPP table: %d\n", ret);
+
 	msm_host->rx_buf = devm_kzalloc(&pdev->dev, SZ_4K, GFP_KERNEL);
 	if (!msm_host->rx_buf) {
 		ret = -ENOMEM;
@@ -1896,6 +1967,7 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host)
 	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
 
 	DBG("");
+	dev_pm_opp_of_remove_table(&msm_host->pdev->dev);
 	dsi_tx_buf_free(msm_host);
 	if (msm_host->workqueue) {
 		flush_workqueue(msm_host->workqueue);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [RFC v2 11/11] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (9 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 10/11] drm/msm: dsi: " Rajendra Nayak
@ 2019-03-20  9:49 ` Rajendra Nayak
  2019-04-10  3:51 ` [RFC v2 00/11] DVFS in the OPP core Viresh Kumar
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-03-20  9:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: ulf.hansson, Rajendra Nayak, linux-scsi, linux-pm, linux-arm-msm,
	rafael, dianders, dri-devel, linux-spi, linux-serial,
	viresh.kumar, swboyd

Add the OPP tables for DSI and MDP based on the perf state/clk
requirements, and add the power-domains property to specify the
scalable power domain.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 59 ++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index a3af4a1757b4..675954fde391 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1857,6 +1857,59 @@
 			#reset-cells = <1>;
 		};
 
+		mdp_opp_table: mdp-opp-table {
+			compatible = "operating-points-v2";
+
+			opp-19200000 {
+				opp-hz = /bits/ 64 <19200000>;
+				required-opps = <&rpmhpd_opp_min_svs>;
+			};
+
+			opp-171428571 {
+				opp-hz = /bits/ 64 <171428571>;
+				required-opps = <&rpmhpd_opp_low_svs>;
+			};
+
+			opp-344000000 {
+				opp-hz = /bits/ 64 <344000000>;
+				required-opps = <&rpmhpd_opp_svs_l1>;
+			};
+
+			opp-430000000 {
+				opp-hz = /bits/ 64 <430000000>;
+				required-opps = <&rpmhpd_opp_nom>;
+			};
+		};
+
+		dsi_opp_table: dsi-opp-table {
+			compatible = "operating-points-v2";
+
+			opp-19200000 {
+				opp-hz = /bits/ 64 <19200000>;
+				required-opps = <&rpmhpd_opp_min_svs>;
+			};
+
+			opp-180000000 {
+				opp-hz = /bits/ 64 <180000000>;
+				required-opps = <&rpmhpd_opp_low_svs>;
+			};
+
+			opp-275000000 {
+				opp-hz = /bits/ 64 <275000000>;
+				required-opps = <&rpmhpd_opp_svs>;
+			};
+
+			opp-328580000 {
+				opp-hz = /bits/ 64 <328580000>;
+				required-opps = <&rpmhpd_opp_svs_l1>;
+			};
+
+			opp-358000000 {
+				opp-hz = /bits/ 64 <358000000>;
+				required-opps = <&rpmhpd_opp_nom>;
+			};
+		};
+
 		mdss: mdss@ae00000 {
 			compatible = "qcom,sdm845-mdss";
 			reg = <0 0x0ae00000 0 0x1000>;
@@ -1901,6 +1954,8 @@
 						  <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
 				assigned-clock-rates = <300000000>,
 						       <19200000>;
+				operating-points-v2 = <&mdp_opp_table>;
+				power-domains = <&rpmhpd SDM845_CX>;
 
 				interrupt-parent = <&mdss>;
 				interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
@@ -1947,6 +2002,8 @@
 					      "core",
 					      "iface",
 					      "bus";
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SDM845_CX>;
 
 				phys = <&dsi0_phy>;
 				phy-names = "dsi";
@@ -2013,6 +2070,8 @@
 					      "core",
 					      "iface",
 					      "bus";
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SDM845_CX>;
 
 				phys = <&dsi1_phy>;
 				phy-names = "dsi";
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state
  2019-03-20  9:49 ` [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state Rajendra Nayak
@ 2019-04-10  3:49   ` Viresh Kumar
  2019-04-10  3:49     ` Viresh Kumar
  0 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-04-10  3:49 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> On some qualcomm platforms DPU needs to express a perforamnce state
> requirement on a power domain depennding on the clock rates.
> Use OPP table from DT to register with OPP framework and use
> dev_pm_opp_set_rate() to set the clk/perf state.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 7 ++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 9 +++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 9f20f397f77d..db21a86b242b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -15,6 +15,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/errno.h>
>  #include <linux/mutex.h>
> +#include <linux/pm_opp.h>
>  #include <linux/sort.h>
>  #include <linux/clk.h>
>  #include <linux/bitmap.h>
> @@ -298,7 +299,11 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate)
>  		rate = core_clk->max_rate;
>  
>  	core_clk->rate = rate;
> -	return msm_dss_clk_set_rate(core_clk, 1);
> +
> +	if (dev_pm_opp_get_opp_table(&kms->pdev->dev))

This takes a reference of the OPP table, you need to call put thing as well to
balance it off.

> +		return dev_pm_opp_set_rate(&kms->pdev->dev, core_clk->rate);
> +	else
> +		return msm_dss_clk_set_rate(core_clk, 1);
>  }
-- 
viresh

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

* Re: [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state
  2019-04-10  3:49   ` Viresh Kumar
@ 2019-04-10  3:49     ` Viresh Kumar
  0 siblings, 0 replies; 38+ messages in thread
From: Viresh Kumar @ 2019-04-10  3:49 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> On some qualcomm platforms DPU needs to express a perforamnce state
> requirement on a power domain depennding on the clock rates.
> Use OPP table from DT to register with OPP framework and use
> dev_pm_opp_set_rate() to set the clk/perf state.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 7 ++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       | 9 +++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 9f20f397f77d..db21a86b242b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -15,6 +15,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/errno.h>
>  #include <linux/mutex.h>
> +#include <linux/pm_opp.h>
>  #include <linux/sort.h>
>  #include <linux/clk.h>
>  #include <linux/bitmap.h>
> @@ -298,7 +299,11 @@ static int _dpu_core_perf_set_core_clk_rate(struct dpu_kms *kms, u64 rate)
>  		rate = core_clk->max_rate;
>  
>  	core_clk->rate = rate;
> -	return msm_dss_clk_set_rate(core_clk, 1);
> +
> +	if (dev_pm_opp_get_opp_table(&kms->pdev->dev))

This takes a reference of the OPP table, you need to call put thing as well to
balance it off.

> +		return dev_pm_opp_set_rate(&kms->pdev->dev, core_clk->rate);
> +	else
> +		return msm_dss_clk_set_rate(core_clk, 1);
>  }
-- 
viresh

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

* Re: [RFC v2 00/11] DVFS in the OPP core
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (10 preceding siblings ...)
  2019-03-20  9:49 ` [RFC v2 11/11] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains Rajendra Nayak
@ 2019-04-10  3:51 ` Viresh Kumar
  2019-04-10  3:51   ` Viresh Kumar
  2019-05-21  6:22 ` Viresh Kumar
  2019-06-17  4:26 ` Viresh Kumar
  13 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-04-10  3:51 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
> 
> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
> API using clk framework to round the frequency passed and making it
> accept 0 as a valid frequency indicating the frequency isn't required
> anymore. It just has a few more drivers converted to use this approach
> like dsi/dpu and ufs.
> ufs demonstrates the case of having to handle multiple power domains, one
> of which is scalable.

I though we discussed about enabling/disabling clk as well from OPP core to
simply driver ? I was expecting that to be part of this version :(

-- 
viresh

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

* Re: [RFC v2 00/11] DVFS in the OPP core
  2019-04-10  3:51 ` [RFC v2 00/11] DVFS in the OPP core Viresh Kumar
@ 2019-04-10  3:51   ` Viresh Kumar
  0 siblings, 0 replies; 38+ messages in thread
From: Viresh Kumar @ 2019-04-10  3:51 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
> 
> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
> API using clk framework to round the frequency passed and making it
> accept 0 as a valid frequency indicating the frequency isn't required
> anymore. It just has a few more drivers converted to use this approach
> like dsi/dpu and ufs.
> ufs demonstrates the case of having to handle multiple power domains, one
> of which is scalable.

I though we discussed about enabling/disabling clk as well from OPP core to
simply driver ? I was expecting that to be part of this version :(

-- 
viresh

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

* Re: [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains
  2019-03-20  9:49 ` [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains Rajendra Nayak
@ 2019-05-14  7:53   ` Ulf Hansson
  0 siblings, 0 replies; 38+ messages in thread
From: Ulf Hansson @ 2019-05-14  7:53 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: Linux Kernel Mailing List, linux-arm-msm, Linux PM, linux-serial,
	linux-spi, dri-devel, linux-scsi, Stephen Boyd, Viresh Kumar,
	Doug Anderson, Rafael J. Wysocki

On Wed, 20 Mar 2019 at 10:50, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
> Add the additional power domain and the OPP table for ufs on sdm845
> so the driver can set the appropriate performance state of the
> power domain while setting the clock rate.
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 027ffe6e93e8..a3af4a1757b4 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1140,6 +1140,21 @@
>                         };
>                 };
>
> +               ufs_opp_table: ufs-opp-table {
> +                       compatible = "operating-points-v2";
> +
> +                       opp-50000000 {
> +                               opp-hz = /bits/ 64 <50000000>;
> +                               required-opps = <&rpmhpd_opp_min_svs>;
> +                       };
> +
> +                       opp-200000000 {
> +                               opp-hz = /bits/ 64 <200000000>;
> +                               required-opps = <&rpmhpd_opp_nom>;
> +
> +                       };
> +               };
> +
>                 ufs_mem_hc: ufshc@1d84000 {
>                         compatible = "qcom,sdm845-ufshc", "qcom,ufshc",
>                                      "jedec,ufs-2.0";
> @@ -1148,7 +1163,7 @@
>                         phys = <&ufs_mem_phy_lanes>;
>                         phy-names = "ufsphy";
>                         lanes-per-direction = <2>;
> -                       power-domains = <&gcc UFS_PHY_GDSC>;
> +                       power-domains = <&gcc UFS_PHY_GDSC>, <&rpmhpd SDM845_CX>;

You probably want to use "power-domain-names" as well.

[...]

Kind regards
Uffe

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

* Re: [RFC v2 00/11] DVFS in the OPP core
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (11 preceding siblings ...)
  2019-04-10  3:51 ` [RFC v2 00/11] DVFS in the OPP core Viresh Kumar
@ 2019-05-21  6:22 ` Viresh Kumar
  2019-05-24  6:03   ` Rajendra Nayak
  2019-06-17  4:26 ` Viresh Kumar
  13 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-05-21  6:22 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael,
	vincent.guittot

On 20-03-19, 15:19, Rajendra Nayak wrote:
> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
> 
> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
> API using clk framework to round the frequency passed and making it
> accept 0 as a valid frequency indicating the frequency isn't required
> anymore. It just has a few more drivers converted to use this approach
> like dsi/dpu and ufs.
> ufs demonstrates the case of having to handle multiple power domains, one
> of which is scalable.
> 
> The patches are based on 5.1-rc1 and depend on some ufs fixes I posted
> earlier [2] and a DT patch to include the rpmpd header [3]
> 
> [1] https://lkml.org/lkml/2019/1/28/2086
> [2] https://lkml.org/lkml/2019/3/8/70
> [3] https://lkml.org/lkml/2019/3/20/120

Hi Rajendra,

I am inclined to apply/push this series for 5.3-rc1, will it be
possible for you to spend some time on this at priority ?

-- 
viresh

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

* Re: [RFC v2 00/11] DVFS in the OPP core
  2019-05-21  6:22 ` Viresh Kumar
@ 2019-05-24  6:03   ` Rajendra Nayak
  0 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-05-24  6:03 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael,
	vincent.guittot


On 5/21/2019 11:52 AM, Viresh Kumar wrote:
> On 20-03-19, 15:19, Rajendra Nayak wrote:
>> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
>>
>> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
>> API using clk framework to round the frequency passed and making it
>> accept 0 as a valid frequency indicating the frequency isn't required
>> anymore. It just has a few more drivers converted to use this approach
>> like dsi/dpu and ufs.
>> ufs demonstrates the case of having to handle multiple power domains, one
>> of which is scalable.
>>
>> The patches are based on 5.1-rc1 and depend on some ufs fixes I posted
>> earlier [2] and a DT patch to include the rpmpd header [3]
>>
>> [1] https://lkml.org/lkml/2019/1/28/2086
>> [2] https://lkml.org/lkml/2019/3/8/70
>> [3] https://lkml.org/lkml/2019/3/20/120
> 
> Hi Rajendra,
> 
> I am inclined to apply/push this series for 5.3-rc1, will it be
> possible for you to spend some time on this at priority ?

Hey Viresh, I was on vacation, just got back. I will refresh this series
and address your previous feedback, I haven't received much feedback for the
driver changes :/ but we can atleast review and get the OPP layer changes
finalized. thanks.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-03-20  9:49 ` [RFC v2 01/11] OPP: Don't overwrite rounded clk rate Rajendra Nayak
@ 2019-06-11 10:54   ` Viresh Kumar
  2019-06-12  7:42     ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-06-11 10:54 UTC (permalink / raw)
  To: swboyd, Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> From: Stephen Boyd <swboyd@chromium.org>
> 
> Doing this allows us to call this API with any rate requested and have
> it not need to match in the OPP table. Instead, we'll round the rate up
> to the nearest OPP that we see so that we can get the voltage or level
> that's required for that OPP. This supports users of OPP that want to
> specify the 'fmax' tables of a device instead of every single frequency
> that they need. And for devices that required the exact frequency, we
> can rely on the clk framework to round the rate to the nearest supported
> frequency instead of the OPP framework to do so.
> 
> Note that this may affect drivers that don't want the clk framework to
> do rounding, but instead want the OPP table to do the rounding for them.
> Do we have that case? Should we add some flag to the OPP table to
> indicate this and then not have that flag set when there isn't an OPP
> table for the device and also introduce a property like 'opp-use-clk' to
> tell the table that it should use the clk APIs to round rates instead of
> OPP?
> 
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/opp/core.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 0420f7e8ad5b..bc9a7762dd4c 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -703,7 +703,7 @@ static int _set_required_opps(struct device *dev,
>  int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>  {
>  	struct opp_table *opp_table;
> -	unsigned long freq, old_freq;
> +	unsigned long freq, opp_freq, old_freq, old_opp_freq;
>  	struct dev_pm_opp *old_opp, *opp;
>  	struct clk *clk;
>  	int ret;
> @@ -742,13 +742,15 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>  		goto put_opp_table;
>  	}
>  
> -	old_opp = _find_freq_ceil(opp_table, &old_freq);
> +	old_opp_freq = old_freq;
> +	old_opp = _find_freq_ceil(opp_table, &old_opp_freq);
>  	if (IS_ERR(old_opp)) {
>  		dev_err(dev, "%s: failed to find current OPP for freq %lu (%ld)\n",
>  			__func__, old_freq, PTR_ERR(old_opp));
>  	}
>  
> -	opp = _find_freq_ceil(opp_table, &freq);
> +	opp_freq = freq;
> +	opp = _find_freq_ceil(opp_table, &opp_freq);
>  	if (IS_ERR(opp)) {
>  		ret = PTR_ERR(opp);
>  		dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n",

I see a logical problem with this patch.

Suppose the clock driver supports following frequencies: 500M, 800M,
1G, 1.2G and the OPP table contains following list: 500M, 1G, 1.2G
(i.e. missing 800M).

Now 800M should never get programmed as it isn't part of the OPP
table. But if you pass 600M to opp-set-rate, then it will end up
selecting 800M as clock driver will round up to the closest value.

Even if no one is doing this right now, it is a sensible usecase,
specially during testing of patches and I don't think we should avoid
it.

What exactly is the use case for which we need this patch ? What kind
of driver ? Some detail can be helpful to find another solution that
fixes this problem.

-- 
viresh

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-11 10:54   ` Viresh Kumar
@ 2019-06-12  7:42     ` Rajendra Nayak
  2019-06-12  8:25       ` Viresh Kumar
  0 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-06-12  7:42 UTC (permalink / raw)
  To: Viresh Kumar, swboyd
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael


On 6/11/2019 4:24 PM, Viresh Kumar wrote:
> On 20-03-19, 15:19, Rajendra Nayak wrote:
>> From: Stephen Boyd <swboyd@chromium.org>
>>
>> Doing this allows us to call this API with any rate requested and have
>> it not need to match in the OPP table. Instead, we'll round the rate up
>> to the nearest OPP that we see so that we can get the voltage or level
>> that's required for that OPP. This supports users of OPP that want to
>> specify the 'fmax' tables of a device instead of every single frequency
>> that they need. And for devices that required the exact frequency, we
>> can rely on the clk framework to round the rate to the nearest supported
>> frequency instead of the OPP framework to do so.
>>
>> Note that this may affect drivers that don't want the clk framework to
>> do rounding, but instead want the OPP table to do the rounding for them.
>> Do we have that case? Should we add some flag to the OPP table to
>> indicate this and then not have that flag set when there isn't an OPP
>> table for the device and also introduce a property like 'opp-use-clk' to
>> tell the table that it should use the clk APIs to round rates instead of
>> OPP?
>>
>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---

[]...

> 
> I see a logical problem with this patch.
> 
> Suppose the clock driver supports following frequencies: 500M, 800M,
> 1G, 1.2G and the OPP table contains following list: 500M, 1G, 1.2G
> (i.e. missing 800M).
> 
> Now 800M should never get programmed as it isn't part of the OPP
> table. But if you pass 600M to opp-set-rate, then it will end up
> selecting 800M as clock driver will round up to the closest value.

correct

> 
> Even if no one is doing this right now, it is a sensible usecase,
> specially during testing of patches and I don't think we should avoid
> it.
> 
> What exactly is the use case for which we need this patch ? 
Like the changelog says 'This supports users of OPP that want to
specify the 'fmax' tables of a device instead of every single frequency
that they need'

so the 'fmax' tables basically say what the max frequency the device can
operate at for a given performance state/voltage level.

so in your example it would be for instance

500M, Perf state = 2
1G, Perf state = 3
1.2G, Perf state = 4

Now when the device wants to operate at say 800Mhz, you need to set the
Perf state to 3, so this patch basically avoids you having to put those additional
OPPs in the table which would otherwise look something like this

500M, Perf state = 2
800M, Perf state = 3 <-- redundant OPP
1G, Perf state = 3
1.2G, Perf state = 4

Your example had just 1 missing entry in the 'fmax' tables in reality its a lot more,
atleast on all qualcomm platforms.


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-12  7:42     ` Rajendra Nayak
@ 2019-06-12  8:25       ` Viresh Kumar
  2019-06-13  9:54         ` Viresh Kumar
  0 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-06-12  8:25 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: swboyd, linux-kernel, linux-arm-msm, linux-pm, linux-serial,
	linux-spi, dri-devel, linux-scsi, ulf.hansson, dianders, rafael

On 12-06-19, 13:12, Rajendra Nayak wrote:
> so the 'fmax' tables basically say what the max frequency the device can
> operate at for a given performance state/voltage level.
> 
> so in your example it would be for instance
> 
> 500M, Perf state = 2
> 1G, Perf state = 3
> 1.2G, Perf state = 4
> 
> Now when the device wants to operate at say 800Mhz, you need to set the
> Perf state to 3, so this patch basically avoids you having to put those additional
> OPPs in the table which would otherwise look something like this
> 
> 500M, Perf state = 2
> 800M, Perf state = 3 <-- redundant OPP
> 1G, Perf state = 3
> 1.2G, Perf state = 4
> 
> Your example had just 1 missing entry in the 'fmax' tables in reality its a lot more,
> atleast on all qualcomm platforms.

Okay, I have applied this patch (alone) to the OPP tree with minor
modifications in commit log and diff.

-- 
viresh

-------------------------8<-------------------------

From: Stephen Boyd <swboyd@chromium.org>
Date: Wed, 20 Mar 2019 15:19:08 +0530
Subject: [PATCH] opp: Don't overwrite rounded clk rate

Doing this allows us to call this API with any rate requested and have
it no need to match in the OPP table. Instead, we'll round the rate up
to the nearest OPP, so that we can get the voltage or performance level
required for that OPP. This supports users of the OPP core that want to
specify the possible 'fmax' values corresponding to the voltage or
performance levels of each OPP. And for devices that required the exact
frequency, we can rely on the clk framework to round the rate to the
nearest supported frequency instead of the OPP framework to do so.

Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
[ Viresh: Massaged changelog and use temp_opp variable instead ]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 764e05a2fa66..0fbc77f05048 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -757,7 +757,7 @@ static int _set_required_opps(struct device *dev,
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 {
 	struct opp_table *opp_table;
-	unsigned long freq, old_freq;
+	unsigned long freq, old_freq, temp_freq;
 	struct dev_pm_opp *old_opp, *opp;
 	struct clk *clk;
 	int ret;
@@ -796,13 +796,15 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		goto put_opp_table;
 	}
 
-	old_opp = _find_freq_ceil(opp_table, &old_freq);
+	temp_freq = old_freq;
+	old_opp = _find_freq_ceil(opp_table, &temp_freq);
 	if (IS_ERR(old_opp)) {
 		dev_err(dev, "%s: failed to find current OPP for freq %lu (%ld)\n",
 			__func__, old_freq, PTR_ERR(old_opp));
 	}
 
-	opp = _find_freq_ceil(opp_table, &freq);
+	temp_freq = freq;
+	opp = _find_freq_ceil(opp_table, &temp_freq);
 	if (IS_ERR(opp)) {
 		ret = PTR_ERR(opp);
 		dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n",

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-12  8:25       ` Viresh Kumar
@ 2019-06-13  9:54         ` Viresh Kumar
  2019-06-14  5:27           ` Viresh Kumar
  2019-06-14  5:54           ` Rajendra Nayak
  0 siblings, 2 replies; 38+ messages in thread
From: Viresh Kumar @ 2019-06-13  9:54 UTC (permalink / raw)
  To: swboyd, Rajendra Nayak, vincent.guittot, mturquette
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael

On 12-06-19, 13:55, Viresh Kumar wrote:
> Okay, I have applied this patch (alone) to the OPP tree with minor
> modifications in commit log and diff.

And I have removed it now :)

I am confused as hell on what we should be doing and what we are doing
right now. And if we should do better.

Let me explain with an example.

- The clock provider supports following frequencies: 500, 600, 700,
  800, 900, 1000 MHz.

- The OPP table contains/supports only a subset: 500, 700, 1000 MHz.

Now, the request to change the frequency starts from cpufreq
governors, like schedutil when they calls:

__cpufreq_driver_target(policy, 599 MHz, CPUFREQ_RELATION_L);

CPUFREQ_RELATION_L means: lowest frequency at or above target. And so
I would expect the frequency to get set to 600MHz (if we look at clock
driver) or 700MHz (if we look at OPP table). I think we should decide
this thing from the OPP table only as that's what the platform guys
want us to use. So, we should end up with 700 MHz.

Then we land into dev_pm_opp_set_rate(), which does this (which is
code copied from earlier version of cpufreq-dt driver):

- clk_round_rate(clk, 599 MHz).

  clk_round_rate() returns the highest frequency lower than target. So
  it must return 500 MHz (I haven't tested this yet, all theoretical).

- _find_freq_ceil(opp_table, 500 MHz).

  This works like CPUFREQ_RELATION_L, so we find lowest frequency >=
  target freq. And so we should get: 500 MHz itself as OPP table has
  it.

- clk_set_rate(clk, 500 MHz).

  This must be doing round-rate again, but I think we will settle with
  500 MHz eventually.


Now the questionnaire:

- Is this whole exercise correct ?
- We shouldn't have landed on 500 MHz, right ?
- Is there anything wrong with the above theory (I am going to test it soon though).
- Why do we need to do the first clock_round_rate() ? Should we remove
  it ?



Now lets move to this patch, which makes it more confusing.

The OPP tables for CPUs and GPUs should already be somewhat like fmax
tables for particular voltage values and that's why both cpufreq and
OPP core try to find a frequency higher than target so we choose the
most optimum one power-efficiency wise.

For cases where the OPP table is only a subset of the clk-providers
table (almost always), if we let the clock provider to find the
nearest frequency (which is lower) we will run the CPU/GPU at a
not-so-optimal frequency. i.e. if 500, 600, 700 MHz all need voltage
to be 1.2 V, we should be running at 700 always, while we may end up
running at 500 MHz.

This kind of behavior (introduced by this patch) is important for
other devices which want to run at the nearest frequency to target
one, but not for CPUs/GPUs. So, we need to tag these IO devices
separately, maybe from DT ? So we select the closest match instead of
most optimal one.

But lets fix the existing issues first and then think about this
patch.

-- 
viresh

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-13  9:54         ` Viresh Kumar
@ 2019-06-14  5:27           ` Viresh Kumar
  2019-06-17  3:50             ` Viresh Kumar
  2019-06-14  5:54           ` Rajendra Nayak
  1 sibling, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-06-14  5:27 UTC (permalink / raw)
  To: swboyd, Rajendra Nayak, vincent.guittot, mturquette
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael

On 13-06-19, 15:24, Viresh Kumar wrote:
> I am confused as hell on what we should be doing and what we are doing
> right now. And if we should do better.
> 
> Let me explain with an example.
> 
> - The clock provider supports following frequencies: 500, 600, 700,
>   800, 900, 1000 MHz.
> 
> - The OPP table contains/supports only a subset: 500, 700, 1000 MHz.
> 
> Now, the request to change the frequency starts from cpufreq
> governors, like schedutil when they calls:
> 
> __cpufreq_driver_target(policy, 599 MHz, CPUFREQ_RELATION_L);
> 
> CPUFREQ_RELATION_L means: lowest frequency at or above target. And so
> I would expect the frequency to get set to 600MHz (if we look at clock
> driver) or 700MHz (if we look at OPP table). I think we should decide
> this thing from the OPP table only as that's what the platform guys
> want us to use. So, we should end up with 700 MHz.
> 
> Then we land into dev_pm_opp_set_rate(), which does this (which is
> code copied from earlier version of cpufreq-dt driver):
> 
> - clk_round_rate(clk, 599 MHz).
> 
>   clk_round_rate() returns the highest frequency lower than target. So
>   it must return 500 MHz (I haven't tested this yet, all theoretical).
> 
> - _find_freq_ceil(opp_table, 500 MHz).
> 
>   This works like CPUFREQ_RELATION_L, so we find lowest frequency >=
>   target freq. And so we should get: 500 MHz itself as OPP table has
>   it.
> 
> - clk_set_rate(clk, 500 MHz).
> 
>   This must be doing round-rate again, but I think we will settle with
>   500 MHz eventually.
> 
> 
> Now the questionnaire:
> 
> - Is this whole exercise correct ?

No, I missed the call to cpufreq_frequency_table_target() in
__cpufreq_driver_target() which finds the exact frequency from cpufreq table
(which was created using opp table) and so we never screw up here. Sorry for
confusing everyone on this :(

> Now lets move to this patch, which makes it more confusing.
> 
> The OPP tables for CPUs and GPUs should already be somewhat like fmax
> tables for particular voltage values and that's why both cpufreq and
> OPP core try to find a frequency higher than target so we choose the
> most optimum one power-efficiency wise.
> 
> For cases where the OPP table is only a subset of the clk-providers
> table (almost always), if we let the clock provider to find the
> nearest frequency (which is lower) we will run the CPU/GPU at a
> not-so-optimal frequency. i.e. if 500, 600, 700 MHz all need voltage
> to be 1.2 V, we should be running at 700 always, while we may end up
> running at 500 MHz.

This won't happen for CPUs because of the reason I explained earlier. cpufreq
core does the rounding before calling dev_pm_opp_set_rate(). And no other
devices use dev_pm_opp_set_rate() right now apart from CPUs, so we are not going
to break anything.

> This kind of behavior (introduced by this patch) is important for
> other devices which want to run at the nearest frequency to target
> one, but not for CPUs/GPUs. So, we need to tag these IO devices
> separately, maybe from DT ? So we select the closest match instead of
> most optimal one.

Hmm, so this patch won't break anything and I am inclined to apply it again :)

Does anyone see any other issues with it, which I might be missing ?

-- 
viresh

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-13  9:54         ` Viresh Kumar
  2019-06-14  5:27           ` Viresh Kumar
@ 2019-06-14  5:54           ` Rajendra Nayak
  1 sibling, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-06-14  5:54 UTC (permalink / raw)
  To: Viresh Kumar, swboyd, vincent.guittot, mturquette
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael


> Now, the request to change the frequency starts from cpufreq
> governors, like schedutil when they calls:
> 
> __cpufreq_driver_target(policy, 599 MHz, CPUFREQ_RELATION_L);
> 
> CPUFREQ_RELATION_L means: lowest frequency at or above target. And so
> I would expect the frequency to get set to 600MHz (if we look at clock
> driver) or 700MHz (if we look at OPP table). I think we should decide
> this thing from the OPP table only as that's what the platform guys
> want us to use. So, we should end up with 700 MHz.
> 
> Then we land into dev_pm_opp_set_rate(), which does this (which is
> code copied from earlier version of cpufreq-dt driver):

so before we land into dev_pm_opp_set_rate() from a __cpufreq_driver_target()
I guess we do have a cpufreq driver callback that gets called in between?
which is either .target_index or .target

In case of .target_index, the cpufreq core looks for a OPP index
and we would land up with 700Mhz i guess, so we are good.

In case of .target though the 'relation' CPUFREQ_RELATION_L does get passed over
to the cpufreq driver which I am guessing is expected to handle it in some way to
make sure the target frequency set is not less than whats requested? instead of
simply passing the requested frequency over to dev_pm_opp_set_rate()?

Looking at all the existing cpufreq drivers upstream, while most support .target_index
the 3 which do support .target seem to completely ignore this 'relation' input that's
passed to them.

drivers/cpufreq/cppc_cpufreq.c:	.target = cppc_cpufreq_set_target,
drivers/cpufreq/cpufreq-nforce2.c:	.target = nforce2_target,
drivers/cpufreq/pcc-cpufreq.c:	.target = pcc_cpufreq_target,

> This kind of behavior (introduced by this patch) is important for
> other devices which want to run at the nearest frequency to target
> one, but not for CPUs/GPUs. So, we need to tag these IO devices
> separately, maybe from DT ? So we select the closest match instead of
> most optimal one.

yes we do need some way to distinguish between CPU/GPU devices and other
IO devices. CPU/GPU can always run at fmax for a given voltage, that's not true
for IO devices and I don't see how we can satisfy both cases without
clearly knowing if we are serving a processor or an IO device, unless the
higher layers (cpufreq/devfreq) are able to handle this somehow without
expecting the OPP layer to handle the differences.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
  2019-03-20  9:49 ` [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid Rajendra Nayak
@ 2019-06-14  6:32   ` Viresh Kumar
  2019-06-17  4:04     ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-06-14  6:32 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> For devices with performance state, we use dev_pm_opp_set_rate()
> to set the appropriate clk rate and the performance state.
> We do need a way to *remove* the performance state vote when
> we idle the device and turn the clocks off. Use dev_pm_opp_set_rate()
> with freq=0 to achieve this.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/opp/core.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

What about this instead ?

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 2fe96c2363a3..9accf8bb6afc 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -711,7 +711,7 @@ static int _set_required_opps(struct device *dev,
 
        /* Single genpd case */
        if (!genpd_virt_devs) {
-               pstate = opp->required_opps[0]->pstate;
+               pstate = likely(opp) ? opp->required_opps[0]->pstate : 0;
                ret = dev_pm_genpd_set_performance_state(dev, pstate);
                if (ret) {
                        dev_err(dev, "Failed to set performance state of %s: %d (%d)\n",
@@ -729,7 +729,7 @@ static int _set_required_opps(struct device *dev,
        mutex_lock(&opp_table->genpd_virt_dev_lock);
 
        for (i = 0; i < opp_table->required_opp_count; i++) {
-               pstate = opp->required_opps[i]->pstate;
+               pstate = likely(opp) ? opp->required_opps[i]->pstate : 0;
 
                if (!genpd_virt_devs[i])
                        continue;
@@ -770,14 +770,13 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
        if (unlikely(!target_freq)) {
                if (opp_table->required_opp_tables) {
-                       /* drop the performance state vote */
-                       dev_pm_genpd_set_performance_state(dev, 0);
-                       return 0;
+                       ret = _set_required_opps(dev, opp_table, NULL);
                } else {
-                       dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
-                               target_freq);
-                       return -EINVAL;
+                       dev_err(dev, "target frequency can't be 0\n");
+                       ret = -EINVAL;
                }
+
+               goto put_opp_table;
        }
 
        clk = opp_table->clk;

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-14  5:27           ` Viresh Kumar
@ 2019-06-17  3:50             ` Viresh Kumar
  2019-06-17  4:07               ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-06-17  3:50 UTC (permalink / raw)
  To: swboyd, Rajendra Nayak, vincent.guittot, mturquette
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael

On 14-06-19, 10:57, Viresh Kumar wrote:
> Hmm, so this patch won't break anything and I am inclined to apply it again :)
> 
> Does anyone see any other issues with it, which I might be missing ?

I have updated the commit log a bit more to clarify on things, please let me
know if it looks okay.

    opp: Don't overwrite rounded clk rate
    
    The OPP table normally contains 'fmax' values corresponding to the
    voltage or performance levels of each OPP, but we don't necessarily want
    all the devices to run at fmax all the time. Running at fmax makes sense
    for devices like CPU/GPU, which have a finite amount of work to do and
    since a specific amount of energy is consumed at an OPP, its better to
    run at the highest possible frequency for that voltage value.
    
    On the other hand, we have IO devices which need to run at specific
    frequencies only for their proper functioning, instead of maximum
    possible frequency.
    
    The OPP core currently roundup to the next possible OPP for a frequency
    and select the fmax value. To support the IO devices by the OPP core,
    lets do the roundup to fetch the voltage or performance state values,
    but not use the OPP frequency value. Rather use the value returned by
    clk_round_rate().
    
    The current user, cpufreq, of dev_pm_opp_set_rate() already does the
    rounding to the next OPP before calling this routine and it won't
    have any side affects because of this change.
    
    Signed-off-by: Stephen Boyd <swboyd@chromium.org>
    Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
    [ Viresh: Massaged changelog and use temp_opp variable instead ]
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>


-- 
viresh

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

* Re: [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
  2019-06-14  6:32   ` Viresh Kumar
@ 2019-06-17  4:04     ` Rajendra Nayak
  0 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-06-17  4:04 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael



On 6/14/2019 12:02 PM, Viresh Kumar wrote:
> On 20-03-19, 15:19, Rajendra Nayak wrote:
>> For devices with performance state, we use dev_pm_opp_set_rate()
>> to set the appropriate clk rate and the performance state.
>> We do need a way to *remove* the performance state vote when
>> we idle the device and turn the clocks off. Use dev_pm_opp_set_rate()
>> with freq=0 to achieve this.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>>   drivers/opp/core.c | 18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> What about this instead ?

yes, this works, thanks for updating the patch.

> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 2fe96c2363a3..9accf8bb6afc 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -711,7 +711,7 @@ static int _set_required_opps(struct device *dev,
>   
>          /* Single genpd case */
>          if (!genpd_virt_devs) {
> -               pstate = opp->required_opps[0]->pstate;
> +               pstate = likely(opp) ? opp->required_opps[0]->pstate : 0;
>                  ret = dev_pm_genpd_set_performance_state(dev, pstate);
>                  if (ret) {
>                          dev_err(dev, "Failed to set performance state of %s: %d (%d)\n",
> @@ -729,7 +729,7 @@ static int _set_required_opps(struct device *dev,
>          mutex_lock(&opp_table->genpd_virt_dev_lock);
>   
>          for (i = 0; i < opp_table->required_opp_count; i++) {
> -               pstate = opp->required_opps[i]->pstate;
> +               pstate = likely(opp) ? opp->required_opps[i]->pstate : 0;
>   
>                  if (!genpd_virt_devs[i])
>                          continue;
> @@ -770,14 +770,13 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>   
>          if (unlikely(!target_freq)) {
>                  if (opp_table->required_opp_tables) {
> -                       /* drop the performance state vote */
> -                       dev_pm_genpd_set_performance_state(dev, 0);
> -                       return 0;
> +                       ret = _set_required_opps(dev, opp_table, NULL);
>                  } else {
> -                       dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
> -                               target_freq);
> -                       return -EINVAL;
> +                       dev_err(dev, "target frequency can't be 0\n");
> +                       ret = -EINVAL;
>                  }
> +
> +               goto put_opp_table;
>          }
>   
>          clk = opp_table->clk;
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-17  3:50             ` Viresh Kumar
@ 2019-06-17  4:07               ` Rajendra Nayak
  2019-06-17  4:17                 ` Viresh Kumar
  0 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2019-06-17  4:07 UTC (permalink / raw)
  To: Viresh Kumar, swboyd, vincent.guittot, mturquette
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, ulf.hansson, dianders, rafael



On 6/17/2019 9:20 AM, Viresh Kumar wrote:
> On 14-06-19, 10:57, Viresh Kumar wrote:
>> Hmm, so this patch won't break anything and I am inclined to apply it again :)
>>
>> Does anyone see any other issues with it, which I might be missing ?
> 
> I have updated the commit log a bit more to clarify on things, please let me
> know if it looks okay.
> 
>      opp: Don't overwrite rounded clk rate
>      
>      The OPP table normally contains 'fmax' values corresponding to the
>      voltage or performance levels of each OPP, but we don't necessarily want
>      all the devices to run at fmax all the time. Running at fmax makes sense
>      for devices like CPU/GPU, which have a finite amount of work to do and
>      since a specific amount of energy is consumed at an OPP, its better to
>      run at the highest possible frequency for that voltage value.
>      
>      On the other hand, we have IO devices which need to run at specific
>      frequencies only for their proper functioning, instead of maximum
>      possible frequency.
>      
>      The OPP core currently roundup to the next possible OPP for a frequency
>      and select the fmax value. To support the IO devices by the OPP core,
>      lets do the roundup to fetch the voltage or performance state values,
>      but not use the OPP frequency value. Rather use the value returned by
>      clk_round_rate().
>      
>      The current user, cpufreq, of dev_pm_opp_set_rate() already does the
>      rounding to the next OPP before calling this routine and it won't
>      have any side affects because of this change.

Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate()
would not be able to distinguish between its users trying to scale CPU/GPU's vs IO
devices, so its the callers responsibility to round it accordingly before calling the
API?

>      
>      Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>      Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>      [ Viresh: Massaged changelog and use temp_opp variable instead ]
>      Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-17  4:07               ` Rajendra Nayak
@ 2019-06-17  4:17                 ` Viresh Kumar
  2019-06-17  4:25                   ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: Viresh Kumar @ 2019-06-17  4:17 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: swboyd, vincent.guittot, mturquette, linux-kernel, linux-arm-msm,
	linux-pm, linux-serial, linux-spi, dri-devel, linux-scsi,
	ulf.hansson, dianders, rafael

On 17-06-19, 09:37, Rajendra Nayak wrote:
> 
> 
> On 6/17/2019 9:20 AM, Viresh Kumar wrote:
> > On 14-06-19, 10:57, Viresh Kumar wrote:
> > > Hmm, so this patch won't break anything and I am inclined to apply it again :)
> > > 
> > > Does anyone see any other issues with it, which I might be missing ?
> > 
> > I have updated the commit log a bit more to clarify on things, please let me
> > know if it looks okay.
> > 
> >      opp: Don't overwrite rounded clk rate
> >      The OPP table normally contains 'fmax' values corresponding to the
> >      voltage or performance levels of each OPP, but we don't necessarily want
> >      all the devices to run at fmax all the time. Running at fmax makes sense
> >      for devices like CPU/GPU, which have a finite amount of work to do and
> >      since a specific amount of energy is consumed at an OPP, its better to
> >      run at the highest possible frequency for that voltage value.
> >      On the other hand, we have IO devices which need to run at specific
> >      frequencies only for their proper functioning, instead of maximum
> >      possible frequency.
> >      The OPP core currently roundup to the next possible OPP for a frequency
> >      and select the fmax value. To support the IO devices by the OPP core,
> >      lets do the roundup to fetch the voltage or performance state values,
> >      but not use the OPP frequency value. Rather use the value returned by
> >      clk_round_rate().
> >      The current user, cpufreq, of dev_pm_opp_set_rate() already does the
> >      rounding to the next OPP before calling this routine and it won't
> >      have any side affects because of this change.
> 
> Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate()
> would not be able to distinguish between its users trying to scale CPU/GPU's vs IO
> devices, so its the callers responsibility to round it accordingly before calling the
> API?

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0fbc77f05048..bae94bfa1e96 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -751,8 +751,11 @@ static int _set_required_opps(struct device *dev,
  * @dev:        device for which we do this operation
  * @target_freq: frequency to achieve
  *
- * This configures the power-supplies and clock source to the levels specified
- * by the OPP corresponding to the target_freq.
+ * This configures the power-supplies to the levels specified by the OPP
+ * corresponding to the target_freq, and programs the clock to a value <=
+ * target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax
+ * provided by the opp, should have already rounded to the target OPP's
+ * frequency.
  */

-- 
viresh

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

* Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
  2019-06-17  4:17                 ` Viresh Kumar
@ 2019-06-17  4:25                   ` Rajendra Nayak
  0 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2019-06-17  4:25 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: swboyd, vincent.guittot, mturquette, linux-kernel, linux-arm-msm,
	linux-pm, linux-serial, linux-spi, dri-devel, linux-scsi,
	ulf.hansson, dianders, rafael



On 6/17/2019 9:47 AM, Viresh Kumar wrote:
> On 17-06-19, 09:37, Rajendra Nayak wrote:
>>
>>
>> On 6/17/2019 9:20 AM, Viresh Kumar wrote:
>>> On 14-06-19, 10:57, Viresh Kumar wrote:
>>>> Hmm, so this patch won't break anything and I am inclined to apply it again :)
>>>>
>>>> Does anyone see any other issues with it, which I might be missing ?
>>>
>>> I have updated the commit log a bit more to clarify on things, please let me
>>> know if it looks okay.
>>>
>>>       opp: Don't overwrite rounded clk rate
>>>       The OPP table normally contains 'fmax' values corresponding to the
>>>       voltage or performance levels of each OPP, but we don't necessarily want
>>>       all the devices to run at fmax all the time. Running at fmax makes sense
>>>       for devices like CPU/GPU, which have a finite amount of work to do and
>>>       since a specific amount of energy is consumed at an OPP, its better to
>>>       run at the highest possible frequency for that voltage value.
>>>       On the other hand, we have IO devices which need to run at specific
>>>       frequencies only for their proper functioning, instead of maximum
>>>       possible frequency.
>>>       The OPP core currently roundup to the next possible OPP for a frequency
>>>       and select the fmax value. To support the IO devices by the OPP core,
>>>       lets do the roundup to fetch the voltage or performance state values,
>>>       but not use the OPP frequency value. Rather use the value returned by
>>>       clk_round_rate().
>>>       The current user, cpufreq, of dev_pm_opp_set_rate() already does the
>>>       rounding to the next OPP before calling this routine and it won't
>>>       have any side affects because of this change.
>>
>> Looks good to me. Should this also be documented someplace that dev_pm_opp_set_rate()
>> would not be able to distinguish between its users trying to scale CPU/GPU's vs IO
>> devices, so its the callers responsibility to round it accordingly before calling the
>> API?
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 0fbc77f05048..bae94bfa1e96 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -751,8 +751,11 @@ static int _set_required_opps(struct device *dev,
>    * @dev:        device for which we do this operation
>    * @target_freq: frequency to achieve
>    *
> - * This configures the power-supplies and clock source to the levels specified
> - * by the OPP corresponding to the target_freq.
> + * This configures the power-supplies to the levels specified by the OPP
> + * corresponding to the target_freq, and programs the clock to a value <=
> + * target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax
> + * provided by the opp, should have already rounded to the target OPP's
> + * frequency.
>    */

Perfect, thanks.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 00/11] DVFS in the OPP core
  2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
                   ` (12 preceding siblings ...)
  2019-05-21  6:22 ` Viresh Kumar
@ 2019-06-17  4:26 ` Viresh Kumar
  13 siblings, 0 replies; 38+ messages in thread
From: Viresh Kumar @ 2019-06-17  4:26 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: linux-kernel, linux-arm-msm, linux-pm, linux-serial, linux-spi,
	dri-devel, linux-scsi, swboyd, ulf.hansson, dianders, rafael

On 20-03-19, 15:19, Rajendra Nayak wrote:
> This is a v2 of the RFC posted earlier by Stephen Boyd [1]
> 
> As part of v2 I still follow the same approach of dev_pm_opp_set_rate()
> API using clk framework to round the frequency passed and making it
> accept 0 as a valid frequency indicating the frequency isn't required
> anymore. It just has a few more drivers converted to use this approach
> like dsi/dpu and ufs.
> ufs demonstrates the case of having to handle multiple power domains, one
> of which is scalable.
> 
> The patches are based on 5.1-rc1 and depend on some ufs fixes I posted
> earlier [2] and a DT patch to include the rpmpd header [3]
> 
> [1] https://lkml.org/lkml/2019/1/28/2086
> [2] https://lkml.org/lkml/2019/3/8/70
> [3] https://lkml.org/lkml/2019/3/20/120
> 
> Rajendra Nayak (10):
>   OPP: Make dev_pm_opp_set_rate() with freq=0 as valid
> 
> Stephen Boyd (1):
>   OPP: Don't overwrite rounded clk rate

I have applied modified version of these two patches to the OPP tree now.
Thanks.

-- 
viresh

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

* Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2019-03-20  9:49 ` [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state Rajendra Nayak
@ 2020-08-11 23:11   ` John Stultz
  2020-08-12  1:33     ` John Stultz
  0 siblings, 1 reply; 38+ messages in thread
From: John Stultz @ 2020-08-11 23:11 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: lkml, Ulf Hansson, linux-scsi, Linux PM list, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, dri-devel, linux-spi,
	linux-serial, Viresh Kumar, Stephen Boyd, Amit Pundir,
	Bjorn Andersson

On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
> geni serial needs to express a perforamnce state requirement on CX
> depending on the frequency of the clock rates. Use OPP table from
> DT to register with OPP framework and use dev_pm_opp_set_rate() to
> set the clk/perf state.
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>

Hey,
  I just wanted to follow up on this patch, as I've bisected it
(a5819b548af0) down as having broken qca bluetooth on the Dragonboard
845c.

I haven't yet had time to debug it yet, but wanted to raise the issue
in case anyone else has seen similar trouble.

thanks
-john

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

* Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2020-08-11 23:11   ` John Stultz
@ 2020-08-12  1:33     ` John Stultz
  2020-08-12  5:48       ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: John Stultz @ 2020-08-12  1:33 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: lkml, Ulf Hansson, linux-scsi, Linux PM list, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, dri-devel, linux-spi,
	linux-serial, Viresh Kumar, Stephen Boyd, Amit Pundir,
	Bjorn Andersson

On Tue, Aug 11, 2020 at 4:11 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
> >
> > geni serial needs to express a perforamnce state requirement on CX
> > depending on the frequency of the clock rates. Use OPP table from
> > DT to register with OPP framework and use dev_pm_opp_set_rate() to
> > set the clk/perf state.
> >
> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
>
> Hey,
>   I just wanted to follow up on this patch, as I've bisected it
> (a5819b548af0) down as having broken qca bluetooth on the Dragonboard
> 845c.
>
> I haven't yet had time to debug it yet, but wanted to raise the issue
> in case anyone else has seen similar trouble.

So I dug in a bit further, and this chunk seems to be causing the issue:
> @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
>                 goto out_restart_rx;
>
>         uport->uartclk = clk_rate;
> -       clk_set_rate(port->se.clk, clk_rate);
> +       dev_pm_opp_set_rate(port->dev, clk_rate);
>         ser_clk_cfg = SER_CLK_EN;
>         ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
>


With that applied, I see the following errors in dmesg and bluetooth
fails to function:
[    4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
failed to find OPP for freq 102400000 (-34)
[    4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
failed to find OPP for freq 102400000 (-34)

With just that chunk reverted on linus/HEAD, bluetooth seems to work ok.

thanks
-john

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

* Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2020-08-12  1:33     ` John Stultz
@ 2020-08-12  5:48       ` Rajendra Nayak
  2020-08-12  7:35         ` Amit Pundir
  0 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2020-08-12  5:48 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Ulf Hansson, linux-scsi, Linux PM list, linux-arm-msm,
	Rafael J. Wysocki, Doug Anderson, dri-devel, linux-spi,
	linux-serial, Viresh Kumar, Stephen Boyd, Amit Pundir,
	Bjorn Andersson


On 8/12/2020 7:03 AM, John Stultz wrote:
> On Tue, Aug 11, 2020 at 4:11 PM John Stultz <john.stultz@linaro.org> wrote:
>>
>> On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>>
>>> geni serial needs to express a perforamnce state requirement on CX
>>> depending on the frequency of the clock rates. Use OPP table from
>>> DT to register with OPP framework and use dev_pm_opp_set_rate() to
>>> set the clk/perf state.
>>>
>>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>> ---
>>>   drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
>>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>
>> Hey,
>>    I just wanted to follow up on this patch, as I've bisected it
>> (a5819b548af0) down as having broken qca bluetooth on the Dragonboard
>> 845c.
>>
>> I haven't yet had time to debug it yet, but wanted to raise the issue
>> in case anyone else has seen similar trouble.
> 
> So I dug in a bit further, and this chunk seems to be causing the issue:
>> @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
>>                  goto out_restart_rx;
>>
>>          uport->uartclk = clk_rate;
>> -       clk_set_rate(port->se.clk, clk_rate);
>> +       dev_pm_opp_set_rate(port->dev, clk_rate);
>>          ser_clk_cfg = SER_CLK_EN;
>>          ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
>>
> 
> 
> With that applied, I see the following errors in dmesg and bluetooth
> fails to function:
> [    4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
> failed to find OPP for freq 102400000 (-34)
> [    4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
> failed to find OPP for freq 102400000 (-34)
> 
> With just that chunk reverted on linus/HEAD, bluetooth seems to work ok.

This seems like the same issue that was also reported on venus [1] because the
clock frequency tables apparently don;t exactly match the achievable clock
frequencies (which we also used to construct the OPP tables)

Can you try updating the OPP table for QUP to have 102400000 instead of the
current 100000000 and see if that fixes it?

[1] https://lkml.org/lkml/2020/7/27/507

> 
> thanks
> -john
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2020-08-12  5:48       ` Rajendra Nayak
@ 2020-08-12  7:35         ` Amit Pundir
  2020-08-12  7:39           ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: Amit Pundir @ 2020-08-12  7:35 UTC (permalink / raw)
  To: Rajendra Nayak
  Cc: John Stultz, lkml, Ulf Hansson, linux-scsi, Linux PM list,
	linux-arm-msm, Rafael J. Wysocki, Doug Anderson, dri-devel,
	linux-spi, linux-serial, Viresh Kumar, Stephen Boyd,
	Bjorn Andersson

Hi Rajendra,

On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 8/12/2020 7:03 AM, John Stultz wrote:
> > On Tue, Aug 11, 2020 at 4:11 PM John Stultz <john.stultz@linaro.org> wrote:
> >>
> >> On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
> >>>
> >>> geni serial needs to express a perforamnce state requirement on CX
> >>> depending on the frequency of the clock rates. Use OPP table from
> >>> DT to register with OPP framework and use dev_pm_opp_set_rate() to
> >>> set the clk/perf state.
> >>>
> >>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> >>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >>> ---
> >>>   drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
> >>>   1 file changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>
> >> Hey,
> >>    I just wanted to follow up on this patch, as I've bisected it
> >> (a5819b548af0) down as having broken qca bluetooth on the Dragonboard
> >> 845c.
> >>
> >> I haven't yet had time to debug it yet, but wanted to raise the issue
> >> in case anyone else has seen similar trouble.
> >
> > So I dug in a bit further, and this chunk seems to be causing the issue:
> >> @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
> >>                  goto out_restart_rx;
> >>
> >>          uport->uartclk = clk_rate;
> >> -       clk_set_rate(port->se.clk, clk_rate);
> >> +       dev_pm_opp_set_rate(port->dev, clk_rate);
> >>          ser_clk_cfg = SER_CLK_EN;
> >>          ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
> >>
> >
> >
> > With that applied, I see the following errors in dmesg and bluetooth
> > fails to function:
> > [    4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
> > failed to find OPP for freq 102400000 (-34)
> > [    4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
> > failed to find OPP for freq 102400000 (-34)
> >
> > With just that chunk reverted on linus/HEAD, bluetooth seems to work ok.
>
> This seems like the same issue that was also reported on venus [1] because the
> clock frequency tables apparently don;t exactly match the achievable clock
> frequencies (which we also used to construct the OPP tables)
>
> Can you try updating the OPP table for QUP to have 102400000 instead of the
> current 100000000 and see if that fixes it?

That worked. Thanks.

Should this change be common to base sdm845.dtsi or platform specific dts?
For what it's worth, we see this BT breakage on PocoF1 phone too.

Regards,
Amit Pundir


>
> [1] https://lkml.org/lkml/2020/7/27/507
>
> >
> > thanks
> > -john
> >
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2020-08-12  7:35         ` Amit Pundir
@ 2020-08-12  7:39           ` Rajendra Nayak
  2020-08-12  9:26             ` Rajendra Nayak
  0 siblings, 1 reply; 38+ messages in thread
From: Rajendra Nayak @ 2020-08-12  7:39 UTC (permalink / raw)
  To: Amit Pundir
  Cc: John Stultz, lkml, Ulf Hansson, linux-scsi, Linux PM list,
	linux-arm-msm, Rafael J. Wysocki, Doug Anderson, dri-devel,
	linux-spi, linux-serial, Viresh Kumar, Stephen Boyd,
	Bjorn Andersson


On 8/12/2020 1:05 PM, Amit Pundir wrote:
> Hi Rajendra,
> 
> On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>
>>
>> On 8/12/2020 7:03 AM, John Stultz wrote:
>>> On Tue, Aug 11, 2020 at 4:11 PM John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>> On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>>>>
>>>>> geni serial needs to express a perforamnce state requirement on CX
>>>>> depending on the frequency of the clock rates. Use OPP table from
>>>>> DT to register with OPP framework and use dev_pm_opp_set_rate() to
>>>>> set the clk/perf state.
>>>>>
>>>>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>>>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>>>> ---
>>>>>    drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
>>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>>> Hey,
>>>>     I just wanted to follow up on this patch, as I've bisected it
>>>> (a5819b548af0) down as having broken qca bluetooth on the Dragonboard
>>>> 845c.
>>>>
>>>> I haven't yet had time to debug it yet, but wanted to raise the issue
>>>> in case anyone else has seen similar trouble.
>>>
>>> So I dug in a bit further, and this chunk seems to be causing the issue:
>>>> @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
>>>>                   goto out_restart_rx;
>>>>
>>>>           uport->uartclk = clk_rate;
>>>> -       clk_set_rate(port->se.clk, clk_rate);
>>>> +       dev_pm_opp_set_rate(port->dev, clk_rate);
>>>>           ser_clk_cfg = SER_CLK_EN;
>>>>           ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
>>>>
>>>
>>>
>>> With that applied, I see the following errors in dmesg and bluetooth
>>> fails to function:
>>> [    4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
>>> failed to find OPP for freq 102400000 (-34)
>>> [    4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
>>> failed to find OPP for freq 102400000 (-34)
>>>
>>> With just that chunk reverted on linus/HEAD, bluetooth seems to work ok.
>>
>> This seems like the same issue that was also reported on venus [1] because the
>> clock frequency tables apparently don;t exactly match the achievable clock
>> frequencies (which we also used to construct the OPP tables)
>>
>> Can you try updating the OPP table for QUP to have 102400000 instead of the
>> current 100000000 and see if that fixes it?
> 
> That worked. Thanks.
> 
> Should this change be common to base sdm845.dtsi or platform specific dts?
> For what it's worth, we see this BT breakage on PocoF1 phone too.

Thanks for confirming, it will have to be part of the SoC dtsi, and I am
guessing a similar change is perhaps also needed on sc7180.
I will send a patch out to fix the OPP tables for both.

> 
> Regards,
> Amit Pundir
> 
> 
>>
>> [1] https://lkml.org/lkml/2020/7/27/507
>>
>>>
>>> thanks
>>> -john
>>>
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

* Re: [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state
  2020-08-12  7:39           ` Rajendra Nayak
@ 2020-08-12  9:26             ` Rajendra Nayak
  0 siblings, 0 replies; 38+ messages in thread
From: Rajendra Nayak @ 2020-08-12  9:26 UTC (permalink / raw)
  To: Amit Pundir
  Cc: John Stultz, lkml, Ulf Hansson, linux-scsi, Linux PM list,
	linux-arm-msm, Rafael J. Wysocki, Doug Anderson, dri-devel,
	linux-spi, linux-serial, Viresh Kumar, Stephen Boyd,
	Bjorn Andersson


On 8/12/2020 1:09 PM, Rajendra Nayak wrote:
> 
> On 8/12/2020 1:05 PM, Amit Pundir wrote:
>> Hi Rajendra,
>>
>> On Wed, 12 Aug 2020 at 11:18, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>>
>>>
>>> On 8/12/2020 7:03 AM, John Stultz wrote:
>>>> On Tue, Aug 11, 2020 at 4:11 PM John Stultz <john.stultz@linaro.org> wrote:
>>>>>
>>>>> On Wed, Mar 20, 2019 at 2:49 AM Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>>>>>
>>>>>> geni serial needs to express a perforamnce state requirement on CX
>>>>>> depending on the frequency of the clock rates. Use OPP table from
>>>>>> DT to register with OPP framework and use dev_pm_opp_set_rate() to
>>>>>> set the clk/perf state.
>>>>>>
>>>>>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>>>>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>>>>> ---
>>>>>>    drivers/tty/serial/qcom_geni_serial.c | 15 +++++++++++++--
>>>>>>    1 file changed, 13 insertions(+), 2 deletions(-)
>>>>>>
>>>>>
>>>>> Hey,
>>>>>     I just wanted to follow up on this patch, as I've bisected it
>>>>> (a5819b548af0) down as having broken qca bluetooth on the Dragonboard
>>>>> 845c.
>>>>>
>>>>> I haven't yet had time to debug it yet, but wanted to raise the issue
>>>>> in case anyone else has seen similar trouble.
>>>>
>>>> So I dug in a bit further, and this chunk seems to be causing the issue:
>>>>> @@ -961,7 +963,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
>>>>>                   goto out_restart_rx;
>>>>>
>>>>>           uport->uartclk = clk_rate;
>>>>> -       clk_set_rate(port->se.clk, clk_rate);
>>>>> +       dev_pm_opp_set_rate(port->dev, clk_rate);
>>>>>           ser_clk_cfg = SER_CLK_EN;
>>>>>           ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
>>>>>
>>>>
>>>>
>>>> With that applied, I see the following errors in dmesg and bluetooth
>>>> fails to function:
>>>> [    4.763467] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
>>>> failed to find OPP for freq 102400000 (-34)
>>>> [    4.773493] qcom_geni_serial 898000.serial: dev_pm_opp_set_rate:
>>>> failed to find OPP for freq 102400000 (-34)
>>>>
>>>> With just that chunk reverted on linus/HEAD, bluetooth seems to work ok.
>>>
>>> This seems like the same issue that was also reported on venus [1] because the
>>> clock frequency tables apparently don;t exactly match the achievable clock
>>> frequencies (which we also used to construct the OPP tables)
>>>
>>> Can you try updating the OPP table for QUP to have 102400000 instead of the
>>> current 100000000 and see if that fixes it?
>>
>> That worked. Thanks.
>>
>> Should this change be common to base sdm845.dtsi or platform specific dts?
>> For what it's worth, we see this BT breakage on PocoF1 phone too.
> 
> Thanks for confirming, it will have to be part of the SoC dtsi, and I am
> guessing a similar change is perhaps also needed on sc7180.
> I will send a patch out to fix the OPP tables for both.

I spent some more time looking at this and it does not look like this is the
rounding issues with clock FMAX tables. I had these tables picked from downstream
clock code and it turns out these tables were reworked at clock init based on
the silicon rev, so I need to fix up the OPP tables accordingly which will add
a new OPP entry for 102.4Mhz. I'll post a patch shortly.

> 
>>
>> Regards,
>> Amit Pundir
>>
>>
>>>
>>> [1] https://lkml.org/lkml/2020/7/27/507
>>>
>>>>
>>>> thanks
>>>> -john
>>>>
>>>
>>> -- 
>>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>>> of Code Aurora Forum, hosted by The Linux Foundation
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

end of thread, other threads:[~2020-08-12  9:26 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 01/11] OPP: Don't overwrite rounded clk rate Rajendra Nayak
2019-06-11 10:54   ` Viresh Kumar
2019-06-12  7:42     ` Rajendra Nayak
2019-06-12  8:25       ` Viresh Kumar
2019-06-13  9:54         ` Viresh Kumar
2019-06-14  5:27           ` Viresh Kumar
2019-06-17  3:50             ` Viresh Kumar
2019-06-17  4:07               ` Rajendra Nayak
2019-06-17  4:17                 ` Viresh Kumar
2019-06-17  4:25                   ` Rajendra Nayak
2019-06-14  5:54           ` Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid Rajendra Nayak
2019-06-14  6:32   ` Viresh Kumar
2019-06-17  4:04     ` Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state Rajendra Nayak
2020-08-11 23:11   ` John Stultz
2020-08-12  1:33     ` John Stultz
2020-08-12  5:48       ` Rajendra Nayak
2020-08-12  7:35         ` Amit Pundir
2020-08-12  7:39           ` Rajendra Nayak
2020-08-12  9:26             ` Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 04/11] spi: spi-geni-qcom: " Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 05/11] arm64: dts: sdm845: Add OPP table for all qup devices Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 06/11] scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 07/11] scsi: ufs: Add support for specifying OPP tables in DT Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains Rajendra Nayak
2019-05-14  7:53   ` Ulf Hansson
2019-03-20  9:49 ` [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state Rajendra Nayak
2019-04-10  3:49   ` Viresh Kumar
2019-04-10  3:49     ` Viresh Kumar
2019-03-20  9:49 ` [RFC v2 10/11] drm/msm: dsi: " Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 11/11] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains Rajendra Nayak
2019-04-10  3:51 ` [RFC v2 00/11] DVFS in the OPP core Viresh Kumar
2019-04-10  3:51   ` Viresh Kumar
2019-05-21  6:22 ` Viresh Kumar
2019-05-24  6:03   ` Rajendra Nayak
2019-06-17  4:26 ` Viresh Kumar

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