[PATCH 0/2] DSPBRIDGE: cleanup OPP handling

[PATCH 0/2] DSPBRIDGE: cleanup OPP handling

[Nishanth Menon]

Applies on DSPbridge branch:
this is a two patch series of which patch 2/2 is provided as two options:
a) using legacy implementation
b) cleaner one using pm-wip-opp APIs.

only (b) has been tested on a 3630 platform.

Nishanth Menon (2):
  DSPBRIDGE: remove dependency of mpu freq
  DSPBRIDGE: pm: use old implementation for opps
	OR
  DSPBRIDGE: pm: use pm-wip-opp APIs for opp list [RFC]

 arch/arm/mach-omap2/dspbridge.c                |  125 ++++++++++++++++++++++++
 arch/arm/plat-omap/include/dspbridge/host_os.h |   13 +++-
 drivers/dsp/bridge/rmgr/drv_interface.c        |   38 -------
 drivers/dsp/bridge/rmgr/node.c                 |    4 +-
 drivers/dsp/bridge/rmgr/proc.c                 |    4 +-
 drivers/dsp/bridge/wmd/io_sm.c                 |   20 ++---
 6 files changed, 148 insertions(+), 56 deletions(-)

Cc: Ameya Palande <ameya.palande@nokia.com>
Cc: Deepak Chitriki <deepak.chitriki@ti.com>
Cc: Felipe Contreras <felipe.contreras@nokia.com>
Cc: Hiroshi Doyu <hiroshi.doyu@nokia.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>

Regards,
Nishanth Menon

[PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq

[Nishanth Menon]

Current DSPBridge uses hardcoded logic which is not scalable
across OMAP3 silicons. introduce a structure based implementation
this allows for dsp frequency table indices to be generated runtime
instead of being compile time.

Cc: Ameya Palande <ameya.palande@nokia.com>
Cc: Deepak Chitriki <deepak.chitriki@ti.com>
Cc: Felipe Contreras <felipe.contreras@nokia.com>
Cc: Hiroshi Doyu <hiroshi.doyu@nokia.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/dspbridge.c                |   28 +++++++++++++++++
 arch/arm/plat-omap/include/dspbridge/host_os.h |   13 +++++++-
 drivers/dsp/bridge/rmgr/drv_interface.c        |   38 ------------------------
 drivers/dsp/bridge/rmgr/node.c                 |    4 +-
 drivers/dsp/bridge/rmgr/proc.c                 |    4 +-
 drivers/dsp/bridge/wmd/io_sm.c                 |   20 ++++--------
 6 files changed, 51 insertions(+), 56 deletions(-)

diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
index ea109a3..26b860f 100644
--- a/arch/arm/mach-omap2/dspbridge.c
+++ b/arch/arm/mach-omap2/dspbridge.c
@@ -30,6 +30,23 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
 #endif
 };
 
+/**
+ * get_opp_table() - populate the pdata with opp info
+ * @pdata: pointer to pdata
+ *
+ * OPP table implementation is a variant b/w platforms.
+ * the platform file now incorporates this into the build
+ * itself and uses the interface to talk to platform specific
+ * functions
+ */
+static int get_opp_table(struct dspbridge_platform_data *pdata)
+{
+#ifdef CONFIG_BRIDGE_DVFS
+	/* Do nothing now  - fill based on PM implementation */
+#endif
+	return 0;
+}
+
 static int __init dspbridge_init(void)
 {
 	struct platform_device *pdev;
@@ -48,6 +65,10 @@ static int __init dspbridge_init(void)
 	if (!pdev)
 		goto err_out;
 
+	err = get_opp_table(pdata);
+	if (err)
+		goto err_out;
+
 	err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
 	if (err)
 		goto err_out;
@@ -60,6 +81,10 @@ static int __init dspbridge_init(void)
 	return 0;
 
 err_out:
+	kfree(pdata->mpu_speeds);
+	kfree(pdata->dsp_freq_table);
+	pdata->mpu_speeds = NULL;
+	pdata->dsp_freq_table = NULL;
 	platform_device_put(pdev);
 	return err;
 }
@@ -67,6 +92,9 @@ module_init(dspbridge_init);
 
 static void __exit dspbridge_exit(void)
 {
+	struct dspbridge_platform_data *pdata = &dspbridge_pdata;
+	kfree(pdata->mpu_speeds);
+	kfree(pdata->dsp_freq_table);
 	platform_device_unregister(dspbridge_pdev);
 }
 module_exit(dspbridge_exit);
diff --git a/arch/arm/plat-omap/include/dspbridge/host_os.h b/arch/arm/plat-omap/include/dspbridge/host_os.h
index 066c4d7..ebf6a80 100644
--- a/arch/arm/plat-omap/include/dspbridge/host_os.h
+++ b/arch/arm/plat-omap/include/dspbridge/host_os.h
@@ -54,12 +54,23 @@
 #define INT_MAIL_MPU_IRQ        26
 #define INT_DSP_MMU_IRQ        28
 
+struct dsp_shm_freq_table {
+	unsigned long u_volts;
+	unsigned long dsp_freq;
+	unsigned long thresh_min_freq;
+	unsigned long thresh_max_freq;
+};
 struct dspbridge_platform_data {
 	void 	(*dsp_set_min_opp)(u8 opp_id);
 	u8 	(*dsp_get_opp)(void);
 	void 	(*cpu_set_freq)(unsigned long f);
 	unsigned long (*cpu_get_freq)(void);
-	unsigned long mpu_speed[6];
+	unsigned long *mpu_speeds;
+	u8 mpu_num_speeds;
+	unsigned long mpu_min_speed;
+	unsigned long mpu_max_speed;
+	struct dsp_shm_freq_table *dsp_freq_table;
+	u8 dsp_num_speeds;
 
 	u32 phys_mempool_base;
 	u32 phys_mempool_size;
diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index d8fe250..9330f44 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -169,44 +169,12 @@ static struct file_operations bridge_fops = {
 static u32 timeOut = 1000;
 #ifdef CONFIG_BRIDGE_DVFS
 static struct clk *clk_handle;
-s32 dsp_max_opps = VDD1_OPP5;
 #endif
 
-/* Maximum Opps that can be requested by IVA*/
-/*vdd1 rate table*/
-#ifdef CONFIG_BRIDGE_DVFS
-const struct omap_opp  vdd1_rate_table_bridge[] = {
-	{0, 0, 0},
-	/*OPP1*/
-	{S125M, VDD1_OPP1, 0},
-	/*OPP2*/
-	{S250M, VDD1_OPP2, 0},
-	/*OPP3*/
-	{S500M, VDD1_OPP3, 0},
-	/*OPP4*/
-	{S550M, VDD1_OPP4, 0},
-	/*OPP5*/
-	{S600M, VDD1_OPP5, 0},
-};
-#endif
 #endif
 
 struct dspbridge_platform_data *omap_dspbridge_pdata;
 
-u32 vdd1_dsp_freq[6][4] = {
-	{0, 0, 0, 0},
-	/*OPP1*/
-	{0, 90000, 0, 86000},
-	/*OPP2*/
-	{0, 180000, 80000, 170000},
-	/*OPP3*/
-	{0, 360000, 160000, 340000},
-	/*OPP4*/
-	{0, 396000, 325000, 376000},
-	/*OPP5*/
-	{0, 430000, 355000, 430000},
-};
-
 #ifdef CONFIG_BRIDGE_DVFS
 static int dspbridge_post_scale(struct notifier_block *op, unsigned long level,
 				void *ptr)
@@ -228,9 +196,6 @@ static int __devinit omap34xx_bridge_probe(struct platform_device *pdev)
 	u32 temp;
 	dev_t   dev = 0 ;
 	int     result;
-#ifdef CONFIG_BRIDGE_DVFS
-	int i = 0;
-#endif
 	struct dspbridge_platform_data *pdata = pdev->dev.platform_data;
 
 	omap_dspbridge_dev = pdev;
@@ -341,9 +306,6 @@ static int __devinit omap34xx_bridge_probe(struct platform_device *pdev)
 	}
 	if (DSP_SUCCEEDED(initStatus)) {
 #ifdef CONFIG_BRIDGE_DVFS
-		for (i = 0; i < 6; i++)
-			pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
-
 		clk_handle = clk_get(NULL, "iva2_ck");
 		if (!clk_handle) {
 			GT_0trace(driverTrace, GT_7CLASS,
diff --git a/drivers/dsp/bridge/rmgr/node.c b/drivers/dsp/bridge/rmgr/node.c
index e8ba0dd..5b06c37 100644
--- a/drivers/dsp/bridge/rmgr/node.c
+++ b/drivers/dsp/bridge/rmgr/node.c
@@ -1263,7 +1263,7 @@ DSP_STATUS NODE_Create(struct NODE_OBJECT *hNode)
 		/* Boost the OPP level to max level that DSP can be requested */
 #if defined(CONFIG_BRIDGE_DVFS) && !defined(CONFIG_CPU_FREQ)
 		if (pdata->cpu_set_freq) {
-			(*pdata->cpu_set_freq)(pdata->mpu_speed[VDD1_OPP3]);
+			(*pdata->cpu_set_freq)(pdata->mpu_max_speed);
 
 			if (pdata->dsp_get_opp) {
 				GT_1trace(NODE_debugMask, GT_4CLASS, "opp level"
@@ -1289,7 +1289,7 @@ DSP_STATUS NODE_Create(struct NODE_OBJECT *hNode)
 		/* Request the lowest OPP level*/
 #if defined(CONFIG_BRIDGE_DVFS) && !defined(CONFIG_CPU_FREQ)
 		if (pdata->cpu_set_freq) {
-			(*pdata->cpu_set_freq)(pdata->mpu_speed[VDD1_OPP1]);
+			(*pdata->cpu_set_freq)(pdata->mpu_min_speed);
 
 			if (pdata->dsp_get_opp) {
 				GT_1trace(NODE_debugMask, GT_4CLASS, "opp level"
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index 9eae2f9..7b60630 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -1134,7 +1134,7 @@ DSP_STATUS PROC_Load(DSP_HPROCESSOR hProcessor, IN CONST s32 iArgc,
 	/* Boost the OPP level to Maximum level supported by baseport*/
 #if defined(CONFIG_BRIDGE_DVFS) && !defined(CONFIG_CPU_FREQ)
 	if (pdata->cpu_set_freq)
-		(*pdata->cpu_set_freq)(pdata->mpu_speed[VDD1_OPP5]);
+		(*pdata->cpu_set_freq)(pdata->mpu_max_speed);
 #endif
 		status = COD_LoadBase(hCodMgr, iArgc, (char **)aArgv,
 				     DEV_BrdWriteFxn,
@@ -1156,7 +1156,7 @@ DSP_STATUS PROC_Load(DSP_HPROCESSOR hProcessor, IN CONST s32 iArgc,
 	/* Requesting the lowest opp supported*/
 #if defined(CONFIG_BRIDGE_DVFS) && !defined(CONFIG_CPU_FREQ)
 	if (pdata->cpu_set_freq)
-		(*pdata->cpu_set_freq)(pdata->mpu_speed[VDD1_OPP1]);
+		(*pdata->cpu_set_freq)(pdata->mpu_min_speed);
 #endif
 
 	}
diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
index 4511ec5..6c140c5 100644
--- a/drivers/dsp/bridge/wmd/io_sm.c
+++ b/drivers/dsp/bridge/wmd/io_sm.c
@@ -159,13 +159,6 @@ static DSP_STATUS registerSHMSegs(struct IO_MGR *hIOMgr,
 				  struct COD_MANAGER *hCodMan,
 				  u32 dwGPPBasePA);
 
-#ifdef CONFIG_BRIDGE_DVFS
-/* The maximum number of OPPs that are supported */
-extern s32 dsp_max_opps;
-/* The Vdd1 opp table information */
-extern u32 vdd1_dsp_freq[6][4] ;
-#endif
-
 #if GT_TRACE
 static struct GT_Mask dsp_trace_mask = { NULL, NULL }; /* GT trace variable */
 #endif
@@ -1839,29 +1832,30 @@ DSP_STATUS IO_SHMsetting(IN struct IO_MGR *hIOMgr, IN enum SHM_DESCTYPE desc,
 		 * Update the shared memory with the voltage, frequency,
 		 * min and max frequency values for an OPP.
 		 */
-		for (i = 0; i <= dsp_max_opps; i++) {
+		for (i = 0; i <= pdata->dsp_num_speeds; i++) {
 			hIOMgr->pSharedMem->oppTableStruct.oppPoint[i].voltage =
-				vdd1_dsp_freq[i][0];
+				pdata->dsp_freq_table[i].u_volts;
 			DBG_Trace(DBG_LEVEL5, "OPP shared memory -voltage: "
 				 "%d\n", hIOMgr->pSharedMem->oppTableStruct.
 				 oppPoint[i].voltage);
 			hIOMgr->pSharedMem->oppTableStruct.oppPoint[i].
-				frequency = vdd1_dsp_freq[i][1];
+				frequency = pdata->dsp_freq_table[i].dsp_freq;
 			DBG_Trace(DBG_LEVEL5, "OPP shared memory -frequency: "
 				 "%d\n", hIOMgr->pSharedMem->oppTableStruct.
 				 oppPoint[i].frequency);
 			hIOMgr->pSharedMem->oppTableStruct.oppPoint[i].minFreq =
-				vdd1_dsp_freq[i][2];
+				pdata->dsp_freq_table[i].thresh_min_freq;
 			DBG_Trace(DBG_LEVEL5, "OPP shared memory -min value: "
 				 "%d\n", hIOMgr->pSharedMem->oppTableStruct.
 				  oppPoint[i].minFreq);
 			hIOMgr->pSharedMem->oppTableStruct.oppPoint[i].maxFreq =
-				vdd1_dsp_freq[i][3];
+				pdata->dsp_freq_table[i].thresh_max_freq;
 			DBG_Trace(DBG_LEVEL5, "OPP shared memory -max value: "
 				 "%d\n", hIOMgr->pSharedMem->oppTableStruct.
 				 oppPoint[i].maxFreq);
 		}
-		hIOMgr->pSharedMem->oppTableStruct.numOppPts = dsp_max_opps;
+		hIOMgr->pSharedMem->oppTableStruct.numOppPts =
+			pdata->dsp_num_speeds;
 		DBG_Trace(DBG_LEVEL5, "OPP shared memory - max OPP number: "
 			 "%d\n", hIOMgr->pSharedMem->oppTableStruct.numOppPts);
 		/* Update the current OPP number */
-- 
1.6.3.3

[PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps

[Nishanth Menon]

Use the old implementation for OPP list generation and update.
this is a 1-1 implementation of what we had previously without
support for 3630.

Cc: Ameya Palande <ameya.palande@nokia.com>
Cc: Deepak Chitriki <deepak.chitriki@ti.com>
Cc: Felipe Contreras <felipe.contreras@nokia.com>
Cc: Hiroshi Doyu <hiroshi.doyu@nokia.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
This probably belongs to dspbridge-pm branch - I dont personally dont like
doing opp handling the way it is done currently, but dont want to mess around
with the current implementation.

 arch/arm/mach-omap2/dspbridge.c |   59 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
index 26b860f..120d8a2 100644
--- a/arch/arm/mach-omap2/dspbridge.c
+++ b/arch/arm/mach-omap2/dspbridge.c
@@ -42,7 +42,64 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
 static int get_opp_table(struct dspbridge_platform_data *pdata)
 {
 #ifdef CONFIG_BRIDGE_DVFS
-	/* Do nothing now  - fill based on PM implementation */
+	/* legacy values for 3430 */
+	u32 vdd1_dsp_freq[6][4] = {
+		{0, 0, 0, 0},
+		/*OPP1*/
+		{0, 90000, 0, 86000},
+		/*OPP2*/
+		{0, 180000, 80000, 170000},
+		/*OPP3*/
+		{0, 360000, 160000, 340000},
+		/*OPP4*/
+		{0, 396000, 325000, 376000},
+		/*OPP5*/
+		{0, 430000, 355000, 430000},
+	};
+	struct omap_opp vdd1_rate_table_bridge[] = {
+		{0, 0, 0},
+		/*OPP1*/
+		{S125M, VDD1_OPP1, 0},
+		/*OPP2*/
+		{S250M, VDD1_OPP2, 0},
+		/*OPP3*/
+		{S500M, VDD1_OPP3, 0},
+		/*OPP4*/
+		{S550M, VDD1_OPP4, 0},
+		/*OPP5*/
+		{S600M, VDD1_OPP5, 0},
+	};
+	pdata->dsp_num_speeds = VDD1_OPP5;
+	pdata->mpu_speeds = kzalloc(sizeof(u32) * pdata->dsp_num_speeds,
+			GFP_KERNEL);
+	if (!pdata->mpu_speeds) {
+		pr_err("unable to allocate memory for the mpu"
+		"frequencies\n");
+		return -ENOMEM;
+	}
+	pdata->dsp_freq_table = kzalloc(
+			sizeof(struct dsp_shm_freq_table) *
+			(pdata->dsp_num_speeds + 1), GFP_KERNEL);
+	if (!pdata->dsp_freq_table) {
+		pr_err("unable to allocate memory for the dsp"
+		"frequencies\n");
+		return -ENOMEM;
+	}
+	for (i = 0; i < 6; i++)
+		pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
+	pdata->mpu_max_speed = pdata->mpu_speed[VDD1_OPP5];
+	pdata->mpu_min_speed = pdata->mpu_speed[VDD1_OPP1];
+	pdata->dsp_num_speeds = VDD1_OPP5;
+	for (i = 0; i <= pdata->dsp_num_speeds; i++) {
+		pdata->dsp_freq_table[i].u_volts =
+				vdd1_dsp_freq[i][0];
+		frequency = pdata->dsp_freq_table[i].dsp_freq =
+			frequency = vdd1_dsp_freq[i][1];
+		pdata->dsp_freq_table[i].thresh_min_freq =
+			vdd1_dsp_freq[i][2];
+		pdata->dsp_freq_table[i].thresh_max_freq =
+			vdd1_dsp_freq[i][3];
+	}
 #endif
 	return 0;
 }
-- 
1.6.3.3

[RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list

[Nishanth Menon]

Use the PM-WIP-OPP branch apis for generating the opp list for DSP.
This allows for scaling accross 3430,3440,3450 and 3630 series.

Caveat: does not handle dynamic addition of opp after the
bridgedriver is inserted - this can be done once the framework for
the same is made available

Cc: Ameya Palande <ameya.palande@nokia.com>
Cc: Deepak Chitriki <deepak.chitriki@ti.com>
Cc: Felipe Contreras <felipe.contreras@nokia.com>
Cc: Hiroshi Doyu <hiroshi.doyu@nokia.com>
Cc: Omar Ramirez Luna <omar.ramirez@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
caveat: this does not use the ENUM implementation yet, this can be once it is
merged to pm-wip-opp branch.

 arch/arm/mach-omap2/dspbridge.c |   99 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 98 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
index 26b860f..935afd7 100644
--- a/arch/arm/mach-omap2/dspbridge.c
+++ b/arch/arm/mach-omap2/dspbridge.c
@@ -15,6 +15,20 @@
 
 #ifdef CONFIG_BRIDGE_DVFS
 #include <mach/omap-pm.h>
+#include <plat/opp.h>
+/*
+ * The DSP load balancer works on the following logic:
+ * Opp frequencies:
+ * 0 <---------> Freq 1 <------------> Freq 2 <---------> Freq 3
+ * DSP Thresholds for the frequencies:
+ * 0M<-------X-> Freq 1 <-------M--X-> Freq 2 <----M--X-> Freq 3
+ * Where, M is the minimal threshold and X is maximum threshold
+ *
+ * if from Freq x to Freq y; where x > y, transition happens on M
+ * if from Freq x to Freq y; where x < y, transition happens on X
+ */
+#define BRIDGE_THRESH_HIGH_PERCENT	95
+#define BRIDGE_THRESH_LOW_PERCENT	88
 #endif
 
 #include <dspbridge/host_os.h>
@@ -42,7 +56,90 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
 static int get_opp_table(struct dspbridge_platform_data *pdata)
 {
 #ifdef CONFIG_BRIDGE_DVFS
-	/* Do nothing now  - fill based on PM implementation */
+	int mpu_freqs;
+	int dsp_freqs;
+	int i;
+	struct omap_opp *opp;
+	unsigned long freq, old_rate;
+
+	mpu_freqs = opp_get_opp_count(mpu_opps);
+	dsp_freqs = opp_get_opp_count(dsp_opps);
+	if (mpu_freqs < 0 || dsp_freqs < 0 || mpu_freqs != dsp_freqs) {
+		pr_err("mpu and dsp frequencies are inconsistent! "
+			"mpu_freqs=%d dsp_freqs=%d\n", mpu_freqs, dsp_freqs);
+		return -EINVAL;
+	}
+	/* allocate memory if we have opps initialized */
+	pdata->mpu_speeds = kzalloc(sizeof(u32) * mpu_freqs,
+			GFP_KERNEL);
+	if (!pdata->mpu_speeds) {
+		pr_err("unable to allocate memory for the mpu"
+		"frequencies\n");
+		return -ENOMEM;
+	}
+	opp = mpu_opps;
+	i = 0;
+	freq = 0;
+	while (!IS_ERR(opp = opp_find_freq_ceil(opp, &freq))) {
+		pdata->mpu_speeds[i] = freq;
+		freq++;
+		i++;
+	}
+	pdata->mpu_num_speeds = mpu_freqs;
+	pdata->mpu_min_speed = pdata->mpu_speeds[0];
+	pdata->mpu_max_speed = pdata->mpu_speeds[mpu_freqs - 1];
+	/* need an initial terminator */
+	pdata->dsp_freq_table = kzalloc(
+			sizeof(struct dsp_shm_freq_table) *
+			(dsp_freqs + 1), GFP_KERNEL);
+	if (!pdata->dsp_freq_table) {
+		pr_err("unable to allocate memory for the dsp"
+			"frequencies\n");
+		return -ENOMEM;
+	}
+
+	opp = dsp_opps;
+	i = 1;
+	freq = 0;
+	old_rate = 0;
+	/*
+	 * the freq table is in terms of khz.. so we need to
+	 * divide by 1000
+	 */
+	while (!IS_ERR(opp = opp_find_freq_ceil(opp, &freq))) {
+		/* dsp frequencies are in khz */
+		u32 rate = freq / 1000;
+		/*
+		 * On certain 34xx silicons, certain OPPs are duplicated
+		 * for DSP - handle those by copying previous opp value
+		 */
+		if (rate == old_rate) {
+			memcpy(&pdata->dsp_freq_table[i],
+				&pdata->dsp_freq_table[i-1],
+				sizeof(struct dsp_shm_freq_table));
+		} else {
+			pdata->dsp_freq_table[i].dsp_freq = rate;
+			pdata->dsp_freq_table[i].u_volts =
+				opp_get_voltage(opp);
+			/*
+			 * min threshold:
+			 * NOTE: index 1 needs a min of 0! else no
+			 * scaling happens at DSP!
+			 */
+			pdata->dsp_freq_table[i].thresh_min_freq =
+				((old_rate * BRIDGE_THRESH_LOW_PERCENT) / 100);
+
+			/* max threshold */
+			pdata->dsp_freq_table[i].thresh_max_freq =
+				((rate * BRIDGE_THRESH_HIGH_PERCENT) / 100);
+		}
+		old_rate = rate;
+		freq++;
+		i++;
+	}
+	/* the last entry should map with maximum rate */
+	pdata->dsp_freq_table[i - 1].thresh_max_freq = old_rate;
+	pdata->dsp_num_speeds = dsp_freqs;
 #endif
 	return 0;
 }
-- 
1.6.3.3

Re: [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps

[Romit Dasgupta]

> 
>  arch/arm/mach-omap2/dspbridge.c |   59 ++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 58 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
> index 26b860f..120d8a2 100644
> --- a/arch/arm/mach-omap2/dspbridge.c
> +++ b/arch/arm/mach-omap2/dspbridge.c
> @@ -42,7 +42,64 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
>  static int get_opp_table(struct dspbridge_platform_data *pdata)
>  {
>  #ifdef CONFIG_BRIDGE_DVFS
> -	/* Do nothing now  - fill based on PM implementation */
> +	/* legacy values for 3430 */
> +	u32 vdd1_dsp_freq[6][4] = {
> +		{0, 0, 0, 0},
> +		/*OPP1*/
> +		{0, 90000, 0, 86000},
> +		/*OPP2*/
> +		{0, 180000, 80000, 170000},
> +		/*OPP3*/
> +		{0, 360000, 160000, 340000},
> +		/*OPP4*/
> +		{0, 396000, 325000, 376000},
> +		/*OPP5*/
> +		{0, 430000, 355000, 430000},
> +	};
> +	struct omap_opp vdd1_rate_table_bridge[] = {
> +		{0, 0, 0},
> +		/*OPP1*/
> +		{S125M, VDD1_OPP1, 0},
> +		/*OPP2*/
> +		{S250M, VDD1_OPP2, 0},
> +		/*OPP3*/
> +		{S500M, VDD1_OPP3, 0},
> +		/*OPP4*/
> +		{S550M, VDD1_OPP4, 0},
> +		/*OPP5*/
> +		{S600M, VDD1_OPP5, 0},
> +	};
> +	pdata->dsp_num_speeds = VDD1_OPP5;
Why dont you use ARRAY_SIZE - 1 ?
> +	pdata->mpu_speeds = kzalloc(sizeof(u32) * pdata->dsp_num_speeds,
> +			GFP_KERNEL);
I understand pdata->dsp_num_speeds == pdata->mpu_num_speeds. But don't you think
passing pdata->mpu_num_speeds makes more sense here?
> +	if (!pdata->mpu_speeds) {
> +		pr_err("unable to allocate memory for the mpu"
> +		"frequencies\n");
> +		return -ENOMEM;
> +	}
As I mentioned in my earlier email. You return to the caller here but you free
pdata->dsp_freq_table in the caller even if pdata->dsp_freq_table is not allocated.

> +	pdata->dsp_freq_table = kzalloc(
> +			sizeof(struct dsp_shm_freq_table) *
> +			(pdata->dsp_num_speeds + 1), GFP_KERNEL);
> +	if (!pdata->dsp_freq_table) {
> +		pr_err("unable to allocate memory for the dsp"
> +		"frequencies\n");
> +		return -ENOMEM;
> +	}
> +	for (i = 0; i < 6; i++)
Why are you hard coding numeric 6 here?

> +		pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
> +	pdata->mpu_max_speed = pdata->mpu_speed[VDD1_OPP5];
You can use ARRAY_SIZE.
> +	pdata->mpu_min_speed = pdata->mpu_speed[VDD1_OPP1];
> +	pdata->dsp_num_speeds = VDD1_OPP5;
Same..ARRAY_SIZE...
> +	for (i = 0; i <= pdata->dsp_num_speeds; i++) {
> +		pdata->dsp_freq_table[i].u_volts =
> +				vdd1_dsp_freq[i][0];
> +		frequency = pdata->dsp_freq_table[i].dsp_freq =
> +			frequency = vdd1_dsp_freq[i][1];
> +		pdata->dsp_freq_table[i].thresh_min_freq =
> +			vdd1_dsp_freq[i][2];
> +		pdata->dsp_freq_table[i].thresh_max_freq =
> +			vdd1_dsp_freq[i][3];
> +	}
>  #endif
>  	return 0;
>  }

Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq

[Romit Dasgupta]

> 
> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
> +
>  static int __init dspbridge_init(void)
>  {
>  	struct platform_device *pdev;
> @@ -48,6 +65,10 @@ static int __init dspbridge_init(void)
>  	if (!pdev)
>  		goto err_out;
>  
> +	err = get_opp_table(pdata);
> +	if (err)
> +		goto err_out;
> +
>  	err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
>  	if (err)
>  		goto err_out;
> @@ -60,6 +81,10 @@ static int __init dspbridge_init(void)
>  	return 0;
>  
>  err_out:
> +	kfree(pdata->mpu_speeds);
> +	kfree(pdata->dsp_freq_table);
Are we sure that pdata->dsp_freq_table is NULL before initialization?
Looking at your second part of the patch. You seem to invoke kfree for
pdata->dsp_freq_table even if pdata->dsp_freq_table is not allocated.
So my question is :

Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq

[Romit Dasgupta]

Romit Dasgupta wrote:
>> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
>> +
>>  static int __init dspbridge_init(void)
>>  {
>>  	struct platform_device *pdev;
>> @@ -48,6 +65,10 @@ static int __init dspbridge_init(void)
>>  	if (!pdev)
>>  		goto err_out;
>>  
>> +	err = get_opp_table(pdata);
>> +	if (err)
>> +		goto err_out;
>> +
>>  	err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
>>  	if (err)
>>  		goto err_out;
>> @@ -60,6 +81,10 @@ static int __init dspbridge_init(void)
>>  	return 0;
>>  
>>  err_out:
>> +	kfree(pdata->mpu_speeds);
>> +	kfree(pdata->dsp_freq_table);
> Are we sure that pdata->dsp_freq_table is NULL before initialization?
> Looking at your second part of the patch. You seem to invoke kfree for
> pdata->dsp_freq_table even if pdata->dsp_freq_table is not allocated.
> So my question is :
missed the last part of the mail. If pdata->dsp_freq_table is NULL to start
with. This is ok. Otherwise this needs to be changed.

Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq

[Nishanth Menon]

Dasgupta, Romit had written, on 01/21/2010 01:40 AM, the following:
> Romit Dasgupta wrote:
>>> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
>>> +
>>>  static int __init dspbridge_init(void)
>>>  {
>>>  	struct platform_device *pdev;
>>> @@ -48,6 +65,10 @@ static int __init dspbridge_init(void)
>>>  	if (!pdev)
>>>  		goto err_out;
>>>  
>>> +	err = get_opp_table(pdata);
>>> +	if (err)
>>> +		goto err_out;
>>> +
>>>  	err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
>>>  	if (err)
>>>  		goto err_out;
>>> @@ -60,6 +81,10 @@ static int __init dspbridge_init(void)
>>>  	return 0;
>>>  
>>>  err_out:
>>> +	kfree(pdata->mpu_speeds);
>>> +	kfree(pdata->dsp_freq_table);
>> Are we sure that pdata->dsp_freq_table is NULL before initialization?
>> Looking at your second part of the patch. You seem to invoke kfree for
>> pdata->dsp_freq_table even if pdata->dsp_freq_table is not allocated.
>> So my question is :
> missed the last part of the mail. If pdata->dsp_freq_table is NULL to start
> with. This is ok. Otherwise this needs to be changed.
;) yep it is NULL to start with as the same file passes pdata from:
static struct dspbridge_platform_data dspbridge_pdata __initdata
and kfree is NULL safe :D.

-- 
Regards,
Nishanth Menon

Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq

[Romit Dasgupta]

> @@ -67,6 +92,9 @@ module_init(dspbridge_init);
>  
>  static void __exit dspbridge_exit(void)
>  {
> +	struct dspbridge_platform_data *pdata = &dspbridge_pdata;
> +	kfree(pdata->mpu_speeds);
> +	kfree(pdata->dsp_freq_table);
>  	platform_device_unregister(dspbridge_pdev);
>  }
>  module_exit(dspbridge_exit);

Ok if pdata->mpu_speeds, pdata->dsp_freq_table are NULL to start with, then you
need to make them NULL after kfree as well. Otherwise next time you insert the
module it may fail if pdata_mpu_speeds is initialized but pdata->dsp_freq_table
is not.

Re: [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps

[Nishanth Menon]

Dasgupta, Romit had written, on 01/21/2010 01:38 AM, the following:
>>  arch/arm/mach-omap2/dspbridge.c |   59 ++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 58 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
>> index 26b860f..120d8a2 100644
>> --- a/arch/arm/mach-omap2/dspbridge.c
>> +++ b/arch/arm/mach-omap2/dspbridge.c
>> @@ -42,7 +42,64 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
>>  static int get_opp_table(struct dspbridge_platform_data *pdata)
>>  {
>>  #ifdef CONFIG_BRIDGE_DVFS
>> -	/* Do nothing now  - fill based on PM implementation */
>> +	/* legacy values for 3430 */
>> +	u32 vdd1_dsp_freq[6][4] = {
>> +		{0, 0, 0, 0},
>> +		/*OPP1*/
>> +		{0, 90000, 0, 86000},
>> +		/*OPP2*/
>> +		{0, 180000, 80000, 170000},
>> +		/*OPP3*/
>> +		{0, 360000, 160000, 340000},
>> +		/*OPP4*/
>> +		{0, 396000, 325000, 376000},
>> +		/*OPP5*/
>> +		{0, 430000, 355000, 430000},
>> +	};
>> +	struct omap_opp vdd1_rate_table_bridge[] = {
>> +		{0, 0, 0},
>> +		/*OPP1*/
>> +		{S125M, VDD1_OPP1, 0},
>> +		/*OPP2*/
>> +		{S250M, VDD1_OPP2, 0},
>> +		/*OPP3*/
>> +		{S500M, VDD1_OPP3, 0},
>> +		/*OPP4*/
>> +		{S550M, VDD1_OPP4, 0},
>> +		/*OPP5*/
>> +		{S600M, VDD1_OPP5, 0},
>> +	};
>> +	pdata->dsp_num_speeds = VDD1_OPP5;
> Why dont you use ARRAY_SIZE - 1 ?
This is a 1-1 transposition of old code here. further improvement 
patches is a good thing to have (though I find the usage of ARRAY_SIZE 
still wont make it cpu independent).

>> +	pdata->mpu_speeds = kzalloc(sizeof(u32) * pdata->dsp_num_speeds,
>> +			GFP_KERNEL);
> I understand pdata->dsp_num_speeds == pdata->mpu_num_speeds. But don't you think
> passing pdata->mpu_num_speeds makes more sense here?
thanks for catching this -> yeah, it is right to use mpu_num_speeds 
(even though the nums are the same)

>> +	if (!pdata->mpu_speeds) {
>> +		pr_err("unable to allocate memory for the mpu"
>> +		"frequencies\n");
>> +		return -ENOMEM;
>> +	}
> As I mentioned in my earlier email. You return to the caller here but you free
> pdata->dsp_freq_table in the caller even if pdata->dsp_freq_table is not allocated.
yes, that is coz kfree is NULL safe. you can verify that by writing a 
simple code and testing on checkpatch:
if (ptr)
	kfree(ptr);

> 
>> +	pdata->dsp_freq_table = kzalloc(
>> +			sizeof(struct dsp_shm_freq_table) *
>> +			(pdata->dsp_num_speeds + 1), GFP_KERNEL);
>> +	if (!pdata->dsp_freq_table) {
>> +		pr_err("unable to allocate memory for the dsp"
>> +		"frequencies\n");
>> +		return -ENOMEM;
>> +	}
>> +	for (i = 0; i < 6; i++)
> Why are you hard coding numeric 6 here?
carry over of old code - intention is *not* to optimize this.

> 
>> +		pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
>> +	pdata->mpu_max_speed = pdata->mpu_speed[VDD1_OPP5];
> You can use ARRAY_SIZE.
>> +	pdata->mpu_min_speed = pdata->mpu_speed[VDD1_OPP1];
>> +	pdata->dsp_num_speeds = VDD1_OPP5;
> Same..ARRAY_SIZE...
intention was to retain the old logic here.
>> +	for (i = 0; i <= pdata->dsp_num_speeds; i++) {
>> +		pdata->dsp_freq_table[i].u_volts =
>> +				vdd1_dsp_freq[i][0];
>> +		frequency = pdata->dsp_freq_table[i].dsp_freq =
>> +			frequency = vdd1_dsp_freq[i][1];
>> +		pdata->dsp_freq_table[i].thresh_min_freq =
>> +			vdd1_dsp_freq[i][2];
>> +		pdata->dsp_freq_table[i].thresh_max_freq =
>> +			vdd1_dsp_freq[i][3];
>> +	}
>>  #endif
>>  	return 0;
>>  }


-- 
Regards,
Nishanth Menon

Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq

[Nishanth Menon]

Dasgupta, Romit had written, on 01/21/2010 01:45 AM, the following:
>> @@ -67,6 +92,9 @@ module_init(dspbridge_init);
>>  
>>  static void __exit dspbridge_exit(void)
>>  {
>> +	struct dspbridge_platform_data *pdata = &dspbridge_pdata;
>> +	kfree(pdata->mpu_speeds);
>> +	kfree(pdata->dsp_freq_table);
>>  	platform_device_unregister(dspbridge_pdev);
>>  }
>>  module_exit(dspbridge_exit);
> 
> Ok if pdata->mpu_speeds, pdata->dsp_freq_table are NULL to start with, then you
> need to make them NULL after kfree as well. Otherwise next time you insert the
> module it may fail if pdata_mpu_speeds is initialized but pdata->dsp_freq_table
> is not.
Good catch. thanks.

-- 
Regards,
Nishanth Menon

Re: [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list

[Romit Dasgupta]

Menon, Nishanth wrote:
> Use the PM-WIP-OPP branch apis for generating the opp list for DSP.
> This allows for scaling accross 3430,3440,3450 and 3630 series.
> 
> @@ -42,7 +56,90 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
>  static int get_opp_table(struct dspbridge_platform_data *pdata)
>  {
Is get_opp_table used anywhere other than dspbridge_init? I dont see any usage
outside this now.

Can't you make this __init?

Re: [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list

[Nishanth Menon]

Dasgupta, Romit had written, on 01/21/2010 01:58 AM, the following:
> Menon, Nishanth wrote:
>> Use the PM-WIP-OPP branch apis for generating the opp list for DSP.
>> This allows for scaling accross 3430,3440,3450 and 3630 series.
>>
>> @@ -42,7 +56,90 @@ static struct dspbridge_platform_data dspbridge_pdata __initdata = {
>>  static int get_opp_table(struct dspbridge_platform_data *pdata)
>>  {
> Is get_opp_table used anywhere other than dspbridge_init? I dont see any usage
> outside this now.
it was not meant to be (static).
> 
> Can't you make this __init?
ack. duh to myself.

-- 
Regards,
Nishanth Menon