Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

media: venus: avoid calling core_clk_setrate() concurrently during concurrent video sessions

In existing implementation, core_clk_setrate() is getting called
concurrently in concurrent video sessions. Before the previous call to
core_clk_setrate returns, new call to core_clk_setrate is invoked from
another video session running concurrently. This results in latest
calculated frequency being set (higher/lower) instead of actual frequency
required for that video session. It also results in stability crashes
mention below. These resources are specific to video core, hence keeping
under core lock would ensure that they are estimated for all running video
sessions and called once for the video core.

Crash logs:

[ 1.900089] WARNING: CPU: 4 PID: 1 at drivers/opp/debugfs.c:33 opp_debug_remove_one+0x2c/0x48
[ 1.908493] Modules linked in:
[ 1.911524] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.10.67 #35 f8edb8c30cf2dd6838495dd9ef9be47af7f5f60c
[ 1.921036] Hardware name: Qualcomm Technologies, Inc. sc7280 IDP SKU2 platform (DT)
[ 1.928673] pstate: 60800009 (nZCv daif -PAN +UAO -TCO BTYPE=--)
[ 1.934608] pc : opp_debug_remove_one+0x2c/0x48
[ 1.939080] lr : opp_debug_remove_one+0x2c/0x48
[ 1.943560] sp : ffffffc011d7b7f0
[ 1.946836] pmr_save: 000000e0
[ 1.949854] x29: ffffffc011d7b7f0 x28: ffffffc010733bbc
[ 1.955104] x27: ffffffc010733ba8 x26: ffffff8083cedd00
[ 1.960355] x25: 0000000000000001 x24: 0000000000000000
[ 1.965603] x23: ffffff8083cc2878 x22: ffffff8083ceb900
[ 1.970852] x21: ffffff8083ceb910 x20: ffffff8083cc2800
[ 1.976101] x19: ffffff8083ceb900 x18: 00000000ffff0a10
[ 1.981352] x17: ffffff80837a5620 x16: 00000000000000ec
[ 1.986601] x15: ffffffc010519ad4 x14: 0000000000000003
[ 1.991849] x13: 0000000000000004 x12: 0000000000000001
[ 1.997100] x11: c0000000ffffdfff x10: 00000000ffffffff
[ 2.002348] x9 : d2627c580300dc00 x8 : d2627c580300dc00
[ 2.007596] x7 : 0720072007200720 x6 : ffffff80802ecf00
[ 2.012845] x5 : 0000000000190004 x4 : 0000000000000000
[ 2.018094] x3 : ffffffc011d7b478 x2 : ffffffc011d7b480
[ 2.023343] x1 : 00000000ffffdfff x0 : 0000000000000017
[ 2.028594] Call trace:
[ 2.031022] opp_debug_remove_one+0x2c/0x48
[ 2.035160] dev_pm_opp_put+0x94/0xb0
[ 2.038780] _opp_remove_all+0x7c/0xc8
[ 2.042486] _opp_remove_all_static+0x54/0x7c
[ 2.046796] dev_pm_opp_remove_table+0x74/0x98
[ 2.051183] devm_pm_opp_of_table_release+0x18/0x24
[ 2.056001] devm_action_release+0x1c/0x28
[ 2.060053] release_nodes+0x23c/0x2b8
[ 2.063760] devres_release_group+0xcc/0xd0
[ 2.067900] component_bind+0xac/0x168
[ 2.071608] component_bind_all+0x98/0x124
[ 2.075664] msm_drm_bind+0x1e8/0x678
[ 2.079287] try_to_bring_up_master+0x60/0x134
[ 2.083674] component_master_add_with_match+0xd8/0x120
[ 2.088834] msm_pdev_probe+0x20c/0x2a0
[ 2.092629] platform_drv_probe+0x9c/0xbc
[ 2.096598] really_probe+0x11c/0x46c
[ 2.100217] driver_probe_device+0x8c/0xf0
[ 2.104270] device_driver_attach+0x54/0x78
[ 2.108407] __driver_attach+0x48/0x148
[ 2.112201] bus_for_each_dev+0x88/0xd4
[ 2.115998] driver_attach+0x2c/0x38
[ 2.119534] bus_add_driver+0x10c/0x200
[ 2.123330] driver_register+0x6c/0x104
[ 2.127122] __platform_driver_register+0x4c/0x58
[ 2.131767] msm_drm_register+0x6c/0x70
[ 2.135560] do_one_initcall+0x64/0x23c
[ 2.139357] do_initcall_level+0xac/0x15c
[ 2.143321] do_initcalls+0x5c/0x9c
[ 2.146778] do_basic_setup+0x2c/0x38
[ 2.150401] kernel_init_freeable+0xf8/0x15c
[ 2.154622] kernel_init+0x1c/0x11c
[ 2.158079] ret_from_fork+0x10/0x30
[ 2.161615] ---[ end trace a2cc45a0f784b212 ]---

[ 2.166272] Removing OPP: 300000000

Signed-off-by: Mansur Alisha Shaik <mansur@codeaurora.org>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

authored by

Mansur Alisha Shaik and committed by
Mauro Carvalho Chehab
91f2b7d2 b1f9bb80

+14 -14
+14 -14
drivers/media/platform/qcom/venus/pm_helpers.c
··· 163 163 struct venus_inst *inst = NULL; 164 164 u32 mbs_per_sec = 0; 165 165 166 - mutex_lock(&core->lock); 167 166 list_for_each_entry(inst, &core->instances, list) { 168 167 if (inst->session_type != session_type) 169 168 continue; 170 169 171 170 mbs_per_sec += load_per_instance(inst); 172 171 } 173 - mutex_unlock(&core->lock); 174 172 175 173 return mbs_per_sec; 176 174 } ··· 217 219 struct venus_inst *inst = NULL; 218 220 u32 mbs_per_sec, avg, peak, total_avg = 0, total_peak = 0; 219 221 220 - mutex_lock(&core->lock); 221 222 list_for_each_entry(inst, &core->instances, list) { 222 223 mbs_per_sec = load_per_instance(inst); 223 224 mbs_to_bw(inst, mbs_per_sec, &avg, &peak); 224 225 total_avg += avg; 225 226 total_peak += peak; 226 227 } 227 - mutex_unlock(&core->lock); 228 228 229 229 /* 230 230 * keep minimum bandwidth vote for "video-mem" path, ··· 249 253 struct device *dev = core->dev; 250 254 u32 mbs_per_sec; 251 255 unsigned int i; 252 - int ret; 256 + int ret = 0; 253 257 258 + mutex_lock(&core->lock); 254 259 mbs_per_sec = load_per_type(core, VIDC_SESSION_TYPE_ENC) + 255 260 load_per_type(core, VIDC_SESSION_TYPE_DEC); 256 261 ··· 276 279 if (ret) { 277 280 dev_err(dev, "failed to set clock rate %lu (%d)\n", 278 281 freq, ret); 279 - return ret; 282 + goto exit; 280 283 } 281 284 282 285 ret = load_scale_bw(core); 283 286 if (ret) { 284 287 dev_err(dev, "failed to set bandwidth (%d)\n", 285 288 ret); 286 - return ret; 289 + goto exit; 287 290 } 288 291 289 - return 0; 292 + exit: 293 + mutex_unlock(&core->lock); 294 + return ret; 290 295 } 291 296 292 297 static int core_get_v1(struct venus_core *core) ··· 1115 1116 struct device *dev = core->dev; 1116 1117 unsigned long freq = 0, freq_core1 = 0, freq_core2 = 0; 1117 1118 unsigned long filled_len = 0; 1118 - int i, ret; 1119 + int i, ret = 0; 1119 1120 1120 1121 for (i = 0; i < inst->num_input_bufs; i++) 1121 1122 filled_len = max(filled_len, inst->payloads[i]); 1122 1123 1123 1124 if (inst->session_type == VIDC_SESSION_TYPE_DEC && !filled_len) 1124 - return 0; 1125 + return ret; 1125 1126 1126 1127 freq = calculate_inst_freq(inst, filled_len); 1127 1128 inst->clk_data.freq = freq; ··· 1137 1138 freq_core2 += inst->clk_data.freq; 1138 1139 } 1139 1140 } 1140 - mutex_unlock(&core->lock); 1141 1141 1142 1142 freq = max(freq_core1, freq_core2); 1143 1143 ··· 1161 1163 if (ret) { 1162 1164 dev_err(dev, "failed to set clock rate %lu (%d)\n", 1163 1165 freq, ret); 1164 - return ret; 1166 + goto exit; 1165 1167 } 1166 1168 1167 1169 ret = load_scale_bw(core); 1168 1170 if (ret) { 1169 1171 dev_err(dev, "failed to set bandwidth (%d)\n", 1170 1172 ret); 1171 - return ret; 1173 + goto exit; 1172 1174 } 1173 1175 1174 - return 0; 1176 + exit: 1177 + mutex_unlock(&core->lock); 1178 + return ret; 1175 1179 } 1176 1180 1177 1181 static const struct venus_pm_ops pm_ops_v4 = {