Skip to content

Commit 08b1cf4

Browse files
0xB0Dmchehab
0xB0D
authored and
mchehab
committed
media: venus: core, venc, vdec: Fix probe dependency error
Commit aaaa93e ("media] media: venus: venc: add video encoder files") is the last in a series of three commits to add core.c vdec.c and venc.c adding core, encoder and decoder. The encoder and decoder check for core drvdata as set and return -EPROBE_DEFER if it has not been set, however both the encoder and decoder rely on core.v4l2_dev as valid. core.v4l2_dev will not be valid until v4l2_device_register() has completed in core.c's probe(). Normally this is never seen however, Dmitry reported the following backtrace when compiling drivers and firmware directly into a kernel image. [ 5.259968] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT) [ 5.269850] sd 0:0:0:3: [sdd] Optimal transfer size 524288 bytes [ 5.275505] Workqueue: events deferred_probe_work_func [ 5.275513] pstate: 60400005 (nZCv daif +PAN -UAO -TCO BTYPE=--) [ 5.441211] usb 2-1: new SuperSpeedPlus Gen 2 USB device number 2 using xhci-hcd [ 5.442486] pc : refcount_warn_saturate+0x140/0x148 [ 5.493756] hub 2-1:1.0: USB hub found [ 5.496266] lr : refcount_warn_saturate+0x140/0x148 [ 5.500982] hub 2-1:1.0: 4 ports detected [ 5.503440] sp : ffff80001067b730 [ 5.503442] x29: ffff80001067b730 [ 5.592660] usb 1-1: new high-speed USB device number 2 using xhci-hcd [ 5.598478] x28: ffff6c6bc1c379b8 [ 5.598480] x27: ffffa5c673852960 x26: ffffa5c673852000 [ 5.598484] x25: ffff6c6bc1c37800 x24: 0000000000000001 [ 5.810652] x23: 0000000000000000 x22: ffffa5c673bc7118 [ 5.813777] hub 1-1:1.0: USB hub found [ 5.816108] x21: ffffa5c674440000 x20: 0000000000000001 [ 5.820846] hub 1-1:1.0: 4 ports detected [ 5.825415] x19: ffffa5c6744f4000 x18: ffffffffffffffff [ 5.825418] x17: 0000000000000000 x16: 0000000000000000 [ 5.825421] x15: 00000a4810c193ba x14: 0000000000000000 [ 5.825424] x13: 00000000000002b8 x12: 000000000000f20a [ 5.825427] x11: 000000000000f20a x10: 0000000000000038 [ 5.845447] usb 2-1.1: new SuperSpeed Gen 1 USB device number 3 using xhci-hcd [ 5.845904] [ 5.845905] x9 : 0000000000000000 x8 : ffff6c6d36fae780 [ 5.871208] x7 : ffff6c6d36faf240 x6 : 0000000000000000 [ 5.876664] x5 : 0000000000000004 x4 : 0000000000000085 [ 5.882121] x3 : 0000000000000119 x2 : ffffa5c6741ef478 [ 5.887578] x1 : 3acbb3926faf5f00 x0 : 0000000000000000 [ 5.893036] Call trace: [ 5.895551] refcount_warn_saturate+0x140/0x148 [ 5.900202] __video_register_device+0x64c/0xd10 [ 5.904944] venc_probe+0xc4/0x148 [ 5.908444] platform_probe+0x68/0xe0 [ 5.912210] really_probe+0x118/0x3e0 [ 5.915977] driver_probe_device+0x5c/0xc0 [ 5.920187] __device_attach_driver+0x98/0xb8 [ 5.924661] bus_for_each_drv+0x68/0xd0 [ 5.928604] __device_attach+0xec/0x148 [ 5.932547] device_initial_probe+0x14/0x20 [ 5.936845] bus_probe_device+0x9c/0xa8 [ 5.940788] device_add+0x3e8/0x7c8 [ 5.944376] of_device_add+0x4c/0x60 [ 5.948056] of_platform_device_create_pdata+0xbc/0x140 [ 5.953425] of_platform_bus_create+0x17c/0x3c0 [ 5.958078] of_platform_populate+0x80/0x110 [ 5.962463] venus_probe+0x2ec/0x4d8 [ 5.966143] platform_probe+0x68/0xe0 [ 5.969907] really_probe+0x118/0x3e0 [ 5.973674] driver_probe_device+0x5c/0xc0 [ 5.977882] __device_attach_driver+0x98/0xb8 [ 5.982356] bus_for_each_drv+0x68/0xd0 [ 5.986298] __device_attach+0xec/0x148 [ 5.990242] device_initial_probe+0x14/0x20 [ 5.994539] bus_probe_device+0x9c/0xa8 [ 5.998481] deferred_probe_work_func+0x74/0xb0 [ 6.003132] process_one_work+0x1e8/0x360 [ 6.007254] worker_thread+0x208/0x478 [ 6.011106] kthread+0x150/0x158 [ 6.014431] ret_from_fork+0x10/0x30 [ 6.018111] ---[ end trace f074246b1ecdb466 ]--- This patch fixes by - Only setting drvdata after v4l2_device_register() completes - Moving v4l2_device_register() so that suspend/reume in core::probe() stays as-is - Changes pm_ops->core_function() to take struct venus_core not struct device - Minimal rework of v4l2_device_*register in probe/remove Reported-by: Dmitry Baryshkov <[email protected]> Signed-off-by: Bryan O'Donoghue <[email protected]> Signed-off-by: Stanimir Varbanov <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
1 parent 5a465c5 commit 08b1cf4

File tree

3 files changed

+34
-33
lines changed

3 files changed

+34
-33
lines changed

‎drivers/media/platform/qcom/venus/core.c

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ static int venus_probe(struct platform_device *pdev)
218218
return -ENOMEM;
219219

220220
core->dev = dev;
221-
platform_set_drvdata(pdev, core);
222221

223222
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
224223
core->base = devm_ioremap_resource(dev, r);
@@ -248,7 +247,7 @@ static int venus_probe(struct platform_device *pdev)
248247
return -ENODEV;
249248

250249
if (core->pm_ops->core_get) {
251-
ret = core->pm_ops->core_get(dev);
250+
ret = core->pm_ops->core_get(core);
252251
if (ret)
253252
return ret;
254253
}
@@ -273,6 +272,12 @@ static int venus_probe(struct platform_device *pdev)
273272
if (ret)
274273
goto err_core_put;
275274

275+
ret = v4l2_device_register(dev, &core->v4l2_dev);
276+
if (ret)
277+
goto err_core_deinit;
278+
279+
platform_set_drvdata(pdev, core);
280+
276281
pm_runtime_enable(dev);
277282

278283
ret = pm_runtime_get_sync(dev);
@@ -307,10 +312,6 @@ static int venus_probe(struct platform_device *pdev)
307312
if (ret)
308313
goto err_venus_shutdown;
309314

310-
ret = v4l2_device_register(dev, &core->v4l2_dev);
311-
if (ret)
312-
goto err_core_deinit;
313-
314315
ret = pm_runtime_put_sync(dev);
315316
if (ret) {
316317
pm_runtime_get_noresume(dev);
@@ -323,18 +324,18 @@ static int venus_probe(struct platform_device *pdev)
323324

324325
err_dev_unregister:
325326
v4l2_device_unregister(&core->v4l2_dev);
326-
err_core_deinit:
327-
hfi_core_deinit(core, false);
328327
err_venus_shutdown:
329328
venus_shutdown(core);
330329
err_runtime_disable:
331330
pm_runtime_put_noidle(dev);
332331
pm_runtime_set_suspended(dev);
333332
pm_runtime_disable(dev);
334333
hfi_destroy(core);
334+
err_core_deinit:
335+
hfi_core_deinit(core, false);
335336
err_core_put:
336337
if (core->pm_ops->core_put)
337-
core->pm_ops->core_put(dev);
338+
core->pm_ops->core_put(core);
338339
return ret;
339340
}
340341

@@ -360,11 +361,14 @@ static int venus_remove(struct platform_device *pdev)
360361
pm_runtime_disable(dev);
361362

362363
if (pm_ops->core_put)
363-
pm_ops->core_put(dev);
364+
pm_ops->core_put(core);
365+
366+
v4l2_device_unregister(&core->v4l2_dev);
364367

365368
hfi_destroy(core);
366369

367370
v4l2_device_unregister(&core->v4l2_dev);
371+
368372
mutex_destroy(&core->pm_lock);
369373
mutex_destroy(&core->lock);
370374
venus_dbgfs_deinit(core);
@@ -393,7 +397,7 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev)
393397
return ret;
394398

395399
if (pm_ops->core_power) {
396-
ret = pm_ops->core_power(dev, POWER_OFF);
400+
ret = pm_ops->core_power(core, POWER_OFF);
397401
if (ret)
398402
return ret;
399403
}
@@ -411,7 +415,7 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev)
411415
err_video_path:
412416
icc_set_bw(core->cpucfg_path, kbps_to_icc(1000), 0);
413417
err_cpucfg_path:
414-
pm_ops->core_power(dev, POWER_ON);
418+
pm_ops->core_power(core, POWER_ON);
415419

416420
return ret;
417421
}
@@ -431,7 +435,7 @@ static __maybe_unused int venus_runtime_resume(struct device *dev)
431435
return ret;
432436

433437
if (pm_ops->core_power) {
434-
ret = pm_ops->core_power(dev, POWER_ON);
438+
ret = pm_ops->core_power(core, POWER_ON);
435439
if (ret)
436440
return ret;
437441
}

‎drivers/media/platform/qcom/venus/pm_helpers.c

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -277,16 +277,13 @@ static int load_scale_v1(struct venus_inst *inst)
277277
return 0;
278278
}
279279

280-
static int core_get_v1(struct device *dev)
280+
static int core_get_v1(struct venus_core *core)
281281
{
282-
struct venus_core *core = dev_get_drvdata(dev);
283-
284282
return core_clks_get(core);
285283
}
286284

287-
static int core_power_v1(struct device *dev, int on)
285+
static int core_power_v1(struct venus_core *core, int on)
288286
{
289-
struct venus_core *core = dev_get_drvdata(dev);
290287
int ret = 0;
291288

292289
if (on == POWER_ON)
@@ -753,12 +750,12 @@ static int venc_power_v4(struct device *dev, int on)
753750
return ret;
754751
}
755752

756-
static int vcodec_domains_get(struct device *dev)
753+
static int vcodec_domains_get(struct venus_core *core)
757754
{
758755
int ret;
759756
struct opp_table *opp_table;
760757
struct device **opp_virt_dev;
761-
struct venus_core *core = dev_get_drvdata(dev);
758+
struct device *dev = core->dev;
762759
const struct venus_resources *res = core->res;
763760
struct device *pd;
764761
unsigned int i;
@@ -809,9 +806,8 @@ static int vcodec_domains_get(struct device *dev)
809806
return ret;
810807
}
811808

812-
static void vcodec_domains_put(struct device *dev)
809+
static void vcodec_domains_put(struct venus_core *core)
813810
{
814-
struct venus_core *core = dev_get_drvdata(dev);
815811
const struct venus_resources *res = core->res;
816812
unsigned int i;
817813

@@ -834,9 +830,9 @@ static void vcodec_domains_put(struct device *dev)
834830
dev_pm_opp_detach_genpd(core->opp_table);
835831
}
836832

837-
static int core_get_v4(struct device *dev)
833+
static int core_get_v4(struct venus_core *core)
838834
{
839-
struct venus_core *core = dev_get_drvdata(dev);
835+
struct device *dev = core->dev;
840836
const struct venus_resources *res = core->res;
841837
int ret;
842838

@@ -875,7 +871,7 @@ static int core_get_v4(struct device *dev)
875871
}
876872
}
877873

878-
ret = vcodec_domains_get(dev);
874+
ret = vcodec_domains_get(core);
879875
if (ret) {
880876
if (core->has_opp_table)
881877
dev_pm_opp_of_remove_table(dev);
@@ -886,24 +882,24 @@ static int core_get_v4(struct device *dev)
886882
return 0;
887883
}
888884

889-
static void core_put_v4(struct device *dev)
885+
static void core_put_v4(struct venus_core *core)
890886
{
891-
struct venus_core *core = dev_get_drvdata(dev);
887+
struct device *dev = core->dev;
892888

893889
if (legacy_binding)
894890
return;
895891

896-
vcodec_domains_put(dev);
892+
vcodec_domains_put(core);
897893

898894
if (core->has_opp_table)
899895
dev_pm_opp_of_remove_table(dev);
900896
dev_pm_opp_put_clkname(core->opp_table);
901897

902898
}
903899

904-
static int core_power_v4(struct device *dev, int on)
900+
static int core_power_v4(struct venus_core *core, int on)
905901
{
906-
struct venus_core *core = dev_get_drvdata(dev);
902+
struct device *dev = core->dev;
907903
struct device *pmctrl = core->pmdomains[0];
908904
int ret = 0;
909905

‎drivers/media/platform/qcom/venus/pm_helpers.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@
44
#define __VENUS_PM_HELPERS_H__
55

66
struct device;
7+
struct venus_core;
78

89
#define POWER_ON 1
910
#define POWER_OFF 0
1011

1112
struct venus_pm_ops {
12-
int (*core_get)(struct device *dev);
13-
void (*core_put)(struct device *dev);
14-
int (*core_power)(struct device *dev, int on);
13+
int (*core_get)(struct venus_core *core);
14+
void (*core_put)(struct venus_core *core);
15+
int (*core_power)(struct venus_core *core, int on);
1516

1617
int (*vdec_get)(struct device *dev);
1718
void (*vdec_put)(struct device *dev);

0 commit comments

Comments
 (0)