All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Add support for drm/rockchip to dynamically control the DDR frequency.
@ 2018-07-30  8:11 ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel, Catalin Marinas, Mark Yao,
	Jacob Chen, Brian Norris, Douglas Anderson, Jeffy Chen,
	Nickey Yang, devicetree, Klaus Goger, Randy Li, Chris Zhong,
	Shunqian Zheng, Mark Rutland

Dear all,

The rk3399 platform has a DFI controller that can monitor DDR load and a
DMC driver that talks with the TF-A (Trusted Firmware-A) to dynamically
set the DDR frequency with following flow.

             kernel                          Trusted Firmware-A
                                                  (bl31)
        monitor ddr load
                |
                |
        get_target_rate
                |
                |           pass rate to TF-A
        clk_set_rate(ddr) --------------------->run ddr dvs flow
                                                    |
                                                    |
                 <------------------------------end ddr dvs flow
                |
                |
              return

These patches add support for devfreq to dynamically control the DDR
frequency into the drm rockchip driver. By default it uses the
'simple_ondemand' governor which can adjust the frequency based on the
DDR load.

Waiting for your feedback.

Best regards,
 Enric

Changes in v1:
- [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
- [RFC 1/10] s/Generic/General/ (Robin Murphy)
- [RFC 2/10] Add reviewed and acked tags from Chanwoo Choi and Rob Herring
- [RFC 3/10] Add an explanation for platform SIP calls.
- [RFC 3/10] Change if statement for a switch.
- [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.
- [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
- [RFC 7/10] Removed from the series. I did not found a use case where this matters.
- [RFC 8/10] Move rk3399-dram.h to dt-includes.
- [RFC 8/10] Put sdram default values under the dmc node.
- [RFC 8/10] Removed rk3399-dram-default-timing.dts

Derek Basehore (2):
  devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel
    for DDRfreq.
  devfreq: rk3399_dmc / clk: rockchip: Disable DDR clk timeout on
    suspend.

Enric Balletbo i Serra (3):
  devfreq: rockchip-dfi: Move GRF definitions to a common place.
  dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
  devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.

Lin Huang (2):
  arm64: dts: rk3399: Add dfi and dmc nodes.
  arm64: dts: rockchip: Enable dmc and dfi nodes on gru.

Sean Paul (1):
  drm: rockchip: Add DDR devfreq support.

 .../bindings/devfreq/rk3399_dmc.txt           |   2 +
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  |  21 ++
 .../boot/dts/rockchip/rk3399-op1-opp.dtsi     |  29 +++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  51 ++++-
 drivers/clk/rockchip/clk-ddr.c                | 157 +++++++++++--
 drivers/clk/rockchip/clk.c                    |   2 +-
 drivers/clk/rockchip/clk.h                    |   3 +-
 drivers/devfreq/event/rockchip-dfi.c          |  23 +-
 drivers/devfreq/rk3399_dmc.c                  | 209 +++++++++++++++---
 drivers/devfreq/rk3399_dmc_priv.h             |  16 ++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  46 ++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |   9 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c    |  35 +++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  34 +++
 include/dt-bindings/power/rk3399-dram.h       |  73 ++++++
 include/soc/rockchip/rk3399_dmc.h             |  42 ++++
 include/soc/rockchip/rk3399_grf.h             |  21 ++
 include/soc/rockchip/rockchip_sip.h           |   1 +
 18 files changed, 704 insertions(+), 70 deletions(-)
 create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
 create mode 100644 include/dt-bindings/power/rk3399-dram.h
 create mode 100644 include/soc/rockchip/rk3399_dmc.h
 create mode 100644 include/soc/rockchip/rk3399_grf.h

-- 
2.18.0


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

* [PATCH 0/8] Add support for drm/rockchip to dynamically control the DDR frequency.
@ 2018-07-30  8:11 ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel, Catalin Marinas, Mark Yao,
	Jacob Chen, Brian Norris, Douglas Anderson, Jeffy Chen,
	Nickey Yang, devicetree, Klaus Goger, Randy Li, Chris Zhong

Dear all,

The rk3399 platform has a DFI controller that can monitor DDR load and a
DMC driver that talks with the TF-A (Trusted Firmware-A) to dynamically
set the DDR frequency with following flow.

             kernel                          Trusted Firmware-A
                                                  (bl31)
        monitor ddr load
                |
                |
        get_target_rate
                |
                |           pass rate to TF-A
        clk_set_rate(ddr) --------------------->run ddr dvs flow
                                                    |
                                                    |
                 <------------------------------end ddr dvs flow
                |
                |
              return

These patches add support for devfreq to dynamically control the DDR
frequency into the drm rockchip driver. By default it uses the
'simple_ondemand' governor which can adjust the frequency based on the
DDR load.

Waiting for your feedback.

Best regards,
 Enric

Changes in v1:
- [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
- [RFC 1/10] s/Generic/General/ (Robin Murphy)
- [RFC 2/10] Add reviewed and acked tags from Chanwoo Choi and Rob Herring
- [RFC 3/10] Add an explanation for platform SIP calls.
- [RFC 3/10] Change if statement for a switch.
- [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.
- [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
- [RFC 7/10] Removed from the series. I did not found a use case where this matters.
- [RFC 8/10] Move rk3399-dram.h to dt-includes.
- [RFC 8/10] Put sdram default values under the dmc node.
- [RFC 8/10] Removed rk3399-dram-default-timing.dts

Derek Basehore (2):
  devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel
    for DDRfreq.
  devfreq: rk3399_dmc / clk: rockchip: Disable DDR clk timeout on
    suspend.

Enric Balletbo i Serra (3):
  devfreq: rockchip-dfi: Move GRF definitions to a common place.
  dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
  devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.

Lin Huang (2):
  arm64: dts: rk3399: Add dfi and dmc nodes.
  arm64: dts: rockchip: Enable dmc and dfi nodes on gru.

Sean Paul (1):
  drm: rockchip: Add DDR devfreq support.

 .../bindings/devfreq/rk3399_dmc.txt           |   2 +
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  |  21 ++
 .../boot/dts/rockchip/rk3399-op1-opp.dtsi     |  29 +++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  51 ++++-
 drivers/clk/rockchip/clk-ddr.c                | 157 +++++++++++--
 drivers/clk/rockchip/clk.c                    |   2 +-
 drivers/clk/rockchip/clk.h                    |   3 +-
 drivers/devfreq/event/rockchip-dfi.c          |  23 +-
 drivers/devfreq/rk3399_dmc.c                  | 209 +++++++++++++++---
 drivers/devfreq/rk3399_dmc_priv.h             |  16 ++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  46 ++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |   9 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c    |  35 +++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  34 +++
 include/dt-bindings/power/rk3399-dram.h       |  73 ++++++
 include/soc/rockchip/rk3399_dmc.h             |  42 ++++
 include/soc/rockchip/rk3399_grf.h             |  21 ++
 include/soc/rockchip/rockchip_sip.h           |   1 +
 18 files changed, 704 insertions(+), 70 deletions(-)
 create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
 create mode 100644 include/dt-bindings/power/rk3399-dram.h
 create mode 100644 include/soc/rockchip/rk3399_dmc.h
 create mode 100644 include/soc/rockchip/rk3399_grf.h

-- 
2.18.0

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

* [PATCH 0/8] Add support for drm/rockchip to dynamically control the DDR frequency.
@ 2018-07-30  8:11 ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

Dear all,

The rk3399 platform has a DFI controller that can monitor DDR load and a
DMC driver that talks with the TF-A (Trusted Firmware-A) to dynamically
set the DDR frequency with following flow.

             kernel                          Trusted Firmware-A
                                                  (bl31)
        monitor ddr load
                |
                |
        get_target_rate
                |
                |           pass rate to TF-A
        clk_set_rate(ddr) --------------------->run ddr dvs flow
                                                    |
                                                    |
                 <------------------------------end ddr dvs flow
                |
                |
              return

These patches add support for devfreq to dynamically control the DDR
frequency into the drm rockchip driver. By default it uses the
'simple_ondemand' governor which can adjust the frequency based on the
DDR load.

Waiting for your feedback.

Best regards,
 Enric

Changes in v1:
- [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
- [RFC 1/10] s/Generic/General/ (Robin Murphy)
- [RFC 2/10] Add reviewed and acked tags from Chanwoo Choi and Rob Herring
- [RFC 3/10] Add an explanation for platform SIP calls.
- [RFC 3/10] Change if statement for a switch.
- [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.
- [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
- [RFC 7/10] Removed from the series. I did not found a use case where this matters.
- [RFC 8/10] Move rk3399-dram.h to dt-includes.
- [RFC 8/10] Put sdram default values under the dmc node.
- [RFC 8/10] Removed rk3399-dram-default-timing.dts

Derek Basehore (2):
  devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel
    for DDRfreq.
  devfreq: rk3399_dmc / clk: rockchip: Disable DDR clk timeout on
    suspend.

Enric Balletbo i Serra (3):
  devfreq: rockchip-dfi: Move GRF definitions to a common place.
  dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
  devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.

Lin Huang (2):
  arm64: dts: rk3399: Add dfi and dmc nodes.
  arm64: dts: rockchip: Enable dmc and dfi nodes on gru.

Sean Paul (1):
  drm: rockchip: Add DDR devfreq support.

 .../bindings/devfreq/rk3399_dmc.txt           |   2 +
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  |  21 ++
 .../boot/dts/rockchip/rk3399-op1-opp.dtsi     |  29 +++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  51 ++++-
 drivers/clk/rockchip/clk-ddr.c                | 157 +++++++++++--
 drivers/clk/rockchip/clk.c                    |   2 +-
 drivers/clk/rockchip/clk.h                    |   3 +-
 drivers/devfreq/event/rockchip-dfi.c          |  23 +-
 drivers/devfreq/rk3399_dmc.c                  | 209 +++++++++++++++---
 drivers/devfreq/rk3399_dmc_priv.h             |  16 ++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  46 ++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h   |   9 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c    |  35 +++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |  34 +++
 include/dt-bindings/power/rk3399-dram.h       |  73 ++++++
 include/soc/rockchip/rk3399_dmc.h             |  42 ++++
 include/soc/rockchip/rk3399_grf.h             |  21 ++
 include/soc/rockchip/rockchip_sip.h           |   1 +
 18 files changed, 704 insertions(+), 70 deletions(-)
 create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
 create mode 100644 include/dt-bindings/power/rk3399-dram.h
 create mode 100644 include/soc/rockchip/rk3399_dmc.h
 create mode 100644 include/soc/rockchip/rk3399_grf.h

-- 
2.18.0

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

* [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

Some rk3399 GRF (Generic Register Files) definitions can be used for
different drivers. Move these definitions to a common include so we
don't need to duplicate these definitions.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1:
- [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
- [RFC 1/10] s/Generic/General/ (Robin Murphy)
- [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
- [RFC 7/10] Removed from the series. I did not found a use case where this matters.

 drivers/devfreq/event/rockchip-dfi.c | 23 +++++++----------------
 include/soc/rockchip/rk3399_grf.h    | 21 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 16 deletions(-)
 create mode 100644 include/soc/rockchip/rk3399_grf.h

diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
index 22b113363ffc..2fbbcbeb644f 100644
--- a/drivers/devfreq/event/rockchip-dfi.c
+++ b/drivers/devfreq/event/rockchip-dfi.c
@@ -26,6 +26,8 @@
 #include <linux/list.h>
 #include <linux/of.h>
 
+#include <soc/rockchip/rk3399_grf.h>
+
 #define RK3399_DMC_NUM_CH	2
 
 /* DDRMON_CTRL */
@@ -43,18 +45,6 @@
 #define DDRMON_CH1_COUNT_NUM		0x3c
 #define DDRMON_CH1_DFI_ACCESS_NUM	0x40
 
-/* pmu grf */
-#define PMUGRF_OS_REG2	0x308
-#define DDRTYPE_SHIFT	13
-#define DDRTYPE_MASK	7
-
-enum {
-	DDR3 = 3,
-	LPDDR3 = 6,
-	LPDDR4 = 7,
-	UNUSED = 0xFF
-};
-
 struct dmc_usage {
 	u32 access;
 	u32 total;
@@ -83,16 +73,17 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev)
 	u32 ddr_type;
 
 	/* get ddr type */
-	regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
-	ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
+	regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
+	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
+		    RK3399_PMUGRF_DDRTYPE_MASK;
 
 	/* clear DDRMON_CTRL setting */
 	writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
 
 	/* set ddr type to dfi */
-	if (ddr_type == LPDDR3)
+	if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3)
 		writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
-	else if (ddr_type == LPDDR4)
+	else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4)
 		writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
 
 	/* enable count, use software mode */
diff --git a/include/soc/rockchip/rk3399_grf.h b/include/soc/rockchip/rk3399_grf.h
new file mode 100644
index 000000000000..3eebabcb2812
--- /dev/null
+++ b/include/soc/rockchip/rk3399_grf.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Rockchip General Register Files definitions
+ *
+ * Copyright (c) 2018, Collabora Ltd.
+ * Author: Enric Balletbo i Serra <enric.balletbo@collabora.com>
+ */
+
+#ifndef __SOC_RK3399_GRF_H
+#define __SOC_RK3399_GRF_H
+
+/* PMU GRF Registers */
+#define RK3399_PMUGRF_OS_REG2		0x308
+#define RK3399_PMUGRF_DDRTYPE_SHIFT	13
+#define RK3399_PMUGRF_DDRTYPE_MASK	7
+#define RK3399_PMUGRF_DDRTYPE_DDR3	3
+#define RK3399_PMUGRF_DDRTYPE_LPDDR2	5
+#define RK3399_PMUGRF_DDRTYPE_LPDDR3	6
+#define RK3399_PMUGRF_DDRTYPE_LPDDR4	7
+
+#endif
-- 
2.18.0


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

* [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

Some rk3399 GRF (Generic Register Files) definitions can be used for
different drivers. Move these definitions to a common include so we
don't need to duplicate these definitions.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1:
- [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
- [RFC 1/10] s/Generic/General/ (Robin Murphy)
- [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
- [RFC 7/10] Removed from the series. I did not found a use case where this matters.

 drivers/devfreq/event/rockchip-dfi.c | 23 +++++++----------------
 include/soc/rockchip/rk3399_grf.h    | 21 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 16 deletions(-)
 create mode 100644 include/soc/rockchip/rk3399_grf.h

diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
index 22b113363ffc..2fbbcbeb644f 100644
--- a/drivers/devfreq/event/rockchip-dfi.c
+++ b/drivers/devfreq/event/rockchip-dfi.c
@@ -26,6 +26,8 @@
 #include <linux/list.h>
 #include <linux/of.h>
 
+#include <soc/rockchip/rk3399_grf.h>
+
 #define RK3399_DMC_NUM_CH	2
 
 /* DDRMON_CTRL */
@@ -43,18 +45,6 @@
 #define DDRMON_CH1_COUNT_NUM		0x3c
 #define DDRMON_CH1_DFI_ACCESS_NUM	0x40
 
-/* pmu grf */
-#define PMUGRF_OS_REG2	0x308
-#define DDRTYPE_SHIFT	13
-#define DDRTYPE_MASK	7
-
-enum {
-	DDR3 = 3,
-	LPDDR3 = 6,
-	LPDDR4 = 7,
-	UNUSED = 0xFF
-};
-
 struct dmc_usage {
 	u32 access;
 	u32 total;
@@ -83,16 +73,17 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev)
 	u32 ddr_type;
 
 	/* get ddr type */
-	regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
-	ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
+	regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
+	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
+		    RK3399_PMUGRF_DDRTYPE_MASK;
 
 	/* clear DDRMON_CTRL setting */
 	writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
 
 	/* set ddr type to dfi */
-	if (ddr_type == LPDDR3)
+	if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3)
 		writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
-	else if (ddr_type == LPDDR4)
+	else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4)
 		writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
 
 	/* enable count, use software mode */
diff --git a/include/soc/rockchip/rk3399_grf.h b/include/soc/rockchip/rk3399_grf.h
new file mode 100644
index 000000000000..3eebabcb2812
--- /dev/null
+++ b/include/soc/rockchip/rk3399_grf.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Rockchip General Register Files definitions
+ *
+ * Copyright (c) 2018, Collabora Ltd.
+ * Author: Enric Balletbo i Serra <enric.balletbo@collabora.com>
+ */
+
+#ifndef __SOC_RK3399_GRF_H
+#define __SOC_RK3399_GRF_H
+
+/* PMU GRF Registers */
+#define RK3399_PMUGRF_OS_REG2		0x308
+#define RK3399_PMUGRF_DDRTYPE_SHIFT	13
+#define RK3399_PMUGRF_DDRTYPE_MASK	7
+#define RK3399_PMUGRF_DDRTYPE_DDR3	3
+#define RK3399_PMUGRF_DDRTYPE_LPDDR2	5
+#define RK3399_PMUGRF_DDRTYPE_LPDDR3	6
+#define RK3399_PMUGRF_DDRTYPE_LPDDR4	7
+
+#endif
-- 
2.18.0

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

* [PATCH 2/8] dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel, devicetree, Mark Rutland

The Rockchip DMC (Dynamic Memory Interface) needs to access to the PMU
general register files to know the DRAM type, so add a phandle to the
syscon that manages these registers.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v1:
- [RFC 2/10] Add reviewed and acked tags from Chanwoo Choi and Rob Herring

 Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
index 0ec68141f85a..951789c0cdd6 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
@@ -12,6 +12,8 @@ Required properties:
 			 for details.
 - center-supply:	 DMC supply node.
 - status:		 Marks the node enabled/disabled.
+- rockchip,pmu:		 Phandle to the syscon managing the "PMU general register
+			 files".
 
 Optional properties:
 - interrupts:		 The CPU interrupt number. The interrupt specifier
-- 
2.18.0


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

* [PATCH 2/8] dt-bindings: devfreq: rk3399_dmc: Add rockchip, pmu phandle.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

The Rockchip DMC (Dynamic Memory Interface) needs to access to the PMU
general register files to know the DRAM type, so add a phandle to the
syscon that manages these registers.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v1:
- [RFC 2/10] Add reviewed and acked tags from Chanwoo Choi and Rob Herring

 Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
index 0ec68141f85a..951789c0cdd6 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
@@ -12,6 +12,8 @@ Required properties:
 			 for details.
 - center-supply:	 DMC supply node.
 - status:		 Marks the node enabled/disabled.
+- rockchip,pmu:		 Phandle to the syscon managing the "PMU general register
+			 files".
 
 Optional properties:
 - interrupts:		 The CPU interrupt number. The interrupt specifier
-- 
2.18.0

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

* [PATCH 3/8] devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

Trusted Firmware-A (TF-A) for rk3399 implements a SiP call to get the
on-die termination (ODT) and auto power down parameters from kernel,
this patch adds the functionality to do this. Also, if DDR clock
frequency is lower than the on-die termination (ODT) disable frequency
this driver should disable the DDR ODT.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1:
- [RFC 3/10] Add an explanation for platform SIP calls.
- [RFC 3/10] Change if statement for a switch.
- [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.

 drivers/devfreq/rk3399_dmc.c        | 74 ++++++++++++++++++++++++++++-
 include/soc/rockchip/rockchip_sip.h |  1 +
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index e795ad2b3f6b..c619dc4ac620 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -18,14 +18,17 @@
 #include <linux/devfreq.h>
 #include <linux/devfreq-event.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/rwsem.h>
 #include <linux/suspend.h>
 
+#include <soc/rockchip/rk3399_grf.h>
 #include <soc/rockchip/rockchip_sip.h>
 
 struct dram_timing {
@@ -69,8 +72,11 @@ struct rk3399_dmcfreq {
 	struct mutex lock;
 	struct dram_timing timing;
 	struct regulator *vdd_center;
+	struct regmap *regmap_pmu;
 	unsigned long rate, target_rate;
 	unsigned long volt, target_volt;
+	unsigned int odt_dis_freq;
+	int odt_pd_arg0, odt_pd_arg1;
 };
 
 static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
@@ -80,6 +86,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
 	struct dev_pm_opp *opp;
 	unsigned long old_clk_rate = dmcfreq->rate;
 	unsigned long target_volt, target_rate;
+	struct arm_smccc_res res;
+	bool odt_enable = false;
 	int err;
 
 	opp = devfreq_recommended_opp(dev, freq, flags);
@@ -95,6 +103,19 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
 
 	mutex_lock(&dmcfreq->lock);
 
+	if (target_rate >= dmcfreq->odt_dis_freq)
+		odt_enable = true;
+
+	/*
+	 * This makes a SMC call to the TF-A to set the DDR PD (power-down)
+	 * timings and to enable or disable the ODT (on-die termination)
+	 * resistors.
+	 */
+	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
+		      dmcfreq->odt_pd_arg1,
+		      ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
+		      odt_enable, 0, 0, 0, &res);
+
 	/*
 	 * If frequency scaling from low to high, adjust voltage first.
 	 * If frequency scaling from high to low, adjust frequency first.
@@ -294,11 +315,13 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 {
 	struct arm_smccc_res res;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node, *node;
 	struct rk3399_dmcfreq *data;
 	int ret, index, size;
 	uint32_t *timing;
 	struct dev_pm_opp *opp;
+	u32 ddr_type;
+	u32 val;
 
 	data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
 	if (!data)
@@ -334,6 +357,34 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* Try to find the optional reference to the pmu syscon */
+	node = of_parse_phandle(np, "rockchip,pmu", 0);
+	if (node) {
+		data->regmap_pmu = syscon_node_to_regmap(node);
+		if (IS_ERR(data->regmap_pmu))
+			return PTR_ERR(data->regmap_pmu);
+	}
+
+	/* Get DDR type */
+	regmap_read(data->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
+	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
+		    RK3399_PMUGRF_DDRTYPE_MASK;
+
+	/* Get the odt_dis_freq parameter in function of the DDR type */
+	switch (ddr_type) {
+	case RK3399_PMUGRF_DDRTYPE_DDR3:
+		data->odt_dis_freq = data->timing.ddr3_odt_dis_freq;
+		break;
+	case RK3399_PMUGRF_DDRTYPE_LPDDR3:
+		data->odt_dis_freq = data->timing.lpddr3_odt_dis_freq;
+		break;
+	case RK3399_PMUGRF_DDRTYPE_LPDDR4:
+		data->odt_dis_freq = data->timing.lpddr4_odt_dis_freq;
+		break;
+	default:
+		return -EINVAL;
+	};
+
 	/*
 	 * Get dram timing and pass it to arm trust firmware,
 	 * the dram drvier in arm trust firmware will get these
@@ -358,6 +409,27 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 		      ROCKCHIP_SIP_CONFIG_DRAM_INIT,
 		      0, 0, 0, 0, &res);
 
+	/*
+	 * In TF-A there is a platform SIP call to set the PD (power-down)
+	 * timings and to enable or disable the ODT (on-die termination).
+	 * This call needs three arguments as follows:
+	 *
+	 * arg0:
+	 *     bit[0-7]   : sr_idle
+	 *     bit[8-15]  : sr_mc_gate_idle
+	 *     bit[16-31] : standby idle
+	 * arg1:
+	 *     bit[0-11]  : pd_idle
+	 *     bit[16-27] : srpd_lite_idle
+	 * arg2:
+	 *     bit[0]     : odt enable
+	 */
+	data->odt_pd_arg0 = (data->timing.sr_idle & 0xff) |
+			    ((data->timing.sr_mc_gate_idle & 0xff) << 8) |
+			    ((data->timing.standby_idle & 0xffff) << 16);
+	data->odt_pd_arg1 = (data->timing.pd_idle & 0xfff) |
+			    ((data->timing.srpd_lite_idle & 0xfff) << 16);
+
 	/*
 	 * We add a devfreq driver to our parent since it has a device tree node
 	 * with operating points.
diff --git a/include/soc/rockchip/rockchip_sip.h b/include/soc/rockchip/rockchip_sip.h
index 7e28092c4d3d..ad9482c56797 100644
--- a/include/soc/rockchip/rockchip_sip.h
+++ b/include/soc/rockchip/rockchip_sip.h
@@ -23,5 +23,6 @@
 #define ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE	0x05
 #define ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ	0x06
 #define ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM	0x07
+#define ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD	0x08
 
 #endif
-- 
2.18.0


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

* [PATCH 3/8] devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

Trusted Firmware-A (TF-A) for rk3399 implements a SiP call to get the
on-die termination (ODT) and auto power down parameters from kernel,
this patch adds the functionality to do this. Also, if DDR clock
frequency is lower than the on-die termination (ODT) disable frequency
this driver should disable the DDR ODT.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1:
- [RFC 3/10] Add an explanation for platform SIP calls.
- [RFC 3/10] Change if statement for a switch.
- [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.

 drivers/devfreq/rk3399_dmc.c        | 74 ++++++++++++++++++++++++++++-
 include/soc/rockchip/rockchip_sip.h |  1 +
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index e795ad2b3f6b..c619dc4ac620 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -18,14 +18,17 @@
 #include <linux/devfreq.h>
 #include <linux/devfreq-event.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/rwsem.h>
 #include <linux/suspend.h>
 
+#include <soc/rockchip/rk3399_grf.h>
 #include <soc/rockchip/rockchip_sip.h>
 
 struct dram_timing {
@@ -69,8 +72,11 @@ struct rk3399_dmcfreq {
 	struct mutex lock;
 	struct dram_timing timing;
 	struct regulator *vdd_center;
+	struct regmap *regmap_pmu;
 	unsigned long rate, target_rate;
 	unsigned long volt, target_volt;
+	unsigned int odt_dis_freq;
+	int odt_pd_arg0, odt_pd_arg1;
 };
 
 static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
@@ -80,6 +86,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
 	struct dev_pm_opp *opp;
 	unsigned long old_clk_rate = dmcfreq->rate;
 	unsigned long target_volt, target_rate;
+	struct arm_smccc_res res;
+	bool odt_enable = false;
 	int err;
 
 	opp = devfreq_recommended_opp(dev, freq, flags);
@@ -95,6 +103,19 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
 
 	mutex_lock(&dmcfreq->lock);
 
+	if (target_rate >= dmcfreq->odt_dis_freq)
+		odt_enable = true;
+
+	/*
+	 * This makes a SMC call to the TF-A to set the DDR PD (power-down)
+	 * timings and to enable or disable the ODT (on-die termination)
+	 * resistors.
+	 */
+	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
+		      dmcfreq->odt_pd_arg1,
+		      ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
+		      odt_enable, 0, 0, 0, &res);
+
 	/*
 	 * If frequency scaling from low to high, adjust voltage first.
 	 * If frequency scaling from high to low, adjust frequency first.
@@ -294,11 +315,13 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 {
 	struct arm_smccc_res res;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node, *node;
 	struct rk3399_dmcfreq *data;
 	int ret, index, size;
 	uint32_t *timing;
 	struct dev_pm_opp *opp;
+	u32 ddr_type;
+	u32 val;
 
 	data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
 	if (!data)
@@ -334,6 +357,34 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* Try to find the optional reference to the pmu syscon */
+	node = of_parse_phandle(np, "rockchip,pmu", 0);
+	if (node) {
+		data->regmap_pmu = syscon_node_to_regmap(node);
+		if (IS_ERR(data->regmap_pmu))
+			return PTR_ERR(data->regmap_pmu);
+	}
+
+	/* Get DDR type */
+	regmap_read(data->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
+	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
+		    RK3399_PMUGRF_DDRTYPE_MASK;
+
+	/* Get the odt_dis_freq parameter in function of the DDR type */
+	switch (ddr_type) {
+	case RK3399_PMUGRF_DDRTYPE_DDR3:
+		data->odt_dis_freq = data->timing.ddr3_odt_dis_freq;
+		break;
+	case RK3399_PMUGRF_DDRTYPE_LPDDR3:
+		data->odt_dis_freq = data->timing.lpddr3_odt_dis_freq;
+		break;
+	case RK3399_PMUGRF_DDRTYPE_LPDDR4:
+		data->odt_dis_freq = data->timing.lpddr4_odt_dis_freq;
+		break;
+	default:
+		return -EINVAL;
+	};
+
 	/*
 	 * Get dram timing and pass it to arm trust firmware,
 	 * the dram drvier in arm trust firmware will get these
@@ -358,6 +409,27 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 		      ROCKCHIP_SIP_CONFIG_DRAM_INIT,
 		      0, 0, 0, 0, &res);
 
+	/*
+	 * In TF-A there is a platform SIP call to set the PD (power-down)
+	 * timings and to enable or disable the ODT (on-die termination).
+	 * This call needs three arguments as follows:
+	 *
+	 * arg0:
+	 *     bit[0-7]   : sr_idle
+	 *     bit[8-15]  : sr_mc_gate_idle
+	 *     bit[16-31] : standby idle
+	 * arg1:
+	 *     bit[0-11]  : pd_idle
+	 *     bit[16-27] : srpd_lite_idle
+	 * arg2:
+	 *     bit[0]     : odt enable
+	 */
+	data->odt_pd_arg0 = (data->timing.sr_idle & 0xff) |
+			    ((data->timing.sr_mc_gate_idle & 0xff) << 8) |
+			    ((data->timing.standby_idle & 0xffff) << 16);
+	data->odt_pd_arg1 = (data->timing.pd_idle & 0xfff) |
+			    ((data->timing.srpd_lite_idle & 0xfff) << 16);
+
 	/*
 	 * We add a devfreq driver to our parent since it has a device tree node
 	 * with operating points.
diff --git a/include/soc/rockchip/rockchip_sip.h b/include/soc/rockchip/rockchip_sip.h
index 7e28092c4d3d..ad9482c56797 100644
--- a/include/soc/rockchip/rockchip_sip.h
+++ b/include/soc/rockchip/rockchip_sip.h
@@ -23,5 +23,6 @@
 #define ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE	0x05
 #define ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ	0x06
 #define ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM	0x07
+#define ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD	0x08
 
 #endif
-- 
2.18.0

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

* [PATCH 4/8] devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel for DDRfreq.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

From: Derek Basehore <dbasehore@chromium.org>

This changes the kernel to sync with vblank for the display in the
kernel. This was done in Trusted Firmware-A (TF-A) before, but that locks
up one CPU for up to one display frame (1/60 second). That's bad for
performance and power, so this moves waiting to the kernel where the
waiting thread can properly wait on a completion.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 drivers/clk/rockchip/clk-ddr.c    | 142 +++++++++++++++++++++++++-----
 drivers/clk/rockchip/clk.c        |   2 +-
 drivers/clk/rockchip/clk.h        |   3 +-
 drivers/devfreq/rk3399_dmc.c      | 125 ++++++++++++++++++++------
 drivers/devfreq/rk3399_dmc_priv.h |  15 ++++
 include/soc/rockchip/rk3399_dmc.h |  42 +++++++++
 6 files changed, 277 insertions(+), 52 deletions(-)
 create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
 create mode 100644 include/soc/rockchip/rk3399_dmc.h

diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index e8075359366b..707be1bd8910 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -17,7 +17,9 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/ktime.h>
 #include <linux/slab.h>
+#include <soc/rockchip/rk3399_dmc.h>
 #include <soc/rockchip/rockchip_sip.h>
 #include "clk.h"
 
@@ -30,39 +32,104 @@ struct rockchip_ddrclk {
 	int		div_shift;
 	int		div_width;
 	int		ddr_flag;
-	spinlock_t	*lock;
+	unsigned long	cached_rate;
+	struct work_struct set_rate_work;
+	struct mutex	lock;
+	struct raw_notifier_head sync_chain;
 };
 
 #define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw)
+#define DMC_DEFAULT_TIMEOUT_NS		NSEC_PER_SEC
+#define DDRCLK_SET_RATE_MAX_RETRIES	3
 
-static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
-					unsigned long prate)
+static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 {
-	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
-	unsigned long flags;
+	struct rockchip_ddrclk *ddrclk = container_of(work,
+			struct rockchip_ddrclk, set_rate_work);
+	ktime_t timeout = ktime_add_ns(ktime_get(), DMC_DEFAULT_TIMEOUT_NS);
 	struct arm_smccc_res res;
+	int ret, i;
+
+	mutex_lock(&ddrclk->lock);
+	for (i = 0; i < DDRCLK_SET_RATE_MAX_RETRIES; i++) {
+		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0, &timeout);
+		if (ret == NOTIFY_BAD)
+			goto out;
+
+		/*
+		 * Check the timeout with irqs disabled. This is so we don't get
+		 * preempted after checking the timeout. That could cause things
+		 * like garbage values for the display if we change the DDR rate
+		 * at the wrong time.
+		 */
+		local_irq_disable();
+		if (ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
+				timeout)) {
+			local_irq_enable();
+			continue;
+		}
+
+		arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, ddrclk->cached_rate, 0,
+			      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
+			      0, 0, 0, 0, &res);
+		local_irq_enable();
+		break;
+	}
+out:
+	mutex_unlock(&ddrclk->lock);
+}
 
-	spin_lock_irqsave(ddrclk->lock, flags);
-	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
-		      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
-		      0, 0, 0, 0, &res);
-	spin_unlock_irqrestore(ddrclk->lock, flags);
+int rockchip_ddrclk_register_sync_nb(struct clk *clk, struct notifier_block *nb)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk;
+	int ret;
 
-	return res.a0;
+	if (!hw || !nb)
+		return -EINVAL;
+
+	ddrclk = to_rockchip_ddrclk_hw(hw);
+	if (!ddrclk)
+		return -EINVAL;
+
+	mutex_lock(&ddrclk->lock);
+	ret = raw_notifier_chain_register(&ddrclk->sync_chain, nb);
+	mutex_unlock(&ddrclk->lock);
+
+	return ret;
 }
+EXPORT_SYMBOL_GPL(rockchip_ddrclk_register_sync_nb);
 
-static unsigned long
-rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
-				unsigned long parent_rate)
+int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
+				       struct notifier_block *nb)
 {
-	struct arm_smccc_res res;
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk;
+	int ret;
 
-	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
-		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
-		      0, 0, 0, 0, &res);
+	if (!hw || !nb)
+		return -EINVAL;
 
-	return res.a0;
+	ddrclk = to_rockchip_ddrclk_hw(hw);
+	if (!ddrclk)
+		return -EINVAL;
+
+	mutex_lock(&ddrclk->lock);
+	ret = raw_notifier_chain_unregister(&ddrclk->sync_chain, nb);
+	mutex_unlock(&ddrclk->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_ddrclk_unregister_sync_nb);
+
+void rockchip_ddrclk_wait_set_rate(struct clk *clk)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+
+	flush_work(&ddrclk->set_rate_work);
 }
+EXPORT_SYMBOL_GPL(rockchip_ddrclk_wait_set_rate);
 
 static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
 					   unsigned long rate,
@@ -77,6 +144,30 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
 	return res.a0;
 }
 
+static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
+					unsigned long prate)
+{
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+	long rate;
+
+	rate = rockchip_ddrclk_sip_round_rate(hw, drate, &prate);
+	if (rate < 0)
+		return rate;
+
+	ddrclk->cached_rate = rate;
+	queue_work(system_highpri_wq, &ddrclk->set_rate_work);
+	return 0;
+}
+
+static unsigned long
+rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+
+	return ddrclk->cached_rate;
+}
+
 static u8 rockchip_ddrclk_get_parent(struct clk_hw *hw)
 {
 	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
@@ -105,12 +196,12 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 					 u8 num_parents, int mux_offset,
 					 int mux_shift, int mux_width,
 					 int div_shift, int div_width,
-					 int ddr_flag, void __iomem *reg_base,
-					 spinlock_t *lock)
+					 int ddr_flag, void __iomem *reg_base)
 {
 	struct rockchip_ddrclk *ddrclk;
 	struct clk_init_data init;
 	struct clk *clk;
+	struct arm_smccc_res res;
 
 	ddrclk = kzalloc(sizeof(*ddrclk), GFP_KERNEL);
 	if (!ddrclk)
@@ -134,7 +225,6 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 	}
 
 	ddrclk->reg_base = reg_base;
-	ddrclk->lock = lock;
 	ddrclk->hw.init = &init;
 	ddrclk->mux_offset = mux_offset;
 	ddrclk->mux_shift = mux_shift;
@@ -142,6 +232,14 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 	ddrclk->div_shift = div_shift;
 	ddrclk->div_width = div_width;
 	ddrclk->ddr_flag = ddr_flag;
+	mutex_init(&ddrclk->lock);
+	INIT_WORK(&ddrclk->set_rate_work, rockchip_ddrclk_set_rate_func);
+	RAW_INIT_NOTIFIER_HEAD(&ddrclk->sync_chain);
+
+	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
+		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
+		      0, 0, 0, 0, &res);
+	ddrclk->cached_rate = res.a0;
 
 	clk = clk_register(NULL, &ddrclk->hw);
 	if (IS_ERR(clk))
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 326b3fa44f5d..1b7775aff191 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -541,7 +541,7 @@ void __init rockchip_clk_register_branches(
 				list->muxdiv_offset, list->mux_shift,
 				list->mux_width, list->div_shift,
 				list->div_width, list->div_flags,
-				ctx->reg_base, &ctx->lock);
+				ctx->reg_base);
 			break;
 		}
 
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index ef601dded32c..5e4ce49ef337 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -326,8 +326,7 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 					 u8 num_parents, int mux_offset,
 					 int mux_shift, int mux_width,
 					 int div_shift, int div_width,
-					 int ddr_flags, void __iomem *reg_base,
-					 spinlock_t *lock);
+					 int ddr_flags, void __iomem *reg_base);
 
 #define ROCKCHIP_INVERTER_HIWORD_MASK	BIT(0)
 
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index c619dc4ac620..b4ac9ee605be 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -28,9 +28,12 @@
 #include <linux/rwsem.h>
 #include <linux/suspend.h>
 
+#include <soc/rockchip/rk3399_dmc.h>
 #include <soc/rockchip/rk3399_grf.h>
 #include <soc/rockchip/rockchip_sip.h>
 
+#include "rk3399_dmc_priv.h"
+
 struct dram_timing {
 	unsigned int ddr3_speed_bin;
 	unsigned int pd_idle;
@@ -70,6 +73,7 @@ struct rk3399_dmcfreq {
 	struct clk *dmc_clk;
 	struct devfreq_event_dev *edev;
 	struct mutex lock;
+	struct mutex en_lock;
 	struct dram_timing timing;
 	struct regulator *vdd_center;
 	struct regmap *regmap_pmu;
@@ -77,6 +81,8 @@ struct rk3399_dmcfreq {
 	unsigned long volt, target_volt;
 	unsigned int odt_dis_freq;
 	int odt_pd_arg0, odt_pd_arg1;
+	int num_sync_nb;
+	int disable_count;
 };
 
 static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
@@ -139,6 +145,13 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
 		goto out;
 	}
 
+	/*
+	 * Setting the dpll is asynchronous since clk_set_rate grabs a global
+	 * common clk lock and set_rate for the dpll takes up to one display
+	 * frame to complete. We still need to wait for the set_rate to complete
+	 * here, though, before we change voltage.
+	 */
+	rockchip_ddrclk_wait_set_rate(dmcfreq->dmc_clk);
 	/*
 	 * Check the dpll rate,
 	 * There only two result we will get,
@@ -205,40 +218,15 @@ static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
 static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
 {
 	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
-	int ret = 0;
-
-	ret = devfreq_event_disable_edev(dmcfreq->edev);
-	if (ret < 0) {
-		dev_err(dev, "failed to disable the devfreq-event devices\n");
-		return ret;
-	}
 
-	ret = devfreq_suspend_device(dmcfreq->devfreq);
-	if (ret < 0) {
-		dev_err(dev, "failed to suspend the devfreq devices\n");
-		return ret;
-	}
-
-	return 0;
+	return rockchip_dmcfreq_block(dmcfreq->devfreq);
 }
 
 static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
 {
 	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
-	int ret = 0;
 
-	ret = devfreq_event_enable_edev(dmcfreq->edev);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable the devfreq-event devices\n");
-		return ret;
-	}
-
-	ret = devfreq_resume_device(dmcfreq->devfreq);
-	if (ret < 0) {
-		dev_err(dev, "failed to resume the devfreq devices\n");
-		return ret;
-	}
-	return ret;
+	return rockchip_dmcfreq_unblock(dmcfreq->devfreq);
 }
 
 static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
@@ -311,6 +299,88 @@ static int of_get_ddr_timings(struct dram_timing *timing,
 	return ret;
 }
 
+int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
+					struct notifier_block *nb)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret;
+
+	mutex_lock(&dmcfreq->en_lock);
+	/*
+	 * We have a short amount of time (~1ms or less typically) to run
+	 * dmcfreq after we sync with the notifier, so syncing with more than
+	 * one notifier is not generally possible. Thus, if more than one sync
+	 * notifier is registered, disable dmcfreq.
+	 */
+	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+		devfreq_suspend_device(devfreq);
+
+	ret = rockchip_ddrclk_register_sync_nb(dmcfreq->dmc_clk, nb);
+	if (ret == 0)
+		dmcfreq->num_sync_nb++;
+	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+		devfreq_resume_device(devfreq);
+
+	mutex_unlock(&dmcfreq->en_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_register_clk_sync_nb);
+
+int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
+					  struct notifier_block *nb)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret;
+
+	mutex_lock(&dmcfreq->en_lock);
+	ret = rockchip_ddrclk_unregister_sync_nb(dmcfreq->dmc_clk, nb);
+	if (ret == 0) {
+		dmcfreq->num_sync_nb--;
+		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+			devfreq_resume_device(devfreq);
+	}
+
+	mutex_unlock(&dmcfreq->en_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unregister_clk_sync_nb);
+
+int rockchip_dmcfreq_block(struct devfreq *devfreq)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&dmcfreq->en_lock);
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0)
+		ret = devfreq_suspend_device(devfreq);
+
+	if (!ret)
+		dmcfreq->disable_count++;
+	mutex_unlock(&dmcfreq->en_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_block);
+
+int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&dmcfreq->en_lock);
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1)
+		ret = devfreq_resume_device(devfreq);
+
+	if (!ret)
+		dmcfreq->disable_count--;
+	WARN_ON(dmcfreq->disable_count < 0);
+	mutex_unlock(&dmcfreq->en_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unblock);
+
 static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 {
 	struct arm_smccc_res res;
@@ -328,6 +398,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	mutex_init(&data->lock);
+	mutex_init(&data->en_lock);
 
 	data->vdd_center = devm_regulator_get(dev, "center");
 	if (IS_ERR(data->vdd_center)) {
diff --git a/drivers/devfreq/rk3399_dmc_priv.h b/drivers/devfreq/rk3399_dmc_priv.h
new file mode 100644
index 000000000000..8ac0340a4990
--- /dev/null
+++ b/drivers/devfreq/rk3399_dmc_priv.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2017-2018 Google, Inc.
+ * Author: Derek Basehore <dbasehore@chromium.org>
+ */
+
+#ifndef __RK3399_DMC_PRIV_H
+#define __RK3399_DMC_PRIV_H
+
+void rockchip_ddrclk_wait_set_rate(struct clk *clk);
+int rockchip_ddrclk_register_sync_nb(struct clk *clk,
+				     struct notifier_block *nb);
+int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
+				       struct notifier_block *nb);
+#endif
diff --git a/include/soc/rockchip/rk3399_dmc.h b/include/soc/rockchip/rk3399_dmc.h
new file mode 100644
index 000000000000..8b28563710d1
--- /dev/null
+++ b/include/soc/rockchip/rk3399_dmc.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Lin Huang <hl@rock-chips.com>
+ */
+
+#ifndef __SOC_RK3399_DMC_H
+#define __SOC_RK3399_DMC_H
+
+#include <linux/devfreq.h>
+#include <linux/notifier.h>
+
+#define DMC_MIN_SET_RATE_NS	(250 * NSEC_PER_USEC)
+#define DMC_MIN_VBLANK_NS	(DMC_MIN_SET_RATE_NS + 50 * NSEC_PER_USEC)
+
+#if IS_ENABLED(CONFIG_ARM_RK3399_DMC_DEVFREQ)
+int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
+					  struct notifier_block *nb);
+
+int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
+					    struct notifier_block *nb);
+
+int rockchip_dmcfreq_block(struct devfreq *devfreq);
+
+int rockchip_dmcfreq_unblock(struct devfreq *devfreq);
+#else
+static inline int
+rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
+				      struct notifier_block *nb)
+{ return 0; }
+
+static inline int
+rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
+					struct notifier_block *nb)
+{ return 0; }
+
+static inline int rockchip_dmcfreq_block(struct devfreq *devfreq) { return 0; }
+
+static inline int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
+{ return 0; }
+#endif
+#endif
-- 
2.18.0


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

* [PATCH 4/8] devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel for DDRfreq.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Derek Basehore <dbasehore@chromium.org>

This changes the kernel to sync with vblank for the display in the
kernel. This was done in Trusted Firmware-A (TF-A) before, but that locks
up one CPU for up to one display frame (1/60 second). That's bad for
performance and power, so this moves waiting to the kernel where the
waiting thread can properly wait on a completion.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 drivers/clk/rockchip/clk-ddr.c    | 142 +++++++++++++++++++++++++-----
 drivers/clk/rockchip/clk.c        |   2 +-
 drivers/clk/rockchip/clk.h        |   3 +-
 drivers/devfreq/rk3399_dmc.c      | 125 ++++++++++++++++++++------
 drivers/devfreq/rk3399_dmc_priv.h |  15 ++++
 include/soc/rockchip/rk3399_dmc.h |  42 +++++++++
 6 files changed, 277 insertions(+), 52 deletions(-)
 create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
 create mode 100644 include/soc/rockchip/rk3399_dmc.h

diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index e8075359366b..707be1bd8910 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -17,7 +17,9 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/ktime.h>
 #include <linux/slab.h>
+#include <soc/rockchip/rk3399_dmc.h>
 #include <soc/rockchip/rockchip_sip.h>
 #include "clk.h"
 
@@ -30,39 +32,104 @@ struct rockchip_ddrclk {
 	int		div_shift;
 	int		div_width;
 	int		ddr_flag;
-	spinlock_t	*lock;
+	unsigned long	cached_rate;
+	struct work_struct set_rate_work;
+	struct mutex	lock;
+	struct raw_notifier_head sync_chain;
 };
 
 #define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw)
+#define DMC_DEFAULT_TIMEOUT_NS		NSEC_PER_SEC
+#define DDRCLK_SET_RATE_MAX_RETRIES	3
 
-static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
-					unsigned long prate)
+static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 {
-	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
-	unsigned long flags;
+	struct rockchip_ddrclk *ddrclk = container_of(work,
+			struct rockchip_ddrclk, set_rate_work);
+	ktime_t timeout = ktime_add_ns(ktime_get(), DMC_DEFAULT_TIMEOUT_NS);
 	struct arm_smccc_res res;
+	int ret, i;
+
+	mutex_lock(&ddrclk->lock);
+	for (i = 0; i < DDRCLK_SET_RATE_MAX_RETRIES; i++) {
+		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0, &timeout);
+		if (ret == NOTIFY_BAD)
+			goto out;
+
+		/*
+		 * Check the timeout with irqs disabled. This is so we don't get
+		 * preempted after checking the timeout. That could cause things
+		 * like garbage values for the display if we change the DDR rate
+		 * at the wrong time.
+		 */
+		local_irq_disable();
+		if (ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
+				timeout)) {
+			local_irq_enable();
+			continue;
+		}
+
+		arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, ddrclk->cached_rate, 0,
+			      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
+			      0, 0, 0, 0, &res);
+		local_irq_enable();
+		break;
+	}
+out:
+	mutex_unlock(&ddrclk->lock);
+}
 
-	spin_lock_irqsave(ddrclk->lock, flags);
-	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
-		      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
-		      0, 0, 0, 0, &res);
-	spin_unlock_irqrestore(ddrclk->lock, flags);
+int rockchip_ddrclk_register_sync_nb(struct clk *clk, struct notifier_block *nb)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk;
+	int ret;
 
-	return res.a0;
+	if (!hw || !nb)
+		return -EINVAL;
+
+	ddrclk = to_rockchip_ddrclk_hw(hw);
+	if (!ddrclk)
+		return -EINVAL;
+
+	mutex_lock(&ddrclk->lock);
+	ret = raw_notifier_chain_register(&ddrclk->sync_chain, nb);
+	mutex_unlock(&ddrclk->lock);
+
+	return ret;
 }
+EXPORT_SYMBOL_GPL(rockchip_ddrclk_register_sync_nb);
 
-static unsigned long
-rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
-				unsigned long parent_rate)
+int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
+				       struct notifier_block *nb)
 {
-	struct arm_smccc_res res;
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk;
+	int ret;
 
-	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
-		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
-		      0, 0, 0, 0, &res);
+	if (!hw || !nb)
+		return -EINVAL;
 
-	return res.a0;
+	ddrclk = to_rockchip_ddrclk_hw(hw);
+	if (!ddrclk)
+		return -EINVAL;
+
+	mutex_lock(&ddrclk->lock);
+	ret = raw_notifier_chain_unregister(&ddrclk->sync_chain, nb);
+	mutex_unlock(&ddrclk->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_ddrclk_unregister_sync_nb);
+
+void rockchip_ddrclk_wait_set_rate(struct clk *clk)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+
+	flush_work(&ddrclk->set_rate_work);
 }
+EXPORT_SYMBOL_GPL(rockchip_ddrclk_wait_set_rate);
 
 static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
 					   unsigned long rate,
@@ -77,6 +144,30 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
 	return res.a0;
 }
 
+static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
+					unsigned long prate)
+{
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+	long rate;
+
+	rate = rockchip_ddrclk_sip_round_rate(hw, drate, &prate);
+	if (rate < 0)
+		return rate;
+
+	ddrclk->cached_rate = rate;
+	queue_work(system_highpri_wq, &ddrclk->set_rate_work);
+	return 0;
+}
+
+static unsigned long
+rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+
+	return ddrclk->cached_rate;
+}
+
 static u8 rockchip_ddrclk_get_parent(struct clk_hw *hw)
 {
 	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
@@ -105,12 +196,12 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 					 u8 num_parents, int mux_offset,
 					 int mux_shift, int mux_width,
 					 int div_shift, int div_width,
-					 int ddr_flag, void __iomem *reg_base,
-					 spinlock_t *lock)
+					 int ddr_flag, void __iomem *reg_base)
 {
 	struct rockchip_ddrclk *ddrclk;
 	struct clk_init_data init;
 	struct clk *clk;
+	struct arm_smccc_res res;
 
 	ddrclk = kzalloc(sizeof(*ddrclk), GFP_KERNEL);
 	if (!ddrclk)
@@ -134,7 +225,6 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 	}
 
 	ddrclk->reg_base = reg_base;
-	ddrclk->lock = lock;
 	ddrclk->hw.init = &init;
 	ddrclk->mux_offset = mux_offset;
 	ddrclk->mux_shift = mux_shift;
@@ -142,6 +232,14 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 	ddrclk->div_shift = div_shift;
 	ddrclk->div_width = div_width;
 	ddrclk->ddr_flag = ddr_flag;
+	mutex_init(&ddrclk->lock);
+	INIT_WORK(&ddrclk->set_rate_work, rockchip_ddrclk_set_rate_func);
+	RAW_INIT_NOTIFIER_HEAD(&ddrclk->sync_chain);
+
+	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
+		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
+		      0, 0, 0, 0, &res);
+	ddrclk->cached_rate = res.a0;
 
 	clk = clk_register(NULL, &ddrclk->hw);
 	if (IS_ERR(clk))
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 326b3fa44f5d..1b7775aff191 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -541,7 +541,7 @@ void __init rockchip_clk_register_branches(
 				list->muxdiv_offset, list->mux_shift,
 				list->mux_width, list->div_shift,
 				list->div_width, list->div_flags,
-				ctx->reg_base, &ctx->lock);
+				ctx->reg_base);
 			break;
 		}
 
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index ef601dded32c..5e4ce49ef337 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -326,8 +326,7 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 					 u8 num_parents, int mux_offset,
 					 int mux_shift, int mux_width,
 					 int div_shift, int div_width,
-					 int ddr_flags, void __iomem *reg_base,
-					 spinlock_t *lock);
+					 int ddr_flags, void __iomem *reg_base);
 
 #define ROCKCHIP_INVERTER_HIWORD_MASK	BIT(0)
 
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index c619dc4ac620..b4ac9ee605be 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -28,9 +28,12 @@
 #include <linux/rwsem.h>
 #include <linux/suspend.h>
 
+#include <soc/rockchip/rk3399_dmc.h>
 #include <soc/rockchip/rk3399_grf.h>
 #include <soc/rockchip/rockchip_sip.h>
 
+#include "rk3399_dmc_priv.h"
+
 struct dram_timing {
 	unsigned int ddr3_speed_bin;
 	unsigned int pd_idle;
@@ -70,6 +73,7 @@ struct rk3399_dmcfreq {
 	struct clk *dmc_clk;
 	struct devfreq_event_dev *edev;
 	struct mutex lock;
+	struct mutex en_lock;
 	struct dram_timing timing;
 	struct regulator *vdd_center;
 	struct regmap *regmap_pmu;
@@ -77,6 +81,8 @@ struct rk3399_dmcfreq {
 	unsigned long volt, target_volt;
 	unsigned int odt_dis_freq;
 	int odt_pd_arg0, odt_pd_arg1;
+	int num_sync_nb;
+	int disable_count;
 };
 
 static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
@@ -139,6 +145,13 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
 		goto out;
 	}
 
+	/*
+	 * Setting the dpll is asynchronous since clk_set_rate grabs a global
+	 * common clk lock and set_rate for the dpll takes up to one display
+	 * frame to complete. We still need to wait for the set_rate to complete
+	 * here, though, before we change voltage.
+	 */
+	rockchip_ddrclk_wait_set_rate(dmcfreq->dmc_clk);
 	/*
 	 * Check the dpll rate,
 	 * There only two result we will get,
@@ -205,40 +218,15 @@ static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
 static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
 {
 	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
-	int ret = 0;
-
-	ret = devfreq_event_disable_edev(dmcfreq->edev);
-	if (ret < 0) {
-		dev_err(dev, "failed to disable the devfreq-event devices\n");
-		return ret;
-	}
 
-	ret = devfreq_suspend_device(dmcfreq->devfreq);
-	if (ret < 0) {
-		dev_err(dev, "failed to suspend the devfreq devices\n");
-		return ret;
-	}
-
-	return 0;
+	return rockchip_dmcfreq_block(dmcfreq->devfreq);
 }
 
 static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
 {
 	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
-	int ret = 0;
 
-	ret = devfreq_event_enable_edev(dmcfreq->edev);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable the devfreq-event devices\n");
-		return ret;
-	}
-
-	ret = devfreq_resume_device(dmcfreq->devfreq);
-	if (ret < 0) {
-		dev_err(dev, "failed to resume the devfreq devices\n");
-		return ret;
-	}
-	return ret;
+	return rockchip_dmcfreq_unblock(dmcfreq->devfreq);
 }
 
 static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
@@ -311,6 +299,88 @@ static int of_get_ddr_timings(struct dram_timing *timing,
 	return ret;
 }
 
+int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
+					struct notifier_block *nb)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret;
+
+	mutex_lock(&dmcfreq->en_lock);
+	/*
+	 * We have a short amount of time (~1ms or less typically) to run
+	 * dmcfreq after we sync with the notifier, so syncing with more than
+	 * one notifier is not generally possible. Thus, if more than one sync
+	 * notifier is registered, disable dmcfreq.
+	 */
+	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+		devfreq_suspend_device(devfreq);
+
+	ret = rockchip_ddrclk_register_sync_nb(dmcfreq->dmc_clk, nb);
+	if (ret == 0)
+		dmcfreq->num_sync_nb++;
+	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+		devfreq_resume_device(devfreq);
+
+	mutex_unlock(&dmcfreq->en_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_register_clk_sync_nb);
+
+int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
+					  struct notifier_block *nb)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret;
+
+	mutex_lock(&dmcfreq->en_lock);
+	ret = rockchip_ddrclk_unregister_sync_nb(dmcfreq->dmc_clk, nb);
+	if (ret == 0) {
+		dmcfreq->num_sync_nb--;
+		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+			devfreq_resume_device(devfreq);
+	}
+
+	mutex_unlock(&dmcfreq->en_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unregister_clk_sync_nb);
+
+int rockchip_dmcfreq_block(struct devfreq *devfreq)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&dmcfreq->en_lock);
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0)
+		ret = devfreq_suspend_device(devfreq);
+
+	if (!ret)
+		dmcfreq->disable_count++;
+	mutex_unlock(&dmcfreq->en_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_block);
+
+int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
+{
+	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
+	int ret = 0;
+
+	mutex_lock(&dmcfreq->en_lock);
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1)
+		ret = devfreq_resume_device(devfreq);
+
+	if (!ret)
+		dmcfreq->disable_count--;
+	WARN_ON(dmcfreq->disable_count < 0);
+	mutex_unlock(&dmcfreq->en_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unblock);
+
 static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 {
 	struct arm_smccc_res res;
@@ -328,6 +398,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	mutex_init(&data->lock);
+	mutex_init(&data->en_lock);
 
 	data->vdd_center = devm_regulator_get(dev, "center");
 	if (IS_ERR(data->vdd_center)) {
diff --git a/drivers/devfreq/rk3399_dmc_priv.h b/drivers/devfreq/rk3399_dmc_priv.h
new file mode 100644
index 000000000000..8ac0340a4990
--- /dev/null
+++ b/drivers/devfreq/rk3399_dmc_priv.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2017-2018 Google, Inc.
+ * Author: Derek Basehore <dbasehore@chromium.org>
+ */
+
+#ifndef __RK3399_DMC_PRIV_H
+#define __RK3399_DMC_PRIV_H
+
+void rockchip_ddrclk_wait_set_rate(struct clk *clk);
+int rockchip_ddrclk_register_sync_nb(struct clk *clk,
+				     struct notifier_block *nb);
+int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
+				       struct notifier_block *nb);
+#endif
diff --git a/include/soc/rockchip/rk3399_dmc.h b/include/soc/rockchip/rk3399_dmc.h
new file mode 100644
index 000000000000..8b28563710d1
--- /dev/null
+++ b/include/soc/rockchip/rk3399_dmc.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Lin Huang <hl@rock-chips.com>
+ */
+
+#ifndef __SOC_RK3399_DMC_H
+#define __SOC_RK3399_DMC_H
+
+#include <linux/devfreq.h>
+#include <linux/notifier.h>
+
+#define DMC_MIN_SET_RATE_NS	(250 * NSEC_PER_USEC)
+#define DMC_MIN_VBLANK_NS	(DMC_MIN_SET_RATE_NS + 50 * NSEC_PER_USEC)
+
+#if IS_ENABLED(CONFIG_ARM_RK3399_DMC_DEVFREQ)
+int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
+					  struct notifier_block *nb);
+
+int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
+					    struct notifier_block *nb);
+
+int rockchip_dmcfreq_block(struct devfreq *devfreq);
+
+int rockchip_dmcfreq_unblock(struct devfreq *devfreq);
+#else
+static inline int
+rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
+				      struct notifier_block *nb)
+{ return 0; }
+
+static inline int
+rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
+					struct notifier_block *nb)
+{ return 0; }
+
+static inline int rockchip_dmcfreq_block(struct devfreq *devfreq) { return 0; }
+
+static inline int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
+{ return 0; }
+#endif
+#endif
-- 
2.18.0

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

* [PATCH 5/8] devfreq: rk3399_dmc / clk: rockchip: Disable DDR clk timeout on suspend.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

From: Derek Basehore <dbasehore@chromium.org>

This disables the timeout for the DDR clk when the governor is
suspended or stopped. This makes sure that the suspend frequency is
set. It also makes sure that the userspace governor will be able to
change the frequency without failing.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 drivers/clk/rockchip/clk-ddr.c    | 21 ++++++++++++++++++---
 drivers/devfreq/rk3399_dmc.c      | 20 +++++++++++++++-----
 drivers/devfreq/rk3399_dmc_priv.h |  1 +
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index 707be1bd8910..de00590af167 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -32,6 +32,7 @@ struct rockchip_ddrclk {
 	int		div_shift;
 	int		div_width;
 	int		ddr_flag;
+	bool		timeout_en;
 	unsigned long	cached_rate;
 	struct work_struct set_rate_work;
 	struct mutex	lock;
@@ -52,8 +53,9 @@ static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 
 	mutex_lock(&ddrclk->lock);
 	for (i = 0; i < DDRCLK_SET_RATE_MAX_RETRIES; i++) {
-		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0, &timeout);
-		if (ret == NOTIFY_BAD)
+		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0,
+					      &timeout);
+		if (ddrclk->timeout_en && ret == NOTIFY_BAD)
 			goto out;
 
 		/*
@@ -63,7 +65,8 @@ static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 		 * at the wrong time.
 		 */
 		local_irq_disable();
-		if (ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
+		if (ddrclk->timeout_en &&
+		    ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
 				timeout)) {
 			local_irq_enable();
 			continue;
@@ -79,6 +82,17 @@ static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 	mutex_unlock(&ddrclk->lock);
 }
 
+void rockchip_ddrclk_set_timeout_en(struct clk *clk, bool enable)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+
+	mutex_lock(&ddrclk->lock);
+	ddrclk->timeout_en = enable;
+	mutex_unlock(&ddrclk->lock);
+}
+EXPORT_SYMBOL(rockchip_ddrclk_set_timeout_en);
+
 int rockchip_ddrclk_register_sync_nb(struct clk *clk, struct notifier_block *nb)
 {
 	struct clk_hw *hw = __clk_get_hw(clk);
@@ -232,6 +246,7 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 	ddrclk->div_shift = div_shift;
 	ddrclk->div_width = div_width;
 	ddrclk->ddr_flag = ddr_flag;
+	ddrclk->timeout_en = true;
 	mutex_init(&ddrclk->lock);
 	INIT_WORK(&ddrclk->set_rate_work, rockchip_ddrclk_set_rate_func);
 	RAW_INIT_NOTIFIER_HEAD(&ddrclk->sync_chain);
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index b4ac9ee605be..323c888b542a 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -312,14 +312,18 @@ int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
 	 * one notifier is not generally possible. Thus, if more than one sync
 	 * notifier is registered, disable dmcfreq.
 	 */
-	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, false);
 		devfreq_suspend_device(devfreq);
+	}
 
 	ret = rockchip_ddrclk_register_sync_nb(dmcfreq->dmc_clk, nb);
 	if (ret == 0)
 		dmcfreq->num_sync_nb++;
-	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, true);
 		devfreq_resume_device(devfreq);
+	}
 
 	mutex_unlock(&dmcfreq->en_lock);
 	return ret;
@@ -336,8 +340,10 @@ int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
 	ret = rockchip_ddrclk_unregister_sync_nb(dmcfreq->dmc_clk, nb);
 	if (ret == 0) {
 		dmcfreq->num_sync_nb--;
-		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0) {
+			rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, true);
 			devfreq_resume_device(devfreq);
+		}
 	}
 
 	mutex_unlock(&dmcfreq->en_lock);
@@ -352,8 +358,10 @@ int rockchip_dmcfreq_block(struct devfreq *devfreq)
 	int ret = 0;
 
 	mutex_lock(&dmcfreq->en_lock);
-	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0)
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, false);
 		ret = devfreq_suspend_device(devfreq);
+	}
 
 	if (!ret)
 		dmcfreq->disable_count++;
@@ -369,8 +377,10 @@ int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
 	int ret = 0;
 
 	mutex_lock(&dmcfreq->en_lock);
-	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1)
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, true);
 		ret = devfreq_resume_device(devfreq);
+	}
 
 	if (!ret)
 		dmcfreq->disable_count--;
diff --git a/drivers/devfreq/rk3399_dmc_priv.h b/drivers/devfreq/rk3399_dmc_priv.h
index 8ac0340a4990..1ad46f4b15cc 100644
--- a/drivers/devfreq/rk3399_dmc_priv.h
+++ b/drivers/devfreq/rk3399_dmc_priv.h
@@ -7,6 +7,7 @@
 #ifndef __RK3399_DMC_PRIV_H
 #define __RK3399_DMC_PRIV_H
 
+void rockchip_ddrclk_set_timeout_en(struct clk *clk, bool enable);
 void rockchip_ddrclk_wait_set_rate(struct clk *clk);
 int rockchip_ddrclk_register_sync_nb(struct clk *clk,
 				     struct notifier_block *nb);
-- 
2.18.0


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

* [PATCH 5/8] devfreq: rk3399_dmc / clk: rockchip: Disable DDR clk timeout on suspend.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Derek Basehore <dbasehore@chromium.org>

This disables the timeout for the DDR clk when the governor is
suspended or stopped. This makes sure that the suspend frequency is
set. It also makes sure that the userspace governor will be able to
change the frequency without failing.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 drivers/clk/rockchip/clk-ddr.c    | 21 ++++++++++++++++++---
 drivers/devfreq/rk3399_dmc.c      | 20 +++++++++++++++-----
 drivers/devfreq/rk3399_dmc_priv.h |  1 +
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index 707be1bd8910..de00590af167 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -32,6 +32,7 @@ struct rockchip_ddrclk {
 	int		div_shift;
 	int		div_width;
 	int		ddr_flag;
+	bool		timeout_en;
 	unsigned long	cached_rate;
 	struct work_struct set_rate_work;
 	struct mutex	lock;
@@ -52,8 +53,9 @@ static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 
 	mutex_lock(&ddrclk->lock);
 	for (i = 0; i < DDRCLK_SET_RATE_MAX_RETRIES; i++) {
-		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0, &timeout);
-		if (ret == NOTIFY_BAD)
+		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0,
+					      &timeout);
+		if (ddrclk->timeout_en && ret == NOTIFY_BAD)
 			goto out;
 
 		/*
@@ -63,7 +65,8 @@ static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 		 * at the wrong time.
 		 */
 		local_irq_disable();
-		if (ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
+		if (ddrclk->timeout_en &&
+		    ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
 				timeout)) {
 			local_irq_enable();
 			continue;
@@ -79,6 +82,17 @@ static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
 	mutex_unlock(&ddrclk->lock);
 }
 
+void rockchip_ddrclk_set_timeout_en(struct clk *clk, bool enable)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
+
+	mutex_lock(&ddrclk->lock);
+	ddrclk->timeout_en = enable;
+	mutex_unlock(&ddrclk->lock);
+}
+EXPORT_SYMBOL(rockchip_ddrclk_set_timeout_en);
+
 int rockchip_ddrclk_register_sync_nb(struct clk *clk, struct notifier_block *nb)
 {
 	struct clk_hw *hw = __clk_get_hw(clk);
@@ -232,6 +246,7 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
 	ddrclk->div_shift = div_shift;
 	ddrclk->div_width = div_width;
 	ddrclk->ddr_flag = ddr_flag;
+	ddrclk->timeout_en = true;
 	mutex_init(&ddrclk->lock);
 	INIT_WORK(&ddrclk->set_rate_work, rockchip_ddrclk_set_rate_func);
 	RAW_INIT_NOTIFIER_HEAD(&ddrclk->sync_chain);
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index b4ac9ee605be..323c888b542a 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -312,14 +312,18 @@ int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
 	 * one notifier is not generally possible. Thus, if more than one sync
 	 * notifier is registered, disable dmcfreq.
 	 */
-	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, false);
 		devfreq_suspend_device(devfreq);
+	}
 
 	ret = rockchip_ddrclk_register_sync_nb(dmcfreq->dmc_clk, nb);
 	if (ret == 0)
 		dmcfreq->num_sync_nb++;
-	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, true);
 		devfreq_resume_device(devfreq);
+	}
 
 	mutex_unlock(&dmcfreq->en_lock);
 	return ret;
@@ -336,8 +340,10 @@ int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
 	ret = rockchip_ddrclk_unregister_sync_nb(dmcfreq->dmc_clk, nb);
 	if (ret == 0) {
 		dmcfreq->num_sync_nb--;
-		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
+		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0) {
+			rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, true);
 			devfreq_resume_device(devfreq);
+		}
 	}
 
 	mutex_unlock(&dmcfreq->en_lock);
@@ -352,8 +358,10 @@ int rockchip_dmcfreq_block(struct devfreq *devfreq)
 	int ret = 0;
 
 	mutex_lock(&dmcfreq->en_lock);
-	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0)
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, false);
 		ret = devfreq_suspend_device(devfreq);
+	}
 
 	if (!ret)
 		dmcfreq->disable_count++;
@@ -369,8 +377,10 @@ int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
 	int ret = 0;
 
 	mutex_lock(&dmcfreq->en_lock);
-	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1)
+	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1) {
+		rockchip_ddrclk_set_timeout_en(dmcfreq->dmc_clk, true);
 		ret = devfreq_resume_device(devfreq);
+	}
 
 	if (!ret)
 		dmcfreq->disable_count--;
diff --git a/drivers/devfreq/rk3399_dmc_priv.h b/drivers/devfreq/rk3399_dmc_priv.h
index 8ac0340a4990..1ad46f4b15cc 100644
--- a/drivers/devfreq/rk3399_dmc_priv.h
+++ b/drivers/devfreq/rk3399_dmc_priv.h
@@ -7,6 +7,7 @@
 #ifndef __RK3399_DMC_PRIV_H
 #define __RK3399_DMC_PRIV_H
 
+void rockchip_ddrclk_set_timeout_en(struct clk *clk, bool enable);
 void rockchip_ddrclk_wait_set_rate(struct clk *clk);
 int rockchip_ddrclk_register_sync_nb(struct clk *clk,
 				     struct notifier_block *nb);
-- 
2.18.0

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

* [PATCH 6/8] drm: rockchip: Add DDR devfreq support.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

From: Sean Paul <seanpaul@chromium.org>

Add support for devfreq to dynamically control the DDR frequency. It
will activate when there is one CRTC active, and disable if more than
one becomes active (to avoid flickering on one of the screens).

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 46 +++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  9 ++++
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 35 ++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 34 +++++++++++++++
 4 files changed, 124 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f814d37b1db2..e774f3c7c325 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -19,6 +19,8 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
+#include <linux/devfreq.h>
+#include <linux/devfreq-event.h>
 #include <linux/dma-mapping.h>
 #include <linux/dma-iommu.h>
 #include <linux/pm_runtime.h>
@@ -77,6 +79,46 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 	iommu_detach_device(domain, dev);
 }
 
+#if IS_ENABLED(CONFIG_ARM_RK3399_DMC_DEVFREQ)
+static int rockchip_drm_init_devfreq(struct device *dev,
+				     struct rockchip_drm_private *priv)
+{
+	struct devfreq *devfreq;
+	struct devfreq_event_dev *edev;
+	int ret;
+
+	devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
+	if (IS_ERR(devfreq)) {
+		ret = PTR_ERR(devfreq);
+		if (ret == -ENODEV) {
+			DRM_DEV_INFO(dev, "devfreq missing, skip\n");
+			return 0;
+		}
+		return ret;
+	}
+
+	edev = devfreq_event_get_edev_by_phandle(devfreq->dev.parent, 0);
+	if (IS_ERR(edev)) {
+		ret = PTR_ERR(edev);
+		if (ret == -ENODEV) {
+			DRM_DEV_INFO(dev, "devfreq edev missing, skip\n");
+			return 0;
+		}
+		return ret;
+	}
+
+	priv->devfreq = devfreq;
+	priv->devfreq_event_dev = edev;
+	return 0;
+}
+#else
+static int rockchip_drm_init_devfreq(struct device *dev,
+				     struct rockchip_drm_private *priv)
+{
+	return 0;
+}
+#endif
+
 static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
 {
 	struct rockchip_drm_private *private = drm_dev->dev_private;
@@ -136,6 +178,10 @@ static int rockchip_drm_bind(struct device *dev)
 	INIT_LIST_HEAD(&private->psr_list);
 	mutex_init(&private->psr_list_lock);
 
+	ret = rockchip_drm_init_devfreq(dev, private);
+	if (ret)
+		goto err_free;
+
 	ret = rockchip_drm_init_iommu(drm_dev);
 	if (ret)
 		goto err_free;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 3a6ebfc26036..cc13fac59644 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -57,6 +57,10 @@ struct rockchip_drm_private {
 	struct drm_mm mm;
 	struct list_head psr_list;
 	struct mutex psr_list_lock;
+
+	struct devfreq *devfreq;
+	struct devfreq_event_dev *devfreq_event_dev;
+	bool dmc_disable_flag;
 };
 
 int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
@@ -65,6 +69,11 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev);
 int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
 
+uint32_t rockchip_drm_get_vblank_ns(struct drm_display_mode *mode);
+
+void rockchip_drm_enable_dmc(struct rockchip_drm_private *priv);
+void rockchip_drm_disable_dmc(struct rockchip_drm_private *priv);
+
 extern struct platform_driver cdn_dp_driver;
 extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
 extern struct platform_driver dw_mipi_dsi_driver;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index ea18cb2a76c0..7b6f7227d476 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -19,6 +19,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <soc/rockchip/rk3399_dmc.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -145,6 +146,16 @@ rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state)
 		rockchip_drm_psr_inhibit_get(encoder);
 }
 
+uint32_t rockchip_drm_get_vblank_ns(struct drm_display_mode *mode)
+{
+	uint64_t vblank_time = mode->vtotal - mode->vdisplay;
+
+	vblank_time *= (uint64_t)NSEC_PER_SEC * mode->htotal;
+	do_div(vblank_time, mode->clock * 1000);
+
+	return vblank_time;
+}
+
 static void
 rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state)
 {
@@ -167,9 +178,28 @@ static void
 rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
 {
 	struct drm_device *dev = old_state->dev;
+	struct rockchip_drm_private *priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	struct drm_display_mode *mode;
+	bool force_dmc_off = false;
+
+	drm_for_each_crtc(crtc, dev) {
+		if (crtc->state->active) {
+			mode = &crtc->state->adjusted_mode;
+			if (rockchip_drm_get_vblank_ns(mode) <
+			    DMC_MIN_VBLANK_NS)
+				force_dmc_off = true;
+		}
+	}
 
 	rockchip_drm_psr_inhibit_get_state(old_state);
 
+	/* If disabling dmc, disable it before committing mode set changes. */
+	if (force_dmc_off && !priv->dmc_disable_flag) {
+		rockchip_dmcfreq_block(priv->devfreq);
+		priv->dmc_disable_flag = true;
+	}
+
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
 
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
@@ -184,6 +214,11 @@ rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
 	drm_atomic_helper_wait_for_vblanks(dev, old_state);
 
 	drm_atomic_helper_cleanup_planes(dev, old_state);
+
+	if (!force_dmc_off && priv->dmc_disable_flag) {
+		rockchip_dmcfreq_unblock(priv->devfreq);
+		priv->dmc_disable_flag = false;
+	}
 }
 
 static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = {
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 45847d4a2e14..e9f91278137d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -36,6 +36,8 @@
 #include <linux/reset.h>
 #include <linux/delay.h>
 
+#include <soc/rockchip/rk3399_dmc.h>
+
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_gem.h"
 #include "rockchip_drm_fb.h"
@@ -106,7 +108,9 @@ struct vop {
 	struct drm_flip_work fb_unref_work;
 	unsigned long pending;
 
+	ktime_t line_flag_timestamp;
 	struct completion line_flag_completion;
+	struct notifier_block dmc_nb;
 
 	const struct vop_data *data;
 
@@ -572,10 +576,12 @@ static int vop_enable(struct drm_crtc *crtc)
 static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
 				    struct drm_crtc_state *old_state)
 {
+	struct rockchip_drm_private *priv = crtc->dev->dev_private;
 	struct vop *vop = to_vop(crtc);
 
 	WARN_ON(vop->event);
 
+	rockchip_dmcfreq_unregister_clk_sync_nb(priv->devfreq, &vop->dmc_nb);
 	mutex_lock(&vop->vop_lock);
 	drm_crtc_vblank_off(crtc);
 
@@ -872,6 +878,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
 	const struct vop_data *vop_data = vop->data;
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc->state);
 	struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode;
+	struct rockchip_drm_private *priv = crtc->dev->dev_private;
 	u16 hsync_len = adjusted_mode->hsync_end - adjusted_mode->hsync_start;
 	u16 hdisplay = adjusted_mode->hdisplay;
 	u16 htotal = adjusted_mode->htotal;
@@ -962,6 +969,31 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	VOP_REG_SET(vop, common, standby, 0);
 	mutex_unlock(&vop->vop_lock);
+	rockchip_dmcfreq_register_clk_sync_nb(priv->devfreq, &vop->dmc_nb);
+}
+
+static int dmc_notify(struct notifier_block *nb,
+		      unsigned long action,
+		      void *data)
+{
+	struct vop *vop = container_of(nb, struct vop, dmc_nb);
+	struct drm_crtc *crtc = &vop->crtc;
+	ktime_t *timeout = data;
+	int ret;
+
+	if (WARN_ON(!vop->is_enabled))
+		return NOTIFY_BAD;
+
+	ret = rockchip_drm_wait_vact_end(crtc, 100);
+	*timeout = ktime_add_ns(vop->line_flag_timestamp,
+				rockchip_drm_get_vblank_ns(&crtc->mode));
+	if (ret) {
+		dev_err(vop->dev, "%s: Line flag interrupt did not arrive\n",
+			__func__);
+		return NOTIFY_BAD;
+	}
+
+	return NOTIFY_STOP;
 }
 
 static bool vop_fs_irq_is_pending(struct vop *vop)
@@ -1201,6 +1233,7 @@ static irqreturn_t vop_isr(int irq, void *data)
 	}
 
 	if (active_irqs & LINE_FLAG_INTR) {
+		vop->line_flag_timestamp = ktime_get();
 		complete(&vop->line_flag_completion);
 		active_irqs &= ~LINE_FLAG_INTR;
 		ret = IRQ_HANDLED;
@@ -1577,6 +1610,7 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
 	spin_lock_init(&vop->reg_lock);
 	spin_lock_init(&vop->irq_lock);
 	mutex_init(&vop->vop_lock);
+	vop->dmc_nb.notifier_call = dmc_notify;
 
 	ret = vop_create_crtc(vop);
 	if (ret)
-- 
2.18.0


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

* [PATCH 6/8] drm: rockchip: Add DDR devfreq support.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sean Paul <seanpaul@chromium.org>

Add support for devfreq to dynamically control the DDR frequency. It
will activate when there is one CRTC active, and disable if more than
one becomes active (to avoid flickering on one of the screens).

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 46 +++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  9 ++++
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 35 ++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 34 +++++++++++++++
 4 files changed, 124 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f814d37b1db2..e774f3c7c325 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -19,6 +19,8 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
+#include <linux/devfreq.h>
+#include <linux/devfreq-event.h>
 #include <linux/dma-mapping.h>
 #include <linux/dma-iommu.h>
 #include <linux/pm_runtime.h>
@@ -77,6 +79,46 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 	iommu_detach_device(domain, dev);
 }
 
+#if IS_ENABLED(CONFIG_ARM_RK3399_DMC_DEVFREQ)
+static int rockchip_drm_init_devfreq(struct device *dev,
+				     struct rockchip_drm_private *priv)
+{
+	struct devfreq *devfreq;
+	struct devfreq_event_dev *edev;
+	int ret;
+
+	devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
+	if (IS_ERR(devfreq)) {
+		ret = PTR_ERR(devfreq);
+		if (ret == -ENODEV) {
+			DRM_DEV_INFO(dev, "devfreq missing, skip\n");
+			return 0;
+		}
+		return ret;
+	}
+
+	edev = devfreq_event_get_edev_by_phandle(devfreq->dev.parent, 0);
+	if (IS_ERR(edev)) {
+		ret = PTR_ERR(edev);
+		if (ret == -ENODEV) {
+			DRM_DEV_INFO(dev, "devfreq edev missing, skip\n");
+			return 0;
+		}
+		return ret;
+	}
+
+	priv->devfreq = devfreq;
+	priv->devfreq_event_dev = edev;
+	return 0;
+}
+#else
+static int rockchip_drm_init_devfreq(struct device *dev,
+				     struct rockchip_drm_private *priv)
+{
+	return 0;
+}
+#endif
+
 static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
 {
 	struct rockchip_drm_private *private = drm_dev->dev_private;
@@ -136,6 +178,10 @@ static int rockchip_drm_bind(struct device *dev)
 	INIT_LIST_HEAD(&private->psr_list);
 	mutex_init(&private->psr_list_lock);
 
+	ret = rockchip_drm_init_devfreq(dev, private);
+	if (ret)
+		goto err_free;
+
 	ret = rockchip_drm_init_iommu(drm_dev);
 	if (ret)
 		goto err_free;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 3a6ebfc26036..cc13fac59644 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -57,6 +57,10 @@ struct rockchip_drm_private {
 	struct drm_mm mm;
 	struct list_head psr_list;
 	struct mutex psr_list_lock;
+
+	struct devfreq *devfreq;
+	struct devfreq_event_dev *devfreq_event_dev;
+	bool dmc_disable_flag;
 };
 
 int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
@@ -65,6 +69,11 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev);
 int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
 
+uint32_t rockchip_drm_get_vblank_ns(struct drm_display_mode *mode);
+
+void rockchip_drm_enable_dmc(struct rockchip_drm_private *priv);
+void rockchip_drm_disable_dmc(struct rockchip_drm_private *priv);
+
 extern struct platform_driver cdn_dp_driver;
 extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
 extern struct platform_driver dw_mipi_dsi_driver;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index ea18cb2a76c0..7b6f7227d476 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -19,6 +19,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <soc/rockchip/rk3399_dmc.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -145,6 +146,16 @@ rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state)
 		rockchip_drm_psr_inhibit_get(encoder);
 }
 
+uint32_t rockchip_drm_get_vblank_ns(struct drm_display_mode *mode)
+{
+	uint64_t vblank_time = mode->vtotal - mode->vdisplay;
+
+	vblank_time *= (uint64_t)NSEC_PER_SEC * mode->htotal;
+	do_div(vblank_time, mode->clock * 1000);
+
+	return vblank_time;
+}
+
 static void
 rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state)
 {
@@ -167,9 +178,28 @@ static void
 rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
 {
 	struct drm_device *dev = old_state->dev;
+	struct rockchip_drm_private *priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	struct drm_display_mode *mode;
+	bool force_dmc_off = false;
+
+	drm_for_each_crtc(crtc, dev) {
+		if (crtc->state->active) {
+			mode = &crtc->state->adjusted_mode;
+			if (rockchip_drm_get_vblank_ns(mode) <
+			    DMC_MIN_VBLANK_NS)
+				force_dmc_off = true;
+		}
+	}
 
 	rockchip_drm_psr_inhibit_get_state(old_state);
 
+	/* If disabling dmc, disable it before committing mode set changes. */
+	if (force_dmc_off && !priv->dmc_disable_flag) {
+		rockchip_dmcfreq_block(priv->devfreq);
+		priv->dmc_disable_flag = true;
+	}
+
 	drm_atomic_helper_commit_modeset_disables(dev, old_state);
 
 	drm_atomic_helper_commit_modeset_enables(dev, old_state);
@@ -184,6 +214,11 @@ rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state)
 	drm_atomic_helper_wait_for_vblanks(dev, old_state);
 
 	drm_atomic_helper_cleanup_planes(dev, old_state);
+
+	if (!force_dmc_off && priv->dmc_disable_flag) {
+		rockchip_dmcfreq_unblock(priv->devfreq);
+		priv->dmc_disable_flag = false;
+	}
 }
 
 static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = {
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 45847d4a2e14..e9f91278137d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -36,6 +36,8 @@
 #include <linux/reset.h>
 #include <linux/delay.h>
 
+#include <soc/rockchip/rk3399_dmc.h>
+
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_gem.h"
 #include "rockchip_drm_fb.h"
@@ -106,7 +108,9 @@ struct vop {
 	struct drm_flip_work fb_unref_work;
 	unsigned long pending;
 
+	ktime_t line_flag_timestamp;
 	struct completion line_flag_completion;
+	struct notifier_block dmc_nb;
 
 	const struct vop_data *data;
 
@@ -572,10 +576,12 @@ static int vop_enable(struct drm_crtc *crtc)
 static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
 				    struct drm_crtc_state *old_state)
 {
+	struct rockchip_drm_private *priv = crtc->dev->dev_private;
 	struct vop *vop = to_vop(crtc);
 
 	WARN_ON(vop->event);
 
+	rockchip_dmcfreq_unregister_clk_sync_nb(priv->devfreq, &vop->dmc_nb);
 	mutex_lock(&vop->vop_lock);
 	drm_crtc_vblank_off(crtc);
 
@@ -872,6 +878,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
 	const struct vop_data *vop_data = vop->data;
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc->state);
 	struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode;
+	struct rockchip_drm_private *priv = crtc->dev->dev_private;
 	u16 hsync_len = adjusted_mode->hsync_end - adjusted_mode->hsync_start;
 	u16 hdisplay = adjusted_mode->hdisplay;
 	u16 htotal = adjusted_mode->htotal;
@@ -962,6 +969,31 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	VOP_REG_SET(vop, common, standby, 0);
 	mutex_unlock(&vop->vop_lock);
+	rockchip_dmcfreq_register_clk_sync_nb(priv->devfreq, &vop->dmc_nb);
+}
+
+static int dmc_notify(struct notifier_block *nb,
+		      unsigned long action,
+		      void *data)
+{
+	struct vop *vop = container_of(nb, struct vop, dmc_nb);
+	struct drm_crtc *crtc = &vop->crtc;
+	ktime_t *timeout = data;
+	int ret;
+
+	if (WARN_ON(!vop->is_enabled))
+		return NOTIFY_BAD;
+
+	ret = rockchip_drm_wait_vact_end(crtc, 100);
+	*timeout = ktime_add_ns(vop->line_flag_timestamp,
+				rockchip_drm_get_vblank_ns(&crtc->mode));
+	if (ret) {
+		dev_err(vop->dev, "%s: Line flag interrupt did not arrive\n",
+			__func__);
+		return NOTIFY_BAD;
+	}
+
+	return NOTIFY_STOP;
 }
 
 static bool vop_fs_irq_is_pending(struct vop *vop)
@@ -1201,6 +1233,7 @@ static irqreturn_t vop_isr(int irq, void *data)
 	}
 
 	if (active_irqs & LINE_FLAG_INTR) {
+		vop->line_flag_timestamp = ktime_get();
 		complete(&vop->line_flag_completion);
 		active_irqs &= ~LINE_FLAG_INTR;
 		ret = IRQ_HANDLED;
@@ -1577,6 +1610,7 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
 	spin_lock_init(&vop->reg_lock);
 	spin_lock_init(&vop->irq_lock);
 	mutex_init(&vop->vop_lock);
+	vop->dmc_nb.notifier_call = dmc_notify;
 
 	ret = vop_create_crtc(vop);
 	if (ret)
-- 
2.18.0

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

* [PATCH 7/8] arm64: dts: rk3399: Add dfi and dmc nodes.
  2018-07-30  8:11 ` Enric Balletbo i Serra
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel, Nickey Yang, devicetree, Mark Yao,
	Jacob Chen, Brian Norris, Klaus Goger, Randy Li,
	Douglas Anderson, Catalin Marinas, Mark Rutland

From: Lin Huang <hl@rock-chips.com>

These are required to support DDR DVFS on rk3399 platform. The patch also
introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
with default DRAM settings.

Signed-off-by: Lin Huang <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1:
- [RFC 8/10] Move rk3399-dram.h to dt-includes.
- [RFC 8/10] Put sdram default values under the dmc node.
- [RFC 8/10] Removed rk3399-dram-default-timing.dts

 .../boot/dts/rockchip/rk3399-op1-opp.dtsi     | 29 ++++++++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      | 49 +++++++++++++
 include/dt-bindings/power/rk3399-dram.h       | 73 +++++++++++++++++++
 3 files changed, 151 insertions(+)
 create mode 100644 include/dt-bindings/power/rk3399-dram.h

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
index 69cc9b05baa5..c9e7032b01a8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
@@ -110,6 +110,31 @@
 			opp-microvolt = <1075000>;
 		};
 	};
+
+	dmc_opp_table: dmc_opp_table {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <900000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <900000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <666000000>;
+			opp-microvolt = <900000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <900000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <928000000>;
+			opp-microvolt = <900000>;
+		};
+	};
 };
 
 &cpu_l0 {
@@ -139,3 +164,7 @@
 &gpu {
 	operating-points-v2 = <&gpu_opp_table>;
 };
+
+&dmc {
+	operating-points-v2 = <&dmc_opp_table>;
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index b6cc50ab2cb8..e9d9aa7aa2ac 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -8,6 +8,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/power/rk3399-dram.h>
 #include <dt-bindings/power/rk3399-power.h>
 #include <dt-bindings/thermal/thermal.h>
 
@@ -1833,6 +1834,54 @@
 		status = "disabled";
 	};
 
+	dfi: dfi@ff630000 {
+		reg = <0x00 0xff630000 0x00 0x4000>;
+		compatible = "rockchip,rk3399-dfi";
+		rockchip,pmu = <&pmugrf>;
+		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru PCLK_DDR_MON>;
+		clock-names = "pclk_ddr_mon";
+		status = "disabled";
+	};
+
+	dmc: dmc {
+		compatible = "rockchip,rk3399-dmc";
+		rockchip,pmu = <&pmugrf>;
+		devfreq-events = <&dfi>;
+		clocks = <&cru SCLK_DDRC>;
+		clock-names = "dmc_clk";
+		status = "disabled";
+		rockchip,ddr3_speed_bin = <21>;
+		rockchip,pd_idle = <0x40>;
+		rockchip,sr_idle = <0x2>;
+		rockchip,sr_mc_gate_idle = <0x3>;
+		rockchip,srpd_lite_idle	= <0x4>;
+		rockchip,standby_idle = <0x2000>;
+		rockchip,dram_dll_dis_freq = <300000000>;
+		rockchip,phy_dll_dis_freq = <125000000>;
+		rockchip,auto_pd_dis_freq = <666000000>;
+		rockchip,ddr3_odt_dis_freq = <333000000>;
+		rockchip,ddr3_drv = <DDR3_DS_40ohm>;
+		rockchip,ddr3_odt = <DDR3_ODT_120ohm>;
+		rockchip,phy_ddr3_ca_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_ddr3_dq_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_ddr3_odt = <PHY_DRV_ODT_240>;
+		rockchip,lpddr3_odt_dis_freq = <333000000>;
+		rockchip,lpddr3_drv = <LP3_DS_34ohm>;
+		rockchip,lpddr3_odt = <LP3_ODT_240ohm>;
+		rockchip,phy_lpddr3_ca_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_lpddr3_dq_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_lpddr3_odt = <PHY_DRV_ODT_240>;
+		rockchip,lpddr4_odt_dis_freq = <333000000>;
+		rockchip,lpddr4_drv = <LP4_PDDS_60ohm>;
+		rockchip,lpddr4_dq_odt = <LP4_DQ_ODT_40ohm>;
+		rockchip,lpddr4_ca_odt = <LP4_CA_ODT_40ohm>;
+		rockchip,phy_lpddr4_ca_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_lpddr4_ck_cs_drv = <PHY_DRV_ODT_80>;
+		rockchip,phy_lpddr4_dq_drv = <PHY_DRV_ODT_80>;
+		rockchip,phy_lpddr4_odt = <PHY_DRV_ODT_60>;
+	};
+
 	pinctrl: pinctrl {
 		compatible = "rockchip,rk3399-pinctrl";
 		rockchip,grf = <&grf>;
diff --git a/include/dt-bindings/power/rk3399-dram.h b/include/dt-bindings/power/rk3399-dram.h
new file mode 100644
index 000000000000..4b3d4a79923b
--- /dev/null
+++ b/include/dt-bindings/power/rk3399-dram.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR X11) */
+/*
+ * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Lin Huang <hl@rock-chips.com>
+ */
+
+#ifndef _DTS_DRAM_ROCKCHIP_RK3399_H
+#define _DTS_DRAM_ROCKCHIP_RK3399_H
+
+#define DDR3_DS_34ohm		34
+#define DDR3_DS_40ohm		40
+
+#define DDR3_ODT_DIS		0
+#define DDR3_ODT_40ohm		40
+#define DDR3_ODT_60ohm		60
+#define DDR3_ODT_120ohm		120
+
+#define LP2_DS_34ohm		34
+#define LP2_DS_40ohm		40
+#define LP2_DS_48ohm		48
+#define LP2_DS_60ohm		60
+#define LP2_DS_68_6ohm		68	/* optional */
+#define LP2_DS_80ohm		80
+#define LP2_DS_120ohm		120	/* optional */
+
+#define LP3_DS_34ohm		34
+#define LP3_DS_40ohm		40
+#define LP3_DS_48ohm		48
+#define LP3_DS_60ohm		60
+#define LP3_DS_80ohm		80
+#define LP3_DS_34D_40U		3440
+#define LP3_DS_40D_48U		4048
+#define LP3_DS_34D_48U		3448
+
+#define LP3_ODT_DIS		0
+#define LP3_ODT_60ohm		60
+#define LP3_ODT_120ohm		120
+#define LP3_ODT_240ohm		240
+
+#define LP4_PDDS_40ohm		40
+#define LP4_PDDS_48ohm		48
+#define LP4_PDDS_60ohm		60
+#define LP4_PDDS_80ohm		80
+#define LP4_PDDS_120ohm		120
+#define LP4_PDDS_240ohm		240
+
+#define LP4_DQ_ODT_40ohm	40
+#define LP4_DQ_ODT_48ohm	48
+#define LP4_DQ_ODT_60ohm	60
+#define LP4_DQ_ODT_80ohm	80
+#define LP4_DQ_ODT_120ohm	120
+#define LP4_DQ_ODT_240ohm	240
+#define LP4_DQ_ODT_DIS		0
+
+#define LP4_CA_ODT_40ohm	40
+#define LP4_CA_ODT_48ohm	48
+#define LP4_CA_ODT_60ohm	60
+#define LP4_CA_ODT_80ohm	80
+#define LP4_CA_ODT_120ohm	120
+#define LP4_CA_ODT_240ohm	240
+#define LP4_CA_ODT_DIS		0
+
+#define PHY_DRV_ODT_Hi_Z	0
+#define PHY_DRV_ODT_240		240
+#define PHY_DRV_ODT_120		120
+#define PHY_DRV_ODT_80		80
+#define PHY_DRV_ODT_60		60
+#define PHY_DRV_ODT_48		48
+#define PHY_DRV_ODT_40		40
+#define PHY_DRV_ODT_34_3	34
+
+#endif /* _DTS_DRAM_ROCKCHIP_RK3399_H */
-- 
2.18.0


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

* [PATCH 7/8] arm64: dts: rk3399: Add dfi and dmc nodes.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Lin Huang <hl@rock-chips.com>

These are required to support DDR DVFS on rk3399 platform. The patch also
introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
with default DRAM settings.

Signed-off-by: Lin Huang <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1:
- [RFC 8/10] Move rk3399-dram.h to dt-includes.
- [RFC 8/10] Put sdram default values under the dmc node.
- [RFC 8/10] Removed rk3399-dram-default-timing.dts

 .../boot/dts/rockchip/rk3399-op1-opp.dtsi     | 29 ++++++++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      | 49 +++++++++++++
 include/dt-bindings/power/rk3399-dram.h       | 73 +++++++++++++++++++
 3 files changed, 151 insertions(+)
 create mode 100644 include/dt-bindings/power/rk3399-dram.h

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
index 69cc9b05baa5..c9e7032b01a8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
@@ -110,6 +110,31 @@
 			opp-microvolt = <1075000>;
 		};
 	};
+
+	dmc_opp_table: dmc_opp_table {
+		compatible = "operating-points-v2";
+
+		opp00 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <900000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <900000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <666000000>;
+			opp-microvolt = <900000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <900000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <928000000>;
+			opp-microvolt = <900000>;
+		};
+	};
 };
 
 &cpu_l0 {
@@ -139,3 +164,7 @@
 &gpu {
 	operating-points-v2 = <&gpu_opp_table>;
 };
+
+&dmc {
+	operating-points-v2 = <&dmc_opp_table>;
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index b6cc50ab2cb8..e9d9aa7aa2ac 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -8,6 +8,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/power/rk3399-dram.h>
 #include <dt-bindings/power/rk3399-power.h>
 #include <dt-bindings/thermal/thermal.h>
 
@@ -1833,6 +1834,54 @@
 		status = "disabled";
 	};
 
+	dfi: dfi at ff630000 {
+		reg = <0x00 0xff630000 0x00 0x4000>;
+		compatible = "rockchip,rk3399-dfi";
+		rockchip,pmu = <&pmugrf>;
+		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru PCLK_DDR_MON>;
+		clock-names = "pclk_ddr_mon";
+		status = "disabled";
+	};
+
+	dmc: dmc {
+		compatible = "rockchip,rk3399-dmc";
+		rockchip,pmu = <&pmugrf>;
+		devfreq-events = <&dfi>;
+		clocks = <&cru SCLK_DDRC>;
+		clock-names = "dmc_clk";
+		status = "disabled";
+		rockchip,ddr3_speed_bin = <21>;
+		rockchip,pd_idle = <0x40>;
+		rockchip,sr_idle = <0x2>;
+		rockchip,sr_mc_gate_idle = <0x3>;
+		rockchip,srpd_lite_idle	= <0x4>;
+		rockchip,standby_idle = <0x2000>;
+		rockchip,dram_dll_dis_freq = <300000000>;
+		rockchip,phy_dll_dis_freq = <125000000>;
+		rockchip,auto_pd_dis_freq = <666000000>;
+		rockchip,ddr3_odt_dis_freq = <333000000>;
+		rockchip,ddr3_drv = <DDR3_DS_40ohm>;
+		rockchip,ddr3_odt = <DDR3_ODT_120ohm>;
+		rockchip,phy_ddr3_ca_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_ddr3_dq_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_ddr3_odt = <PHY_DRV_ODT_240>;
+		rockchip,lpddr3_odt_dis_freq = <333000000>;
+		rockchip,lpddr3_drv = <LP3_DS_34ohm>;
+		rockchip,lpddr3_odt = <LP3_ODT_240ohm>;
+		rockchip,phy_lpddr3_ca_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_lpddr3_dq_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_lpddr3_odt = <PHY_DRV_ODT_240>;
+		rockchip,lpddr4_odt_dis_freq = <333000000>;
+		rockchip,lpddr4_drv = <LP4_PDDS_60ohm>;
+		rockchip,lpddr4_dq_odt = <LP4_DQ_ODT_40ohm>;
+		rockchip,lpddr4_ca_odt = <LP4_CA_ODT_40ohm>;
+		rockchip,phy_lpddr4_ca_drv = <PHY_DRV_ODT_40>;
+		rockchip,phy_lpddr4_ck_cs_drv = <PHY_DRV_ODT_80>;
+		rockchip,phy_lpddr4_dq_drv = <PHY_DRV_ODT_80>;
+		rockchip,phy_lpddr4_odt = <PHY_DRV_ODT_60>;
+	};
+
 	pinctrl: pinctrl {
 		compatible = "rockchip,rk3399-pinctrl";
 		rockchip,grf = <&grf>;
diff --git a/include/dt-bindings/power/rk3399-dram.h b/include/dt-bindings/power/rk3399-dram.h
new file mode 100644
index 000000000000..4b3d4a79923b
--- /dev/null
+++ b/include/dt-bindings/power/rk3399-dram.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR X11) */
+/*
+ * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Lin Huang <hl@rock-chips.com>
+ */
+
+#ifndef _DTS_DRAM_ROCKCHIP_RK3399_H
+#define _DTS_DRAM_ROCKCHIP_RK3399_H
+
+#define DDR3_DS_34ohm		34
+#define DDR3_DS_40ohm		40
+
+#define DDR3_ODT_DIS		0
+#define DDR3_ODT_40ohm		40
+#define DDR3_ODT_60ohm		60
+#define DDR3_ODT_120ohm		120
+
+#define LP2_DS_34ohm		34
+#define LP2_DS_40ohm		40
+#define LP2_DS_48ohm		48
+#define LP2_DS_60ohm		60
+#define LP2_DS_68_6ohm		68	/* optional */
+#define LP2_DS_80ohm		80
+#define LP2_DS_120ohm		120	/* optional */
+
+#define LP3_DS_34ohm		34
+#define LP3_DS_40ohm		40
+#define LP3_DS_48ohm		48
+#define LP3_DS_60ohm		60
+#define LP3_DS_80ohm		80
+#define LP3_DS_34D_40U		3440
+#define LP3_DS_40D_48U		4048
+#define LP3_DS_34D_48U		3448
+
+#define LP3_ODT_DIS		0
+#define LP3_ODT_60ohm		60
+#define LP3_ODT_120ohm		120
+#define LP3_ODT_240ohm		240
+
+#define LP4_PDDS_40ohm		40
+#define LP4_PDDS_48ohm		48
+#define LP4_PDDS_60ohm		60
+#define LP4_PDDS_80ohm		80
+#define LP4_PDDS_120ohm		120
+#define LP4_PDDS_240ohm		240
+
+#define LP4_DQ_ODT_40ohm	40
+#define LP4_DQ_ODT_48ohm	48
+#define LP4_DQ_ODT_60ohm	60
+#define LP4_DQ_ODT_80ohm	80
+#define LP4_DQ_ODT_120ohm	120
+#define LP4_DQ_ODT_240ohm	240
+#define LP4_DQ_ODT_DIS		0
+
+#define LP4_CA_ODT_40ohm	40
+#define LP4_CA_ODT_48ohm	48
+#define LP4_CA_ODT_60ohm	60
+#define LP4_CA_ODT_80ohm	80
+#define LP4_CA_ODT_120ohm	120
+#define LP4_CA_ODT_240ohm	240
+#define LP4_CA_ODT_DIS		0
+
+#define PHY_DRV_ODT_Hi_Z	0
+#define PHY_DRV_ODT_240		240
+#define PHY_DRV_ODT_120		120
+#define PHY_DRV_ODT_80		80
+#define PHY_DRV_ODT_60		60
+#define PHY_DRV_ODT_48		48
+#define PHY_DRV_ODT_40		40
+#define PHY_DRV_ODT_34_3	34
+
+#endif /* _DTS_DRAM_ROCKCHIP_RK3399_H */
-- 
2.18.0

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

* [PATCH 8/8] arm64: dts: rockchip: Enable dmc and dfi nodes on gru.
  2018-07-30  8:11 ` Enric Balletbo i Serra
  (?)
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel, Nickey Yang, devicetree, Mark Yao,
	Jacob Chen, Brian Norris, Klaus Goger, Mark Rutland, Randy Li,
	Douglas Anderson, Chris Zhong, Catalin Marinas, Shunqian Zheng,
	Jeffy Chen

From: Lin Huang <hl@rock-chips.com>

Enable the DMC (Dynamic Memory Controller) and the DFI (DDR PHY Interface)
nodes on gru/kevin boards so we can support DDR DVFS.

Signed-off-by: Lin Huang <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 21 ++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi     |  2 +-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 1da8f7c1501e..7a7dcca11497 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -295,6 +295,12 @@
 	status = "okay";
 };
 
+&dmc_opp_table {
+	opp04 {
+		opp-suspend;
+	};
+};
+
 /*
  * Set some suspend operating points to avoid OVP in suspend
  *
@@ -374,6 +380,10 @@
 		<200000000>;
 };
 
+&display_subsystem {
+	devfreq = <&dmc>;
+};
+
 &emmc_phy {
 	status = "okay";
 };
@@ -495,6 +505,17 @@ ap_i2c_audio: &i2c8 {
 	status = "okay";
 };
 
+&dfi {
+	status = "okay";
+};
+
+&dmc {
+	status = "okay";
+	center-supply = <&ppvar_centerlogic>;
+	upthreshold = <25>;
+	downdifferential = <15>;
+};
+
 &sdhci {
 	/*
 	 * Signal integrity isn't great at 200 MHz and 150 MHz (DDR) gives the
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index e9d9aa7aa2ac..20b64ca638cb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -128,7 +128,7 @@
 		};
 	};
 
-	display-subsystem {
+	display_subsystem: display-subsystem {
 		compatible = "rockchip,display-subsystem";
 		ports = <&vopl_out>, <&vopb_out>;
 	};
-- 
2.18.0


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

* [PATCH 8/8] arm64: dts: rockchip: Enable dmc and dfi nodes on gru.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel, Nickey Yang, devicetree, Mark Yao,
	Jacob Chen, Brian Norris, Klaus Goger, Mark Rutland, Randy Li,
	Douglas Anderson, Chris Zhong, Catalin Marinas, Shu

From: Lin Huang <hl@rock-chips.com>

Enable the DMC (Dynamic Memory Controller) and the DFI (DDR PHY Interface)
nodes on gru/kevin boards so we can support DDR DVFS.

Signed-off-by: Lin Huang <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 21 ++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi     |  2 +-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 1da8f7c1501e..7a7dcca11497 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -295,6 +295,12 @@
 	status = "okay";
 };
 
+&dmc_opp_table {
+	opp04 {
+		opp-suspend;
+	};
+};
+
 /*
  * Set some suspend operating points to avoid OVP in suspend
  *
@@ -374,6 +380,10 @@
 		<200000000>;
 };
 
+&display_subsystem {
+	devfreq = <&dmc>;
+};
+
 &emmc_phy {
 	status = "okay";
 };
@@ -495,6 +505,17 @@ ap_i2c_audio: &i2c8 {
 	status = "okay";
 };
 
+&dfi {
+	status = "okay";
+};
+
+&dmc {
+	status = "okay";
+	center-supply = <&ppvar_centerlogic>;
+	upthreshold = <25>;
+	downdifferential = <15>;
+};
+
 &sdhci {
 	/*
 	 * Signal integrity isn't great at 200 MHz and 150 MHz (DDR) gives the
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index e9d9aa7aa2ac..20b64ca638cb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -128,7 +128,7 @@
 		};
 	};
 
-	display-subsystem {
+	display_subsystem: display-subsystem {
 		compatible = "rockchip,display-subsystem";
 		ports = <&vopl_out>, <&vopb_out>;
 	};
-- 
2.18.0

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

* [PATCH 8/8] arm64: dts: rockchip: Enable dmc and dfi nodes on gru.
@ 2018-07-30  8:11   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-07-30  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Lin Huang <hl@rock-chips.com>

Enable the DMC (Dynamic Memory Controller) and the DFI (DDR PHY Interface)
nodes on gru/kevin boards so we can support DDR DVFS.

Signed-off-by: Lin Huang <hl@rock-chips.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v1: None

 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 21 ++++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi     |  2 +-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 1da8f7c1501e..7a7dcca11497 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -295,6 +295,12 @@
 	status = "okay";
 };
 
+&dmc_opp_table {
+	opp04 {
+		opp-suspend;
+	};
+};
+
 /*
  * Set some suspend operating points to avoid OVP in suspend
  *
@@ -374,6 +380,10 @@
 		<200000000>;
 };
 
+&display_subsystem {
+	devfreq = <&dmc>;
+};
+
 &emmc_phy {
 	status = "okay";
 };
@@ -495,6 +505,17 @@ ap_i2c_audio: &i2c8 {
 	status = "okay";
 };
 
+&dfi {
+	status = "okay";
+};
+
+&dmc {
+	status = "okay";
+	center-supply = <&ppvar_centerlogic>;
+	upthreshold = <25>;
+	downdifferential = <15>;
+};
+
 &sdhci {
 	/*
 	 * Signal integrity isn't great at 200 MHz and 150 MHz (DDR) gives the
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index e9d9aa7aa2ac..20b64ca638cb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -128,7 +128,7 @@
 		};
 	};
 
-	display-subsystem {
+	display_subsystem: display-subsystem {
 		compatible = "rockchip,display-subsystem";
 		ports = <&vopl_out>, <&vopb_out>;
 	};
-- 
2.18.0

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

* Re: [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place.
  2018-07-30  8:11   ` Enric Balletbo i Serra
@ 2018-08-01  8:36     ` Chanwoo Choi
  -1 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2018-08-01  8:36 UTC (permalink / raw)
  To: Enric Balletbo i Serra, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

Hi Enric,

On 2018년 07월 30일 17:11, Enric Balletbo i Serra wrote:
> Some rk3399 GRF (Generic Register Files) definitions can be used for
> different drivers. Move these definitions to a common include so we
> don't need to duplicate these definitions.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes in v1:
> - [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Even if you add the changes log, you are missing my Acked-by tag.

> - [RFC 1/10] s/Generic/General/ (Robin Murphy)
> - [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
> - [RFC 7/10] Removed from the series. I did not found a use case where this matters.
> 
>  drivers/devfreq/event/rockchip-dfi.c | 23 +++++++----------------
>  include/soc/rockchip/rk3399_grf.h    | 21 +++++++++++++++++++++
>  2 files changed, 28 insertions(+), 16 deletions(-)
>  create mode 100644 include/soc/rockchip/rk3399_grf.h
> 
> diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
> index 22b113363ffc..2fbbcbeb644f 100644
> --- a/drivers/devfreq/event/rockchip-dfi.c
> +++ b/drivers/devfreq/event/rockchip-dfi.c
> @@ -26,6 +26,8 @@
>  #include <linux/list.h>
>  #include <linux/of.h>
>  
> +#include <soc/rockchip/rk3399_grf.h>
> +
>  #define RK3399_DMC_NUM_CH	2
>  
>  /* DDRMON_CTRL */
> @@ -43,18 +45,6 @@
>  #define DDRMON_CH1_COUNT_NUM		0x3c
>  #define DDRMON_CH1_DFI_ACCESS_NUM	0x40
>  
> -/* pmu grf */
> -#define PMUGRF_OS_REG2	0x308
> -#define DDRTYPE_SHIFT	13
> -#define DDRTYPE_MASK	7
> -
> -enum {
> -	DDR3 = 3,
> -	LPDDR3 = 6,
> -	LPDDR4 = 7,
> -	UNUSED = 0xFF
> -};
> -
>  struct dmc_usage {
>  	u32 access;
>  	u32 total;
> @@ -83,16 +73,17 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev)
>  	u32 ddr_type;
>  
>  	/* get ddr type */
> -	regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
> -	ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
> +	regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
> +	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
> +		    RK3399_PMUGRF_DDRTYPE_MASK;
>  
>  	/* clear DDRMON_CTRL setting */
>  	writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
>  
>  	/* set ddr type to dfi */
> -	if (ddr_type == LPDDR3)
> +	if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3)
>  		writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
> -	else if (ddr_type == LPDDR4)
> +	else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4)
>  		writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
>  
>  	/* enable count, use software mode */
> diff --git a/include/soc/rockchip/rk3399_grf.h b/include/soc/rockchip/rk3399_grf.h
> new file mode 100644
> index 000000000000..3eebabcb2812
> --- /dev/null
> +++ b/include/soc/rockchip/rk3399_grf.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Rockchip General Register Files definitions
> + *
> + * Copyright (c) 2018, Collabora Ltd.
> + * Author: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> + */
> +
> +#ifndef __SOC_RK3399_GRF_H
> +#define __SOC_RK3399_GRF_H
> +
> +/* PMU GRF Registers */
> +#define RK3399_PMUGRF_OS_REG2		0x308
> +#define RK3399_PMUGRF_DDRTYPE_SHIFT	13
> +#define RK3399_PMUGRF_DDRTYPE_MASK	7
> +#define RK3399_PMUGRF_DDRTYPE_DDR3	3
> +#define RK3399_PMUGRF_DDRTYPE_LPDDR2	5
> +#define RK3399_PMUGRF_DDRTYPE_LPDDR3	6
> +#define RK3399_PMUGRF_DDRTYPE_LPDDR4	7
> +
> +#endif
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

* [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place.
@ 2018-08-01  8:36     ` Chanwoo Choi
  0 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2018-08-01  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Enric,

On 2018? 07? 30? 17:11, Enric Balletbo i Serra wrote:
> Some rk3399 GRF (Generic Register Files) definitions can be used for
> different drivers. Move these definitions to a common include so we
> don't need to duplicate these definitions.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes in v1:
> - [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Even if you add the changes log, you are missing my Acked-by tag.

> - [RFC 1/10] s/Generic/General/ (Robin Murphy)
> - [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
> - [RFC 7/10] Removed from the series. I did not found a use case where this matters.
> 
>  drivers/devfreq/event/rockchip-dfi.c | 23 +++++++----------------
>  include/soc/rockchip/rk3399_grf.h    | 21 +++++++++++++++++++++
>  2 files changed, 28 insertions(+), 16 deletions(-)
>  create mode 100644 include/soc/rockchip/rk3399_grf.h
> 
> diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
> index 22b113363ffc..2fbbcbeb644f 100644
> --- a/drivers/devfreq/event/rockchip-dfi.c
> +++ b/drivers/devfreq/event/rockchip-dfi.c
> @@ -26,6 +26,8 @@
>  #include <linux/list.h>
>  #include <linux/of.h>
>  
> +#include <soc/rockchip/rk3399_grf.h>
> +
>  #define RK3399_DMC_NUM_CH	2
>  
>  /* DDRMON_CTRL */
> @@ -43,18 +45,6 @@
>  #define DDRMON_CH1_COUNT_NUM		0x3c
>  #define DDRMON_CH1_DFI_ACCESS_NUM	0x40
>  
> -/* pmu grf */
> -#define PMUGRF_OS_REG2	0x308
> -#define DDRTYPE_SHIFT	13
> -#define DDRTYPE_MASK	7
> -
> -enum {
> -	DDR3 = 3,
> -	LPDDR3 = 6,
> -	LPDDR4 = 7,
> -	UNUSED = 0xFF
> -};
> -
>  struct dmc_usage {
>  	u32 access;
>  	u32 total;
> @@ -83,16 +73,17 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev)
>  	u32 ddr_type;
>  
>  	/* get ddr type */
> -	regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
> -	ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
> +	regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
> +	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
> +		    RK3399_PMUGRF_DDRTYPE_MASK;
>  
>  	/* clear DDRMON_CTRL setting */
>  	writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
>  
>  	/* set ddr type to dfi */
> -	if (ddr_type == LPDDR3)
> +	if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3)
>  		writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
> -	else if (ddr_type == LPDDR4)
> +	else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4)
>  		writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
>  
>  	/* enable count, use software mode */
> diff --git a/include/soc/rockchip/rk3399_grf.h b/include/soc/rockchip/rk3399_grf.h
> new file mode 100644
> index 000000000000..3eebabcb2812
> --- /dev/null
> +++ b/include/soc/rockchip/rk3399_grf.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Rockchip General Register Files definitions
> + *
> + * Copyright (c) 2018, Collabora Ltd.
> + * Author: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> + */
> +
> +#ifndef __SOC_RK3399_GRF_H
> +#define __SOC_RK3399_GRF_H
> +
> +/* PMU GRF Registers */
> +#define RK3399_PMUGRF_OS_REG2		0x308
> +#define RK3399_PMUGRF_DDRTYPE_SHIFT	13
> +#define RK3399_PMUGRF_DDRTYPE_MASK	7
> +#define RK3399_PMUGRF_DDRTYPE_DDR3	3
> +#define RK3399_PMUGRF_DDRTYPE_LPDDR2	5
> +#define RK3399_PMUGRF_DDRTYPE_LPDDR3	6
> +#define RK3399_PMUGRF_DDRTYPE_LPDDR4	7
> +
> +#endif
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

* Re: [PATCH 3/8] devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.
  2018-07-30  8:11   ` Enric Balletbo i Serra
@ 2018-08-01  9:08     ` Chanwoo Choi
  -1 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2018-08-01  9:08 UTC (permalink / raw)
  To: Enric Balletbo i Serra, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

Hi Enric,

On 2018년 07월 30일 17:11, Enric Balletbo i Serra wrote:
> Trusted Firmware-A (TF-A) for rk3399 implements a SiP call to get the
> on-die termination (ODT) and auto power down parameters from kernel,
> this patch adds the functionality to do this. Also, if DDR clock
> frequency is lower than the on-die termination (ODT) disable frequency
> this driver should disable the DDR ODT.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

> ---
> 
> Changes in v1:
> - [RFC 3/10] Add an explanation for platform SIP calls.
> - [RFC 3/10] Change if statement for a switch.
> - [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.
> 
>  drivers/devfreq/rk3399_dmc.c        | 74 ++++++++++++++++++++++++++++-
>  include/soc/rockchip/rockchip_sip.h |  1 +
>  2 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index e795ad2b3f6b..c619dc4ac620 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -18,14 +18,17 @@
>  #include <linux/devfreq.h>
>  #include <linux/devfreq-event.h>
>  #include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_opp.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/rwsem.h>
>  #include <linux/suspend.h>
>  
> +#include <soc/rockchip/rk3399_grf.h>
>  #include <soc/rockchip/rockchip_sip.h>
>  
>  struct dram_timing {
> @@ -69,8 +72,11 @@ struct rk3399_dmcfreq {
>  	struct mutex lock;
>  	struct dram_timing timing;
>  	struct regulator *vdd_center;
> +	struct regmap *regmap_pmu;
>  	unsigned long rate, target_rate;
>  	unsigned long volt, target_volt;
> +	unsigned int odt_dis_freq;
> +	int odt_pd_arg0, odt_pd_arg1;
>  };
>  
>  static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> @@ -80,6 +86,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>  	struct dev_pm_opp *opp;
>  	unsigned long old_clk_rate = dmcfreq->rate;
>  	unsigned long target_volt, target_rate;
> +	struct arm_smccc_res res;
> +	bool odt_enable = false;
>  	int err;
>  
>  	opp = devfreq_recommended_opp(dev, freq, flags);
> @@ -95,6 +103,19 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>  
>  	mutex_lock(&dmcfreq->lock);
>  
> +	if (target_rate >= dmcfreq->odt_dis_freq)
> +		odt_enable = true;
> +
> +	/*
> +	 * This makes a SMC call to the TF-A to set the DDR PD (power-down)
> +	 * timings and to enable or disable the ODT (on-die termination)
> +	 * resistors.
> +	 */
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
> +		      dmcfreq->odt_pd_arg1,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
> +		      odt_enable, 0, 0, 0, &res);
> +
>  	/*
>  	 * If frequency scaling from low to high, adjust voltage first.
>  	 * If frequency scaling from high to low, adjust frequency first.
> @@ -294,11 +315,13 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  {
>  	struct arm_smccc_res res;
>  	struct device *dev = &pdev->dev;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.of_node, *node;
>  	struct rk3399_dmcfreq *data;
>  	int ret, index, size;
>  	uint32_t *timing;
>  	struct dev_pm_opp *opp;
> +	u32 ddr_type;
> +	u32 val;
>  
>  	data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
>  	if (!data)
> @@ -334,6 +357,34 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* Try to find the optional reference to the pmu syscon */
> +	node = of_parse_phandle(np, "rockchip,pmu", 0);
> +	if (node) {
> +		data->regmap_pmu = syscon_node_to_regmap(node);
> +		if (IS_ERR(data->regmap_pmu))
> +			return PTR_ERR(data->regmap_pmu);
> +	}
> +
> +	/* Get DDR type */
> +	regmap_read(data->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
> +	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
> +		    RK3399_PMUGRF_DDRTYPE_MASK;
> +
> +	/* Get the odt_dis_freq parameter in function of the DDR type */
> +	switch (ddr_type) {
> +	case RK3399_PMUGRF_DDRTYPE_DDR3:
> +		data->odt_dis_freq = data->timing.ddr3_odt_dis_freq;
> +		break;
> +	case RK3399_PMUGRF_DDRTYPE_LPDDR3:
> +		data->odt_dis_freq = data->timing.lpddr3_odt_dis_freq;
> +		break;
> +	case RK3399_PMUGRF_DDRTYPE_LPDDR4:
> +		data->odt_dis_freq = data->timing.lpddr4_odt_dis_freq;
> +		break;
> +	default:
> +		return -EINVAL;
> +	};
> +
>  	/*
>  	 * Get dram timing and pass it to arm trust firmware,
>  	 * the dram drvier in arm trust firmware will get these
> @@ -358,6 +409,27 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  		      ROCKCHIP_SIP_CONFIG_DRAM_INIT,
>  		      0, 0, 0, 0, &res);
>  
> +	/*
> +	 * In TF-A there is a platform SIP call to set the PD (power-down)
> +	 * timings and to enable or disable the ODT (on-die termination).
> +	 * This call needs three arguments as follows:
> +	 *
> +	 * arg0:
> +	 *     bit[0-7]   : sr_idle
> +	 *     bit[8-15]  : sr_mc_gate_idle
> +	 *     bit[16-31] : standby idle
> +	 * arg1:
> +	 *     bit[0-11]  : pd_idle
> +	 *     bit[16-27] : srpd_lite_idle
> +	 * arg2:
> +	 *     bit[0]     : odt enable
> +	 */
> +	data->odt_pd_arg0 = (data->timing.sr_idle & 0xff) |
> +			    ((data->timing.sr_mc_gate_idle & 0xff) << 8) |
> +			    ((data->timing.standby_idle & 0xffff) << 16);
> +	data->odt_pd_arg1 = (data->timing.pd_idle & 0xfff) |
> +			    ((data->timing.srpd_lite_idle & 0xfff) << 16);
> +
>  	/*
>  	 * We add a devfreq driver to our parent since it has a device tree node
>  	 * with operating points.
> diff --git a/include/soc/rockchip/rockchip_sip.h b/include/soc/rockchip/rockchip_sip.h
> index 7e28092c4d3d..ad9482c56797 100644
> --- a/include/soc/rockchip/rockchip_sip.h
> +++ b/include/soc/rockchip/rockchip_sip.h
> @@ -23,5 +23,6 @@
>  #define ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE	0x05
>  #define ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ	0x06
>  #define ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM	0x07
> +#define ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD	0x08
>  
>  #endif
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

* [PATCH 3/8] devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A.
@ 2018-08-01  9:08     ` Chanwoo Choi
  0 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2018-08-01  9:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Enric,

On 2018? 07? 30? 17:11, Enric Balletbo i Serra wrote:
> Trusted Firmware-A (TF-A) for rk3399 implements a SiP call to get the
> on-die termination (ODT) and auto power down parameters from kernel,
> this patch adds the functionality to do this. Also, if DDR clock
> frequency is lower than the on-die termination (ODT) disable frequency
> this driver should disable the DDR ODT.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

> ---
> 
> Changes in v1:
> - [RFC 3/10] Add an explanation for platform SIP calls.
> - [RFC 3/10] Change if statement for a switch.
> - [RFC 3/10] Rename ddr_flag to odt_enable to be more clear.
> 
>  drivers/devfreq/rk3399_dmc.c        | 74 ++++++++++++++++++++++++++++-
>  include/soc/rockchip/rockchip_sip.h |  1 +
>  2 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index e795ad2b3f6b..c619dc4ac620 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -18,14 +18,17 @@
>  #include <linux/devfreq.h>
>  #include <linux/devfreq-event.h>
>  #include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_opp.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/rwsem.h>
>  #include <linux/suspend.h>
>  
> +#include <soc/rockchip/rk3399_grf.h>
>  #include <soc/rockchip/rockchip_sip.h>
>  
>  struct dram_timing {
> @@ -69,8 +72,11 @@ struct rk3399_dmcfreq {
>  	struct mutex lock;
>  	struct dram_timing timing;
>  	struct regulator *vdd_center;
> +	struct regmap *regmap_pmu;
>  	unsigned long rate, target_rate;
>  	unsigned long volt, target_volt;
> +	unsigned int odt_dis_freq;
> +	int odt_pd_arg0, odt_pd_arg1;
>  };
>  
>  static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> @@ -80,6 +86,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>  	struct dev_pm_opp *opp;
>  	unsigned long old_clk_rate = dmcfreq->rate;
>  	unsigned long target_volt, target_rate;
> +	struct arm_smccc_res res;
> +	bool odt_enable = false;
>  	int err;
>  
>  	opp = devfreq_recommended_opp(dev, freq, flags);
> @@ -95,6 +103,19 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>  
>  	mutex_lock(&dmcfreq->lock);
>  
> +	if (target_rate >= dmcfreq->odt_dis_freq)
> +		odt_enable = true;
> +
> +	/*
> +	 * This makes a SMC call to the TF-A to set the DDR PD (power-down)
> +	 * timings and to enable or disable the ODT (on-die termination)
> +	 * resistors.
> +	 */
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
> +		      dmcfreq->odt_pd_arg1,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
> +		      odt_enable, 0, 0, 0, &res);
> +
>  	/*
>  	 * If frequency scaling from low to high, adjust voltage first.
>  	 * If frequency scaling from high to low, adjust frequency first.
> @@ -294,11 +315,13 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  {
>  	struct arm_smccc_res res;
>  	struct device *dev = &pdev->dev;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.of_node, *node;
>  	struct rk3399_dmcfreq *data;
>  	int ret, index, size;
>  	uint32_t *timing;
>  	struct dev_pm_opp *opp;
> +	u32 ddr_type;
> +	u32 val;
>  
>  	data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
>  	if (!data)
> @@ -334,6 +357,34 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* Try to find the optional reference to the pmu syscon */
> +	node = of_parse_phandle(np, "rockchip,pmu", 0);
> +	if (node) {
> +		data->regmap_pmu = syscon_node_to_regmap(node);
> +		if (IS_ERR(data->regmap_pmu))
> +			return PTR_ERR(data->regmap_pmu);
> +	}
> +
> +	/* Get DDR type */
> +	regmap_read(data->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
> +	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
> +		    RK3399_PMUGRF_DDRTYPE_MASK;
> +
> +	/* Get the odt_dis_freq parameter in function of the DDR type */
> +	switch (ddr_type) {
> +	case RK3399_PMUGRF_DDRTYPE_DDR3:
> +		data->odt_dis_freq = data->timing.ddr3_odt_dis_freq;
> +		break;
> +	case RK3399_PMUGRF_DDRTYPE_LPDDR3:
> +		data->odt_dis_freq = data->timing.lpddr3_odt_dis_freq;
> +		break;
> +	case RK3399_PMUGRF_DDRTYPE_LPDDR4:
> +		data->odt_dis_freq = data->timing.lpddr4_odt_dis_freq;
> +		break;
> +	default:
> +		return -EINVAL;
> +	};
> +
>  	/*
>  	 * Get dram timing and pass it to arm trust firmware,
>  	 * the dram drvier in arm trust firmware will get these
> @@ -358,6 +409,27 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  		      ROCKCHIP_SIP_CONFIG_DRAM_INIT,
>  		      0, 0, 0, 0, &res);
>  
> +	/*
> +	 * In TF-A there is a platform SIP call to set the PD (power-down)
> +	 * timings and to enable or disable the ODT (on-die termination).
> +	 * This call needs three arguments as follows:
> +	 *
> +	 * arg0:
> +	 *     bit[0-7]   : sr_idle
> +	 *     bit[8-15]  : sr_mc_gate_idle
> +	 *     bit[16-31] : standby idle
> +	 * arg1:
> +	 *     bit[0-11]  : pd_idle
> +	 *     bit[16-27] : srpd_lite_idle
> +	 * arg2:
> +	 *     bit[0]     : odt enable
> +	 */
> +	data->odt_pd_arg0 = (data->timing.sr_idle & 0xff) |
> +			    ((data->timing.sr_mc_gate_idle & 0xff) << 8) |
> +			    ((data->timing.standby_idle & 0xffff) << 16);
> +	data->odt_pd_arg1 = (data->timing.pd_idle & 0xfff) |
> +			    ((data->timing.srpd_lite_idle & 0xfff) << 16);
> +
>  	/*
>  	 * We add a devfreq driver to our parent since it has a device tree node
>  	 * with operating points.
> diff --git a/include/soc/rockchip/rockchip_sip.h b/include/soc/rockchip/rockchip_sip.h
> index 7e28092c4d3d..ad9482c56797 100644
> --- a/include/soc/rockchip/rockchip_sip.h
> +++ b/include/soc/rockchip/rockchip_sip.h
> @@ -23,5 +23,6 @@
>  #define ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE	0x05
>  #define ROCKCHIP_SIP_CONFIG_DRAM_CLR_IRQ	0x06
>  #define ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM	0x07
> +#define ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD	0x08
>  
>  #endif
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

* Re: [PATCH 4/8] devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel for DDRfreq.
  2018-07-30  8:11   ` Enric Balletbo i Serra
@ 2018-08-02  2:35     ` Chanwoo Choi
  -1 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2018-08-02  2:35 UTC (permalink / raw)
  To: Enric Balletbo i Serra, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

Hi Enric,

I'm not sure this approach is right. Actually, I don't prefer
the definitions of specific functions for only some device drivers.
This approach connect with device drivers without subsystem.
It looks like the direct call function.

Generally, all device driver in linux kernel have to
use the provided API from subsystem as following:
[A subsystem] --------------------- [B subsystem]
       |                                  |
[A device driver]                   [B device driver]


But, this patch looks like following connection.
I think that it might make the complicated codes.
We cannot develop the all device drivers with direct call function
without standard subsystem interface.

[A subsystem] --------------------- [B subsystem]
       |                                  |
[A device driver] ----------------- [B device driver]


Regards,
Chanwoo Choi


On 2018년 07월 30일 17:11, Enric Balletbo i Serra wrote:
> From: Derek Basehore <dbasehore@chromium.org>
> 
> This changes the kernel to sync with vblank for the display in the
> kernel. This was done in Trusted Firmware-A (TF-A) before, but that locks
> up one CPU for up to one display frame (1/60 second). That's bad for
> performance and power, so this moves waiting to the kernel where the
> waiting thread can properly wait on a completion.
> 
> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes in v1: None
> 
>  drivers/clk/rockchip/clk-ddr.c    | 142 +++++++++++++++++++++++++-----
>  drivers/clk/rockchip/clk.c        |   2 +-
>  drivers/clk/rockchip/clk.h        |   3 +-
>  drivers/devfreq/rk3399_dmc.c      | 125 ++++++++++++++++++++------
>  drivers/devfreq/rk3399_dmc_priv.h |  15 ++++
>  include/soc/rockchip/rk3399_dmc.h |  42 +++++++++
>  6 files changed, 277 insertions(+), 52 deletions(-)
>  create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
>  create mode 100644 include/soc/rockchip/rk3399_dmc.h
> 
> diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
> index e8075359366b..707be1bd8910 100644
> --- a/drivers/clk/rockchip/clk-ddr.c
> +++ b/drivers/clk/rockchip/clk-ddr.c
> @@ -17,7 +17,9 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/ktime.h>
>  #include <linux/slab.h>
> +#include <soc/rockchip/rk3399_dmc.h>
>  #include <soc/rockchip/rockchip_sip.h>
>  #include "clk.h"
>  
> @@ -30,39 +32,104 @@ struct rockchip_ddrclk {
>  	int		div_shift;
>  	int		div_width;
>  	int		ddr_flag;
> -	spinlock_t	*lock;
> +	unsigned long	cached_rate;
> +	struct work_struct set_rate_work;
> +	struct mutex	lock;
> +	struct raw_notifier_head sync_chain;
>  };
>  
>  #define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw)
> +#define DMC_DEFAULT_TIMEOUT_NS		NSEC_PER_SEC
> +#define DDRCLK_SET_RATE_MAX_RETRIES	3
>  
> -static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
> -					unsigned long prate)
> +static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
>  {
> -	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> -	unsigned long flags;
> +	struct rockchip_ddrclk *ddrclk = container_of(work,
> +			struct rockchip_ddrclk, set_rate_work);
> +	ktime_t timeout = ktime_add_ns(ktime_get(), DMC_DEFAULT_TIMEOUT_NS);
>  	struct arm_smccc_res res;
> +	int ret, i;
> +
> +	mutex_lock(&ddrclk->lock);
> +	for (i = 0; i < DDRCLK_SET_RATE_MAX_RETRIES; i++) {
> +		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0, &timeout);
> +		if (ret == NOTIFY_BAD)
> +			goto out;
> +
> +		/*
> +		 * Check the timeout with irqs disabled. This is so we don't get
> +		 * preempted after checking the timeout. That could cause things
> +		 * like garbage values for the display if we change the DDR rate
> +		 * at the wrong time.
> +		 */
> +		local_irq_disable();
> +		if (ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
> +				timeout)) {
> +			local_irq_enable();
> +			continue;
> +		}
> +
> +		arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, ddrclk->cached_rate, 0,
> +			      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
> +			      0, 0, 0, 0, &res);
> +		local_irq_enable();
> +		break;
> +	}
> +out:
> +	mutex_unlock(&ddrclk->lock);
> +}
>  
> -	spin_lock_irqsave(ddrclk->lock, flags);
> -	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
> -		      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
> -		      0, 0, 0, 0, &res);
> -	spin_unlock_irqrestore(ddrclk->lock, flags);
> +int rockchip_ddrclk_register_sync_nb(struct clk *clk, struct notifier_block *nb)
> +{
> +	struct clk_hw *hw = __clk_get_hw(clk);
> +	struct rockchip_ddrclk *ddrclk;
> +	int ret;
>  
> -	return res.a0;
> +	if (!hw || !nb)
> +		return -EINVAL;
> +
> +	ddrclk = to_rockchip_ddrclk_hw(hw);
> +	if (!ddrclk)
> +		return -EINVAL;
> +
> +	mutex_lock(&ddrclk->lock);
> +	ret = raw_notifier_chain_register(&ddrclk->sync_chain, nb);
> +	mutex_unlock(&ddrclk->lock);
> +
> +	return ret;
>  }
> +EXPORT_SYMBOL_GPL(rockchip_ddrclk_register_sync_nb);
>  
> -static unsigned long
> -rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
> -				unsigned long parent_rate)
> +int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
> +				       struct notifier_block *nb)
>  {
> -	struct arm_smccc_res res;
> +	struct clk_hw *hw = __clk_get_hw(clk);
> +	struct rockchip_ddrclk *ddrclk;
> +	int ret;
>  
> -	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
> -		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
> -		      0, 0, 0, 0, &res);
> +	if (!hw || !nb)
> +		return -EINVAL;
>  
> -	return res.a0;
> +	ddrclk = to_rockchip_ddrclk_hw(hw);
> +	if (!ddrclk)
> +		return -EINVAL;
> +
> +	mutex_lock(&ddrclk->lock);
> +	ret = raw_notifier_chain_unregister(&ddrclk->sync_chain, nb);
> +	mutex_unlock(&ddrclk->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_ddrclk_unregister_sync_nb);
> +
> +void rockchip_ddrclk_wait_set_rate(struct clk *clk)
> +{
> +	struct clk_hw *hw = __clk_get_hw(clk);
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +
> +	flush_work(&ddrclk->set_rate_work);
>  }
> +EXPORT_SYMBOL_GPL(rockchip_ddrclk_wait_set_rate);
>  
>  static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
>  					   unsigned long rate,
> @@ -77,6 +144,30 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
>  	return res.a0;
>  }
>  
> +static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
> +					unsigned long prate)
> +{
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +	long rate;
> +
> +	rate = rockchip_ddrclk_sip_round_rate(hw, drate, &prate);
> +	if (rate < 0)
> +		return rate;
> +
> +	ddrclk->cached_rate = rate;
> +	queue_work(system_highpri_wq, &ddrclk->set_rate_work);
> +	return 0;
> +}
> +
> +static unsigned long
> +rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
> +				unsigned long parent_rate)
> +{
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +
> +	return ddrclk->cached_rate;
> +}
> +
>  static u8 rockchip_ddrclk_get_parent(struct clk_hw *hw)
>  {
>  	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> @@ -105,12 +196,12 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  					 u8 num_parents, int mux_offset,
>  					 int mux_shift, int mux_width,
>  					 int div_shift, int div_width,
> -					 int ddr_flag, void __iomem *reg_base,
> -					 spinlock_t *lock)
> +					 int ddr_flag, void __iomem *reg_base)
>  {
>  	struct rockchip_ddrclk *ddrclk;
>  	struct clk_init_data init;
>  	struct clk *clk;
> +	struct arm_smccc_res res;
>  
>  	ddrclk = kzalloc(sizeof(*ddrclk), GFP_KERNEL);
>  	if (!ddrclk)
> @@ -134,7 +225,6 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  	}
>  
>  	ddrclk->reg_base = reg_base;
> -	ddrclk->lock = lock;
>  	ddrclk->hw.init = &init;
>  	ddrclk->mux_offset = mux_offset;
>  	ddrclk->mux_shift = mux_shift;
> @@ -142,6 +232,14 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  	ddrclk->div_shift = div_shift;
>  	ddrclk->div_width = div_width;
>  	ddrclk->ddr_flag = ddr_flag;
> +	mutex_init(&ddrclk->lock);
> +	INIT_WORK(&ddrclk->set_rate_work, rockchip_ddrclk_set_rate_func);
> +	RAW_INIT_NOTIFIER_HEAD(&ddrclk->sync_chain);
> +
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
> +		      0, 0, 0, 0, &res);
> +	ddrclk->cached_rate = res.a0;
>  
>  	clk = clk_register(NULL, &ddrclk->hw);
>  	if (IS_ERR(clk))
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 326b3fa44f5d..1b7775aff191 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -541,7 +541,7 @@ void __init rockchip_clk_register_branches(
>  				list->muxdiv_offset, list->mux_shift,
>  				list->mux_width, list->div_shift,
>  				list->div_width, list->div_flags,
> -				ctx->reg_base, &ctx->lock);
> +				ctx->reg_base);
>  			break;
>  		}
>  
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index ef601dded32c..5e4ce49ef337 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -326,8 +326,7 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  					 u8 num_parents, int mux_offset,
>  					 int mux_shift, int mux_width,
>  					 int div_shift, int div_width,
> -					 int ddr_flags, void __iomem *reg_base,
> -					 spinlock_t *lock);
> +					 int ddr_flags, void __iomem *reg_base);
>  
>  #define ROCKCHIP_INVERTER_HIWORD_MASK	BIT(0)
>  
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index c619dc4ac620..b4ac9ee605be 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -28,9 +28,12 @@
>  #include <linux/rwsem.h>
>  #include <linux/suspend.h>
>  
> +#include <soc/rockchip/rk3399_dmc.h>
>  #include <soc/rockchip/rk3399_grf.h>
>  #include <soc/rockchip/rockchip_sip.h>
>  
> +#include "rk3399_dmc_priv.h"
> +
>  struct dram_timing {
>  	unsigned int ddr3_speed_bin;
>  	unsigned int pd_idle;
> @@ -70,6 +73,7 @@ struct rk3399_dmcfreq {
>  	struct clk *dmc_clk;
>  	struct devfreq_event_dev *edev;
>  	struct mutex lock;
> +	struct mutex en_lock;
>  	struct dram_timing timing;
>  	struct regulator *vdd_center;
>  	struct regmap *regmap_pmu;
> @@ -77,6 +81,8 @@ struct rk3399_dmcfreq {
>  	unsigned long volt, target_volt;
>  	unsigned int odt_dis_freq;
>  	int odt_pd_arg0, odt_pd_arg1;
> +	int num_sync_nb;
> +	int disable_count;
>  };
>  
>  static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> @@ -139,6 +145,13 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>  		goto out;
>  	}
>  
> +	/*
> +	 * Setting the dpll is asynchronous since clk_set_rate grabs a global
> +	 * common clk lock and set_rate for the dpll takes up to one display
> +	 * frame to complete. We still need to wait for the set_rate to complete
> +	 * here, though, before we change voltage.
> +	 */
> +	rockchip_ddrclk_wait_set_rate(dmcfreq->dmc_clk);
>  	/*
>  	 * Check the dpll rate,
>  	 * There only two result we will get,
> @@ -205,40 +218,15 @@ static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
>  static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
>  {
>  	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
> -	int ret = 0;
> -
> -	ret = devfreq_event_disable_edev(dmcfreq->edev);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to disable the devfreq-event devices\n");
> -		return ret;
> -	}
>  
> -	ret = devfreq_suspend_device(dmcfreq->devfreq);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to suspend the devfreq devices\n");
> -		return ret;
> -	}
> -
> -	return 0;
> +	return rockchip_dmcfreq_block(dmcfreq->devfreq);
>  }
>  
>  static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
>  {
>  	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
> -	int ret = 0;
>  
> -	ret = devfreq_event_enable_edev(dmcfreq->edev);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to enable the devfreq-event devices\n");
> -		return ret;
> -	}
> -
> -	ret = devfreq_resume_device(dmcfreq->devfreq);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to resume the devfreq devices\n");
> -		return ret;
> -	}
> -	return ret;
> +	return rockchip_dmcfreq_unblock(dmcfreq->devfreq);
>  }
>  
>  static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
> @@ -311,6 +299,88 @@ static int of_get_ddr_timings(struct dram_timing *timing,
>  	return ret;
>  }
>  
> +int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
> +					struct notifier_block *nb)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	/*
> +	 * We have a short amount of time (~1ms or less typically) to run
> +	 * dmcfreq after we sync with the notifier, so syncing with more than
> +	 * one notifier is not generally possible. Thus, if more than one sync
> +	 * notifier is registered, disable dmcfreq.
> +	 */
> +	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
> +		devfreq_suspend_device(devfreq);
> +
> +	ret = rockchip_ddrclk_register_sync_nb(dmcfreq->dmc_clk, nb);
> +	if (ret == 0)
> +		dmcfreq->num_sync_nb++;
> +	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
> +		devfreq_resume_device(devfreq);
> +
> +	mutex_unlock(&dmcfreq->en_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_register_clk_sync_nb);
> +
> +int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
> +					  struct notifier_block *nb)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	ret = rockchip_ddrclk_unregister_sync_nb(dmcfreq->dmc_clk, nb);
> +	if (ret == 0) {
> +		dmcfreq->num_sync_nb--;
> +		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
> +			devfreq_resume_device(devfreq);
> +	}
> +
> +	mutex_unlock(&dmcfreq->en_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unregister_clk_sync_nb);
> +
> +int rockchip_dmcfreq_block(struct devfreq *devfreq)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret = 0;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0)
> +		ret = devfreq_suspend_device(devfreq);
> +
> +	if (!ret)
> +		dmcfreq->disable_count++;
> +	mutex_unlock(&dmcfreq->en_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_block);
> +
> +int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret = 0;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1)
> +		ret = devfreq_resume_device(devfreq);
> +
> +	if (!ret)
> +		dmcfreq->disable_count--;
> +	WARN_ON(dmcfreq->disable_count < 0);
> +	mutex_unlock(&dmcfreq->en_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unblock);
> +
>  static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  {
>  	struct arm_smccc_res res;
> @@ -328,6 +398,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	mutex_init(&data->lock);
> +	mutex_init(&data->en_lock);
>  
>  	data->vdd_center = devm_regulator_get(dev, "center");
>  	if (IS_ERR(data->vdd_center)) {
> diff --git a/drivers/devfreq/rk3399_dmc_priv.h b/drivers/devfreq/rk3399_dmc_priv.h
> new file mode 100644
> index 000000000000..8ac0340a4990
> --- /dev/null
> +++ b/drivers/devfreq/rk3399_dmc_priv.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2017-2018 Google, Inc.
> + * Author: Derek Basehore <dbasehore@chromium.org>
> + */
> +
> +#ifndef __RK3399_DMC_PRIV_H
> +#define __RK3399_DMC_PRIV_H
> +
> +void rockchip_ddrclk_wait_set_rate(struct clk *clk);
> +int rockchip_ddrclk_register_sync_nb(struct clk *clk,
> +				     struct notifier_block *nb);
> +int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
> +				       struct notifier_block *nb);
> +#endif
> diff --git a/include/soc/rockchip/rk3399_dmc.h b/include/soc/rockchip/rk3399_dmc.h
> new file mode 100644
> index 000000000000..8b28563710d1
> --- /dev/null
> +++ b/include/soc/rockchip/rk3399_dmc.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#ifndef __SOC_RK3399_DMC_H
> +#define __SOC_RK3399_DMC_H
> +
> +#include <linux/devfreq.h>
> +#include <linux/notifier.h>
> +
> +#define DMC_MIN_SET_RATE_NS	(250 * NSEC_PER_USEC)
> +#define DMC_MIN_VBLANK_NS	(DMC_MIN_SET_RATE_NS + 50 * NSEC_PER_USEC)
> +
> +#if IS_ENABLED(CONFIG_ARM_RK3399_DMC_DEVFREQ)
> +int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
> +					  struct notifier_block *nb);
> +
> +int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
> +					    struct notifier_block *nb);
> +
> +int rockchip_dmcfreq_block(struct devfreq *devfreq);
> +
> +int rockchip_dmcfreq_unblock(struct devfreq *devfreq);
> +#else
> +static inline int
> +rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
> +				      struct notifier_block *nb)
> +{ return 0; }
> +
> +static inline int
> +rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
> +					struct notifier_block *nb)
> +{ return 0; }
> +
> +static inline int rockchip_dmcfreq_block(struct devfreq *devfreq) { return 0; }
> +
> +static inline int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
> +{ return 0; }
> +#endif
> +#endif
> 

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

* [PATCH 4/8] devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel for DDRfreq.
@ 2018-08-02  2:35     ` Chanwoo Choi
  0 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2018-08-02  2:35 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Enric,

I'm not sure this approach is right. Actually, I don't prefer
the definitions of specific functions for only some device drivers.
This approach connect with device drivers without subsystem.
It looks like the direct call function.

Generally, all device driver in linux kernel have to
use the provided API from subsystem as following:
[A subsystem] --------------------- [B subsystem]
       |                                  |
[A device driver]                   [B device driver]


But, this patch looks like following connection.
I think that it might make the complicated codes.
We cannot develop the all device drivers with direct call function
without standard subsystem interface.

[A subsystem] --------------------- [B subsystem]
       |                                  |
[A device driver] ----------------- [B device driver]


Regards,
Chanwoo Choi


On 2018? 07? 30? 17:11, Enric Balletbo i Serra wrote:
> From: Derek Basehore <dbasehore@chromium.org>
> 
> This changes the kernel to sync with vblank for the display in the
> kernel. This was done in Trusted Firmware-A (TF-A) before, but that locks
> up one CPU for up to one display frame (1/60 second). That's bad for
> performance and power, so this moves waiting to the kernel where the
> waiting thread can properly wait on a completion.
> 
> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes in v1: None
> 
>  drivers/clk/rockchip/clk-ddr.c    | 142 +++++++++++++++++++++++++-----
>  drivers/clk/rockchip/clk.c        |   2 +-
>  drivers/clk/rockchip/clk.h        |   3 +-
>  drivers/devfreq/rk3399_dmc.c      | 125 ++++++++++++++++++++------
>  drivers/devfreq/rk3399_dmc_priv.h |  15 ++++
>  include/soc/rockchip/rk3399_dmc.h |  42 +++++++++
>  6 files changed, 277 insertions(+), 52 deletions(-)
>  create mode 100644 drivers/devfreq/rk3399_dmc_priv.h
>  create mode 100644 include/soc/rockchip/rk3399_dmc.h
> 
> diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
> index e8075359366b..707be1bd8910 100644
> --- a/drivers/clk/rockchip/clk-ddr.c
> +++ b/drivers/clk/rockchip/clk-ddr.c
> @@ -17,7 +17,9 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/ktime.h>
>  #include <linux/slab.h>
> +#include <soc/rockchip/rk3399_dmc.h>
>  #include <soc/rockchip/rockchip_sip.h>
>  #include "clk.h"
>  
> @@ -30,39 +32,104 @@ struct rockchip_ddrclk {
>  	int		div_shift;
>  	int		div_width;
>  	int		ddr_flag;
> -	spinlock_t	*lock;
> +	unsigned long	cached_rate;
> +	struct work_struct set_rate_work;
> +	struct mutex	lock;
> +	struct raw_notifier_head sync_chain;
>  };
>  
>  #define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw)
> +#define DMC_DEFAULT_TIMEOUT_NS		NSEC_PER_SEC
> +#define DDRCLK_SET_RATE_MAX_RETRIES	3
>  
> -static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
> -					unsigned long prate)
> +static void rockchip_ddrclk_set_rate_func(struct work_struct *work)
>  {
> -	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> -	unsigned long flags;
> +	struct rockchip_ddrclk *ddrclk = container_of(work,
> +			struct rockchip_ddrclk, set_rate_work);
> +	ktime_t timeout = ktime_add_ns(ktime_get(), DMC_DEFAULT_TIMEOUT_NS);
>  	struct arm_smccc_res res;
> +	int ret, i;
> +
> +	mutex_lock(&ddrclk->lock);
> +	for (i = 0; i < DDRCLK_SET_RATE_MAX_RETRIES; i++) {
> +		ret = raw_notifier_call_chain(&ddrclk->sync_chain, 0, &timeout);
> +		if (ret == NOTIFY_BAD)
> +			goto out;
> +
> +		/*
> +		 * Check the timeout with irqs disabled. This is so we don't get
> +		 * preempted after checking the timeout. That could cause things
> +		 * like garbage values for the display if we change the DDR rate
> +		 * at the wrong time.
> +		 */
> +		local_irq_disable();
> +		if (ktime_after(ktime_add_ns(ktime_get(), DMC_MIN_VBLANK_NS),
> +				timeout)) {
> +			local_irq_enable();
> +			continue;
> +		}
> +
> +		arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, ddrclk->cached_rate, 0,
> +			      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
> +			      0, 0, 0, 0, &res);
> +		local_irq_enable();
> +		break;
> +	}
> +out:
> +	mutex_unlock(&ddrclk->lock);
> +}
>  
> -	spin_lock_irqsave(ddrclk->lock, flags);
> -	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
> -		      ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
> -		      0, 0, 0, 0, &res);
> -	spin_unlock_irqrestore(ddrclk->lock, flags);
> +int rockchip_ddrclk_register_sync_nb(struct clk *clk, struct notifier_block *nb)
> +{
> +	struct clk_hw *hw = __clk_get_hw(clk);
> +	struct rockchip_ddrclk *ddrclk;
> +	int ret;
>  
> -	return res.a0;
> +	if (!hw || !nb)
> +		return -EINVAL;
> +
> +	ddrclk = to_rockchip_ddrclk_hw(hw);
> +	if (!ddrclk)
> +		return -EINVAL;
> +
> +	mutex_lock(&ddrclk->lock);
> +	ret = raw_notifier_chain_register(&ddrclk->sync_chain, nb);
> +	mutex_unlock(&ddrclk->lock);
> +
> +	return ret;
>  }
> +EXPORT_SYMBOL_GPL(rockchip_ddrclk_register_sync_nb);
>  
> -static unsigned long
> -rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
> -				unsigned long parent_rate)
> +int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
> +				       struct notifier_block *nb)
>  {
> -	struct arm_smccc_res res;
> +	struct clk_hw *hw = __clk_get_hw(clk);
> +	struct rockchip_ddrclk *ddrclk;
> +	int ret;
>  
> -	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
> -		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
> -		      0, 0, 0, 0, &res);
> +	if (!hw || !nb)
> +		return -EINVAL;
>  
> -	return res.a0;
> +	ddrclk = to_rockchip_ddrclk_hw(hw);
> +	if (!ddrclk)
> +		return -EINVAL;
> +
> +	mutex_lock(&ddrclk->lock);
> +	ret = raw_notifier_chain_unregister(&ddrclk->sync_chain, nb);
> +	mutex_unlock(&ddrclk->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_ddrclk_unregister_sync_nb);
> +
> +void rockchip_ddrclk_wait_set_rate(struct clk *clk)
> +{
> +	struct clk_hw *hw = __clk_get_hw(clk);
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +
> +	flush_work(&ddrclk->set_rate_work);
>  }
> +EXPORT_SYMBOL_GPL(rockchip_ddrclk_wait_set_rate);
>  
>  static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
>  					   unsigned long rate,
> @@ -77,6 +144,30 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
>  	return res.a0;
>  }
>  
> +static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, unsigned long drate,
> +					unsigned long prate)
> +{
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +	long rate;
> +
> +	rate = rockchip_ddrclk_sip_round_rate(hw, drate, &prate);
> +	if (rate < 0)
> +		return rate;
> +
> +	ddrclk->cached_rate = rate;
> +	queue_work(system_highpri_wq, &ddrclk->set_rate_work);
> +	return 0;
> +}
> +
> +static unsigned long
> +rockchip_ddrclk_sip_recalc_rate(struct clk_hw *hw,
> +				unsigned long parent_rate)
> +{
> +	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> +
> +	return ddrclk->cached_rate;
> +}
> +
>  static u8 rockchip_ddrclk_get_parent(struct clk_hw *hw)
>  {
>  	struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
> @@ -105,12 +196,12 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  					 u8 num_parents, int mux_offset,
>  					 int mux_shift, int mux_width,
>  					 int div_shift, int div_width,
> -					 int ddr_flag, void __iomem *reg_base,
> -					 spinlock_t *lock)
> +					 int ddr_flag, void __iomem *reg_base)
>  {
>  	struct rockchip_ddrclk *ddrclk;
>  	struct clk_init_data init;
>  	struct clk *clk;
> +	struct arm_smccc_res res;
>  
>  	ddrclk = kzalloc(sizeof(*ddrclk), GFP_KERNEL);
>  	if (!ddrclk)
> @@ -134,7 +225,6 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  	}
>  
>  	ddrclk->reg_base = reg_base;
> -	ddrclk->lock = lock;
>  	ddrclk->hw.init = &init;
>  	ddrclk->mux_offset = mux_offset;
>  	ddrclk->mux_shift = mux_shift;
> @@ -142,6 +232,14 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  	ddrclk->div_shift = div_shift;
>  	ddrclk->div_width = div_width;
>  	ddrclk->ddr_flag = ddr_flag;
> +	mutex_init(&ddrclk->lock);
> +	INIT_WORK(&ddrclk->set_rate_work, rockchip_ddrclk_set_rate_func);
> +	RAW_INIT_NOTIFIER_HEAD(&ddrclk->sync_chain);
> +
> +	arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
> +		      ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
> +		      0, 0, 0, 0, &res);
> +	ddrclk->cached_rate = res.a0;
>  
>  	clk = clk_register(NULL, &ddrclk->hw);
>  	if (IS_ERR(clk))
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 326b3fa44f5d..1b7775aff191 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -541,7 +541,7 @@ void __init rockchip_clk_register_branches(
>  				list->muxdiv_offset, list->mux_shift,
>  				list->mux_width, list->div_shift,
>  				list->div_width, list->div_flags,
> -				ctx->reg_base, &ctx->lock);
> +				ctx->reg_base);
>  			break;
>  		}
>  
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index ef601dded32c..5e4ce49ef337 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -326,8 +326,7 @@ struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,
>  					 u8 num_parents, int mux_offset,
>  					 int mux_shift, int mux_width,
>  					 int div_shift, int div_width,
> -					 int ddr_flags, void __iomem *reg_base,
> -					 spinlock_t *lock);
> +					 int ddr_flags, void __iomem *reg_base);
>  
>  #define ROCKCHIP_INVERTER_HIWORD_MASK	BIT(0)
>  
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index c619dc4ac620..b4ac9ee605be 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -28,9 +28,12 @@
>  #include <linux/rwsem.h>
>  #include <linux/suspend.h>
>  
> +#include <soc/rockchip/rk3399_dmc.h>
>  #include <soc/rockchip/rk3399_grf.h>
>  #include <soc/rockchip/rockchip_sip.h>
>  
> +#include "rk3399_dmc_priv.h"
> +
>  struct dram_timing {
>  	unsigned int ddr3_speed_bin;
>  	unsigned int pd_idle;
> @@ -70,6 +73,7 @@ struct rk3399_dmcfreq {
>  	struct clk *dmc_clk;
>  	struct devfreq_event_dev *edev;
>  	struct mutex lock;
> +	struct mutex en_lock;
>  	struct dram_timing timing;
>  	struct regulator *vdd_center;
>  	struct regmap *regmap_pmu;
> @@ -77,6 +81,8 @@ struct rk3399_dmcfreq {
>  	unsigned long volt, target_volt;
>  	unsigned int odt_dis_freq;
>  	int odt_pd_arg0, odt_pd_arg1;
> +	int num_sync_nb;
> +	int disable_count;
>  };
>  
>  static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> @@ -139,6 +145,13 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>  		goto out;
>  	}
>  
> +	/*
> +	 * Setting the dpll is asynchronous since clk_set_rate grabs a global
> +	 * common clk lock and set_rate for the dpll takes up to one display
> +	 * frame to complete. We still need to wait for the set_rate to complete
> +	 * here, though, before we change voltage.
> +	 */
> +	rockchip_ddrclk_wait_set_rate(dmcfreq->dmc_clk);
>  	/*
>  	 * Check the dpll rate,
>  	 * There only two result we will get,
> @@ -205,40 +218,15 @@ static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
>  static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
>  {
>  	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
> -	int ret = 0;
> -
> -	ret = devfreq_event_disable_edev(dmcfreq->edev);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to disable the devfreq-event devices\n");
> -		return ret;
> -	}
>  
> -	ret = devfreq_suspend_device(dmcfreq->devfreq);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to suspend the devfreq devices\n");
> -		return ret;
> -	}
> -
> -	return 0;
> +	return rockchip_dmcfreq_block(dmcfreq->devfreq);
>  }
>  
>  static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
>  {
>  	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
> -	int ret = 0;
>  
> -	ret = devfreq_event_enable_edev(dmcfreq->edev);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to enable the devfreq-event devices\n");
> -		return ret;
> -	}
> -
> -	ret = devfreq_resume_device(dmcfreq->devfreq);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to resume the devfreq devices\n");
> -		return ret;
> -	}
> -	return ret;
> +	return rockchip_dmcfreq_unblock(dmcfreq->devfreq);
>  }
>  
>  static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
> @@ -311,6 +299,88 @@ static int of_get_ddr_timings(struct dram_timing *timing,
>  	return ret;
>  }
>  
> +int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
> +					struct notifier_block *nb)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	/*
> +	 * We have a short amount of time (~1ms or less typically) to run
> +	 * dmcfreq after we sync with the notifier, so syncing with more than
> +	 * one notifier is not generally possible. Thus, if more than one sync
> +	 * notifier is registered, disable dmcfreq.
> +	 */
> +	if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
> +		devfreq_suspend_device(devfreq);
> +
> +	ret = rockchip_ddrclk_register_sync_nb(dmcfreq->dmc_clk, nb);
> +	if (ret == 0)
> +		dmcfreq->num_sync_nb++;
> +	else if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
> +		devfreq_resume_device(devfreq);
> +
> +	mutex_unlock(&dmcfreq->en_lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_register_clk_sync_nb);
> +
> +int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
> +					  struct notifier_block *nb)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	ret = rockchip_ddrclk_unregister_sync_nb(dmcfreq->dmc_clk, nb);
> +	if (ret == 0) {
> +		dmcfreq->num_sync_nb--;
> +		if (dmcfreq->num_sync_nb == 1 && dmcfreq->disable_count <= 0)
> +			devfreq_resume_device(devfreq);
> +	}
> +
> +	mutex_unlock(&dmcfreq->en_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unregister_clk_sync_nb);
> +
> +int rockchip_dmcfreq_block(struct devfreq *devfreq)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret = 0;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count <= 0)
> +		ret = devfreq_suspend_device(devfreq);
> +
> +	if (!ret)
> +		dmcfreq->disable_count++;
> +	mutex_unlock(&dmcfreq->en_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_block);
> +
> +int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
> +{
> +	struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(devfreq->dev.parent);
> +	int ret = 0;
> +
> +	mutex_lock(&dmcfreq->en_lock);
> +	if (dmcfreq->num_sync_nb <= 1 && dmcfreq->disable_count == 1)
> +		ret = devfreq_resume_device(devfreq);
> +
> +	if (!ret)
> +		dmcfreq->disable_count--;
> +	WARN_ON(dmcfreq->disable_count < 0);
> +	mutex_unlock(&dmcfreq->en_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(rockchip_dmcfreq_unblock);
> +
>  static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  {
>  	struct arm_smccc_res res;
> @@ -328,6 +398,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	mutex_init(&data->lock);
> +	mutex_init(&data->en_lock);
>  
>  	data->vdd_center = devm_regulator_get(dev, "center");
>  	if (IS_ERR(data->vdd_center)) {
> diff --git a/drivers/devfreq/rk3399_dmc_priv.h b/drivers/devfreq/rk3399_dmc_priv.h
> new file mode 100644
> index 000000000000..8ac0340a4990
> --- /dev/null
> +++ b/drivers/devfreq/rk3399_dmc_priv.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2017-2018 Google, Inc.
> + * Author: Derek Basehore <dbasehore@chromium.org>
> + */
> +
> +#ifndef __RK3399_DMC_PRIV_H
> +#define __RK3399_DMC_PRIV_H
> +
> +void rockchip_ddrclk_wait_set_rate(struct clk *clk);
> +int rockchip_ddrclk_register_sync_nb(struct clk *clk,
> +				     struct notifier_block *nb);
> +int rockchip_ddrclk_unregister_sync_nb(struct clk *clk,
> +				       struct notifier_block *nb);
> +#endif
> diff --git a/include/soc/rockchip/rk3399_dmc.h b/include/soc/rockchip/rk3399_dmc.h
> new file mode 100644
> index 000000000000..8b28563710d1
> --- /dev/null
> +++ b/include/soc/rockchip/rk3399_dmc.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#ifndef __SOC_RK3399_DMC_H
> +#define __SOC_RK3399_DMC_H
> +
> +#include <linux/devfreq.h>
> +#include <linux/notifier.h>
> +
> +#define DMC_MIN_SET_RATE_NS	(250 * NSEC_PER_USEC)
> +#define DMC_MIN_VBLANK_NS	(DMC_MIN_SET_RATE_NS + 50 * NSEC_PER_USEC)
> +
> +#if IS_ENABLED(CONFIG_ARM_RK3399_DMC_DEVFREQ)
> +int rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
> +					  struct notifier_block *nb);
> +
> +int rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
> +					    struct notifier_block *nb);
> +
> +int rockchip_dmcfreq_block(struct devfreq *devfreq);
> +
> +int rockchip_dmcfreq_unblock(struct devfreq *devfreq);
> +#else
> +static inline int
> +rockchip_dmcfreq_register_clk_sync_nb(struct devfreq *devfreq,
> +				      struct notifier_block *nb)
> +{ return 0; }
> +
> +static inline int
> +rockchip_dmcfreq_unregister_clk_sync_nb(struct devfreq *devfreq,
> +					struct notifier_block *nb)
> +{ return 0; }
> +
> +static inline int rockchip_dmcfreq_block(struct devfreq *devfreq) { return 0; }
> +
> +static inline int rockchip_dmcfreq_unblock(struct devfreq *devfreq)
> +{ return 0; }
> +#endif
> +#endif
> 

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

* Re: [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place.
  2018-08-01  8:36     ` Chanwoo Choi
@ 2018-08-07 10:50       ` Enric Balletbo i Serra
  -1 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-08-07 10:50 UTC (permalink / raw)
  To: Chanwoo Choi, MyungJoo Ham, Kyungmin Park, Rob Herring,
	Will Deacon, Heiko Stuebner, Michael Turquette, Stephen Boyd,
	Sandy Huang, David Airlie
  Cc: linux-pm, linux-kernel, Derek Basehore, linux-clk,
	linux-rockchip, dri-devel, Lin Huang, kernel, robin.murphy,
	Sean Paul, linux-arm-kernel

Hi,

On 01/08/18 10:36, Chanwoo Choi wrote:
> Hi Enric,
> 
> On 2018년 07월 30일 17:11, Enric Balletbo i Serra wrote:
>> Some rk3399 GRF (Generic Register Files) definitions can be used for
>> different drivers. Move these definitions to a common include so we
>> don't need to duplicate these definitions.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> Changes in v1:
>> - [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> 
> Even if you add the changes log, you are missing my Acked-by tag.
> 

Ops, right, sorry about that. I'll fix in next version.

>> - [RFC 1/10] s/Generic/General/ (Robin Murphy)
>> - [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
>> - [RFC 7/10] Removed from the series. I did not found a use case where this matters.
>>
>>  drivers/devfreq/event/rockchip-dfi.c | 23 +++++++----------------
>>  include/soc/rockchip/rk3399_grf.h    | 21 +++++++++++++++++++++
>>  2 files changed, 28 insertions(+), 16 deletions(-)
>>  create mode 100644 include/soc/rockchip/rk3399_grf.h
>>
>> diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
>> index 22b113363ffc..2fbbcbeb644f 100644
>> --- a/drivers/devfreq/event/rockchip-dfi.c
>> +++ b/drivers/devfreq/event/rockchip-dfi.c
>> @@ -26,6 +26,8 @@
>>  #include <linux/list.h>
>>  #include <linux/of.h>
>>  
>> +#include <soc/rockchip/rk3399_grf.h>
>> +
>>  #define RK3399_DMC_NUM_CH	2
>>  
>>  /* DDRMON_CTRL */
>> @@ -43,18 +45,6 @@
>>  #define DDRMON_CH1_COUNT_NUM		0x3c
>>  #define DDRMON_CH1_DFI_ACCESS_NUM	0x40
>>  
>> -/* pmu grf */
>> -#define PMUGRF_OS_REG2	0x308
>> -#define DDRTYPE_SHIFT	13
>> -#define DDRTYPE_MASK	7
>> -
>> -enum {
>> -	DDR3 = 3,
>> -	LPDDR3 = 6,
>> -	LPDDR4 = 7,
>> -	UNUSED = 0xFF
>> -};
>> -
>>  struct dmc_usage {
>>  	u32 access;
>>  	u32 total;
>> @@ -83,16 +73,17 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev)
>>  	u32 ddr_type;
>>  
>>  	/* get ddr type */
>> -	regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
>> -	ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
>> +	regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
>> +	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
>> +		    RK3399_PMUGRF_DDRTYPE_MASK;
>>  
>>  	/* clear DDRMON_CTRL setting */
>>  	writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
>>  
>>  	/* set ddr type to dfi */
>> -	if (ddr_type == LPDDR3)
>> +	if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3)
>>  		writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
>> -	else if (ddr_type == LPDDR4)
>> +	else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4)
>>  		writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
>>  
>>  	/* enable count, use software mode */
>> diff --git a/include/soc/rockchip/rk3399_grf.h b/include/soc/rockchip/rk3399_grf.h
>> new file mode 100644
>> index 000000000000..3eebabcb2812
>> --- /dev/null
>> +++ b/include/soc/rockchip/rk3399_grf.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Rockchip General Register Files definitions
>> + *
>> + * Copyright (c) 2018, Collabora Ltd.
>> + * Author: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> + */
>> +
>> +#ifndef __SOC_RK3399_GRF_H
>> +#define __SOC_RK3399_GRF_H
>> +
>> +/* PMU GRF Registers */
>> +#define RK3399_PMUGRF_OS_REG2		0x308
>> +#define RK3399_PMUGRF_DDRTYPE_SHIFT	13
>> +#define RK3399_PMUGRF_DDRTYPE_MASK	7
>> +#define RK3399_PMUGRF_DDRTYPE_DDR3	3
>> +#define RK3399_PMUGRF_DDRTYPE_LPDDR2	5
>> +#define RK3399_PMUGRF_DDRTYPE_LPDDR3	6
>> +#define RK3399_PMUGRF_DDRTYPE_LPDDR4	7
>> +
>> +#endif
>>
> 
> 

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

* [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place.
@ 2018-08-07 10:50       ` Enric Balletbo i Serra
  0 siblings, 0 replies; 31+ messages in thread
From: Enric Balletbo i Serra @ 2018-08-07 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 01/08/18 10:36, Chanwoo Choi wrote:
> Hi Enric,
> 
> On 2018? 07? 30? 17:11, Enric Balletbo i Serra wrote:
>> Some rk3399 GRF (Generic Register Files) definitions can be used for
>> different drivers. Move these definitions to a common include so we
>> don't need to duplicate these definitions.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> Changes in v1:
>> - [RFC 1/10] Add Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> 
> Even if you add the changes log, you are missing my Acked-by tag.
> 

Ops, right, sorry about that. I'll fix in next version.

>> - [RFC 1/10] s/Generic/General/ (Robin Murphy)
>> - [RFC 4/10] Removed from the series. I did not found a use case where not holding the mutex causes the issue.
>> - [RFC 7/10] Removed from the series. I did not found a use case where this matters.
>>
>>  drivers/devfreq/event/rockchip-dfi.c | 23 +++++++----------------
>>  include/soc/rockchip/rk3399_grf.h    | 21 +++++++++++++++++++++
>>  2 files changed, 28 insertions(+), 16 deletions(-)
>>  create mode 100644 include/soc/rockchip/rk3399_grf.h
>>
>> diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
>> index 22b113363ffc..2fbbcbeb644f 100644
>> --- a/drivers/devfreq/event/rockchip-dfi.c
>> +++ b/drivers/devfreq/event/rockchip-dfi.c
>> @@ -26,6 +26,8 @@
>>  #include <linux/list.h>
>>  #include <linux/of.h>
>>  
>> +#include <soc/rockchip/rk3399_grf.h>
>> +
>>  #define RK3399_DMC_NUM_CH	2
>>  
>>  /* DDRMON_CTRL */
>> @@ -43,18 +45,6 @@
>>  #define DDRMON_CH1_COUNT_NUM		0x3c
>>  #define DDRMON_CH1_DFI_ACCESS_NUM	0x40
>>  
>> -/* pmu grf */
>> -#define PMUGRF_OS_REG2	0x308
>> -#define DDRTYPE_SHIFT	13
>> -#define DDRTYPE_MASK	7
>> -
>> -enum {
>> -	DDR3 = 3,
>> -	LPDDR3 = 6,
>> -	LPDDR4 = 7,
>> -	UNUSED = 0xFF
>> -};
>> -
>>  struct dmc_usage {
>>  	u32 access;
>>  	u32 total;
>> @@ -83,16 +73,17 @@ static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev)
>>  	u32 ddr_type;
>>  
>>  	/* get ddr type */
>> -	regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
>> -	ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
>> +	regmap_read(info->regmap_pmu, RK3399_PMUGRF_OS_REG2, &val);
>> +	ddr_type = (val >> RK3399_PMUGRF_DDRTYPE_SHIFT) &
>> +		    RK3399_PMUGRF_DDRTYPE_MASK;
>>  
>>  	/* clear DDRMON_CTRL setting */
>>  	writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
>>  
>>  	/* set ddr type to dfi */
>> -	if (ddr_type == LPDDR3)
>> +	if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR3)
>>  		writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
>> -	else if (ddr_type == LPDDR4)
>> +	else if (ddr_type == RK3399_PMUGRF_DDRTYPE_LPDDR4)
>>  		writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
>>  
>>  	/* enable count, use software mode */
>> diff --git a/include/soc/rockchip/rk3399_grf.h b/include/soc/rockchip/rk3399_grf.h
>> new file mode 100644
>> index 000000000000..3eebabcb2812
>> --- /dev/null
>> +++ b/include/soc/rockchip/rk3399_grf.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Rockchip General Register Files definitions
>> + *
>> + * Copyright (c) 2018, Collabora Ltd.
>> + * Author: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> + */
>> +
>> +#ifndef __SOC_RK3399_GRF_H
>> +#define __SOC_RK3399_GRF_H
>> +
>> +/* PMU GRF Registers */
>> +#define RK3399_PMUGRF_OS_REG2		0x308
>> +#define RK3399_PMUGRF_DDRTYPE_SHIFT	13
>> +#define RK3399_PMUGRF_DDRTYPE_MASK	7
>> +#define RK3399_PMUGRF_DDRTYPE_DDR3	3
>> +#define RK3399_PMUGRF_DDRTYPE_LPDDR2	5
>> +#define RK3399_PMUGRF_DDRTYPE_LPDDR3	6
>> +#define RK3399_PMUGRF_DDRTYPE_LPDDR4	7
>> +
>> +#endif
>>
> 
> 

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

* Re: [PATCH 7/8] arm64: dts: rk3399: Add dfi and dmc nodes.
  2018-07-30  8:11   ` Enric Balletbo i Serra
  (?)
@ 2018-08-09 22:41     ` Rob Herring
  -1 siblings, 0 replies; 31+ messages in thread
From: Rob Herring @ 2018-08-09 22:41 UTC (permalink / raw)
  To: enric.balletbo
  Cc: Mark Rutland, Douglas Anderson, David Airlie, Catalin Marinas,
	dri-devel, linux-kernel, Klaus Goger, MyungJoo Ham, kernel,
	linux-clk, linux-rockchip, Brian Norris, Chanwoo Choi,
	Nickey Yang, Jacob Chen, devicetree, Randy Li, linux-pm,
	Derek Basehore, Mark Yao, linux-arm-kernel, Lin Huang,
	Kyungmin Park, robin.murphy

Hi, this is an automated email from Rob's (experimental) review bot. I
found a couple of common problems with your patch. Please see below.

On Mon, 30 Jul 2018 10:11:23 +0200, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
> 
> These are required to support DDR DVFS on rk3399 platform. The patch also
> introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
> with default DRAM settings.
> 
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

The preferred subject prefix is "dt-bindings: <binding dir>: ...".

> ---
> 
> Changes in v1:
> - [RFC 8/10] Move rk3399-dram.h to dt-includes.
> - [RFC 8/10] Put sdram default values under the dmc node.
> - [RFC 8/10] Removed rk3399-dram-default-timing.dts
> 
>  .../boot/dts/rockchip/rk3399-op1-opp.dtsi     | 29 ++++++++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      | 49 +++++++++++++
>  include/dt-bindings/power/rk3399-dram.h       | 73 +++++++++++++++++++
>  3 files changed, 151 insertions(+)
>  create mode 100644 include/dt-bindings/power/rk3399-dram.h
> 

DT bindings (including binding headers) should be a separate patch. See
Documentation/devicetree/bindings/submitting-patches.txt.

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

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

* Re: [PATCH 7/8] arm64: dts: rk3399: Add dfi and dmc nodes.
@ 2018-08-09 22:41     ` Rob Herring
  0 siblings, 0 replies; 31+ messages in thread
From: Rob Herring @ 2018-08-09 22:41 UTC (permalink / raw)
  To: enric.balletbo
  Cc: , Mark Rutland, Douglas Anderson, David Airlie, Catalin Marinas,
	dri-devel, linux-kernel, Klaus Goger, kernel, linux-clk,
	linux-rockchip, Brian Norris, Chanwoo Choi, Nickey Yang,
	MyungJoo Ham, devicetree, Randy Li, linux-pm, Derek Basehore,
	Sean Paul, Mark Yao, linux-arm-kernel, Jacob Chen, Lin Huang,
	Sandy Huang, Kyungmin Park, robin.murphy

Hi, this is an automated email from Rob's (experimental) review bot. I
found a couple of common problems with your patch. Please see below.

On Mon, 30 Jul 2018 10:11:23 +0200, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
> 
> These are required to support DDR DVFS on rk3399 platform. The patch also
> introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
> with default DRAM settings.
> 
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

The preferred subject prefix is "dt-bindings: <binding dir>: ...".

> ---
> 
> Changes in v1:
> - [RFC 8/10] Move rk3399-dram.h to dt-includes.
> - [RFC 8/10] Put sdram default values under the dmc node.
> - [RFC 8/10] Removed rk3399-dram-default-timing.dts
> 
>  .../boot/dts/rockchip/rk3399-op1-opp.dtsi     | 29 ++++++++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      | 49 +++++++++++++
>  include/dt-bindings/power/rk3399-dram.h       | 73 +++++++++++++++++++
>  3 files changed, 151 insertions(+)
>  create mode 100644 include/dt-bindings/power/rk3399-dram.h
> 

DT bindings (including binding headers) should be a separate patch. See
Documentation/devicetree/bindings/submitting-patches.txt.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 7/8] arm64: dts: rk3399: Add dfi and dmc nodes.
@ 2018-08-09 22:41     ` Rob Herring
  0 siblings, 0 replies; 31+ messages in thread
From: Rob Herring @ 2018-08-09 22:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi, this is an automated email from Rob's (experimental) review bot. I
found a couple of common problems with your patch. Please see below.

On Mon, 30 Jul 2018 10:11:23 +0200, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
> 
> These are required to support DDR DVFS on rk3399 platform. The patch also
> introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
> with default DRAM settings.
> 
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

The preferred subject prefix is "dt-bindings: <binding dir>: ...".

> ---
> 
> Changes in v1:
> - [RFC 8/10] Move rk3399-dram.h to dt-includes.
> - [RFC 8/10] Put sdram default values under the dmc node.
> - [RFC 8/10] Removed rk3399-dram-default-timing.dts
> 
>  .../boot/dts/rockchip/rk3399-op1-opp.dtsi     | 29 ++++++++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi      | 49 +++++++++++++
>  include/dt-bindings/power/rk3399-dram.h       | 73 +++++++++++++++++++
>  3 files changed, 151 insertions(+)
>  create mode 100644 include/dt-bindings/power/rk3399-dram.h
> 

DT bindings (including binding headers) should be a separate patch. See
Documentation/devicetree/bindings/submitting-patches.txt.

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

end of thread, other threads:[~2018-08-09 22:41 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30  8:11 [PATCH 0/8] Add support for drm/rockchip to dynamically control the DDR frequency Enric Balletbo i Serra
2018-07-30  8:11 ` Enric Balletbo i Serra
2018-07-30  8:11 ` Enric Balletbo i Serra
2018-07-30  8:11 ` [PATCH 1/8] devfreq: rockchip-dfi: Move GRF definitions to a common place Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-08-01  8:36   ` Chanwoo Choi
2018-08-01  8:36     ` Chanwoo Choi
2018-08-07 10:50     ` Enric Balletbo i Serra
2018-08-07 10:50       ` Enric Balletbo i Serra
2018-07-30  8:11 ` [PATCH 2/8] dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle Enric Balletbo i Serra
2018-07-30  8:11   ` [PATCH 2/8] dt-bindings: devfreq: rk3399_dmc: Add rockchip, pmu phandle Enric Balletbo i Serra
2018-07-30  8:11 ` [PATCH 3/8] devfreq: rk3399_dmc: Pass ODT and auto power down parameters to TF-A Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-08-01  9:08   ` Chanwoo Choi
2018-08-01  9:08     ` Chanwoo Choi
2018-07-30  8:11 ` [PATCH 4/8] devfreq: rk3399_dmc / clk: rockchip: Sync with vblank in the kernel for DDRfreq Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-08-02  2:35   ` Chanwoo Choi
2018-08-02  2:35     ` Chanwoo Choi
2018-07-30  8:11 ` [PATCH 5/8] devfreq: rk3399_dmc / clk: rockchip: Disable DDR clk timeout on suspend Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-07-30  8:11 ` [PATCH 6/8] drm: rockchip: Add DDR devfreq support Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-07-30  8:11 ` [PATCH 7/8] arm64: dts: rk3399: Add dfi and dmc nodes Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-08-09 22:41   ` Rob Herring
2018-08-09 22:41     ` Rob Herring
2018-08-09 22:41     ` Rob Herring
2018-07-30  8:11 ` [PATCH 8/8] arm64: dts: rockchip: Enable dmc and dfi nodes on gru Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra
2018-07-30  8:11   ` Enric Balletbo i Serra

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