[RFC PATCH 0/7] Add driver for dvfsrc and add support for active state of scpsys on mt8183

[RFC PATCH 0/7] Add driver for dvfsrc and add support for active state of scpsys on mt8183

[Henry Chen]

The patchsets add support for MediaTek hardware module named DVFSRC
(dynamic voltage and frequency scaling resource collector). The DVFSRC is
a HW module which is used to collect all the requests from both software
and hardware and turn into the decision of minimum operating voltage and
minimum DRAM frequency to fulfill those requests.

So, This series is to implement the dvfsrc driver to collect all the
requests of operating voltage or DRAM bandwidth from other device drivers
likes GPU/Camera through 2 frameworks basically:

1. PM_QOS_MEMORY_BANDWIDTH from PM QOS: to aggregate the bandwidth
   requirements from different clients
2. Active state management of power domains[1]: to handle the operating
   voltage opp requirement from different power domains

[1] https://lwn.net/Articles/744047/

[RFC PATCH 1/7] dt-bindings: soc: Add DVFSRC driver bindings

[Henry Chen]

Document the binding for enabling DVFSRC on MediaTek SoC.

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 .../devicetree/bindings/soc/mediatek/dvfsrc.txt    | 26 ++++++++++++++++++++++
 include/dt-bindings/soc/mtk,dvfsrc.h               | 18 +++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt
 create mode 100644 include/dt-bindings/soc/mtk,dvfsrc.h

diff --git a/Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt b/Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt
new file mode 100644
index 0000000..402c885
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt
@@ -0,0 +1,26 @@
+MediaTek DVFSRC Driver
+
+The Dynamic Voltage and Frequency Scaling Resource Collector (DVFSRC) is a
+HW module which is used to collect all the requests from both software and
+hardware and turn into the decision of minimum operating voltage and minimum
+DRAM frequency to fulfill those requests.
+
+Required Properties:
+- compatible: Should be one of the following
+	- "mediatek,mt8183-dvfsrc": For MT8183 SoC
+- reg: Address range of the DVFSRC unit
+- dram_type: Refer to <include/dt-bindings/soc/mtk,dvfsrc.h> for the
+	different dram type support.
+- clock-names: Must include the following entries:
+	"dvfsrc": DVFSRC module clock
+- clocks: Must contain an entry for each entry in clock-names.
+
+Example:
+
+	dvfsrc_top@10012000 {
+		compatible = "mediatek,mt8183-dvfsrc";
+		reg = <0 0x10012000 0 0x1000>;
+		clocks = <&infracfg CLK_INFRA_DVFSRC>;
+		clock-names = "dvfsrc";
+		dram_type = <MT8183_DVFSRC_OPP_LP4>;
+	};
diff --git a/include/dt-bindings/soc/mtk,dvfsrc.h b/include/dt-bindings/soc/mtk,dvfsrc.h
new file mode 100644
index 0000000..60b3497
--- /dev/null
+++ b/include/dt-bindings/soc/mtk,dvfsrc.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ */
+
+#ifndef _DT_BINDINGS_POWER_MTK_DVFSRC_H
+#define _DT_BINDINGS_POWER_MTK_DVFSRC_H
+
+#define MT8183_DVFSRC_OPP_LP4	0
+#define MT8183_DVFSRC_OPP_LP4X	1
+#define MT8183_DVFSRC_OPP_LP3	2
+
+#define MT8183_DVFSRC_LEVEL_1	1
+#define MT8183_DVFSRC_LEVEL_2	2
+#define MT8183_DVFSRC_LEVEL_3	3
+#define MT8183_DVFSRC_LEVEL_4	4
+
+#endif /* _DT_BINDINGS_POWER_MTK_DVFSRC_H */
-- 
1.9.1

[RFC PATCH 2/7] dt-bindings: soc: Add opp table on scpsys bindings

[Henry Chen]

Add opp table on scpsys dt-bindings for Mediatek SoC.

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 Documentation/devicetree/bindings/opp/mtk-opp.txt  | 24 +++++++++++++
 .../devicetree/bindings/soc/mediatek/scpsys.txt    | 42 ++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/opp/mtk-opp.txt

diff --git a/Documentation/devicetree/bindings/opp/mtk-opp.txt b/Documentation/devicetree/bindings/opp/mtk-opp.txt
new file mode 100644
index 0000000..036be1c
--- /dev/null
+++ b/Documentation/devicetree/bindings/opp/mtk-opp.txt
@@ -0,0 +1,24 @@
+Mediatek OPP bindings to descibe OPP nodes with level values
+
+OPP tables for devices on Mediatek platforms require an additional
+platform specific level value to be specified.
+This value is passed on to the mediatek Power Management Unit by the
+CPU, which then takes the necessary actions to set a voltage rail
+to an appropriate voltage based on the value passed.
+
+The bindings are based on top of the operating-points-v2 bindings
+described in Documentation/devicetree/bindings/opp/opp.txt
+Additional properties are described below.
+
+* OPP Table Node
+
+Required properties:
+- compatible: Allow OPPs to express their compatibility. It should be:
+  "operating-points-v2-mtk-level"
+
+* OPP Node
+
+Required properties:
+- mtk,level: On Mediatek platforms an OPP node can describe a positive value
+representing a level that's communicated with a our power management hardware
+which then translates it into a certain voltage on a voltage rail.
diff --git a/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt b/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt
index b4728ce..299b526 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt
+++ b/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt
@@ -63,6 +63,10 @@ Optional properties:
 - mfg_2d-supply: Power supply for the mfg_2d power domain
 - mfg-supply: Power supply for the mfg power domain
 
+- operating-points-v2: Phandle to the OPP table for the Power domain.
+	Refer to Documentation/devicetree/bindings/power/power_domain.txt
+	and Documentation/devicetree/bindings/opp/mtk-opp.txt for more details
+
 Example:
 
 	scpsys: scpsys@10006000 {
@@ -75,6 +79,27 @@ Example:
 			 <&topckgen CLK_TOP_VENC_SEL>,
 			 <&topckgen CLK_TOP_VENC_LT_SEL>;
 		clock-names = "mfg", "mm", "venc", "venc_lt";
+		operating-points-v2 = <&dvfsrc_opp_table>;
+
+		dvfsrc_opp_table: opp-table {
+			compatible = "operating-points-v2-mtk-level";
+
+			dvfsrc_vol_min: opp1 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_1>;
+			};
+
+			dvfsrc_freq_medium: opp2 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_2>;
+			};
+
+			dvfsrc_freq_max: opp3 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_3>;
+			};
+
+			dvfsrc_vol_max: opp4 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_4>;
+			};
+		};
 	};
 
 Example consumer:
@@ -82,4 +107,21 @@ Example consumer:
 	afe: mt8173-afe-pcm@11220000 {
 		compatible = "mediatek,mt8173-afe-pcm";
 		power-domains = <&scpsys MT8173_POWER_DOMAIN_AUDIO>;
+		operating-points-v2 = <&aud_opp_table>;
+	};
+
+	aud_opp_table: aud-opp-table {
+		compatible = "operating-points-v2";
+		opp1 {
+			opp-hz = /bits/ 64 <793000000>;
+			required-opps = <&dvfsrc_vol_min>;
+		};
+		opp2 {
+			opp-hz = /bits/ 64 <910000000>;
+			required-opps = <&dvfsrc_vol_max>;
+		};
+		opp3 {
+			opp-hz = /bits/ 64 <1014000000>;
+			required-opps = <&dvfsrc_vol_max>;
+		};
 	};
-- 
1.9.1

[RFC PATCH 3/7] soc: mediatek: add support for the performance state

[Henry Chen]

Support power domain performance state, add header file for scp event.

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 drivers/soc/mediatek/mtk-scpsys.c | 60 +++++++++++++++++++++++++++++++++++++++
 drivers/soc/mediatek/mtk-scpsys.h | 22 ++++++++++++++
 2 files changed, 82 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-scpsys.h

diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index 58d84fe..90102ae 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -11,7 +11,9 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 #include <linux/soc/mediatek/infracfg.h>
 #include <linux/soc/mediatek/scpsys-ext.h>
 
@@ -22,6 +24,7 @@
 #include <dt-bindings/power/mt7623a-power.h>
 #include <dt-bindings/power/mt8173-power.h>
 #include <dt-bindings/power/mt8183-power.h>
+#include "mtk-scpsys.h"
 
 #define MTK_POLL_DELAY_US   10
 #define MTK_POLL_TIMEOUT    (jiffies_to_usecs(HZ))
@@ -187,6 +190,18 @@ struct scp_soc_data {
 	bool bus_prot_reg_update;
 };
 
+BLOCKING_NOTIFIER_HEAD(scpsys_notifier_list);
+
+int register_scpsys_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&scpsys_notifier_list, nb);
+}
+
+int unregister_scpsys_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&scpsys_notifier_list, nb);
+}
+
 static int scpsys_domain_is_on(struct scp_domain *scpd)
 {
 	struct scp *scp = scpd->scp;
@@ -536,6 +551,48 @@ static void init_clks(struct platform_device *pdev, struct clk **clk)
 		clk[i] = devm_clk_get(&pdev->dev, clk_names[i]);
 }
 
+static int mtk_pd_set_performance(struct generic_pm_domain *genpd,
+				  unsigned int state)
+{
+	int i;
+	struct scp_domain *scpd =
+		container_of(genpd, struct scp_domain, genpd);
+	struct scp_event_data scpe;
+	struct scp *scp = scpd->scp;
+	struct genpd_onecell_data *pd_data = &scp->pd_data;
+
+	for (i = 0; i < pd_data->num_domains; i++) {
+		if (genpd == pd_data->domains[i]) {
+			dev_dbg(scp->dev, "%d. %s = %d\n",
+				i, genpd->name, state);
+			break;
+		}
+	}
+
+	scpe.event_type = MTK_SCPSYS_PSTATE;
+	scpe.genpd = genpd;
+	scpe.domain_id = i;
+	blocking_notifier_call_chain(&scpsys_notifier_list, state, &scpe);
+
+	return 0;
+}
+
+static unsigned int mtk_pd_get_performance(struct generic_pm_domain *genpd,
+					   struct dev_pm_opp *opp)
+{
+	struct device_node *np;
+	unsigned int state;
+
+	np = dev_pm_opp_get_of_node(opp);
+
+	if (of_property_read_u32(np, "mtk,level", &state))
+		return 0;
+
+	of_node_put(np);
+
+	return state;
+}
+
 static struct scp *init_scp(struct platform_device *pdev,
 			const struct scp_domain_data *scp_domain_data, int num,
 			const struct scp_ctrl_reg *scp_ctrl_reg,
@@ -659,6 +716,9 @@ static struct scp *init_scp(struct platform_device *pdev,
 		genpd->power_on = scpsys_power_on;
 		if (MTK_SCPD_CAPS(scpd, MTK_SCPD_ACTIVE_WAKEUP))
 			genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+
+		genpd->set_performance_state = mtk_pd_set_performance;
+		genpd->opp_to_performance_state = mtk_pd_get_performance;
 	}
 
 	return scp;
diff --git a/drivers/soc/mediatek/mtk-scpsys.h b/drivers/soc/mediatek/mtk-scpsys.h
new file mode 100644
index 0000000..c1e8325
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-scpsys.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ */
+
+#ifndef __MTK_SCPSYS_H__
+#define __MTK_SCPSYS_H__
+
+struct scp_event_data {
+	int event_type;
+	int domain_id;
+	struct generic_pm_domain *genpd;
+};
+
+enum scp_event_type {
+	MTK_SCPSYS_PSTATE,
+};
+
+int register_scpsys_notifier(struct notifier_block *nb);
+int unregister_scpsys_notifier(struct notifier_block *nb);
+
+#endif /* __MTK_SCPSYS_H__ */
-- 
1.9.1

Re: [RFC PATCH 3/7] soc: mediatek: add support for the performance state

[Nicolas Boichat]

On Wed, Jan 2, 2019 at 10:01 PM Henry Chen <henryc.chen@mediatek.com> wrote:
>
> Support power domain performance state, add header file for scp event.
>
> Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-scpsys.c | 60 +++++++++++++++++++++++++++++++++++++++
>  drivers/soc/mediatek/mtk-scpsys.h | 22 ++++++++++++++
>  2 files changed, 82 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mtk-scpsys.h
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index 58d84fe..90102ae 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -11,7 +11,9 @@
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> +#include <linux/pm_opp.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
>  #include <linux/soc/mediatek/infracfg.h>
>  #include <linux/soc/mediatek/scpsys-ext.h>
>
> @@ -22,6 +24,7 @@
>  #include <dt-bindings/power/mt7623a-power.h>
>  #include <dt-bindings/power/mt8173-power.h>
>  #include <dt-bindings/power/mt8183-power.h>
> +#include "mtk-scpsys.h"
>
>  #define MTK_POLL_DELAY_US   10
>  #define MTK_POLL_TIMEOUT    (jiffies_to_usecs(HZ))
> @@ -187,6 +190,18 @@ struct scp_soc_data {
>         bool bus_prot_reg_update;
>  };
>
> +BLOCKING_NOTIFIER_HEAD(scpsys_notifier_list);
> +
> +int register_scpsys_notifier(struct notifier_block *nb)
> +{
> +       return blocking_notifier_chain_register(&scpsys_notifier_list, nb);
> +}
> +
> +int unregister_scpsys_notifier(struct notifier_block *nb)
> +{
> +       return blocking_notifier_chain_unregister(&scpsys_notifier_list, nb);
> +}
> +
>  static int scpsys_domain_is_on(struct scp_domain *scpd)
>  {
>         struct scp *scp = scpd->scp;
> @@ -536,6 +551,48 @@ static void init_clks(struct platform_device *pdev, struct clk **clk)
>                 clk[i] = devm_clk_get(&pdev->dev, clk_names[i]);
>  }
>
> +static int mtk_pd_set_performance(struct generic_pm_domain *genpd,
> +                                 unsigned int state)
> +{
> +       int i;
> +       struct scp_domain *scpd =
> +               container_of(genpd, struct scp_domain, genpd);

nit: This (just) fits in 80 chars.

> +       struct scp_event_data scpe;
> +       struct scp *scp = scpd->scp;
> +       struct genpd_onecell_data *pd_data = &scp->pd_data;
> +
> +       for (i = 0; i < pd_data->num_domains; i++) {
> +               if (genpd == pd_data->domains[i]) {
> +                       dev_dbg(scp->dev, "%d. %s = %d\n",
> +                               i, genpd->name, state);
> +                       break;
> +               }
> +       }

Do we need a sanity check here, if i == pd_data->num_domains?

> +
> +       scpe.event_type = MTK_SCPSYS_PSTATE;
> +       scpe.genpd = genpd;
> +       scpe.domain_id = i;
> +       blocking_notifier_call_chain(&scpsys_notifier_list, state, &scpe);
> +
> +       return 0;
> +}
> +
> +static unsigned int mtk_pd_get_performance(struct generic_pm_domain *genpd,
> +                                          struct dev_pm_opp *opp)
> +{
> +       struct device_node *np;
> +       unsigned int state;
> +
> +       np = dev_pm_opp_get_of_node(opp);
> +
> +       if (of_property_read_u32(np, "mtk,level", &state))
> +               return 0;
> +
> +       of_node_put(np);
> +
> +       return state;
> +}
> +
>  static struct scp *init_scp(struct platform_device *pdev,
>                         const struct scp_domain_data *scp_domain_data, int num,
>                         const struct scp_ctrl_reg *scp_ctrl_reg,
> @@ -659,6 +716,9 @@ static struct scp *init_scp(struct platform_device *pdev,
>                 genpd->power_on = scpsys_power_on;
>                 if (MTK_SCPD_CAPS(scpd, MTK_SCPD_ACTIVE_WAKEUP))
>                         genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
> +
> +               genpd->set_performance_state = mtk_pd_set_performance;
> +               genpd->opp_to_performance_state = mtk_pd_get_performance;
>         }
>
>         return scp;
> diff --git a/drivers/soc/mediatek/mtk-scpsys.h b/drivers/soc/mediatek/mtk-scpsys.h
> new file mode 100644
> index 0000000..c1e8325
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-scpsys.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (c) 2018 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_SCPSYS_H__
> +#define __MTK_SCPSYS_H__
> +
> +struct scp_event_data {
> +       int event_type;
> +       int domain_id;
> +       struct generic_pm_domain *genpd;
> +};
> +
> +enum scp_event_type {
> +       MTK_SCPSYS_PSTATE,
> +};
> +
> +int register_scpsys_notifier(struct notifier_block *nb);
> +int unregister_scpsys_notifier(struct notifier_block *nb);
> +
> +#endif /* __MTK_SCPSYS_H__ */
> --
> 1.9.1
>

Re: [RFC PATCH 3/7] soc: mediatek: add support for the performance state

[Henry Chen]

On Thu, 2019-01-03 at 09:48 +0800, Nicolas Boichat wrote:
> On Wed, Jan 2, 2019 at 10:01 PM Henry Chen <henryc.chen@mediatek.com> wrote:
> >
> > Support power domain performance state, add header file for scp event.
> >
> > Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-scpsys.c | 60 +++++++++++++++++++++++++++++++++++++++
> >  drivers/soc/mediatek/mtk-scpsys.h | 22 ++++++++++++++
> >  2 files changed, 82 insertions(+)
> >  create mode 100644 drivers/soc/mediatek/mtk-scpsys.h
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index 58d84fe..90102ae 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -11,7 +11,9 @@
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_domain.h>
> > +#include <linux/pm_opp.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/slab.h>
> >  #include <linux/soc/mediatek/infracfg.h>
> >  #include <linux/soc/mediatek/scpsys-ext.h>
> >
> > @@ -22,6 +24,7 @@
> >  #include <dt-bindings/power/mt7623a-power.h>
> >  #include <dt-bindings/power/mt8173-power.h>
> >  #include <dt-bindings/power/mt8183-power.h>
> > +#include "mtk-scpsys.h"
> >
> >  #define MTK_POLL_DELAY_US   10
> >  #define MTK_POLL_TIMEOUT    (jiffies_to_usecs(HZ))
> > @@ -187,6 +190,18 @@ struct scp_soc_data {
> >         bool bus_prot_reg_update;
> >  };
> >
> > +BLOCKING_NOTIFIER_HEAD(scpsys_notifier_list);
> > +
> > +int register_scpsys_notifier(struct notifier_block *nb)
> > +{
> > +       return blocking_notifier_chain_register(&scpsys_notifier_list, nb);
> > +}
> > +
> > +int unregister_scpsys_notifier(struct notifier_block *nb)
> > +{
> > +       return blocking_notifier_chain_unregister(&scpsys_notifier_list, nb);
> > +}
> > +
> >  static int scpsys_domain_is_on(struct scp_domain *scpd)
> >  {
> >         struct scp *scp = scpd->scp;
> > @@ -536,6 +551,48 @@ static void init_clks(struct platform_device *pdev, struct clk **clk)
> >                 clk[i] = devm_clk_get(&pdev->dev, clk_names[i]);
> >  }
> >
> > +static int mtk_pd_set_performance(struct generic_pm_domain *genpd,
> > +                                 unsigned int state)
> > +{
> > +       int i;
> > +       struct scp_domain *scpd =
> > +               container_of(genpd, struct scp_domain, genpd);
> 
> nit: This (just) fits in 80 chars.
Try again and get 81 chars :)
> 
> > +       struct scp_event_data scpe;
> > +       struct scp *scp = scpd->scp;
> > +       struct genpd_onecell_data *pd_data = &scp->pd_data;
> > +
> > +       for (i = 0; i < pd_data->num_domains; i++) {
> > +               if (genpd == pd_data->domains[i]) {
> > +                       dev_dbg(scp->dev, "%d. %s = %d\n",
> > +                               i, genpd->name, state);
> > +                       break;
> > +               }
> > +       }
> 
> Do we need a sanity check here, if i == pd_data->num_domains?
Oops. It needed, thanks for remind.
> 
> > +
> > +       scpe.event_type = MTK_SCPSYS_PSTATE;
> > +       scpe.genpd = genpd;
> > +       scpe.domain_id = i;
> > +       blocking_notifier_call_chain(&scpsys_notifier_list, state, &scpe);
> > +
> > +       return 0;
> > +}
> > +
> > +static unsigned int mtk_pd_get_performance(struct generic_pm_domain *genpd,
> > +                                          struct dev_pm_opp *opp)
> > +{
> > +       struct device_node *np;
> > +       unsigned int state;
> > +
> > +       np = dev_pm_opp_get_of_node(opp);
> > +
> > +       if (of_property_read_u32(np, "mtk,level", &state))
> > +               return 0;
> > +
> > +       of_node_put(np);
> > +
> > +       return state;
> > +}
> > +
> >  static struct scp *init_scp(struct platform_device *pdev,
> >                         const struct scp_domain_data *scp_domain_data, int num,
> >                         const struct scp_ctrl_reg *scp_ctrl_reg,
> > @@ -659,6 +716,9 @@ static struct scp *init_scp(struct platform_device *pdev,
> >                 genpd->power_on = scpsys_power_on;
> >                 if (MTK_SCPD_CAPS(scpd, MTK_SCPD_ACTIVE_WAKEUP))
> >                         genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
> > +
> > +               genpd->set_performance_state = mtk_pd_set_performance;
> > +               genpd->opp_to_performance_state = mtk_pd_get_performance;
> >         }
> >
> >         return scp;
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.h b/drivers/soc/mediatek/mtk-scpsys.h
> > new file mode 100644
> > index 0000000..c1e8325
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mtk-scpsys.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0
> > + *
> > + * Copyright (c) 2018 MediaTek Inc.
> > + */
> > +
> > +#ifndef __MTK_SCPSYS_H__
> > +#define __MTK_SCPSYS_H__
> > +
> > +struct scp_event_data {
> > +       int event_type;
> > +       int domain_id;
> > +       struct generic_pm_domain *genpd;
> > +};
> > +
> > +enum scp_event_type {
> > +       MTK_SCPSYS_PSTATE,
> > +};
> > +
> > +int register_scpsys_notifier(struct notifier_block *nb);
> > +int unregister_scpsys_notifier(struct notifier_block *nb);
> > +
> > +#endif /* __MTK_SCPSYS_H__ */
> > --
> > 1.9.1
> >

Re: [RFC PATCH 3/7] soc: mediatek: add support for the performance state

[Nicolas Boichat]

On Thu, Jan 3, 2019 at 10:16 PM Henry Chen <henryc.chen@mediatek.com> wrote:
>
> On Thu, 2019-01-03 at 09:48 +0800, Nicolas Boichat wrote:
> > On Wed, Jan 2, 2019 at 10:01 PM Henry Chen <henryc.chen@mediatek.com> wrote:
> > >
> > > Support power domain performance state, add header file for scp event.
> > >
> > > Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> > > ---
> > >  drivers/soc/mediatek/mtk-scpsys.c | 60 +++++++++++++++++++++++++++++++++++++++
> > >  drivers/soc/mediatek/mtk-scpsys.h | 22 ++++++++++++++
> > >  2 files changed, 82 insertions(+)
> > >  create mode 100644 drivers/soc/mediatek/mtk-scpsys.h
> > >
> > > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > > index 58d84fe..90102ae 100644
> > > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > > @@ -11,7 +11,9 @@
> > >  #include <linux/of_device.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/pm_domain.h>
> > > +#include <linux/pm_opp.h>
> > >  #include <linux/regulator/consumer.h>
> > > +#include <linux/slab.h>
> > >  #include <linux/soc/mediatek/infracfg.h>
> > >  #include <linux/soc/mediatek/scpsys-ext.h>
> > >
> > > @@ -22,6 +24,7 @@
> > >  #include <dt-bindings/power/mt7623a-power.h>
> > >  #include <dt-bindings/power/mt8173-power.h>
> > >  #include <dt-bindings/power/mt8183-power.h>
> > > +#include "mtk-scpsys.h"
> > >
> > >  #define MTK_POLL_DELAY_US   10
> > >  #define MTK_POLL_TIMEOUT    (jiffies_to_usecs(HZ))
> > > @@ -187,6 +190,18 @@ struct scp_soc_data {
> > >         bool bus_prot_reg_update;
> > >  };
> > >
> > > +BLOCKING_NOTIFIER_HEAD(scpsys_notifier_list);
> > > +
> > > +int register_scpsys_notifier(struct notifier_block *nb)
> > > +{
> > > +       return blocking_notifier_chain_register(&scpsys_notifier_list, nb);
> > > +}
> > > +
> > > +int unregister_scpsys_notifier(struct notifier_block *nb)
> > > +{
> > > +       return blocking_notifier_chain_unregister(&scpsys_notifier_list, nb);
> > > +}
> > > +
> > >  static int scpsys_domain_is_on(struct scp_domain *scpd)
> > >  {
> > >         struct scp *scp = scpd->scp;
> > > @@ -536,6 +551,48 @@ static void init_clks(struct platform_device *pdev, struct clk **clk)
> > >                 clk[i] = devm_clk_get(&pdev->dev, clk_names[i]);
> > >  }
> > >
> > > +static int mtk_pd_set_performance(struct generic_pm_domain *genpd,
> > > +                                 unsigned int state)
> > > +{
> > > +       int i;
> > > +       struct scp_domain *scpd =
> > > +               container_of(genpd, struct scp_domain, genpd);
> >
> > nit: This (just) fits in 80 chars.
> Try again and get 81 chars :)

Really not a big deal, but I did check and I think it fits ,-)

        struct scp_domain *scpd = container_of(genpd, struct scp_domain, genpd);
01234567890123456789012345678901234567890123456789012345678901234567890123456789

> >
> > > +       struct scp_event_data scpe;
> > > +       struct scp *scp = scpd->scp;
> > > +       struct genpd_onecell_data *pd_data = &scp->pd_data;
> > > +
> > > +       for (i = 0; i < pd_data->num_domains; i++) {
> > > +               if (genpd == pd_data->domains[i]) {
> > > +                       dev_dbg(scp->dev, "%d. %s = %d\n",
> > > +                               i, genpd->name, state);
> > > +                       break;
> > > +               }
> > > +       }
> >
> > Do we need a sanity check here, if i == pd_data->num_domains?
> Oops. It needed, thanks for remind.
> >
> > > +
> > > +       scpe.event_type = MTK_SCPSYS_PSTATE;
> > > +       scpe.genpd = genpd;
> > > +       scpe.domain_id = i;
> > > +       blocking_notifier_call_chain(&scpsys_notifier_list, state, &scpe);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static unsigned int mtk_pd_get_performance(struct generic_pm_domain *genpd,
> > > +                                          struct dev_pm_opp *opp)
> > > +{
> > > +       struct device_node *np;
> > > +       unsigned int state;
> > > +
> > > +       np = dev_pm_opp_get_of_node(opp);
> > > +
> > > +       if (of_property_read_u32(np, "mtk,level", &state))
> > > +               return 0;
> > > +
> > > +       of_node_put(np);
> > > +
> > > +       return state;
> > > +}
> > > +
> > >  static struct scp *init_scp(struct platform_device *pdev,
> > >                         const struct scp_domain_data *scp_domain_data, int num,
> > >                         const struct scp_ctrl_reg *scp_ctrl_reg,
> > > @@ -659,6 +716,9 @@ static struct scp *init_scp(struct platform_device *pdev,
> > >                 genpd->power_on = scpsys_power_on;
> > >                 if (MTK_SCPD_CAPS(scpd, MTK_SCPD_ACTIVE_WAKEUP))
> > >                         genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
> > > +
> > > +               genpd->set_performance_state = mtk_pd_set_performance;
> > > +               genpd->opp_to_performance_state = mtk_pd_get_performance;
> > >         }
> > >
> > >         return scp;
> > > diff --git a/drivers/soc/mediatek/mtk-scpsys.h b/drivers/soc/mediatek/mtk-scpsys.h
> > > new file mode 100644
> > > index 0000000..c1e8325
> > > --- /dev/null
> > > +++ b/drivers/soc/mediatek/mtk-scpsys.h
> > > @@ -0,0 +1,22 @@
> > > +/* SPDX-License-Identifier: GPL-2.0
> > > + *
> > > + * Copyright (c) 2018 MediaTek Inc.
> > > + */
> > > +
> > > +#ifndef __MTK_SCPSYS_H__
> > > +#define __MTK_SCPSYS_H__
> > > +
> > > +struct scp_event_data {
> > > +       int event_type;
> > > +       int domain_id;
> > > +       struct generic_pm_domain *genpd;
> > > +};
> > > +
> > > +enum scp_event_type {
> > > +       MTK_SCPSYS_PSTATE,
> > > +};
> > > +
> > > +int register_scpsys_notifier(struct notifier_block *nb);
> > > +int unregister_scpsys_notifier(struct notifier_block *nb);
> > > +
> > > +#endif /* __MTK_SCPSYS_H__ */
> > > --
> > > 1.9.1
> > >
>
>

[RFC PATCH 4/7] arm64: dts: mt8183: add performance state support of scpsys

[Henry Chen]

Add support for performance state of scpsys on mt8183 platform.

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 47926a7..e396410 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/power/mt8183-power.h>
+#include <dt-bindings/soc/mtk,dvfsrc.h>
 
 / {
 	compatible = "mediatek,mt8183";
@@ -243,6 +244,26 @@
 			      "vpu-3", "vpu-4", "vpu-5";
 		infracfg = <&infracfg>;
 		smi_comm = <&smi_common>;
+		operating-points-v2 = <&dvfsrc_opp_table>;
+		dvfsrc_opp_table: opp-table {
+			compatible = "operating-points-v2-mtk-level";
+
+			dvfsrc_vol_min: opp1 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_1>;
+			};
+
+			dvfsrc_freq_medium: opp2 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_2>;
+			};
+
+			dvfsrc_freq_max: opp3 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_3>;
+			};
+
+			dvfsrc_vol_max: opp4 {
+				mtk,level = <MT8183_DVFSRC_LEVEL_4>;
+			};
+		};
 	};
 
 	apmixedsys: syscon@1000c000 {
-- 
1.9.1

[RFC PATCH 5/7] soc: mediatek: add header for mediatek SIP interface

[Henry Chen]

Add a header to collect SIPs and add one SIP call to initialize power
management hardware for the SIP interface defined to access the SPM
handling vcore voltage and ddr rate changes on mt8183 (and most likely
later socs).

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 include/soc/mediatek/mtk_sip.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 include/soc/mediatek/mtk_sip.h

diff --git a/include/soc/mediatek/mtk_sip.h b/include/soc/mediatek/mtk_sip.h
new file mode 100644
index 0000000..5394ff4
--- /dev/null
+++ b/include/soc/mediatek/mtk_sip.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ */
+#ifndef __SOC_MTK_SIP_H
+#define __SOC_MTK_SIP_H
+
+#ifdef CONFIG_ARM64
+#define MTK_SIP_SMC_AARCH_BIT		0x40000000
+#else
+#define MTK_SIP_SMC_AARCH_BIT		0x00000000
+#endif
+
+#define MTK_SIP_SPM			(0x82000220 | MTK_SIP_SMC_AARCH_BIT)
+#define MTK_SIP_SPM_DVFSRC_INIT		0x00
+
+#endif
-- 
1.9.1

[RFC PATCH 6/7] soc: mediatek: add MT8183 dvfsrc support

[Henry Chen]

Add dvfsrc driver for MT8183

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 drivers/soc/mediatek/Kconfig      |  15 ++
 drivers/soc/mediatek/Makefile     |   1 +
 drivers/soc/mediatek/mtk-dvfsrc.c | 473 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 489 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-dvfsrc.c

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index a7d0667..f956f03 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -12,6 +12,21 @@ config MTK_INFRACFG
 	  INFRACFG controller contains various infrastructure registers not
 	  directly associated to any device.
 
+config MTK_DVFSRC
+	bool "MediaTek DVFSRC Support"
+	depends on ARCH_MEDIATEK
+	default ARCH_MEDIATEK
+	select REGMAP
+	select MTK_INFRACFG
+	select PM_GENERIC_DOMAINS if PM
+	depends on MTK_SCPSYS
+	help
+	  Say yes here to add support for the MediaTek DVFSRC found
+	  on different MediaTek SoCs. The DVFSRC is a proprietary
+	  hardware which is used to collect all the requests from
+	  system and turn into the decision of minimum Vcore voltage
+	  and minimum DRAM frequency to fulfill those requests.
+
 config MTK_PMIC_WRAP
 	tristate "MediaTek PMIC Wrapper Support"
 	depends on RESET_CONTROLLER
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 9dc6670..5c010b9 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_MTK_DVFSRC) += mtk-dvfsrc.o
 obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
diff --git a/drivers/soc/mediatek/mtk-dvfsrc.c b/drivers/soc/mediatek/mtk-dvfsrc.c
new file mode 100644
index 0000000..af462a3
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-dvfsrc.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 MediaTek Inc.
+ */
+#include <linux/arm-smccc.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <soc/mediatek/mtk_sip.h>
+#include <dt-bindings/power/mt8183-power.h>
+#include <dt-bindings/soc/mtk,dvfsrc.h>
+#include "mtk-scpsys.h"
+
+#define DVFSRC_IDLE		0x00
+#define DVFSRC_GET_TARGET_LEVEL(x)	(((x) >> 0) & 0x0000ffff)
+#define DVFSRC_GET_CURRENT_LEVEL(x)	(((x) >> 16) & 0x0000ffff)
+
+/* macro for irq */
+#define DVFSRC_IRQ_TIMEOUT_EN		BIT(1)
+
+struct dvfsrc_opp {
+	int vcore_opp;
+	int dram_opp;
+};
+
+struct dvfsrc_domain {
+	int id;
+	int state;
+};
+
+struct mtk_dvfsrc;
+struct dvfsrc_soc_data {
+	const int *regs;
+	int num_opp;
+	int num_domains;
+	int dram_sft;
+	int vcore_sft;
+	const struct dvfsrc_opp **opps;
+	struct dvfsrc_domain *domains;
+	void (*init_soc)(struct mtk_dvfsrc *dvfsrc);
+	int (*get_target_level)(struct mtk_dvfsrc *dvfsrc);
+	int (*get_current_level)(struct mtk_dvfsrc *dvfsrc);
+};
+
+struct mtk_dvfsrc {
+	struct device *dev;
+	struct clk *clk_dvfsrc;
+	const struct dvfsrc_soc_data *dvd;
+	int dram_type;
+	int irq;
+	void __iomem *regs;
+	struct mutex lock;	/* generic mutex for dvfsrc driver */
+
+	struct notifier_block qos_notifier;
+	struct notifier_block scpsys_notifier;
+};
+
+static u32 dvfsrc_read(struct mtk_dvfsrc *dvfs, u32 offset)
+{
+	return readl(dvfs->regs + dvfs->dvd->regs[offset]);
+}
+
+static void dvfsrc_write(struct mtk_dvfsrc *dvfs, u32 offset, u32 val)
+{
+	writel(val, dvfs->regs + dvfs->dvd->regs[offset]);
+}
+
+enum dvfsrc_regs {
+	DVFSRC_BASIC_CONTROL,
+	DVFSRC_SW_REQ,
+	DVFSRC_SW_REQ2,
+	DVFSRC_EMI_REQUEST,
+	DVFSRC_EMI_REQUEST2,
+	DVFSRC_EMI_REQUEST3,
+	DVFSRC_EMI_QOS0,
+	DVFSRC_EMI_QOS1,
+	DVFSRC_EMI_QOS2,
+	DVFSRC_EMI_MD2SPM0,
+	DVFSRC_EMI_MD2SPM1,
+	DVFSRC_VCORE_REQUEST,
+	DVFSRC_VCORE_REQUEST2,
+	DVFSRC_VCORE_MD2SPM0,
+	DVFSRC_INT,
+	DVFSRC_INT_EN,
+	DVFSRC_INT_CLR,
+	DVFSRC_TIMEOUT_NEXTREQ,
+	DVFSRC_LEVEL,
+	DVFSRC_LEVEL_LABEL_0_1,
+	DVFSRC_LEVEL_LABEL_2_3,
+	DVFSRC_LEVEL_LABEL_4_5,
+	DVFSRC_LEVEL_LABEL_6_7,
+	DVFSRC_LEVEL_LABEL_8_9,
+	DVFSRC_LEVEL_LABEL_10_11,
+	DVFSRC_LEVEL_LABEL_12_13,
+	DVFSRC_LEVEL_LABEL_14_15,
+	DVFSRC_SW_BW_0,
+	DVFSRC_QOS_EN,
+	DVFSRC_FORCE,
+	DVFSRC_LAST,
+	DVFSRC_RSRV_0,
+	DVFSRC_RSRV_1,
+};
+
+static int mt8183_regs[] = {
+	[DVFSRC_BASIC_CONTROL] =	0x0,
+	[DVFSRC_SW_REQ] =		0x4,
+	[DVFSRC_SW_REQ2] =		0x8,
+	[DVFSRC_EMI_REQUEST] =		0xC,
+	[DVFSRC_EMI_REQUEST2] =		0x10,
+	[DVFSRC_EMI_REQUEST3] =		0x14,
+	[DVFSRC_EMI_QOS0] =		0x24,
+	[DVFSRC_EMI_QOS1] =		0x28,
+	[DVFSRC_EMI_QOS2] =		0x2C,
+	[DVFSRC_EMI_MD2SPM0] =		0x30,
+	[DVFSRC_EMI_MD2SPM1] =		0x34,
+	[DVFSRC_VCORE_REQUEST] =	0x48,
+	[DVFSRC_VCORE_REQUEST2] =	0x4C,
+	[DVFSRC_VCORE_MD2SPM0] =	0x68,
+	[DVFSRC_INT] =			0x98,
+	[DVFSRC_INT_EN] =		0x9C,
+	[DVFSRC_INT_CLR] =		0xA0,
+	[DVFSRC_TIMEOUT_NEXTREQ] =	0xD8,
+	[DVFSRC_LEVEL] =		0xDC,
+	[DVFSRC_LEVEL_LABEL_0_1] =	0xE0,
+	[DVFSRC_LEVEL_LABEL_2_3] =	0xE4,
+	[DVFSRC_LEVEL_LABEL_4_5] =	0xE8,
+	[DVFSRC_LEVEL_LABEL_6_7] =	0xEC,
+	[DVFSRC_LEVEL_LABEL_8_9] =	0xF0,
+	[DVFSRC_LEVEL_LABEL_10_11] =	0xF4,
+	[DVFSRC_LEVEL_LABEL_12_13] =	0xF8,
+	[DVFSRC_LEVEL_LABEL_14_15] =	0xFC,
+	[DVFSRC_SW_BW_0] =		0x160,
+	[DVFSRC_QOS_EN] =		0x180,
+	[DVFSRC_FORCE] =		0x300,
+	[DVFSRC_LAST] =			0x308,
+	[DVFSRC_RSRV_0] =		0x600,
+	[DVFSRC_RSRV_1] =		0x604,
+};
+
+static bool dvfsrc_is_idle(struct mtk_dvfsrc *dvfsrc)
+{
+	int val = 0;
+
+	if (dvfsrc->dvd->get_target_level)
+		val = dvfsrc->dvd->get_target_level(dvfsrc);
+
+	return val == DVFSRC_IDLE;
+}
+
+static int dvfsrc_wait_for_state(struct mtk_dvfsrc *dvfsrc,
+				 bool (*fp)(struct mtk_dvfsrc *))
+{
+	unsigned long timeout;
+
+	timeout = jiffies + usecs_to_jiffies(1000);
+
+	do {
+		if (fp(dvfsrc))
+			return 0;
+	} while (!time_after(jiffies, timeout));
+
+	return -ETIMEDOUT;
+}
+
+static void mtk_dvfsrc_mt8183_init(struct mtk_dvfsrc *dvfsrc)
+{
+	struct arm_smccc_res res;
+
+	mutex_lock(&dvfsrc->lock);
+
+	arm_smccc_smc(MTK_SIP_SPM, MTK_SIP_SPM_DVFSRC_INIT, 0, 0, 0, 0, 0, 0,
+		      &res);
+
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_0_1, 0x00100000);
+
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_2_3, 0x00210011);
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_4_5, 0x01100100);
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_6_7, 0x01210111);
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_8_9, 0x02100200);
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_10_11, 0x02210211);
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_12_13, 0x03210321);
+	dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_14_15, 0x03210321);
+
+	/* EMI/VCORE HRT, MD2SPM, BW setting */
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS0, 0x32);
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS1, 0x66);
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM0, 0x80F8);
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM1, 0x0);
+	dvfsrc_write(dvfsrc, DVFSRC_VCORE_MD2SPM0, 0x80C0);
+
+	dvfsrc_write(dvfsrc, DVFSRC_RSRV_1, 0x0000001C);
+	dvfsrc_write(dvfsrc, DVFSRC_TIMEOUT_NEXTREQ, 0x00000013);
+	dvfsrc_write(dvfsrc, DVFSRC_INT_EN, 0x2);
+
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST, 0x00290209);
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST2, 0);
+
+	dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST, 0x00150000);
+
+	dvfsrc_write(dvfsrc, DVFSRC_QOS_EN, 0x0000407F);
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST3, 0x09000000);
+
+	dvfsrc_write(dvfsrc, DVFSRC_FORCE, 0x00400000);
+	dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000C07B);
+	dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000017B);
+
+	dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST,
+		     (dvfsrc_read(dvfsrc, DVFSRC_VCORE_REQUEST)
+		     & ~(0x3 << 20)));
+	dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST,
+		     (dvfsrc_read(dvfsrc, DVFSRC_EMI_REQUEST)
+		     & ~(0x3 << 20)));
+
+	mutex_unlock(&dvfsrc->lock);
+}
+
+static int mt8183_get_target_level(struct mtk_dvfsrc *dvfsrc)
+{
+	return DVFSRC_GET_TARGET_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
+}
+
+static int mt8183_get_current_level(struct mtk_dvfsrc *dvfsrc)
+{
+	return DVFSRC_GET_CURRENT_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
+}
+
+static int get_cur_performance_level(struct mtk_dvfsrc *dvfsrc)
+{
+	int bit = 0;
+
+	if (dvfsrc->dvd->get_current_level)
+		bit = dvfsrc->dvd->get_current_level(dvfsrc);
+
+	return ffs(bit);
+}
+
+static int pm_qos_memory_bw_notify(struct notifier_block *b,
+				   unsigned long bw, void *v)
+{
+	struct mtk_dvfsrc *dvfsrc;
+
+	dvfsrc = container_of(b, struct mtk_dvfsrc, qos_notifier);
+	mutex_lock(&dvfsrc->lock);
+
+	dev_dbg(dvfsrc->dev, "data: 0x%lx\n", bw);
+	dvfsrc_write(dvfsrc, DVFSRC_SW_BW_0, bw / 100);
+
+	mutex_unlock(&dvfsrc->lock);
+
+	return NOTIFY_OK;
+}
+
+static int dvfsrc_set_performace(struct notifier_block *b,
+				 unsigned long l, void *v)
+{
+	int i, val, highest = 0, vcore_opp = 0, dram_opp = 0;
+	struct mtk_dvfsrc *dvfsrc;
+	struct scp_event_data *sc = v;
+	struct dvfsrc_domain *d;
+
+	if (sc->event_type != MTK_SCPSYS_PSTATE)
+		return 0;
+
+	dvfsrc = container_of(b, struct mtk_dvfsrc, scpsys_notifier);
+
+	mutex_lock(&dvfsrc->lock);
+	d = dvfsrc->dvd->domains;
+
+	if (l > dvfsrc->dvd->num_opp || l <= 0) {
+		dev_err(dvfsrc->dev, "pstate out of range = %ld\n", l);
+		goto out;
+	}
+
+	for (i = 0, highest = 0; i < dvfsrc->dvd->num_domains - 1; i++, d++) {
+		if (sc->domain_id == d->id)
+			d->state = l;
+		if (d->state > highest)
+			highest = d->state;
+	}
+
+	if (highest == 0) {
+		dev_err(dvfsrc->dev, "domain not match\n");
+		goto out;
+	}
+
+	/* translate pstate to dvfsrc level, and set it to DVFSRC HW */
+	vcore_opp =
+		dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].vcore_opp;
+	dram_opp = dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].dram_opp;
+
+	if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
+		dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
+			 __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
+		dvfsrc_read(dvfsrc, DVFSRC_LAST));
+		goto out;
+	}
+
+	dvfsrc_write(dvfsrc, DVFSRC_SW_REQ,
+		     dram_opp << dvfsrc->dvd->dram_sft |
+		     vcore_opp << dvfsrc->dvd->vcore_sft);
+
+	if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
+		dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
+			 __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
+			 dvfsrc_read(dvfsrc, DVFSRC_LAST));
+		goto out;
+	}
+
+	val = get_cur_performance_level(dvfsrc);
+	if (val < highest) {
+		dev_err(dvfsrc->dev, "current: %d < hightest: %x\n",
+			highest, val);
+	}
+out:
+	mutex_unlock(&dvfsrc->lock);
+
+	return 0;
+}
+
+static void pstate_notifier_register(struct mtk_dvfsrc *dvfsrc)
+{
+	dvfsrc->scpsys_notifier.notifier_call = dvfsrc_set_performace;
+	register_scpsys_notifier(&dvfsrc->scpsys_notifier);
+}
+
+static void pm_qos_notifier_register(struct mtk_dvfsrc *dvfsrc)
+{
+	dvfsrc->qos_notifier.notifier_call = pm_qos_memory_bw_notify;
+	pm_qos_add_notifier(PM_QOS_MEMORY_BANDWIDTH, &dvfsrc->qos_notifier);
+}
+
+static irqreturn_t mtk_dvfsrc_interrupt(int irq, void *dev_id)
+{
+	u32 val;
+	struct mtk_dvfsrc *dvfsrc = dev_id;
+
+	val = dvfsrc_read(dvfsrc, DVFSRC_INT);
+	dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, val);
+	dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, 0x0);
+	if (val & DVFSRC_IRQ_TIMEOUT_EN)
+		dev_warn(dvfsrc->dev, "timeout at spm = %x", val);
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_dvfsrc_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct mtk_dvfsrc *dvfsrc;
+	int ret;
+
+	dvfsrc = devm_kzalloc(&pdev->dev, sizeof(*dvfsrc), GFP_KERNEL);
+	if (!dvfsrc)
+		return -ENOMEM;
+
+	dvfsrc->dvd = of_device_get_match_data(&pdev->dev);
+	dvfsrc->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dvfsrc->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dvfsrc->regs))
+		return PTR_ERR(dvfsrc->regs);
+
+	dvfsrc->clk_dvfsrc = devm_clk_get(dvfsrc->dev, "dvfsrc");
+	if (IS_ERR(dvfsrc->clk_dvfsrc)) {
+		dev_err(dvfsrc->dev, "failed to get clock: %ld\n",
+			PTR_ERR(dvfsrc->clk_dvfsrc));
+		return PTR_ERR(dvfsrc->clk_dvfsrc);
+	}
+
+	ret = clk_prepare_enable(dvfsrc->clk_dvfsrc);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(dvfsrc->dev->of_node, "dram_type",
+				   &dvfsrc->dram_type);
+	if (ret) {
+		dev_err(dvfsrc->dev, "failed to get dram_type: %d\n", ret);
+		clk_disable_unprepare(dvfsrc->clk_dvfsrc);
+		return ret;
+	}
+
+	dvfsrc->irq = platform_get_irq(pdev, 0);
+	ret = request_irq(dvfsrc->irq, mtk_dvfsrc_interrupt
+		, IRQF_TRIGGER_HIGH, "dvfsrc", dvfsrc);
+	if (ret)
+		dev_dbg(dvfsrc->dev, "interrupt not use\n");
+
+	mutex_init(&dvfsrc->lock);
+	if (dvfsrc->dvd->init_soc)
+		dvfsrc->dvd->init_soc(dvfsrc);
+
+	pstate_notifier_register(dvfsrc);
+	pm_qos_notifier_register(dvfsrc);
+	platform_set_drvdata(pdev, dvfsrc);
+
+	return 0;
+}
+
+static const struct dvfsrc_opp dvfsrc_opp_mt8183_lp4[] = {
+	{0, 0}, {1, 0}, {1, 1}, {1, 2},
+};
+
+static const struct dvfsrc_opp dvfsrc_opp_mt8183_1p3[] = {
+	{0, 0}, {0, 1}, {1, 1}, {1, 2},
+};
+
+static const struct dvfsrc_opp *dvfsrc_opp_mt8183[] = {
+	[MT8183_DVFSRC_OPP_LP4] = dvfsrc_opp_mt8183_lp4,
+	[MT8183_DVFSRC_OPP_LP4X] = dvfsrc_opp_mt8183_1p3,
+	[MT8183_DVFSRC_OPP_LP3] = dvfsrc_opp_mt8183_1p3,
+};
+
+static struct dvfsrc_domain dvfsrc_domains_mt8183[] = {
+	{MT8183_POWER_DOMAIN_MFG_ASYNC, 0},
+	{MT8183_POWER_DOMAIN_MFG, 0},
+	{MT8183_POWER_DOMAIN_CAM, 0},
+	{MT8183_POWER_DOMAIN_DISP, 0},
+	{MT8183_POWER_DOMAIN_ISP, 0},
+	{MT8183_POWER_DOMAIN_VDEC, 0},
+	{MT8183_POWER_DOMAIN_VENC, 0},
+};
+
+static const struct dvfsrc_soc_data mt8183_data = {
+	.opps = dvfsrc_opp_mt8183,
+	.num_opp = ARRAY_SIZE(dvfsrc_opp_mt8183_lp4),
+	.regs = mt8183_regs,
+	.domains = dvfsrc_domains_mt8183,
+	.num_domains = ARRAY_SIZE(dvfsrc_domains_mt8183),
+	.init_soc = mtk_dvfsrc_mt8183_init,
+	.get_target_level = mt8183_get_target_level,
+	.get_current_level = mt8183_get_current_level,
+	.dram_sft = 0,
+	.vcore_sft = 2,
+};
+
+static int mtk_dvfsrc_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id mtk_dvfsrc_of_match[] = {
+	{
+		.compatible = "mediatek,mt8183-dvfsrc",
+		.data = &mt8183_data,
+	}, {
+		/* sentinel */
+	},
+};
+
+static struct platform_driver mtk_dvfsrc_driver = {
+	.probe	= mtk_dvfsrc_probe,
+	.remove	= mtk_dvfsrc_remove,
+	.driver = {
+		.name = "mtk-dvfsrc",
+		.of_match_table = of_match_ptr(mtk_dvfsrc_of_match),
+	},
+};
+
+builtin_platform_driver(mtk_dvfsrc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MTK DVFSRC driver");
-- 
1.9.1

Re: [RFC PATCH 6/7] soc: mediatek: add MT8183 dvfsrc support

[Nicolas Boichat]

On Wed, Jan 2, 2019 at 10:01 PM Henry Chen <henryc.chen@mediatek.com> wrote:
>
> Add dvfsrc driver for MT8183
>
> Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> ---
>  drivers/soc/mediatek/Kconfig      |  15 ++
>  drivers/soc/mediatek/Makefile     |   1 +
>  drivers/soc/mediatek/mtk-dvfsrc.c | 473 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 489 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mtk-dvfsrc.c
>
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index a7d0667..f956f03 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -12,6 +12,21 @@ config MTK_INFRACFG
>           INFRACFG controller contains various infrastructure registers not
>           directly associated to any device.
>
> +config MTK_DVFSRC
> +       bool "MediaTek DVFSRC Support"
> +       depends on ARCH_MEDIATEK
> +       default ARCH_MEDIATEK
> +       select REGMAP
> +       select MTK_INFRACFG
> +       select PM_GENERIC_DOMAINS if PM
> +       depends on MTK_SCPSYS
> +       help
> +         Say yes here to add support for the MediaTek DVFSRC found
> +         on different MediaTek SoCs. The DVFSRC is a proprietary
> +         hardware which is used to collect all the requests from
> +         system and turn into the decision of minimum Vcore voltage
> +         and minimum DRAM frequency to fulfill those requests.
> +
>  config MTK_PMIC_WRAP
>         tristate "MediaTek PMIC Wrapper Support"
>         depends on RESET_CONTROLLER
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 9dc6670..5c010b9 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_MTK_DVFSRC) += mtk-dvfsrc.o
>  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> diff --git a/drivers/soc/mediatek/mtk-dvfsrc.c b/drivers/soc/mediatek/mtk-dvfsrc.c
> new file mode 100644
> index 0000000..af462a3
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-dvfsrc.c
> @@ -0,0 +1,473 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 MediaTek Inc.
> + */
> +#include <linux/arm-smccc.h>
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/delay.h>

Alphabetical order.

> +#include <linux/kthread.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <soc/mediatek/mtk_sip.h>
> +#include <dt-bindings/power/mt8183-power.h>
> +#include <dt-bindings/soc/mtk,dvfsrc.h>
> +#include "mtk-scpsys.h"
> +
> +#define DVFSRC_IDLE            0x00
> +#define DVFSRC_GET_TARGET_LEVEL(x)     (((x) >> 0) & 0x0000ffff)
> +#define DVFSRC_GET_CURRENT_LEVEL(x)    (((x) >> 16) & 0x0000ffff)
> +
> +/* macro for irq */
> +#define DVFSRC_IRQ_TIMEOUT_EN          BIT(1)
> +
> +struct dvfsrc_opp {
> +       int vcore_opp;
> +       int dram_opp;
> +};
> +
> +struct dvfsrc_domain {
> +       int id;
> +       int state;
> +};
> +
> +struct mtk_dvfsrc;
> +struct dvfsrc_soc_data {
> +       const int *regs;
> +       int num_opp;
> +       int num_domains;
> +       int dram_sft;
> +       int vcore_sft;
> +       const struct dvfsrc_opp **opps;
> +       struct dvfsrc_domain *domains;
> +       void (*init_soc)(struct mtk_dvfsrc *dvfsrc);
> +       int (*get_target_level)(struct mtk_dvfsrc *dvfsrc);
> +       int (*get_current_level)(struct mtk_dvfsrc *dvfsrc);
> +};
> +
> +struct mtk_dvfsrc {
> +       struct device *dev;
> +       struct clk *clk_dvfsrc;
> +       const struct dvfsrc_soc_data *dvd;
> +       int dram_type;
> +       int irq;
> +       void __iomem *regs;
> +       struct mutex lock;      /* generic mutex for dvfsrc driver */
> +
> +       struct notifier_block qos_notifier;
> +       struct notifier_block scpsys_notifier;
> +};
> +
> +static u32 dvfsrc_read(struct mtk_dvfsrc *dvfs, u32 offset)
> +{
> +       return readl(dvfs->regs + dvfs->dvd->regs[offset]);
> +}
> +
> +static void dvfsrc_write(struct mtk_dvfsrc *dvfs, u32 offset, u32 val)
> +{
> +       writel(val, dvfs->regs + dvfs->dvd->regs[offset]);
> +}
> +
> +enum dvfsrc_regs {
> +       DVFSRC_BASIC_CONTROL,
> +       DVFSRC_SW_REQ,
> +       DVFSRC_SW_REQ2,
> +       DVFSRC_EMI_REQUEST,
> +       DVFSRC_EMI_REQUEST2,
> +       DVFSRC_EMI_REQUEST3,
> +       DVFSRC_EMI_QOS0,
> +       DVFSRC_EMI_QOS1,
> +       DVFSRC_EMI_QOS2,
> +       DVFSRC_EMI_MD2SPM0,
> +       DVFSRC_EMI_MD2SPM1,
> +       DVFSRC_VCORE_REQUEST,
> +       DVFSRC_VCORE_REQUEST2,
> +       DVFSRC_VCORE_MD2SPM0,
> +       DVFSRC_INT,
> +       DVFSRC_INT_EN,
> +       DVFSRC_INT_CLR,
> +       DVFSRC_TIMEOUT_NEXTREQ,
> +       DVFSRC_LEVEL,
> +       DVFSRC_LEVEL_LABEL_0_1,
> +       DVFSRC_LEVEL_LABEL_2_3,
> +       DVFSRC_LEVEL_LABEL_4_5,
> +       DVFSRC_LEVEL_LABEL_6_7,
> +       DVFSRC_LEVEL_LABEL_8_9,
> +       DVFSRC_LEVEL_LABEL_10_11,
> +       DVFSRC_LEVEL_LABEL_12_13,
> +       DVFSRC_LEVEL_LABEL_14_15,
> +       DVFSRC_SW_BW_0,
> +       DVFSRC_QOS_EN,
> +       DVFSRC_FORCE,
> +       DVFSRC_LAST,
> +       DVFSRC_RSRV_0,
> +       DVFSRC_RSRV_1,
> +};
> +
> +static int mt8183_regs[] = {
> +       [DVFSRC_BASIC_CONTROL] =        0x0,
> +       [DVFSRC_SW_REQ] =               0x4,
> +       [DVFSRC_SW_REQ2] =              0x8,
> +       [DVFSRC_EMI_REQUEST] =          0xC,
> +       [DVFSRC_EMI_REQUEST2] =         0x10,
> +       [DVFSRC_EMI_REQUEST3] =         0x14,
> +       [DVFSRC_EMI_QOS0] =             0x24,
> +       [DVFSRC_EMI_QOS1] =             0x28,
> +       [DVFSRC_EMI_QOS2] =             0x2C,
> +       [DVFSRC_EMI_MD2SPM0] =          0x30,
> +       [DVFSRC_EMI_MD2SPM1] =          0x34,
> +       [DVFSRC_VCORE_REQUEST] =        0x48,
> +       [DVFSRC_VCORE_REQUEST2] =       0x4C,
> +       [DVFSRC_VCORE_MD2SPM0] =        0x68,
> +       [DVFSRC_INT] =                  0x98,
> +       [DVFSRC_INT_EN] =               0x9C,
> +       [DVFSRC_INT_CLR] =              0xA0,
> +       [DVFSRC_TIMEOUT_NEXTREQ] =      0xD8,
> +       [DVFSRC_LEVEL] =                0xDC,
> +       [DVFSRC_LEVEL_LABEL_0_1] =      0xE0,
> +       [DVFSRC_LEVEL_LABEL_2_3] =      0xE4,
> +       [DVFSRC_LEVEL_LABEL_4_5] =      0xE8,
> +       [DVFSRC_LEVEL_LABEL_6_7] =      0xEC,
> +       [DVFSRC_LEVEL_LABEL_8_9] =      0xF0,
> +       [DVFSRC_LEVEL_LABEL_10_11] =    0xF4,
> +       [DVFSRC_LEVEL_LABEL_12_13] =    0xF8,
> +       [DVFSRC_LEVEL_LABEL_14_15] =    0xFC,
> +       [DVFSRC_SW_BW_0] =              0x160,
> +       [DVFSRC_QOS_EN] =               0x180,
> +       [DVFSRC_FORCE] =                0x300,
> +       [DVFSRC_LAST] =                 0x308,
> +       [DVFSRC_RSRV_0] =               0x600,
> +       [DVFSRC_RSRV_1] =               0x604,
> +};

Why do you need this indirection table? Do you plan to support HW with
very different register offsets?

> +
> +static bool dvfsrc_is_idle(struct mtk_dvfsrc *dvfsrc)
> +{
> +       int val = 0;

So 0 is DVFSRC_IDLE, so the function returns true?

Maybe this is more readable?

if (!dvfsrc->dvd->get_target_level)
    return true;

return dvfsrc->dvd->get_target_level(dvfsrc) == DVFSRC_IDLE;

> +
> +       if (dvfsrc->dvd->get_target_level)

However... can this be false? In what case?

> +               val = dvfsrc->dvd->get_target_level(dvfsrc);
> +
> +       return val == DVFSRC_IDLE;
> +}
> +
> +static int dvfsrc_wait_for_state(struct mtk_dvfsrc *dvfsrc,
> +                                bool (*fp)(struct mtk_dvfsrc *))
> +{
> +       unsigned long timeout;
> +
> +       timeout = jiffies + usecs_to_jiffies(1000);
> +
> +       do {
> +               if (fp(dvfsrc))
> +                       return 0;
> +       } while (!time_after(jiffies, timeout));
> +
> +       return -ETIMEDOUT;
> +}

Can we use wait_event_timeout here?

> +
> +static void mtk_dvfsrc_mt8183_init(struct mtk_dvfsrc *dvfsrc)
> +{
> +       struct arm_smccc_res res;
> +
> +       mutex_lock(&dvfsrc->lock);
> +
> +       arm_smccc_smc(MTK_SIP_SPM, MTK_SIP_SPM_DVFSRC_INIT, 0, 0, 0, 0, 0, 0,
> +                     &res);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_0_1, 0x00100000);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_2_3, 0x00210011);
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_4_5, 0x01100100);
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_6_7, 0x01210111);
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_8_9, 0x02100200);
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_10_11, 0x02210211);
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_12_13, 0x03210321);
> +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_14_15, 0x03210321);
> +
> +       /* EMI/VCORE HRT, MD2SPM, BW setting */
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS0, 0x32);
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS1, 0x66);
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM0, 0x80F8);
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM1, 0x0);
> +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_MD2SPM0, 0x80C0);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_RSRV_1, 0x0000001C);
> +       dvfsrc_write(dvfsrc, DVFSRC_TIMEOUT_NEXTREQ, 0x00000013);
> +       dvfsrc_write(dvfsrc, DVFSRC_INT_EN, 0x2);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST, 0x00290209);
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST2, 0);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST, 0x00150000);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_QOS_EN, 0x0000407F);
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST3, 0x09000000);
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_FORCE, 0x00400000);
> +       dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000C07B);
> +       dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000017B);

Lots of magic values here, can you describe what they do? (either as
#define, or at the very least with comments)

> +
> +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST,
> +                    (dvfsrc_read(dvfsrc, DVFSRC_VCORE_REQUEST)
> +                    & ~(0x3 << 20)));
> +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST,
> +                    (dvfsrc_read(dvfsrc, DVFSRC_EMI_REQUEST)
> +                    & ~(0x3 << 20)));
> +
> +       mutex_unlock(&dvfsrc->lock);
> +}
> +
> +static int mt8183_get_target_level(struct mtk_dvfsrc *dvfsrc)
> +{
> +       return DVFSRC_GET_TARGET_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
> +}
> +
> +static int mt8183_get_current_level(struct mtk_dvfsrc *dvfsrc)
> +{
> +       return DVFSRC_GET_CURRENT_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
> +}
> +
> +static int get_cur_performance_level(struct mtk_dvfsrc *dvfsrc)
> +{
> +       int bit = 0;
> +
> +       if (dvfsrc->dvd->get_current_level)

Same question as above, is this possible?

> +               bit = dvfsrc->dvd->get_current_level(dvfsrc);
> +
> +       return ffs(bit);

So ffs(0) = 0... Maybe just return 0 if get_current_level is NULL.

> +}
> +
> +static int pm_qos_memory_bw_notify(struct notifier_block *b,
> +                                  unsigned long bw, void *v)
> +{
> +       struct mtk_dvfsrc *dvfsrc;
> +
> +       dvfsrc = container_of(b, struct mtk_dvfsrc, qos_notifier);
> +       mutex_lock(&dvfsrc->lock);
> +
> +       dev_dbg(dvfsrc->dev, "data: 0x%lx\n", bw);
> +       dvfsrc_write(dvfsrc, DVFSRC_SW_BW_0, bw / 100);
> +
> +       mutex_unlock(&dvfsrc->lock);
> +
> +       return NOTIFY_OK;
> +}
> +
> +static int dvfsrc_set_performace(struct notifier_block *b,

set_performaNce

> +                                unsigned long l, void *v)
> +{
> +       int i, val, highest = 0, vcore_opp = 0, dram_opp = 0;

No need to initialize any of these to 0.

> +       struct mtk_dvfsrc *dvfsrc;
> +       struct scp_event_data *sc = v;
> +       struct dvfsrc_domain *d;
> +
> +       if (sc->event_type != MTK_SCPSYS_PSTATE)
> +               return 0;
> +
> +       dvfsrc = container_of(b, struct mtk_dvfsrc, scpsys_notifier);
> +
> +       mutex_lock(&dvfsrc->lock);
> +       d = dvfsrc->dvd->domains;
> +
> +       if (l > dvfsrc->dvd->num_opp || l <= 0) {
> +               dev_err(dvfsrc->dev, "pstate out of range = %ld\n", l);
> +               goto out;
> +       }
> +
> +       for (i = 0, highest = 0; i < dvfsrc->dvd->num_domains - 1; i++, d++) {
> +               if (sc->domain_id == d->id)
> +                       d->state = l;
> +               if (d->state > highest)
> +                       highest = d->state;
> +       }
> +
> +       if (highest == 0) {
> +               dev_err(dvfsrc->dev, "domain not match\n");
> +               goto out;
> +       }
> +
> +       /* translate pstate to dvfsrc level, and set it to DVFSRC HW */
> +       vcore_opp =
> +               dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].vcore_opp;
> +       dram_opp = dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].dram_opp;
> +
> +       if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
> +               dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
> +                        __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
> +               dvfsrc_read(dvfsrc, DVFSRC_LAST));
> +               goto out;
> +       }
> +
> +       dvfsrc_write(dvfsrc, DVFSRC_SW_REQ,
> +                    dram_opp << dvfsrc->dvd->dram_sft |
> +                    vcore_opp << dvfsrc->dvd->vcore_sft);
> +
> +       if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
> +               dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
> +                        __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
> +                        dvfsrc_read(dvfsrc, DVFSRC_LAST));
> +               goto out;
> +       }
> +
> +       val = get_cur_performance_level(dvfsrc);
> +       if (val < highest) {
> +               dev_err(dvfsrc->dev, "current: %d < hightest: %x\n",

highest

> +                       highest, val);
> +       }
> +out:
> +       mutex_unlock(&dvfsrc->lock);
> +
> +       return 0;
> +}
> +
> +static void pstate_notifier_register(struct mtk_dvfsrc *dvfsrc)
> +{
> +       dvfsrc->scpsys_notifier.notifier_call = dvfsrc_set_performace;
> +       register_scpsys_notifier(&dvfsrc->scpsys_notifier);
> +}
> +
> +static void pm_qos_notifier_register(struct mtk_dvfsrc *dvfsrc)
> +{
> +       dvfsrc->qos_notifier.notifier_call = pm_qos_memory_bw_notify;
> +       pm_qos_add_notifier(PM_QOS_MEMORY_BANDWIDTH, &dvfsrc->qos_notifier);
> +}
> +
> +static irqreturn_t mtk_dvfsrc_interrupt(int irq, void *dev_id)
> +{
> +       u32 val;
> +       struct mtk_dvfsrc *dvfsrc = dev_id;
> +
> +       val = dvfsrc_read(dvfsrc, DVFSRC_INT);
> +       dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, val);
> +       dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, 0x0);
> +       if (val & DVFSRC_IRQ_TIMEOUT_EN)
> +               dev_warn(dvfsrc->dev, "timeout at spm = %x", val);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int mtk_dvfsrc_probe(struct platform_device *pdev)
> +{
> +       struct resource *res;
> +       struct mtk_dvfsrc *dvfsrc;
> +       int ret;
> +
> +       dvfsrc = devm_kzalloc(&pdev->dev, sizeof(*dvfsrc), GFP_KERNEL);
> +       if (!dvfsrc)
> +               return -ENOMEM;
> +
> +       dvfsrc->dvd = of_device_get_match_data(&pdev->dev);
> +       dvfsrc->dev = &pdev->dev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       dvfsrc->regs = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(dvfsrc->regs))
> +               return PTR_ERR(dvfsrc->regs);
> +
> +       dvfsrc->clk_dvfsrc = devm_clk_get(dvfsrc->dev, "dvfsrc");
> +       if (IS_ERR(dvfsrc->clk_dvfsrc)) {
> +               dev_err(dvfsrc->dev, "failed to get clock: %ld\n",
> +                       PTR_ERR(dvfsrc->clk_dvfsrc));
> +               return PTR_ERR(dvfsrc->clk_dvfsrc);
> +       }
> +
> +       ret = clk_prepare_enable(dvfsrc->clk_dvfsrc);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_property_read_u32(dvfsrc->dev->of_node, "dram_type",
> +                                  &dvfsrc->dram_type);
> +       if (ret) {
> +               dev_err(dvfsrc->dev, "failed to get dram_type: %d\n", ret);
> +               clk_disable_unprepare(dvfsrc->clk_dvfsrc);
> +               return ret;
> +       }
> +
> +       dvfsrc->irq = platform_get_irq(pdev, 0);
> +       ret = request_irq(dvfsrc->irq, mtk_dvfsrc_interrupt
> +               , IRQF_TRIGGER_HIGH, "dvfsrc", dvfsrc);

comma on the line above

> +       if (ret)
> +               dev_dbg(dvfsrc->dev, "interrupt not use\n");
> +
> +       mutex_init(&dvfsrc->lock);
> +       if (dvfsrc->dvd->init_soc)
> +               dvfsrc->dvd->init_soc(dvfsrc);
> +
> +       pstate_notifier_register(dvfsrc);
> +       pm_qos_notifier_register(dvfsrc);
> +       platform_set_drvdata(pdev, dvfsrc);
> +
> +       return 0;
> +}
> +
> +static const struct dvfsrc_opp dvfsrc_opp_mt8183_lp4[] = {
> +       {0, 0}, {1, 0}, {1, 1}, {1, 2},
> +};
> +
> +static const struct dvfsrc_opp dvfsrc_opp_mt8183_1p3[] = {
> +       {0, 0}, {0, 1}, {1, 1}, {1, 2},
> +};

1p3? Should this be lp3?

> +
> +static const struct dvfsrc_opp *dvfsrc_opp_mt8183[] = {
> +       [MT8183_DVFSRC_OPP_LP4] = dvfsrc_opp_mt8183_lp4,
> +       [MT8183_DVFSRC_OPP_LP4X] = dvfsrc_opp_mt8183_1p3,
> +       [MT8183_DVFSRC_OPP_LP3] = dvfsrc_opp_mt8183_1p3,
> +};
> +
> +static struct dvfsrc_domain dvfsrc_domains_mt8183[] = {
> +       {MT8183_POWER_DOMAIN_MFG_ASYNC, 0},
> +       {MT8183_POWER_DOMAIN_MFG, 0},
> +       {MT8183_POWER_DOMAIN_CAM, 0},
> +       {MT8183_POWER_DOMAIN_DISP, 0},
> +       {MT8183_POWER_DOMAIN_ISP, 0},
> +       {MT8183_POWER_DOMAIN_VDEC, 0},
> +       {MT8183_POWER_DOMAIN_VENC, 0},
> +};
> +
> +static const struct dvfsrc_soc_data mt8183_data = {
> +       .opps = dvfsrc_opp_mt8183,
> +       .num_opp = ARRAY_SIZE(dvfsrc_opp_mt8183_lp4),
> +       .regs = mt8183_regs,
> +       .domains = dvfsrc_domains_mt8183,
> +       .num_domains = ARRAY_SIZE(dvfsrc_domains_mt8183),
> +       .init_soc = mtk_dvfsrc_mt8183_init,
> +       .get_target_level = mt8183_get_target_level,
> +       .get_current_level = mt8183_get_current_level,
> +       .dram_sft = 0,
> +       .vcore_sft = 2,
> +};
> +
> +static int mtk_dvfsrc_remove(struct platform_device *pdev)
> +{
> +       return 0;

Should you at least disable the clock? (since you do it in the error
path in _probe...)

clk_disable_unprepare(dvfsrc->clk_dvfsrc);

> +}
> +
> +static const struct of_device_id mtk_dvfsrc_of_match[] = {
> +       {
> +               .compatible = "mediatek,mt8183-dvfsrc",
> +               .data = &mt8183_data,
> +       }, {
> +               /* sentinel */
> +       },
> +};
> +
> +static struct platform_driver mtk_dvfsrc_driver = {
> +       .probe  = mtk_dvfsrc_probe,
> +       .remove = mtk_dvfsrc_remove,
> +       .driver = {
> +               .name = "mtk-dvfsrc",
> +               .of_match_table = of_match_ptr(mtk_dvfsrc_of_match),
> +       },
> +};
> +
> +builtin_platform_driver(mtk_dvfsrc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("MTK DVFSRC driver");
> --
> 1.9.1
>

Re: [RFC PATCH 6/7] soc: mediatek: add MT8183 dvfsrc support

[Henry Chen]

On Thu, 2019-01-03 at 10:16 +0800, Nicolas Boichat wrote:
> On Wed, Jan 2, 2019 at 10:01 PM Henry Chen <henryc.chen@mediatek.com> wrote:
> >
> > Add dvfsrc driver for MT8183
> >
> > Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> > ---
> >  drivers/soc/mediatek/Kconfig      |  15 ++
> >  drivers/soc/mediatek/Makefile     |   1 +
> >  drivers/soc/mediatek/mtk-dvfsrc.c | 473 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 489 insertions(+)
> >  create mode 100644 drivers/soc/mediatek/mtk-dvfsrc.c
> >
> > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> > index a7d0667..f956f03 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -12,6 +12,21 @@ config MTK_INFRACFG
> >           INFRACFG controller contains various infrastructure registers not
> >           directly associated to any device.
> >
> > +config MTK_DVFSRC
> > +       bool "MediaTek DVFSRC Support"
> > +       depends on ARCH_MEDIATEK
> > +       default ARCH_MEDIATEK
> > +       select REGMAP
> > +       select MTK_INFRACFG
> > +       select PM_GENERIC_DOMAINS if PM
> > +       depends on MTK_SCPSYS
> > +       help
> > +         Say yes here to add support for the MediaTek DVFSRC found
> > +         on different MediaTek SoCs. The DVFSRC is a proprietary
> > +         hardware which is used to collect all the requests from
> > +         system and turn into the decision of minimum Vcore voltage
> > +         and minimum DRAM frequency to fulfill those requests.
> > +
> >  config MTK_PMIC_WRAP
> >         tristate "MediaTek PMIC Wrapper Support"
> >         depends on RESET_CONTROLLER
> > diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> > index 9dc6670..5c010b9 100644
> > --- a/drivers/soc/mediatek/Makefile
> > +++ b/drivers/soc/mediatek/Makefile
> > @@ -1,3 +1,4 @@
> > +obj-$(CONFIG_MTK_DVFSRC) += mtk-dvfsrc.o
> >  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
> >  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> >  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> > diff --git a/drivers/soc/mediatek/mtk-dvfsrc.c b/drivers/soc/mediatek/mtk-dvfsrc.c
> > new file mode 100644
> > index 0000000..af462a3
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mtk-dvfsrc.c
> > @@ -0,0 +1,473 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 MediaTek Inc.
> > + */
> > +#include <linux/arm-smccc.h>
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/irq.h>
> > +#include <linux/delay.h>
> 
> Alphabetical order.
I will fix it.
> 
> > +#include <linux/kthread.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_qos.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +#include <soc/mediatek/mtk_sip.h>
> > +#include <dt-bindings/power/mt8183-power.h>
> > +#include <dt-bindings/soc/mtk,dvfsrc.h>
> > +#include "mtk-scpsys.h"
> > +
> > +#define DVFSRC_IDLE            0x00
> > +#define DVFSRC_GET_TARGET_LEVEL(x)     (((x) >> 0) & 0x0000ffff)
> > +#define DVFSRC_GET_CURRENT_LEVEL(x)    (((x) >> 16) & 0x0000ffff)
> > +
> > +/* macro for irq */
> > +#define DVFSRC_IRQ_TIMEOUT_EN          BIT(1)
> > +
> > +struct dvfsrc_opp {
> > +       int vcore_opp;
> > +       int dram_opp;
> > +};
> > +
> > +struct dvfsrc_domain {
> > +       int id;
> > +       int state;
> > +};
> > +
> > +struct mtk_dvfsrc;
> > +struct dvfsrc_soc_data {
> > +       const int *regs;
> > +       int num_opp;
> > +       int num_domains;
> > +       int dram_sft;
> > +       int vcore_sft;
> > +       const struct dvfsrc_opp **opps;
> > +       struct dvfsrc_domain *domains;
> > +       void (*init_soc)(struct mtk_dvfsrc *dvfsrc);
> > +       int (*get_target_level)(struct mtk_dvfsrc *dvfsrc);
> > +       int (*get_current_level)(struct mtk_dvfsrc *dvfsrc);
> > +};
> > +
> > +struct mtk_dvfsrc {
> > +       struct device *dev;
> > +       struct clk *clk_dvfsrc;
> > +       const struct dvfsrc_soc_data *dvd;
> > +       int dram_type;
> > +       int irq;
> > +       void __iomem *regs;
> > +       struct mutex lock;      /* generic mutex for dvfsrc driver */
> > +
> > +       struct notifier_block qos_notifier;
> > +       struct notifier_block scpsys_notifier;
> > +};
> > +
> > +static u32 dvfsrc_read(struct mtk_dvfsrc *dvfs, u32 offset)
> > +{
> > +       return readl(dvfs->regs + dvfs->dvd->regs[offset]);
> > +}
> > +
> > +static void dvfsrc_write(struct mtk_dvfsrc *dvfs, u32 offset, u32 val)
> > +{
> > +       writel(val, dvfs->regs + dvfs->dvd->regs[offset]);
> > +}
> > +
> > +enum dvfsrc_regs {
> > +       DVFSRC_BASIC_CONTROL,
> > +       DVFSRC_SW_REQ,
> > +       DVFSRC_SW_REQ2,
> > +       DVFSRC_EMI_REQUEST,
> > +       DVFSRC_EMI_REQUEST2,
> > +       DVFSRC_EMI_REQUEST3,
> > +       DVFSRC_EMI_QOS0,
> > +       DVFSRC_EMI_QOS1,
> > +       DVFSRC_EMI_QOS2,
> > +       DVFSRC_EMI_MD2SPM0,
> > +       DVFSRC_EMI_MD2SPM1,
> > +       DVFSRC_VCORE_REQUEST,
> > +       DVFSRC_VCORE_REQUEST2,
> > +       DVFSRC_VCORE_MD2SPM0,
> > +       DVFSRC_INT,
> > +       DVFSRC_INT_EN,
> > +       DVFSRC_INT_CLR,
> > +       DVFSRC_TIMEOUT_NEXTREQ,
> > +       DVFSRC_LEVEL,
> > +       DVFSRC_LEVEL_LABEL_0_1,
> > +       DVFSRC_LEVEL_LABEL_2_3,
> > +       DVFSRC_LEVEL_LABEL_4_5,
> > +       DVFSRC_LEVEL_LABEL_6_7,
> > +       DVFSRC_LEVEL_LABEL_8_9,
> > +       DVFSRC_LEVEL_LABEL_10_11,
> > +       DVFSRC_LEVEL_LABEL_12_13,
> > +       DVFSRC_LEVEL_LABEL_14_15,
> > +       DVFSRC_SW_BW_0,
> > +       DVFSRC_QOS_EN,
> > +       DVFSRC_FORCE,
> > +       DVFSRC_LAST,
> > +       DVFSRC_RSRV_0,
> > +       DVFSRC_RSRV_1,
> > +};
> > +
> > +static int mt8183_regs[] = {
> > +       [DVFSRC_BASIC_CONTROL] =        0x0,
> > +       [DVFSRC_SW_REQ] =               0x4,
> > +       [DVFSRC_SW_REQ2] =              0x8,
> > +       [DVFSRC_EMI_REQUEST] =          0xC,
> > +       [DVFSRC_EMI_REQUEST2] =         0x10,
> > +       [DVFSRC_EMI_REQUEST3] =         0x14,
> > +       [DVFSRC_EMI_QOS0] =             0x24,
> > +       [DVFSRC_EMI_QOS1] =             0x28,
> > +       [DVFSRC_EMI_QOS2] =             0x2C,
> > +       [DVFSRC_EMI_MD2SPM0] =          0x30,
> > +       [DVFSRC_EMI_MD2SPM1] =          0x34,
> > +       [DVFSRC_VCORE_REQUEST] =        0x48,
> > +       [DVFSRC_VCORE_REQUEST2] =       0x4C,
> > +       [DVFSRC_VCORE_MD2SPM0] =        0x68,
> > +       [DVFSRC_INT] =                  0x98,
> > +       [DVFSRC_INT_EN] =               0x9C,
> > +       [DVFSRC_INT_CLR] =              0xA0,
> > +       [DVFSRC_TIMEOUT_NEXTREQ] =      0xD8,
> > +       [DVFSRC_LEVEL] =                0xDC,
> > +       [DVFSRC_LEVEL_LABEL_0_1] =      0xE0,
> > +       [DVFSRC_LEVEL_LABEL_2_3] =      0xE4,
> > +       [DVFSRC_LEVEL_LABEL_4_5] =      0xE8,
> > +       [DVFSRC_LEVEL_LABEL_6_7] =      0xEC,
> > +       [DVFSRC_LEVEL_LABEL_8_9] =      0xF0,
> > +       [DVFSRC_LEVEL_LABEL_10_11] =    0xF4,
> > +       [DVFSRC_LEVEL_LABEL_12_13] =    0xF8,
> > +       [DVFSRC_LEVEL_LABEL_14_15] =    0xFC,
> > +       [DVFSRC_SW_BW_0] =              0x160,
> > +       [DVFSRC_QOS_EN] =               0x180,
> > +       [DVFSRC_FORCE] =                0x300,
> > +       [DVFSRC_LAST] =                 0x308,
> > +       [DVFSRC_RSRV_0] =               0x600,
> > +       [DVFSRC_RSRV_1] =               0x604,
> > +};
> 
> Why do you need this indirection table? Do you plan to support HW with
> very different register offsets?
Yes. The dvfsrc not only on mt8183 but also exist on following Mediatek
SoCs. I afraid some of them may be different because the design is
changed.
> 
> > +
> > +static bool dvfsrc_is_idle(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       int val = 0;
> 
> So 0 is DVFSRC_IDLE, so the function returns true?
> 
> Maybe this is more readable?
Ok. it good to me.
> 
> if (!dvfsrc->dvd->get_target_level)
>     return true;
> 
> return dvfsrc->dvd->get_target_level(dvfsrc) == DVFSRC_IDLE;
> 
> > +
> > +       if (dvfsrc->dvd->get_target_level)
> 
> However... can this be false? In what case?
hmm, it just null function check to prevent someone who forget to hook
the callback.

> 
> > +               val = dvfsrc->dvd->get_target_level(dvfsrc);
> > +
> > +       return val == DVFSRC_IDLE;
> > +}
> > +
> > +static int dvfsrc_wait_for_state(struct mtk_dvfsrc *dvfsrc,
> > +                                bool (*fp)(struct mtk_dvfsrc *))
> > +{
> > +       unsigned long timeout;
> > +
> > +       timeout = jiffies + usecs_to_jiffies(1000);
> > +
> > +       do {
> > +               if (fp(dvfsrc))
> > +                       return 0;
> > +       } while (!time_after(jiffies, timeout));
> > +
> > +       return -ETIMEDOUT;
> > +}
> 
> Can we use wait_event_timeout here?
I will give a try.
> 
> > +
> > +static void mtk_dvfsrc_mt8183_init(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       struct arm_smccc_res res;
> > +
> > +       mutex_lock(&dvfsrc->lock);
> > +
> > +       arm_smccc_smc(MTK_SIP_SPM, MTK_SIP_SPM_DVFSRC_INIT, 0, 0, 0, 0, 0, 0,
> > +                     &res);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_0_1, 0x00100000);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_2_3, 0x00210011);
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_4_5, 0x01100100);
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_6_7, 0x01210111);
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_8_9, 0x02100200);
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_10_11, 0x02210211);
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_12_13, 0x03210321);
> > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_14_15, 0x03210321);
> > +
> > +       /* EMI/VCORE HRT, MD2SPM, BW setting */
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS0, 0x32);
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS1, 0x66);
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM0, 0x80F8);
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM1, 0x0);
> > +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_MD2SPM0, 0x80C0);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_RSRV_1, 0x0000001C);
> > +       dvfsrc_write(dvfsrc, DVFSRC_TIMEOUT_NEXTREQ, 0x00000013);
> > +       dvfsrc_write(dvfsrc, DVFSRC_INT_EN, 0x2);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST, 0x00290209);
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST2, 0);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST, 0x00150000);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_QOS_EN, 0x0000407F);
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST3, 0x09000000);
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_FORCE, 0x00400000);
> > +       dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000C07B);
> > +       dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000017B);
> 
> Lots of magic values here, can you describe what they do? (either as
> #define, or at the very least with comments)
I will give a brief comment on these settings.
> 
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST,
> > +                    (dvfsrc_read(dvfsrc, DVFSRC_VCORE_REQUEST)
> > +                    & ~(0x3 << 20)));
> > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST,
> > +                    (dvfsrc_read(dvfsrc, DVFSRC_EMI_REQUEST)
> > +                    & ~(0x3 << 20)));
> > +
> > +       mutex_unlock(&dvfsrc->lock);
> > +}
> > +
> > +static int mt8183_get_target_level(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       return DVFSRC_GET_TARGET_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
> > +}
> > +
> > +static int mt8183_get_current_level(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       return DVFSRC_GET_CURRENT_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
> > +}
> > +
> > +static int get_cur_performance_level(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       int bit = 0;
> > +
> > +       if (dvfsrc->dvd->get_current_level)
> 
> Same question as above, is this possible?
> 
> > +               bit = dvfsrc->dvd->get_current_level(dvfsrc);
> > +
> > +       return ffs(bit);
> 
> So ffs(0) = 0... Maybe just return 0 if get_current_level is NULL.
ok. it will be change to
	if  (!dvfsrc->dvd->get_current_level)
		return 0;
	return ffs(dvfsrc->dvd->get_current_level(dvfsrc));
> 
> > +}
> > +
> > +static int pm_qos_memory_bw_notify(struct notifier_block *b,
> > +                                  unsigned long bw, void *v)
> > +{
> > +       struct mtk_dvfsrc *dvfsrc;
> > +
> > +       dvfsrc = container_of(b, struct mtk_dvfsrc, qos_notifier);
> > +       mutex_lock(&dvfsrc->lock);
> > +
> > +       dev_dbg(dvfsrc->dev, "data: 0x%lx\n", bw);
> > +       dvfsrc_write(dvfsrc, DVFSRC_SW_BW_0, bw / 100);
> > +
> > +       mutex_unlock(&dvfsrc->lock);
> > +
> > +       return NOTIFY_OK;
> > +}
> > +
> > +static int dvfsrc_set_performace(struct notifier_block *b,
> 
> set_performaNce
Thanks.
> 
> > +                                unsigned long l, void *v)
> > +{
> > +       int i, val, highest = 0, vcore_opp = 0, dram_opp = 0;
> 
> No need to initialize any of these to 0.
Ok.
> 
> > +       struct mtk_dvfsrc *dvfsrc;
> > +       struct scp_event_data *sc = v;
> > +       struct dvfsrc_domain *d;
> > +
> > +       if (sc->event_type != MTK_SCPSYS_PSTATE)
> > +               return 0;
> > +
> > +       dvfsrc = container_of(b, struct mtk_dvfsrc, scpsys_notifier);
> > +
> > +       mutex_lock(&dvfsrc->lock);
> > +       d = dvfsrc->dvd->domains;
> > +
> > +       if (l > dvfsrc->dvd->num_opp || l <= 0) {
> > +               dev_err(dvfsrc->dev, "pstate out of range = %ld\n", l);
> > +               goto out;
> > +       }
> > +
> > +       for (i = 0, highest = 0; i < dvfsrc->dvd->num_domains - 1; i++, d++) {
> > +               if (sc->domain_id == d->id)
> > +                       d->state = l;
> > +               if (d->state > highest)
> > +                       highest = d->state;
> > +       }
> > +
> > +       if (highest == 0) {
> > +               dev_err(dvfsrc->dev, "domain not match\n");
> > +               goto out;
> > +       }
> > +
> > +       /* translate pstate to dvfsrc level, and set it to DVFSRC HW */
> > +       vcore_opp =
> > +               dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].vcore_opp;
> > +       dram_opp = dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].dram_opp;
> > +
> > +       if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
> > +               dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
> > +                        __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
> > +               dvfsrc_read(dvfsrc, DVFSRC_LAST));
> > +               goto out;
> > +       }
> > +
> > +       dvfsrc_write(dvfsrc, DVFSRC_SW_REQ,
> > +                    dram_opp << dvfsrc->dvd->dram_sft |
> > +                    vcore_opp << dvfsrc->dvd->vcore_sft);
> > +
> > +       if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
> > +               dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
> > +                        __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
> > +                        dvfsrc_read(dvfsrc, DVFSRC_LAST));
> > +               goto out;
> > +       }
> > +
> > +       val = get_cur_performance_level(dvfsrc);
> > +       if (val < highest) {
> > +               dev_err(dvfsrc->dev, "current: %d < hightest: %x\n",
> 
> highest
Thanks.
> 
> > +                       highest, val);
> > +       }
> > +out:
> > +       mutex_unlock(&dvfsrc->lock);
> > +
> > +       return 0;
> > +}
> > +
> > +static void pstate_notifier_register(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       dvfsrc->scpsys_notifier.notifier_call = dvfsrc_set_performace;
> > +       register_scpsys_notifier(&dvfsrc->scpsys_notifier);
> > +}
> > +
> > +static void pm_qos_notifier_register(struct mtk_dvfsrc *dvfsrc)
> > +{
> > +       dvfsrc->qos_notifier.notifier_call = pm_qos_memory_bw_notify;
> > +       pm_qos_add_notifier(PM_QOS_MEMORY_BANDWIDTH, &dvfsrc->qos_notifier);
> > +}
> > +
> > +static irqreturn_t mtk_dvfsrc_interrupt(int irq, void *dev_id)
> > +{
> > +       u32 val;
> > +       struct mtk_dvfsrc *dvfsrc = dev_id;
> > +
> > +       val = dvfsrc_read(dvfsrc, DVFSRC_INT);
> > +       dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, val);
> > +       dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, 0x0);
> > +       if (val & DVFSRC_IRQ_TIMEOUT_EN)
> > +               dev_warn(dvfsrc->dev, "timeout at spm = %x", val);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_dvfsrc_probe(struct platform_device *pdev)
> > +{
> > +       struct resource *res;
> > +       struct mtk_dvfsrc *dvfsrc;
> > +       int ret;
> > +
> > +       dvfsrc = devm_kzalloc(&pdev->dev, sizeof(*dvfsrc), GFP_KERNEL);
> > +       if (!dvfsrc)
> > +               return -ENOMEM;
> > +
> > +       dvfsrc->dvd = of_device_get_match_data(&pdev->dev);
> > +       dvfsrc->dev = &pdev->dev;
> > +
> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       dvfsrc->regs = devm_ioremap_resource(&pdev->dev, res);
> > +       if (IS_ERR(dvfsrc->regs))
> > +               return PTR_ERR(dvfsrc->regs);
> > +
> > +       dvfsrc->clk_dvfsrc = devm_clk_get(dvfsrc->dev, "dvfsrc");
> > +       if (IS_ERR(dvfsrc->clk_dvfsrc)) {
> > +               dev_err(dvfsrc->dev, "failed to get clock: %ld\n",
> > +                       PTR_ERR(dvfsrc->clk_dvfsrc));
> > +               return PTR_ERR(dvfsrc->clk_dvfsrc);
> > +       }
> > +
> > +       ret = clk_prepare_enable(dvfsrc->clk_dvfsrc);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = of_property_read_u32(dvfsrc->dev->of_node, "dram_type",
> > +                                  &dvfsrc->dram_type);
> > +       if (ret) {
> > +               dev_err(dvfsrc->dev, "failed to get dram_type: %d\n", ret);
> > +               clk_disable_unprepare(dvfsrc->clk_dvfsrc);
> > +               return ret;
> > +       }
> > +
> > +       dvfsrc->irq = platform_get_irq(pdev, 0);
> > +       ret = request_irq(dvfsrc->irq, mtk_dvfsrc_interrupt
> > +               , IRQF_TRIGGER_HIGH, "dvfsrc", dvfsrc);
> 
> comma on the line above
Thanks.
> 
> > +       if (ret)
> > +               dev_dbg(dvfsrc->dev, "interrupt not use\n");
> > +
> > +       mutex_init(&dvfsrc->lock);
> > +       if (dvfsrc->dvd->init_soc)
> > +               dvfsrc->dvd->init_soc(dvfsrc);
> > +
> > +       pstate_notifier_register(dvfsrc);
> > +       pm_qos_notifier_register(dvfsrc);
> > +       platform_set_drvdata(pdev, dvfsrc);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct dvfsrc_opp dvfsrc_opp_mt8183_lp4[] = {
> > +       {0, 0}, {1, 0}, {1, 1}, {1, 2},
> > +};
> > +
> > +static const struct dvfsrc_opp dvfsrc_opp_mt8183_1p3[] = {
> > +       {0, 0}, {0, 1}, {1, 1}, {1, 2},
> > +};
> 
> 1p3? Should this be lp3?
Yes, thanks.
> 
> > +
> > +static const struct dvfsrc_opp *dvfsrc_opp_mt8183[] = {
> > +       [MT8183_DVFSRC_OPP_LP4] = dvfsrc_opp_mt8183_lp4,
> > +       [MT8183_DVFSRC_OPP_LP4X] = dvfsrc_opp_mt8183_1p3,
> > +       [MT8183_DVFSRC_OPP_LP3] = dvfsrc_opp_mt8183_1p3,
> > +};
> > +
> > +static struct dvfsrc_domain dvfsrc_domains_mt8183[] = {
> > +       {MT8183_POWER_DOMAIN_MFG_ASYNC, 0},
> > +       {MT8183_POWER_DOMAIN_MFG, 0},
> > +       {MT8183_POWER_DOMAIN_CAM, 0},
> > +       {MT8183_POWER_DOMAIN_DISP, 0},
> > +       {MT8183_POWER_DOMAIN_ISP, 0},
> > +       {MT8183_POWER_DOMAIN_VDEC, 0},
> > +       {MT8183_POWER_DOMAIN_VENC, 0},
> > +};
> > +
> > +static const struct dvfsrc_soc_data mt8183_data = {
> > +       .opps = dvfsrc_opp_mt8183,
> > +       .num_opp = ARRAY_SIZE(dvfsrc_opp_mt8183_lp4),
> > +       .regs = mt8183_regs,
> > +       .domains = dvfsrc_domains_mt8183,
> > +       .num_domains = ARRAY_SIZE(dvfsrc_domains_mt8183),
> > +       .init_soc = mtk_dvfsrc_mt8183_init,
> > +       .get_target_level = mt8183_get_target_level,
> > +       .get_current_level = mt8183_get_current_level,
> > +       .dram_sft = 0,
> > +       .vcore_sft = 2,
> > +};
> > +
> > +static int mtk_dvfsrc_remove(struct platform_device *pdev)
> > +{
> > +       return 0;
> 
> Should you at least disable the clock? (since you do it in the error
> path in _probe...)
> 
> clk_disable_unprepare(dvfsrc->clk_dvfsrc);
Yes, you're right. I will add this.
> 
> > +}
> > +
> > +static const struct of_device_id mtk_dvfsrc_of_match[] = {
> > +       {
> > +               .compatible = "mediatek,mt8183-dvfsrc",
> > +               .data = &mt8183_data,
> > +       }, {
> > +               /* sentinel */
> > +       },
> > +};
> > +
> > +static struct platform_driver mtk_dvfsrc_driver = {
> > +       .probe  = mtk_dvfsrc_probe,
> > +       .remove = mtk_dvfsrc_remove,
> > +       .driver = {
> > +               .name = "mtk-dvfsrc",
> > +               .of_match_table = of_match_ptr(mtk_dvfsrc_of_match),
> > +       },
> > +};
> > +
> > +builtin_platform_driver(mtk_dvfsrc_driver);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("MTK DVFSRC driver");
> > --
> > 1.9.1
> >

Re: [RFC PATCH 6/7] soc: mediatek: add MT8183 dvfsrc support

[Nicolas Boichat]

On Thu, Jan 3, 2019 at 10:16 PM Henry Chen <henryc.chen@mediatek.com> wrote:
>
> On Thu, 2019-01-03 at 10:16 +0800, Nicolas Boichat wrote:
> > On Wed, Jan 2, 2019 at 10:01 PM Henry Chen <henryc.chen@mediatek.com> wrote:
> > >
> > > Add dvfsrc driver for MT8183
> > >
> > > Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
> > > ---
> > >  drivers/soc/mediatek/Kconfig      |  15 ++
> > >  drivers/soc/mediatek/Makefile     |   1 +
> > >  drivers/soc/mediatek/mtk-dvfsrc.c | 473 ++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 489 insertions(+)
> > >  create mode 100644 drivers/soc/mediatek/mtk-dvfsrc.c
> > >
> > > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> > > index a7d0667..f956f03 100644
> > > --- a/drivers/soc/mediatek/Kconfig
> > > +++ b/drivers/soc/mediatek/Kconfig
> > > @@ -12,6 +12,21 @@ config MTK_INFRACFG
> > >           INFRACFG controller contains various infrastructure registers not
> > >           directly associated to any device.
> > >
> > > +config MTK_DVFSRC
> > > +       bool "MediaTek DVFSRC Support"
> > > +       depends on ARCH_MEDIATEK
> > > +       default ARCH_MEDIATEK
> > > +       select REGMAP
> > > +       select MTK_INFRACFG
> > > +       select PM_GENERIC_DOMAINS if PM
> > > +       depends on MTK_SCPSYS
> > > +       help
> > > +         Say yes here to add support for the MediaTek DVFSRC found
> > > +         on different MediaTek SoCs. The DVFSRC is a proprietary
> > > +         hardware which is used to collect all the requests from
> > > +         system and turn into the decision of minimum Vcore voltage
> > > +         and minimum DRAM frequency to fulfill those requests.
> > > +
> > >  config MTK_PMIC_WRAP
> > >         tristate "MediaTek PMIC Wrapper Support"
> > >         depends on RESET_CONTROLLER
> > > diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> > > index 9dc6670..5c010b9 100644
> > > --- a/drivers/soc/mediatek/Makefile
> > > +++ b/drivers/soc/mediatek/Makefile
> > > @@ -1,3 +1,4 @@
> > > +obj-$(CONFIG_MTK_DVFSRC) += mtk-dvfsrc.o
> > >  obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
> > >  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> > >  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> > > diff --git a/drivers/soc/mediatek/mtk-dvfsrc.c b/drivers/soc/mediatek/mtk-dvfsrc.c
> > > new file mode 100644
> > > index 0000000..af462a3
> > > --- /dev/null
> > > +++ b/drivers/soc/mediatek/mtk-dvfsrc.c
> > > @@ -0,0 +1,473 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2018 MediaTek Inc.
> > > + */
> > > +#include <linux/arm-smccc.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/io.h>
> > > +#include <linux/irq.h>
> > > +#include <linux/delay.h>
> >
> > Alphabetical order.
> I will fix it.
> >
> > > +#include <linux/kthread.h>
> > > +#include <linux/module.h>
> > > +#include <linux/notifier.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_qos.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/spinlock.h>
> > > +#include <soc/mediatek/mtk_sip.h>
> > > +#include <dt-bindings/power/mt8183-power.h>
> > > +#include <dt-bindings/soc/mtk,dvfsrc.h>
> > > +#include "mtk-scpsys.h"
> > > +
> > > +#define DVFSRC_IDLE            0x00
> > > +#define DVFSRC_GET_TARGET_LEVEL(x)     (((x) >> 0) & 0x0000ffff)
> > > +#define DVFSRC_GET_CURRENT_LEVEL(x)    (((x) >> 16) & 0x0000ffff)
> > > +
> > > +/* macro for irq */
> > > +#define DVFSRC_IRQ_TIMEOUT_EN          BIT(1)
> > > +
> > > +struct dvfsrc_opp {
> > > +       int vcore_opp;
> > > +       int dram_opp;
> > > +};
> > > +
> > > +struct dvfsrc_domain {
> > > +       int id;
> > > +       int state;
> > > +};
> > > +
> > > +struct mtk_dvfsrc;
> > > +struct dvfsrc_soc_data {
> > > +       const int *regs;
> > > +       int num_opp;
> > > +       int num_domains;
> > > +       int dram_sft;
> > > +       int vcore_sft;
> > > +       const struct dvfsrc_opp **opps;
> > > +       struct dvfsrc_domain *domains;
> > > +       void (*init_soc)(struct mtk_dvfsrc *dvfsrc);
> > > +       int (*get_target_level)(struct mtk_dvfsrc *dvfsrc);
> > > +       int (*get_current_level)(struct mtk_dvfsrc *dvfsrc);
> > > +};
> > > +
> > > +struct mtk_dvfsrc {
> > > +       struct device *dev;
> > > +       struct clk *clk_dvfsrc;
> > > +       const struct dvfsrc_soc_data *dvd;
> > > +       int dram_type;
> > > +       int irq;
> > > +       void __iomem *regs;
> > > +       struct mutex lock;      /* generic mutex for dvfsrc driver */
> > > +
> > > +       struct notifier_block qos_notifier;
> > > +       struct notifier_block scpsys_notifier;
> > > +};
> > > +
> > > +static u32 dvfsrc_read(struct mtk_dvfsrc *dvfs, u32 offset)
> > > +{
> > > +       return readl(dvfs->regs + dvfs->dvd->regs[offset]);
> > > +}
> > > +
> > > +static void dvfsrc_write(struct mtk_dvfsrc *dvfs, u32 offset, u32 val)
> > > +{
> > > +       writel(val, dvfs->regs + dvfs->dvd->regs[offset]);
> > > +}
> > > +
> > > +enum dvfsrc_regs {
> > > +       DVFSRC_BASIC_CONTROL,
> > > +       DVFSRC_SW_REQ,
> > > +       DVFSRC_SW_REQ2,
> > > +       DVFSRC_EMI_REQUEST,
> > > +       DVFSRC_EMI_REQUEST2,
> > > +       DVFSRC_EMI_REQUEST3,
> > > +       DVFSRC_EMI_QOS0,
> > > +       DVFSRC_EMI_QOS1,
> > > +       DVFSRC_EMI_QOS2,
> > > +       DVFSRC_EMI_MD2SPM0,
> > > +       DVFSRC_EMI_MD2SPM1,
> > > +       DVFSRC_VCORE_REQUEST,
> > > +       DVFSRC_VCORE_REQUEST2,
> > > +       DVFSRC_VCORE_MD2SPM0,
> > > +       DVFSRC_INT,
> > > +       DVFSRC_INT_EN,
> > > +       DVFSRC_INT_CLR,
> > > +       DVFSRC_TIMEOUT_NEXTREQ,
> > > +       DVFSRC_LEVEL,
> > > +       DVFSRC_LEVEL_LABEL_0_1,
> > > +       DVFSRC_LEVEL_LABEL_2_3,
> > > +       DVFSRC_LEVEL_LABEL_4_5,
> > > +       DVFSRC_LEVEL_LABEL_6_7,
> > > +       DVFSRC_LEVEL_LABEL_8_9,
> > > +       DVFSRC_LEVEL_LABEL_10_11,
> > > +       DVFSRC_LEVEL_LABEL_12_13,
> > > +       DVFSRC_LEVEL_LABEL_14_15,
> > > +       DVFSRC_SW_BW_0,
> > > +       DVFSRC_QOS_EN,
> > > +       DVFSRC_FORCE,
> > > +       DVFSRC_LAST,
> > > +       DVFSRC_RSRV_0,
> > > +       DVFSRC_RSRV_1,
> > > +};
> > > +
> > > +static int mt8183_regs[] = {
> > > +       [DVFSRC_BASIC_CONTROL] =        0x0,
> > > +       [DVFSRC_SW_REQ] =               0x4,
> > > +       [DVFSRC_SW_REQ2] =              0x8,
> > > +       [DVFSRC_EMI_REQUEST] =          0xC,
> > > +       [DVFSRC_EMI_REQUEST2] =         0x10,
> > > +       [DVFSRC_EMI_REQUEST3] =         0x14,
> > > +       [DVFSRC_EMI_QOS0] =             0x24,
> > > +       [DVFSRC_EMI_QOS1] =             0x28,
> > > +       [DVFSRC_EMI_QOS2] =             0x2C,
> > > +       [DVFSRC_EMI_MD2SPM0] =          0x30,
> > > +       [DVFSRC_EMI_MD2SPM1] =          0x34,
> > > +       [DVFSRC_VCORE_REQUEST] =        0x48,
> > > +       [DVFSRC_VCORE_REQUEST2] =       0x4C,
> > > +       [DVFSRC_VCORE_MD2SPM0] =        0x68,
> > > +       [DVFSRC_INT] =                  0x98,
> > > +       [DVFSRC_INT_EN] =               0x9C,
> > > +       [DVFSRC_INT_CLR] =              0xA0,
> > > +       [DVFSRC_TIMEOUT_NEXTREQ] =      0xD8,
> > > +       [DVFSRC_LEVEL] =                0xDC,
> > > +       [DVFSRC_LEVEL_LABEL_0_1] =      0xE0,
> > > +       [DVFSRC_LEVEL_LABEL_2_3] =      0xE4,
> > > +       [DVFSRC_LEVEL_LABEL_4_5] =      0xE8,
> > > +       [DVFSRC_LEVEL_LABEL_6_7] =      0xEC,
> > > +       [DVFSRC_LEVEL_LABEL_8_9] =      0xF0,
> > > +       [DVFSRC_LEVEL_LABEL_10_11] =    0xF4,
> > > +       [DVFSRC_LEVEL_LABEL_12_13] =    0xF8,
> > > +       [DVFSRC_LEVEL_LABEL_14_15] =    0xFC,
> > > +       [DVFSRC_SW_BW_0] =              0x160,
> > > +       [DVFSRC_QOS_EN] =               0x180,
> > > +       [DVFSRC_FORCE] =                0x300,
> > > +       [DVFSRC_LAST] =                 0x308,
> > > +       [DVFSRC_RSRV_0] =               0x600,
> > > +       [DVFSRC_RSRV_1] =               0x604,
> > > +};
> >
> > Why do you need this indirection table? Do you plan to support HW with
> > very different register offsets?
> Yes. The dvfsrc not only on mt8183 but also exist on following Mediatek
> SoCs. I afraid some of them may be different because the design is
> changed.
> >
> > > +
> > > +static bool dvfsrc_is_idle(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       int val = 0;
> >
> > So 0 is DVFSRC_IDLE, so the function returns true?
> >
> > Maybe this is more readable?
> Ok. it good to me.
> >
> > if (!dvfsrc->dvd->get_target_level)
> >     return true;
> >
> > return dvfsrc->dvd->get_target_level(dvfsrc) == DVFSRC_IDLE;
> >
> > > +
> > > +       if (dvfsrc->dvd->get_target_level)
> >
> > However... can this be false? In what case?
> hmm, it just null function check to prevent someone who forget to hook
> the callback.

Well, isn't it better to just crash the kernel in that case? I mean,
the hook is setup just below, so I'd rather know that we forgot to
hook the callback, rather than masking the error...

> >
> > > +               val = dvfsrc->dvd->get_target_level(dvfsrc);
> > > +
> > > +       return val == DVFSRC_IDLE;
> > > +}
> > > +
> > > +static int dvfsrc_wait_for_state(struct mtk_dvfsrc *dvfsrc,
> > > +                                bool (*fp)(struct mtk_dvfsrc *))
> > > +{
> > > +       unsigned long timeout;
> > > +
> > > +       timeout = jiffies + usecs_to_jiffies(1000);
> > > +
> > > +       do {
> > > +               if (fp(dvfsrc))
> > > +                       return 0;
> > > +       } while (!time_after(jiffies, timeout));
> > > +
> > > +       return -ETIMEDOUT;
> > > +}
> >
> > Can we use wait_event_timeout here?
> I will give a try.
> >
> > > +
> > > +static void mtk_dvfsrc_mt8183_init(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       struct arm_smccc_res res;
> > > +
> > > +       mutex_lock(&dvfsrc->lock);
> > > +
> > > +       arm_smccc_smc(MTK_SIP_SPM, MTK_SIP_SPM_DVFSRC_INIT, 0, 0, 0, 0, 0, 0,
> > > +                     &res);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_0_1, 0x00100000);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_2_3, 0x00210011);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_4_5, 0x01100100);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_6_7, 0x01210111);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_8_9, 0x02100200);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_10_11, 0x02210211);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_12_13, 0x03210321);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_LEVEL_LABEL_14_15, 0x03210321);
> > > +
> > > +       /* EMI/VCORE HRT, MD2SPM, BW setting */
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS0, 0x32);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_QOS1, 0x66);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM0, 0x80F8);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_MD2SPM1, 0x0);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_MD2SPM0, 0x80C0);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_RSRV_1, 0x0000001C);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_TIMEOUT_NEXTREQ, 0x00000013);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_INT_EN, 0x2);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST, 0x00290209);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST2, 0);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST, 0x00150000);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_QOS_EN, 0x0000407F);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST3, 0x09000000);
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_FORCE, 0x00400000);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000C07B);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_BASIC_CONTROL, 0x0000017B);
> >
> > Lots of magic values here, can you describe what they do? (either as
> > #define, or at the very least with comments)
> I will give a brief comment on these settings.
> >
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_VCORE_REQUEST,
> > > +                    (dvfsrc_read(dvfsrc, DVFSRC_VCORE_REQUEST)
> > > +                    & ~(0x3 << 20)));
> > > +       dvfsrc_write(dvfsrc, DVFSRC_EMI_REQUEST,
> > > +                    (dvfsrc_read(dvfsrc, DVFSRC_EMI_REQUEST)
> > > +                    & ~(0x3 << 20)));
> > > +
> > > +       mutex_unlock(&dvfsrc->lock);
> > > +}
> > > +
> > > +static int mt8183_get_target_level(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       return DVFSRC_GET_TARGET_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
> > > +}
> > > +
> > > +static int mt8183_get_current_level(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       return DVFSRC_GET_CURRENT_LEVEL(dvfsrc_read(dvfsrc, DVFSRC_LEVEL));
> > > +}
> > > +
> > > +static int get_cur_performance_level(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       int bit = 0;
> > > +
> > > +       if (dvfsrc->dvd->get_current_level)
> >
> > Same question as above, is this possible?
> >
> > > +               bit = dvfsrc->dvd->get_current_level(dvfsrc);
> > > +
> > > +       return ffs(bit);
> >
> > So ffs(0) = 0... Maybe just return 0 if get_current_level is NULL.
> ok. it will be change to
>         if  (!dvfsrc->dvd->get_current_level)
>                 return 0;
>         return ffs(dvfsrc->dvd->get_current_level(dvfsrc));
> >
> > > +}
> > > +
> > > +static int pm_qos_memory_bw_notify(struct notifier_block *b,
> > > +                                  unsigned long bw, void *v)
> > > +{
> > > +       struct mtk_dvfsrc *dvfsrc;
> > > +
> > > +       dvfsrc = container_of(b, struct mtk_dvfsrc, qos_notifier);
> > > +       mutex_lock(&dvfsrc->lock);
> > > +
> > > +       dev_dbg(dvfsrc->dev, "data: 0x%lx\n", bw);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_SW_BW_0, bw / 100);
> > > +
> > > +       mutex_unlock(&dvfsrc->lock);
> > > +
> > > +       return NOTIFY_OK;
> > > +}
> > > +
> > > +static int dvfsrc_set_performace(struct notifier_block *b,
> >
> > set_performaNce
> Thanks.
> >
> > > +                                unsigned long l, void *v)
> > > +{
> > > +       int i, val, highest = 0, vcore_opp = 0, dram_opp = 0;
> >
> > No need to initialize any of these to 0.
> Ok.
> >
> > > +       struct mtk_dvfsrc *dvfsrc;
> > > +       struct scp_event_data *sc = v;
> > > +       struct dvfsrc_domain *d;
> > > +
> > > +       if (sc->event_type != MTK_SCPSYS_PSTATE)
> > > +               return 0;
> > > +
> > > +       dvfsrc = container_of(b, struct mtk_dvfsrc, scpsys_notifier);
> > > +
> > > +       mutex_lock(&dvfsrc->lock);
> > > +       d = dvfsrc->dvd->domains;
> > > +
> > > +       if (l > dvfsrc->dvd->num_opp || l <= 0) {
> > > +               dev_err(dvfsrc->dev, "pstate out of range = %ld\n", l);
> > > +               goto out;
> > > +       }
> > > +
> > > +       for (i = 0, highest = 0; i < dvfsrc->dvd->num_domains - 1; i++, d++) {
> > > +               if (sc->domain_id == d->id)
> > > +                       d->state = l;
> > > +               if (d->state > highest)
> > > +                       highest = d->state;
> > > +       }
> > > +
> > > +       if (highest == 0) {
> > > +               dev_err(dvfsrc->dev, "domain not match\n");
> > > +               goto out;
> > > +       }
> > > +
> > > +       /* translate pstate to dvfsrc level, and set it to DVFSRC HW */
> > > +       vcore_opp =
> > > +               dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].vcore_opp;
> > > +       dram_opp = dvfsrc->dvd->opps[dvfsrc->dram_type][highest - 1].dram_opp;
> > > +
> > > +       if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
> > > +               dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
> > > +                        __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
> > > +               dvfsrc_read(dvfsrc, DVFSRC_LAST));
> > > +               goto out;
> > > +       }
> > > +
> > > +       dvfsrc_write(dvfsrc, DVFSRC_SW_REQ,
> > > +                    dram_opp << dvfsrc->dvd->dram_sft |
> > > +                    vcore_opp << dvfsrc->dvd->vcore_sft);
> > > +
> > > +       if (dvfsrc_wait_for_state(dvfsrc, dvfsrc_is_idle)) {
> > > +               dev_warn(dvfsrc->dev, "[%s] wait idle, last: %d -> %d\n",
> > > +                        __func__, dvfsrc_read(dvfsrc, DVFSRC_LEVEL),
> > > +                        dvfsrc_read(dvfsrc, DVFSRC_LAST));
> > > +               goto out;
> > > +       }
> > > +
> > > +       val = get_cur_performance_level(dvfsrc);
> > > +       if (val < highest) {
> > > +               dev_err(dvfsrc->dev, "current: %d < hightest: %x\n",
> >
> > highest
> Thanks.
> >
> > > +                       highest, val);
> > > +       }
> > > +out:
> > > +       mutex_unlock(&dvfsrc->lock);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static void pstate_notifier_register(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       dvfsrc->scpsys_notifier.notifier_call = dvfsrc_set_performace;
> > > +       register_scpsys_notifier(&dvfsrc->scpsys_notifier);
> > > +}
> > > +
> > > +static void pm_qos_notifier_register(struct mtk_dvfsrc *dvfsrc)
> > > +{
> > > +       dvfsrc->qos_notifier.notifier_call = pm_qos_memory_bw_notify;
> > > +       pm_qos_add_notifier(PM_QOS_MEMORY_BANDWIDTH, &dvfsrc->qos_notifier);
> > > +}
> > > +
> > > +static irqreturn_t mtk_dvfsrc_interrupt(int irq, void *dev_id)
> > > +{
> > > +       u32 val;
> > > +       struct mtk_dvfsrc *dvfsrc = dev_id;
> > > +
> > > +       val = dvfsrc_read(dvfsrc, DVFSRC_INT);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, val);
> > > +       dvfsrc_write(dvfsrc, DVFSRC_INT_CLR, 0x0);
> > > +       if (val & DVFSRC_IRQ_TIMEOUT_EN)
> > > +               dev_warn(dvfsrc->dev, "timeout at spm = %x", val);
> > > +
> > > +       return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int mtk_dvfsrc_probe(struct platform_device *pdev)
> > > +{
> > > +       struct resource *res;
> > > +       struct mtk_dvfsrc *dvfsrc;
> > > +       int ret;
> > > +
> > > +       dvfsrc = devm_kzalloc(&pdev->dev, sizeof(*dvfsrc), GFP_KERNEL);
> > > +       if (!dvfsrc)
> > > +               return -ENOMEM;
> > > +
> > > +       dvfsrc->dvd = of_device_get_match_data(&pdev->dev);
> > > +       dvfsrc->dev = &pdev->dev;
> > > +
> > > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +       dvfsrc->regs = devm_ioremap_resource(&pdev->dev, res);
> > > +       if (IS_ERR(dvfsrc->regs))
> > > +               return PTR_ERR(dvfsrc->regs);
> > > +
> > > +       dvfsrc->clk_dvfsrc = devm_clk_get(dvfsrc->dev, "dvfsrc");
> > > +       if (IS_ERR(dvfsrc->clk_dvfsrc)) {
> > > +               dev_err(dvfsrc->dev, "failed to get clock: %ld\n",
> > > +                       PTR_ERR(dvfsrc->clk_dvfsrc));
> > > +               return PTR_ERR(dvfsrc->clk_dvfsrc);
> > > +       }
> > > +
> > > +       ret = clk_prepare_enable(dvfsrc->clk_dvfsrc);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       ret = of_property_read_u32(dvfsrc->dev->of_node, "dram_type",
> > > +                                  &dvfsrc->dram_type);
> > > +       if (ret) {
> > > +               dev_err(dvfsrc->dev, "failed to get dram_type: %d\n", ret);
> > > +               clk_disable_unprepare(dvfsrc->clk_dvfsrc);
> > > +               return ret;
> > > +       }
> > > +
> > > +       dvfsrc->irq = platform_get_irq(pdev, 0);
> > > +       ret = request_irq(dvfsrc->irq, mtk_dvfsrc_interrupt
> > > +               , IRQF_TRIGGER_HIGH, "dvfsrc", dvfsrc);
> >
> > comma on the line above
> Thanks.
> >
> > > +       if (ret)
> > > +               dev_dbg(dvfsrc->dev, "interrupt not use\n");
> > > +
> > > +       mutex_init(&dvfsrc->lock);
> > > +       if (dvfsrc->dvd->init_soc)
> > > +               dvfsrc->dvd->init_soc(dvfsrc);
> > > +
> > > +       pstate_notifier_register(dvfsrc);
> > > +       pm_qos_notifier_register(dvfsrc);
> > > +       platform_set_drvdata(pdev, dvfsrc);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static const struct dvfsrc_opp dvfsrc_opp_mt8183_lp4[] = {
> > > +       {0, 0}, {1, 0}, {1, 1}, {1, 2},
> > > +};
> > > +
> > > +static const struct dvfsrc_opp dvfsrc_opp_mt8183_1p3[] = {
> > > +       {0, 0}, {0, 1}, {1, 1}, {1, 2},
> > > +};
> >
> > 1p3? Should this be lp3?
> Yes, thanks.
> >
> > > +
> > > +static const struct dvfsrc_opp *dvfsrc_opp_mt8183[] = {
> > > +       [MT8183_DVFSRC_OPP_LP4] = dvfsrc_opp_mt8183_lp4,
> > > +       [MT8183_DVFSRC_OPP_LP4X] = dvfsrc_opp_mt8183_1p3,
> > > +       [MT8183_DVFSRC_OPP_LP3] = dvfsrc_opp_mt8183_1p3,
> > > +};
> > > +
> > > +static struct dvfsrc_domain dvfsrc_domains_mt8183[] = {
> > > +       {MT8183_POWER_DOMAIN_MFG_ASYNC, 0},
> > > +       {MT8183_POWER_DOMAIN_MFG, 0},
> > > +       {MT8183_POWER_DOMAIN_CAM, 0},
> > > +       {MT8183_POWER_DOMAIN_DISP, 0},
> > > +       {MT8183_POWER_DOMAIN_ISP, 0},
> > > +       {MT8183_POWER_DOMAIN_VDEC, 0},
> > > +       {MT8183_POWER_DOMAIN_VENC, 0},
> > > +};
> > > +
> > > +static const struct dvfsrc_soc_data mt8183_data = {
> > > +       .opps = dvfsrc_opp_mt8183,
> > > +       .num_opp = ARRAY_SIZE(dvfsrc_opp_mt8183_lp4),
> > > +       .regs = mt8183_regs,
> > > +       .domains = dvfsrc_domains_mt8183,
> > > +       .num_domains = ARRAY_SIZE(dvfsrc_domains_mt8183),
> > > +       .init_soc = mtk_dvfsrc_mt8183_init,
> > > +       .get_target_level = mt8183_get_target_level,
> > > +       .get_current_level = mt8183_get_current_level,
> > > +       .dram_sft = 0,
> > > +       .vcore_sft = 2,
> > > +};
> > > +
> > > +static int mtk_dvfsrc_remove(struct platform_device *pdev)
> > > +{
> > > +       return 0;
> >
> > Should you at least disable the clock? (since you do it in the error
> > path in _probe...)
> >
> > clk_disable_unprepare(dvfsrc->clk_dvfsrc);
> Yes, you're right. I will add this.
> >
> > > +}
> > > +
> > > +static const struct of_device_id mtk_dvfsrc_of_match[] = {
> > > +       {
> > > +               .compatible = "mediatek,mt8183-dvfsrc",
> > > +               .data = &mt8183_data,
> > > +       }, {
> > > +               /* sentinel */
> > > +       },
> > > +};
> > > +
> > > +static struct platform_driver mtk_dvfsrc_driver = {
> > > +       .probe  = mtk_dvfsrc_probe,
> > > +       .remove = mtk_dvfsrc_remove,
> > > +       .driver = {
> > > +               .name = "mtk-dvfsrc",
> > > +               .of_match_table = of_match_ptr(mtk_dvfsrc_of_match),
> > > +       },
> > > +};
> > > +
> > > +builtin_platform_driver(mtk_dvfsrc_driver);
> > > +
> > > +MODULE_LICENSE("GPL v2");
> > > +MODULE_DESCRIPTION("MTK DVFSRC driver");
> > > --
> > > 1.9.1
> > >
>
>

[RFC PATCH 7/7] arm64: dts: mt8183: add dvfsrc related nodes

[Henry Chen]

Enable dvfsrc on mt8183 platform.

Signed-off-by: Henry Chen <henryc.chen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index e396410..06206fd 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -132,6 +132,14 @@
 		clock-output-names = "clk26m";
 	};
 
+	dvfsrc_top@10012000 {
+		compatible = "mediatek,mt8183-dvfsrc";
+		reg = <0 0x10012000 0 0x1000>;
+		clocks = <&infracfg CLK_INFRA_DVFSRC>;
+		clock-names = "dvfsrc";
+		dram_type = <MT8183_DVFSRC_OPP_LP4>;
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupt-parent = <&gic>;
-- 
1.9.1