* [PATCH 0/2] DSPBRIDGE: cleanup OPP handling @ 2010-01-20 22:19 Nishanth Menon 2010-01-20 22:19 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Nishanth Menon 0 siblings, 1 reply; 13+ messages in thread From: Nishanth Menon @ 2010-01-20 22:19 UTC (permalink / raw) To: linux-omap Cc: Nishanth Menon, Ameya Palande, Deepak Chitriki, Felipe Contreras, Hiroshi Doyu, Omar Ramirez Luna 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq 2010-01-20 22:19 [PATCH 0/2] DSPBRIDGE: cleanup OPP handling Nishanth Menon @ 2010-01-20 22:19 ` Nishanth Menon 2010-01-20 22:19 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Nishanth Menon ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Nishanth Menon @ 2010-01-20 22:19 UTC (permalink / raw) To: linux-omap Cc: Nishanth Menon, Ameya Palande, Deepak Chitriki, Felipe Contreras, Hiroshi Doyu, Omar Ramirez Luna 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps 2010-01-20 22:19 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Nishanth Menon @ 2010-01-20 22:19 ` Nishanth Menon 2010-01-20 22:19 ` [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list Nishanth Menon 2010-01-21 7:38 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Romit Dasgupta 2010-01-21 7:38 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Romit Dasgupta 2010-01-21 7:45 ` Romit Dasgupta 2 siblings, 2 replies; 13+ messages in thread From: Nishanth Menon @ 2010-01-20 22:19 UTC (permalink / raw) To: linux-omap Cc: Nishanth Menon, Ameya Palande, Deepak Chitriki, Felipe Contreras, Hiroshi Doyu, Omar Ramirez Luna 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list 2010-01-20 22:19 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Nishanth Menon @ 2010-01-20 22:19 ` Nishanth Menon 2010-01-21 7:58 ` Romit Dasgupta 2010-01-21 7:38 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Romit Dasgupta 1 sibling, 1 reply; 13+ messages in thread From: Nishanth Menon @ 2010-01-20 22:19 UTC (permalink / raw) To: linux-omap Cc: Nishanth Menon, Ameya Palande, Deepak Chitriki, Felipe Contreras, Hiroshi Doyu, Omar Ramirez Luna 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list 2010-01-20 22:19 ` [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list Nishanth Menon @ 2010-01-21 7:58 ` Romit Dasgupta 2010-01-21 7:59 ` Nishanth Menon 0 siblings, 1 reply; 13+ messages in thread From: Romit Dasgupta @ 2010-01-21 7:58 UTC (permalink / raw) To: Menon, Nishanth Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar 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? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list 2010-01-21 7:58 ` Romit Dasgupta @ 2010-01-21 7:59 ` Nishanth Menon 0 siblings, 0 replies; 13+ messages in thread From: Nishanth Menon @ 2010-01-21 7:59 UTC (permalink / raw) To: Dasgupta, Romit Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps 2010-01-20 22:19 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Nishanth Menon 2010-01-20 22:19 ` [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list Nishanth Menon @ 2010-01-21 7:38 ` Romit Dasgupta 2010-01-21 7:47 ` Nishanth Menon 1 sibling, 1 reply; 13+ messages in thread From: Romit Dasgupta @ 2010-01-21 7:38 UTC (permalink / raw) To: Menon, Nishanth Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar > > 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; > } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps 2010-01-21 7:38 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Romit Dasgupta @ 2010-01-21 7:47 ` Nishanth Menon 0 siblings, 0 replies; 13+ messages in thread From: Nishanth Menon @ 2010-01-21 7:47 UTC (permalink / raw) To: Dasgupta, Romit Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq 2010-01-20 22:19 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Nishanth Menon 2010-01-20 22:19 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Nishanth Menon @ 2010-01-21 7:38 ` Romit Dasgupta 2010-01-21 7:40 ` Romit Dasgupta 2010-01-21 7:45 ` Romit Dasgupta 2 siblings, 1 reply; 13+ messages in thread From: Romit Dasgupta @ 2010-01-21 7:38 UTC (permalink / raw) To: Menon, Nishanth Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar > > 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 : ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq 2010-01-21 7:38 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Romit Dasgupta @ 2010-01-21 7:40 ` Romit Dasgupta 2010-01-21 7:42 ` Nishanth Menon 0 siblings, 1 reply; 13+ messages in thread From: Romit Dasgupta @ 2010-01-21 7:40 UTC (permalink / raw) To: Menon, Nishanth Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq 2010-01-21 7:40 ` Romit Dasgupta @ 2010-01-21 7:42 ` Nishanth Menon 0 siblings, 0 replies; 13+ messages in thread From: Nishanth Menon @ 2010-01-21 7:42 UTC (permalink / raw) To: Dasgupta, Romit Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq 2010-01-20 22:19 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Nishanth Menon 2010-01-20 22:19 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Nishanth Menon 2010-01-21 7:38 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Romit Dasgupta @ 2010-01-21 7:45 ` Romit Dasgupta 2010-01-21 7:48 ` Nishanth Menon 2 siblings, 1 reply; 13+ messages in thread From: Romit Dasgupta @ 2010-01-21 7:45 UTC (permalink / raw) To: Menon, Nishanth Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar > @@ -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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq 2010-01-21 7:45 ` Romit Dasgupta @ 2010-01-21 7:48 ` Nishanth Menon 0 siblings, 0 replies; 13+ messages in thread From: Nishanth Menon @ 2010-01-21 7:48 UTC (permalink / raw) To: Dasgupta, Romit Cc: linux-omap, Ameya Palande, Chitriki Rudramuni, Deepak, Felipe Contreras, Hiroshi Doyu, Ramirez Luna, Omar 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-01-21 7:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-20 22:19 [PATCH 0/2] DSPBRIDGE: cleanup OPP handling Nishanth Menon 2010-01-20 22:19 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Nishanth Menon 2010-01-20 22:19 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Nishanth Menon 2010-01-20 22:19 ` [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list Nishanth Menon 2010-01-21 7:58 ` Romit Dasgupta 2010-01-21 7:59 ` Nishanth Menon 2010-01-21 7:38 ` [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps Romit Dasgupta 2010-01-21 7:47 ` Nishanth Menon 2010-01-21 7:38 ` [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq Romit Dasgupta 2010-01-21 7:40 ` Romit Dasgupta 2010-01-21 7:42 ` Nishanth Menon 2010-01-21 7:45 ` Romit Dasgupta 2010-01-21 7:48 ` Nishanth Menon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox