Hi,
I did some testing with omapdrm on v3.10-rc1. Here are some issues I encountered. For most of them I don't have any idea where to even start looking for a problem, so I hope that you may have some ideas.
dispc_runtime_get/put used in irq context =========================================
dispc_runtime_get/put are used in irq context in omap_irq.c. I can hack around that with if (!in_atomic()), but I have no idea yet how to fix it properly.
drm_crtc warning ================
I got this once when unloading the modules, but haven't happened since:
drm_crtc.c line 3869
WARN_ON(!list_empty(&dev->mode_config.fb_list));
As it only happened once, maybe some kind of race.
omap_gem warning ================
This happens on module unload:
[ 30.167480] ------------[ cut here ]------------ [ 30.167724] WARNING: at drivers/gpu/drm/omapdrm/omap_gem.c:1318 omap_gem_free_object+0x24c/0x25c [omapdrm]() [ 30.182952] Modules linked in: omapdrm(-) drm_kms_helper drm panel_taal panel_tfp410 panel_generic_dpi omapdss [ 30.183166] CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted 3.10.0-rc1-00004-g8ed4760 #110 [ 30.183166] Workqueue: omapdrm apply_worker [omapdrm] [ 30.207641] Backtrace: [ 30.210296] [<c00189ac>] (dump_backtrace+0x0/0x10c) from [<c0018b48>] (show_stack+0x18/0x1c) [ 30.219299] r6:bf0e5988 r5:00000009 r4:00000000 r3:eb872080 [ 30.225433] [<c0018b30>] (show_stack+0x0/0x1c) from [<c0520c04>] (dump_stack+0x20/0x28) [ 30.234039] [<c0520be4>] (dump_stack+0x0/0x28) from [<c0041fb8>] (warn_slowpath_common+0x54/0x74) [ 30.243530] [<c0041f64>] (warn_slowpath_common+0x0/0x74) from [<c0041ffc>] (warn_slowpath_null+0x24/0x2c) [ 30.243774] r8:ebfab99c r7:ebb41800 r6:ebb41830 r5:ebefce40 r4:ebefce40 r3:00000009 [ 30.262176] [<c0041fd8>] (warn_slowpath_null+0x0/0x2c) from [<bf0e5988>] (omap_gem_free_object+0x24c/0x25c [omapdrm]) [ 30.273498] [<bf0e573c>] (omap_gem_free_object+0x0/0x25c [omapdrm]) from [<bf080c0c>] (drm_gem_object_free+0x30/0x38 [drm]) [ 30.285675] [<bf080bdc>] (drm_gem_object_free+0x0/0x38 [drm]) from [<bf0e196c>] (omap_plane_post_apply+0x108/0x10c [omapdrm]) [ 30.285675] [<bf0e1864>] (omap_plane_post_apply+0x0/0x10c [omapdrm]) from [<bf0e100c>] (apply_worker+0x68/0x188 [omapdrm]) [ 30.309509] r6:ebae5c6c r5:ebae5c5c r4:ebae5c5c [ 30.312042] [<bf0e0fa4>] (apply_worker+0x0/0x188 [omapdrm]) from [<c005ef10>] (process_one_work+0x1b8/0x4e8) [ 30.325012] [<c005ed58>] (process_one_work+0x0/0x4e8) from [<c005f654>] (worker_thread+0x138/0x388) [ 30.325378] [<c005f51c>] (worker_thread+0x0/0x388) from [<c0066070>] (kthread+0xac/0xb8) [ 30.343292] [<c0065fc4>] (kthread+0x0/0xb8) from [<c00146e8>] (ret_from_fork+0x14/0x2c) [ 30.351837] r7:00000000 r6:00000000 r5:c0065fc4 r4:eb843d54 [ 30.352111] ---[ end trace 78317663d3b4807c ]---
Deadlock at module unload ========================= When removing the modules, there's a dealock. This is the last line printed:
[ 30.399200] [drm] Module unloaded
Then, when the deadlock detection kicks in:
[ 240.962432] INFO: task rmmod:894 blocked for more than 120 seconds. [ 240.962432] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 240.977508] rmmod D c0524964 0 894 884 0x00000000 [ 240.981719] Backtrace: [ 240.987274] [<c05245ec>] (__schedule+0x0/0x7e8) from [<c0524eb0>] (schedule+0x38/0x78) [ 240.995819] [<c0524e78>] (schedule+0x0/0x78) from [<c05251ac>] (schedule_preempt_disabled+0x28/0x38) [ 241.005706] [<c0525184>] (schedule_preempt_disabled+0x0/0x38) from [<c05238c4>] (mutex_lock_nested+0x1b8/0x3a8) [ 241.016693] r4:c07f5bf8 r3:eb978180 [ 241.020812] [<c052370c>] (mutex_lock_nested+0x0/0x3a8) from [<c033c9d0>] (driver_detach+0x4c/0xc0) [ 241.021026] [<c033c984>] (driver_detach+0x0/0xc0) from [<c033bd24>] (bus_remove_driver+0x94/0xfc) [ 241.040100] r6:c07f5b38 r5:bf0ec410 r4:00000000 r3:00000000 [ 241.042633] [<c033bc90>] (bus_remove_driver+0x0/0xfc) from [<c033d054>] (driver_unregister+0x58/0x78) [ 241.056060] r6:c07c28a4 r5:bf0ec410 r4:00000000 r3:eb978180 [ 241.062164] [<c033cffc>] (driver_unregister+0x0/0x78) from [<c033ddc0>] (platform_driver_unregister+0x14/0x18) [ 241.072875] r5:bf0ebc18 r4:c07c2860 [ 241.075439] [<c033ddac>] (platform_driver_unregister+0x0/0x18) from [<bf0df2c8>] (pdev_remove+0x48/0x54 [omapdrm]) [ 241.087921] [<bf0df280>] (pdev_remove+0x0/0x54 [omapdrm]) from [<c033dc38>] (platform_drv_remove+0x20/0x24) [ 241.098327] r4:c07c2870 r3:bf0df280 [ 241.100860] [<c033dc18>] (platform_drv_remove+0x0/0x24) from [<c033bfc4>] (__device_release_driver+0x78/0xd4) [ 241.112792] [<c033bf4c>] (__device_release_driver+0x0/0xd4) from [<c033ca40>] (driver_detach+0xbc/0xc0) [ 241.113159] r5:bf0ebc18 r4:c07c2870 [ 241.122833] [<c033c984>] (driver_detach+0x0/0xc0) from [<c033bd24>] (bus_remove_driver+0x94/0xfc) [ 241.136108] r6:c07f5b38 r5:bf0ebc18 r4:00000000 r3:00000000 [ 241.142211] [<c033bc90>] (bus_remove_driver+0x0/0xfc) from [<c033d054>] (driver_unregister+0x58/0x78) [ 241.152069] r6:00000880 r5:bf0ebc18 r4:00000000 r3:00000000 [ 241.153472] [<c033cffc>] (driver_unregister+0x0/0x78) from [<c033ddc0>] (platform_driver_unregister+0x14/0x18) [ 241.168823] r5:bf0ec4a0 r4:00000000 [ 241.171356] [<c033ddac>] (platform_driver_unregister+0x0/0x18) from [<bf0e9bb8>] (omap_drm_fini+0x28/0x3c [omapdrm]) [ 241.183990] [<bf0e9b90>] (omap_drm_fini+0x0/0x3c [omapdrm]) from [<c009ef10>] (SyS_delete_module+0x154/0x288) [ 241.194610] [<c009edbc>] (SyS_delete_module+0x0/0x288) from [<c0014620>] (ret_fast_syscall+0x0/0x48) [ 241.204345] r7:00000081 r6:006d7264 r5:70616d6f r4:0001aac4 [ 241.206726] 3 locks held by rmmod/894: [ 241.214416] #0: (&__lockdep_no_validate__){......}, at: [<c033c9d0>] driver_detach+0x4c/0xc0 [ 241.223693] #1: (&__lockdep_no_validate__){......}, at: [<c033c9dc>] driver_detach+0x58/0xc0 [ 241.232940] #2: (&__lockdep_no_validate__){......}, at: [<c033c9d0>] driver_detach+0x4c/0xc0
Tomi
On Wed, Jun 5, 2013 at 4:59 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
Hi,
I did some testing with omapdrm on v3.10-rc1. Here are some issues I encountered. For most of them I don't have any idea where to even start looking for a problem, so I hope that you may have some ideas.
dispc_runtime_get/put used in irq context
dispc_runtime_get/put are used in irq context in omap_irq.c. I can hack around that with if (!in_atomic()), but I have no idea yet how to fix it properly.
Probably should have a flag/refcnt that is set when in the irq hander, since we know that things are on in irq.
drm_crtc warning
I got this once when unloading the modules, but haven't happened since:
drm_crtc.c line 3869
WARN_ON(!list_empty(&dev->mode_config.fb_list));As it only happened once, maybe some kind of race.
Hmm, this is new.. it is certain that there can be a fb with a reference still around, waiting for a vblank/framedone. But I think it should already be removed from userspace perspective and no longer on fb_list. I think I need to go back and have a closer look at 2b677e8c0 where Daniel introduced that WARN_ON().
omap_gem warning
This happens on module unload:
oh, that is easy.. it is because the omap_fbdev.c does an unbalanced omap_gem_get_paddr() to keep the fbcon fb from getting unmapped from tiler (so if we have to restore fbcon mode in an opps, we don't have to worry about grabbing mutexes or anything like that). Maybe somewhere in the cleanup it should do a put_paddr(). Otoh, I have some skepticism about whether module unloading is really going to be that reliable in practice, so meh..
BR, -R
[ 30.167480] ------------[ cut here ]------------ [ 30.167724] WARNING: at drivers/gpu/drm/omapdrm/omap_gem.c:1318 omap_gem_free_object+0x24c/0x25c [omapdrm]() [ 30.182952] Modules linked in: omapdrm(-) drm_kms_helper drm panel_taal panel_tfp410 panel_generic_dpi omapdss [ 30.183166] CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted 3.10.0-rc1-00004-g8ed4760 #110 [ 30.183166] Workqueue: omapdrm apply_worker [omapdrm] [ 30.207641] Backtrace: [ 30.210296] [<c00189ac>] (dump_backtrace+0x0/0x10c) from [<c0018b48>] (show_stack+0x18/0x1c) [ 30.219299] r6:bf0e5988 r5:00000009 r4:00000000 r3:eb872080 [ 30.225433] [<c0018b30>] (show_stack+0x0/0x1c) from [<c0520c04>] (dump_stack+0x20/0x28) [ 30.234039] [<c0520be4>] (dump_stack+0x0/0x28) from [<c0041fb8>] (warn_slowpath_common+0x54/0x74) [ 30.243530] [<c0041f64>] (warn_slowpath_common+0x0/0x74) from [<c0041ffc>] (warn_slowpath_null+0x24/0x2c) [ 30.243774] r8:ebfab99c r7:ebb41800 r6:ebb41830 r5:ebefce40 r4:ebefce40 r3:00000009 [ 30.262176] [<c0041fd8>] (warn_slowpath_null+0x0/0x2c) from [<bf0e5988>] (omap_gem_free_object+0x24c/0x25c [omapdrm]) [ 30.273498] [<bf0e573c>] (omap_gem_free_object+0x0/0x25c [omapdrm]) from [<bf080c0c>] (drm_gem_object_free+0x30/0x38 [drm]) [ 30.285675] [<bf080bdc>] (drm_gem_object_free+0x0/0x38 [drm]) from [<bf0e196c>] (omap_plane_post_apply+0x108/0x10c [omapdrm]) [ 30.285675] [<bf0e1864>] (omap_plane_post_apply+0x0/0x10c [omapdrm]) from [<bf0e100c>] (apply_worker+0x68/0x188 [omapdrm]) [ 30.309509] r6:ebae5c6c r5:ebae5c5c r4:ebae5c5c [ 30.312042] [<bf0e0fa4>] (apply_worker+0x0/0x188 [omapdrm]) from [<c005ef10>] (process_one_work+0x1b8/0x4e8) [ 30.325012] [<c005ed58>] (process_one_work+0x0/0x4e8) from [<c005f654>] (worker_thread+0x138/0x388) [ 30.325378] [<c005f51c>] (worker_thread+0x0/0x388) from [<c0066070>] (kthread+0xac/0xb8) [ 30.343292] [<c0065fc4>] (kthread+0x0/0xb8) from [<c00146e8>] (ret_from_fork+0x14/0x2c) [ 30.351837] r7:00000000 r6:00000000 r5:c0065fc4 r4:eb843d54 [ 30.352111] ---[ end trace 78317663d3b4807c ]---
Deadlock at module unload
When removing the modules, there's a dealock. This is the last line printed:
[ 30.399200] [drm] Module unloaded
Then, when the deadlock detection kicks in:
[ 240.962432] INFO: task rmmod:894 blocked for more than 120 seconds. [ 240.962432] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 240.977508] rmmod D c0524964 0 894 884 0x00000000 [ 240.981719] Backtrace: [ 240.987274] [<c05245ec>] (__schedule+0x0/0x7e8) from [<c0524eb0>] (schedule+0x38/0x78) [ 240.995819] [<c0524e78>] (schedule+0x0/0x78) from [<c05251ac>] (schedule_preempt_disabled+0x28/0x38) [ 241.005706] [<c0525184>] (schedule_preempt_disabled+0x0/0x38) from [<c05238c4>] (mutex_lock_nested+0x1b8/0x3a8) [ 241.016693] r4:c07f5bf8 r3:eb978180 [ 241.020812] [<c052370c>] (mutex_lock_nested+0x0/0x3a8) from [<c033c9d0>] (driver_detach+0x4c/0xc0) [ 241.021026] [<c033c984>] (driver_detach+0x0/0xc0) from [<c033bd24>] (bus_remove_driver+0x94/0xfc) [ 241.040100] r6:c07f5b38 r5:bf0ec410 r4:00000000 r3:00000000 [ 241.042633] [<c033bc90>] (bus_remove_driver+0x0/0xfc) from [<c033d054>] (driver_unregister+0x58/0x78) [ 241.056060] r6:c07c28a4 r5:bf0ec410 r4:00000000 r3:eb978180 [ 241.062164] [<c033cffc>] (driver_unregister+0x0/0x78) from [<c033ddc0>] (platform_driver_unregister+0x14/0x18) [ 241.072875] r5:bf0ebc18 r4:c07c2860 [ 241.075439] [<c033ddac>] (platform_driver_unregister+0x0/0x18) from [<bf0df2c8>] (pdev_remove+0x48/0x54 [omapdrm]) [ 241.087921] [<bf0df280>] (pdev_remove+0x0/0x54 [omapdrm]) from [<c033dc38>] (platform_drv_remove+0x20/0x24) [ 241.098327] r4:c07c2870 r3:bf0df280 [ 241.100860] [<c033dc18>] (platform_drv_remove+0x0/0x24) from [<c033bfc4>] (__device_release_driver+0x78/0xd4) [ 241.112792] [<c033bf4c>] (__device_release_driver+0x0/0xd4) from [<c033ca40>] (driver_detach+0xbc/0xc0) [ 241.113159] r5:bf0ebc18 r4:c07c2870 [ 241.122833] [<c033c984>] (driver_detach+0x0/0xc0) from [<c033bd24>] (bus_remove_driver+0x94/0xfc) [ 241.136108] r6:c07f5b38 r5:bf0ebc18 r4:00000000 r3:00000000 [ 241.142211] [<c033bc90>] (bus_remove_driver+0x0/0xfc) from [<c033d054>] (driver_unregister+0x58/0x78) [ 241.152069] r6:00000880 r5:bf0ebc18 r4:00000000 r3:00000000 [ 241.153472] [<c033cffc>] (driver_unregister+0x0/0x78) from [<c033ddc0>] (platform_driver_unregister+0x14/0x18) [ 241.168823] r5:bf0ec4a0 r4:00000000 [ 241.171356] [<c033ddac>] (platform_driver_unregister+0x0/0x18) from [<bf0e9bb8>] (omap_drm_fini+0x28/0x3c [omapdrm]) [ 241.183990] [<bf0e9b90>] (omap_drm_fini+0x0/0x3c [omapdrm]) from [<c009ef10>] (SyS_delete_module+0x154/0x288) [ 241.194610] [<c009edbc>] (SyS_delete_module+0x0/0x288) from [<c0014620>] (ret_fast_syscall+0x0/0x48) [ 241.204345] r7:00000081 r6:006d7264 r5:70616d6f r4:0001aac4 [ 241.206726] 3 locks held by rmmod/894: [ 241.214416] #0: (&__lockdep_no_validate__){......}, at: [<c033c9d0>] driver_detach+0x4c/0xc0 [ 241.223693] #1: (&__lockdep_no_validate__){......}, at: [<c033c9dc>] driver_detach+0x58/0xc0 [ 241.232940] #2: (&__lockdep_no_validate__){......}, at: [<c033c9d0>] driver_detach+0x4c/0xc0
Tomi
On 05/06/13 13:43, Rob Clark wrote:
On Wed, Jun 5, 2013 at 4:59 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
Hi,
I did some testing with omapdrm on v3.10-rc1. Here are some issues I encountered. For most of them I don't have any idea where to even start looking for a problem, so I hope that you may have some ideas.
dispc_runtime_get/put used in irq context
dispc_runtime_get/put are used in irq context in omap_irq.c. I can hack around that with if (!in_atomic()), but I have no idea yet how to fix it properly.
Probably should have a flag/refcnt that is set when in the irq hander, since we know that things are on in irq.
Maybe.
However, I'd like more an approach where the dispc registers are only programmed when a display is enabled. If the display in question is disabled, no registers would be written.
I think that would also support losing DISPC register context. The DISPC driver currently stores all its registers when the pm_runtime refcount goes to 0, and restores them on pm_runtime_get. That's a bit heavy, and uses the OMAP specific ctx-loss-count support, which should be removed (and if that's removed, the register save/restore becomes even heavier).
Things would be simpler if omapdrm (and omapfb) knows how to program all the relevant registers when a display is enabled, and keeps the necessary state stored in memory (including irq settings).
somewhere in the cleanup it should do a put_paddr(). Otoh, I have some skepticism about whether module unloading is really going to be that reliable in practice, so meh..
Why is that?
Tomi
On Wed, Jun 5, 2013 at 6:43 AM, Rob Clark robdclark@gmail.com wrote:
On Wed, Jun 5, 2013 at 4:59 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
Hi,
I did some testing with omapdrm on v3.10-rc1. Here are some issues I encountered. For most of them I don't have any idea where to even start looking for a problem, so I hope that you may have some ideas.
dispc_runtime_get/put used in irq context
dispc_runtime_get/put are used in irq context in omap_irq.c. I can hack around that with if (!in_atomic()), but I have no idea yet how to fix it properly.
Probably should have a flag/refcnt that is set when in the irq hander, since we know that things are on in irq.
drm_crtc warning
I got this once when unloading the modules, but haven't happened since:
drm_crtc.c line 3869
WARN_ON(!list_empty(&dev->mode_config.fb_list));As it only happened once, maybe some kind of race.
Hmm, this is new.. it is certain that there can be a fb with a reference still around, waiting for a vblank/framedone. But I think it should already be removed from userspace perspective and no longer on fb_list. I think I need to go back and have a closer look at 2b677e8c0 where Daniel introduced that WARN_ON().
omap_gem warning
This happens on module unload:
oh, that is easy.. it is because the omap_fbdev.c does an unbalanced omap_gem_get_paddr() to keep the fbcon fb from getting unmapped from tiler (so if we have to restore fbcon mode in an opps, we don't have to worry about grabbing mutexes or anything like that). Maybe somewhere in the cleanup it should do a put_paddr(). Otoh, I have some skepticism about whether module unloading is really going to be that reliable in practice, so meh..
by which I mean: module unloading is a nice-to-have for development, but not a real use case.. it is more important to fix the issues we have with module loading:
1) drmOpen("omap") will try to modprobe "omap", not "omapdrm" so we need to rename the .ko 2) sorting out the modprobe of panel drivers.. although with the current structure of omapdrm+omapdss I can't think of any clean way to handle this. I suppose we could do a hack with a bunch of request_module()s
these are issues that actually cause problems for distros which want to build a unified kernel with all the drm drivers as modules
BR, -R
BR, -R
[ 30.167480] ------------[ cut here ]------------ [ 30.167724] WARNING: at drivers/gpu/drm/omapdrm/omap_gem.c:1318 omap_gem_free_object+0x24c/0x25c [omapdrm]() [ 30.182952] Modules linked in: omapdrm(-) drm_kms_helper drm panel_taal panel_tfp410 panel_generic_dpi omapdss [ 30.183166] CPU: 0 PID: 6 Comm: kworker/u4:0 Not tainted 3.10.0-rc1-00004-g8ed4760 #110 [ 30.183166] Workqueue: omapdrm apply_worker [omapdrm] [ 30.207641] Backtrace: [ 30.210296] [<c00189ac>] (dump_backtrace+0x0/0x10c) from [<c0018b48>] (show_stack+0x18/0x1c) [ 30.219299] r6:bf0e5988 r5:00000009 r4:00000000 r3:eb872080 [ 30.225433] [<c0018b30>] (show_stack+0x0/0x1c) from [<c0520c04>] (dump_stack+0x20/0x28) [ 30.234039] [<c0520be4>] (dump_stack+0x0/0x28) from [<c0041fb8>] (warn_slowpath_common+0x54/0x74) [ 30.243530] [<c0041f64>] (warn_slowpath_common+0x0/0x74) from [<c0041ffc>] (warn_slowpath_null+0x24/0x2c) [ 30.243774] r8:ebfab99c r7:ebb41800 r6:ebb41830 r5:ebefce40 r4:ebefce40 r3:00000009 [ 30.262176] [<c0041fd8>] (warn_slowpath_null+0x0/0x2c) from [<bf0e5988>] (omap_gem_free_object+0x24c/0x25c [omapdrm]) [ 30.273498] [<bf0e573c>] (omap_gem_free_object+0x0/0x25c [omapdrm]) from [<bf080c0c>] (drm_gem_object_free+0x30/0x38 [drm]) [ 30.285675] [<bf080bdc>] (drm_gem_object_free+0x0/0x38 [drm]) from [<bf0e196c>] (omap_plane_post_apply+0x108/0x10c [omapdrm]) [ 30.285675] [<bf0e1864>] (omap_plane_post_apply+0x0/0x10c [omapdrm]) from [<bf0e100c>] (apply_worker+0x68/0x188 [omapdrm]) [ 30.309509] r6:ebae5c6c r5:ebae5c5c r4:ebae5c5c [ 30.312042] [<bf0e0fa4>] (apply_worker+0x0/0x188 [omapdrm]) from [<c005ef10>] (process_one_work+0x1b8/0x4e8) [ 30.325012] [<c005ed58>] (process_one_work+0x0/0x4e8) from [<c005f654>] (worker_thread+0x138/0x388) [ 30.325378] [<c005f51c>] (worker_thread+0x0/0x388) from [<c0066070>] (kthread+0xac/0xb8) [ 30.343292] [<c0065fc4>] (kthread+0x0/0xb8) from [<c00146e8>] (ret_from_fork+0x14/0x2c) [ 30.351837] r7:00000000 r6:00000000 r5:c0065fc4 r4:eb843d54 [ 30.352111] ---[ end trace 78317663d3b4807c ]---
Deadlock at module unload
When removing the modules, there's a dealock. This is the last line printed:
[ 30.399200] [drm] Module unloaded
Then, when the deadlock detection kicks in:
[ 240.962432] INFO: task rmmod:894 blocked for more than 120 seconds. [ 240.962432] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 240.977508] rmmod D c0524964 0 894 884 0x00000000 [ 240.981719] Backtrace: [ 240.987274] [<c05245ec>] (__schedule+0x0/0x7e8) from [<c0524eb0>] (schedule+0x38/0x78) [ 240.995819] [<c0524e78>] (schedule+0x0/0x78) from [<c05251ac>] (schedule_preempt_disabled+0x28/0x38) [ 241.005706] [<c0525184>] (schedule_preempt_disabled+0x0/0x38) from [<c05238c4>] (mutex_lock_nested+0x1b8/0x3a8) [ 241.016693] r4:c07f5bf8 r3:eb978180 [ 241.020812] [<c052370c>] (mutex_lock_nested+0x0/0x3a8) from [<c033c9d0>] (driver_detach+0x4c/0xc0) [ 241.021026] [<c033c984>] (driver_detach+0x0/0xc0) from [<c033bd24>] (bus_remove_driver+0x94/0xfc) [ 241.040100] r6:c07f5b38 r5:bf0ec410 r4:00000000 r3:00000000 [ 241.042633] [<c033bc90>] (bus_remove_driver+0x0/0xfc) from [<c033d054>] (driver_unregister+0x58/0x78) [ 241.056060] r6:c07c28a4 r5:bf0ec410 r4:00000000 r3:eb978180 [ 241.062164] [<c033cffc>] (driver_unregister+0x0/0x78) from [<c033ddc0>] (platform_driver_unregister+0x14/0x18) [ 241.072875] r5:bf0ebc18 r4:c07c2860 [ 241.075439] [<c033ddac>] (platform_driver_unregister+0x0/0x18) from [<bf0df2c8>] (pdev_remove+0x48/0x54 [omapdrm]) [ 241.087921] [<bf0df280>] (pdev_remove+0x0/0x54 [omapdrm]) from [<c033dc38>] (platform_drv_remove+0x20/0x24) [ 241.098327] r4:c07c2870 r3:bf0df280 [ 241.100860] [<c033dc18>] (platform_drv_remove+0x0/0x24) from [<c033bfc4>] (__device_release_driver+0x78/0xd4) [ 241.112792] [<c033bf4c>] (__device_release_driver+0x0/0xd4) from [<c033ca40>] (driver_detach+0xbc/0xc0) [ 241.113159] r5:bf0ebc18 r4:c07c2870 [ 241.122833] [<c033c984>] (driver_detach+0x0/0xc0) from [<c033bd24>] (bus_remove_driver+0x94/0xfc) [ 241.136108] r6:c07f5b38 r5:bf0ebc18 r4:00000000 r3:00000000 [ 241.142211] [<c033bc90>] (bus_remove_driver+0x0/0xfc) from [<c033d054>] (driver_unregister+0x58/0x78) [ 241.152069] r6:00000880 r5:bf0ebc18 r4:00000000 r3:00000000 [ 241.153472] [<c033cffc>] (driver_unregister+0x0/0x78) from [<c033ddc0>] (platform_driver_unregister+0x14/0x18) [ 241.168823] r5:bf0ec4a0 r4:00000000 [ 241.171356] [<c033ddac>] (platform_driver_unregister+0x0/0x18) from [<bf0e9bb8>] (omap_drm_fini+0x28/0x3c [omapdrm]) [ 241.183990] [<bf0e9b90>] (omap_drm_fini+0x0/0x3c [omapdrm]) from [<c009ef10>] (SyS_delete_module+0x154/0x288) [ 241.194610] [<c009edbc>] (SyS_delete_module+0x0/0x288) from [<c0014620>] (ret_fast_syscall+0x0/0x48) [ 241.204345] r7:00000081 r6:006d7264 r5:70616d6f r4:0001aac4 [ 241.206726] 3 locks held by rmmod/894: [ 241.214416] #0: (&__lockdep_no_validate__){......}, at: [<c033c9d0>] driver_detach+0x4c/0xc0 [ 241.223693] #1: (&__lockdep_no_validate__){......}, at: [<c033c9dc>] driver_detach+0x58/0xc0 [ 241.232940] #2: (&__lockdep_no_validate__){......}, at: [<c033c9d0>] driver_detach+0x4c/0xc0
Tomi
On 05/06/13 13:57, Rob Clark wrote:
- drmOpen("omap") will try to modprobe "omap", not "omapdrm" so we
need to rename the .ko
Has something been changed that broke that? Or was "omapdrm" just a badly chosen name from the start? If drmOpen("omapdrm") works now, doesn't changing the module name break userspace compatibility?
I had a quick look at libdrm. It calls server_info->load_module() but I couldn't figure out where that call actually goes...
- sorting out the modprobe of panel drivers.. although with the
current structure of omapdrm+omapdss I can't think of any clean way to handle this. I suppose we could do a hack with a bunch of request_module()s
If omapdrm and omapdss were merged, what would be the clean way be? Or did you mean some other structure?
I'm no expert on this, but my understanding is that udev (or such) should load the modules for the devices that the board has. If it's a requirement that the drm drivers are loaded only by a call to drmOpen() (why is that?), then maybe udev could load only the panel drivers.
Tomi
On Wed, Jun 5, 2013 at 7:35 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 05/06/13 13:57, Rob Clark wrote:
- drmOpen("omap") will try to modprobe "omap", not "omapdrm" so we
need to rename the .ko
Has something been changed that broke that? Or was "omapdrm" just a badly chosen name from the start? If drmOpen("omapdrm") works now, doesn't changing the module name break userspace compatibility?
it was broken all along, because you need to drmOpen("omap") so the module should have been called omap.ko from the beginning
I had a quick look at libdrm. It calls server_info->load_module() but I couldn't figure out where that call actually goes...
it's registered from xserver, fyi
- sorting out the modprobe of panel drivers.. although with the
current structure of omapdrm+omapdss I can't think of any clean way to handle this. I suppose we could do a hack with a bunch of request_module()s
If omapdrm and omapdss were merged, what would be the clean way be? Or did you mean some other structure?
well, then we'd just build it all into one .ko
I'm no expert on this, but my understanding is that udev (or such) should load the modules for the devices that the board has. If it's a requirement that the drm drivers are loaded only by a call to drmOpen() (why is that?), then maybe udev could load only the panel drivers.
I'm not quite sure the entire history offhand.. maybe drmOpen() pre-dated that? At any rate, the main issue is just ensuring the panel modules (if they are split out into different .ko's) are loaded first
BR, -R
Tomi
On 05/06/13 14:52, Rob Clark wrote:
On Wed, Jun 5, 2013 at 7:35 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 05/06/13 13:57, Rob Clark wrote:
- drmOpen("omap") will try to modprobe "omap", not "omapdrm" so we
need to rename the .ko
Has something been changed that broke that? Or was "omapdrm" just a badly chosen name from the start? If drmOpen("omapdrm") works now, doesn't changing the module name break userspace compatibility?
it was broken all along, because you need to drmOpen("omap") so the module should have been called omap.ko from the beginning
But it wasn't, so wouldn't changing it break userspace compatibility?
Why do you "need" drmOpen("omap")? Is it just a convention to use that kind of name, instead of "omapdrm" style name?
I had a quick look at libdrm. It calls server_info->load_module() but I couldn't figure out where that call actually goes...
it's registered from xserver, fyi
Ah ok. So in my testing, running without X, there's actually no module loading happening. The test tools print something like "trying to load module omapdrm...success." but I guess that only means that the module is already there.
- sorting out the modprobe of panel drivers.. although with the
current structure of omapdrm+omapdss I can't think of any clean way to handle this. I suppose we could do a hack with a bunch of request_module()s
If omapdrm and omapdss were merged, what would be the clean way be? Or did you mean some other structure?
well, then we'd just build it all into one .ko
So you didn't actually mean omapdrm+omapdss, but omapdrm + omapdss + all-the-panel-drivers? And then have some kind of forced method to probe the panels at omapdrm start?
That would be quite a horrible solution, in my opinion =). It would also be even worse when we get platform independent panel drivers, as all the drm drivers using those panel drivers would include all the panel drivers.
I'm no expert on this, but my understanding is that udev (or such) should load the modules for the devices that the board has. If it's a requirement that the drm drivers are loaded only by a call to drmOpen() (why is that?), then maybe udev could load only the panel drivers.
I'm not quite sure the entire history offhand.. maybe drmOpen() pre-dated that? At any rate, the main issue is just ensuring the panel modules (if they are split out into different .ko's) are loaded first
I think there should be a core display facility in the kernel to which the panel drivers register the displays in probe().
At boot time udev would go through /sys, find the panel devices somehow (how? modalias is probably somehow involved..), and load their respective modules.
Later, when X is started, omapdrm and omapdss are loaded, and at that point all the panels are already there.
Tomi
On Wed, Jun 5, 2013 at 8:16 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 05/06/13 14:52, Rob Clark wrote:
On Wed, Jun 5, 2013 at 7:35 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 05/06/13 13:57, Rob Clark wrote:
- drmOpen("omap") will try to modprobe "omap", not "omapdrm" so we
need to rename the .ko
Has something been changed that broke that? Or was "omapdrm" just a badly chosen name from the start? If drmOpen("omapdrm") works now, doesn't changing the module name break userspace compatibility?
it was broken all along, because you need to drmOpen("omap") so the module should have been called omap.ko from the beginning
But it wasn't, so wouldn't changing it break userspace compatibility?
could be udev or something else was triggering the module to load? Not 100% sure offhand.
Why do you "need" drmOpen("omap")? Is it just a convention to use that kind of name, instead of "omapdrm" style name?
all of the /dev/dri/cardN get opened, and then DRM_IOCTL_VERSION ioctl to get the driver name/version, which is compared against the name passed in to drmOpen(). So drmOpen("omapdrm") shouldn't really work
I had a quick look at libdrm. It calls server_info->load_module() but I couldn't figure out where that call actually goes...
it's registered from xserver, fyi
Ah ok. So in my testing, running without X, there's actually no module loading happening. The test tools print something like "trying to load module omapdrm...success." but I guess that only means that the module is already there.
right
- sorting out the modprobe of panel drivers.. although with the
current structure of omapdrm+omapdss I can't think of any clean way to handle this. I suppose we could do a hack with a bunch of request_module()s
If omapdrm and omapdss were merged, what would be the clean way be? Or did you mean some other structure?
well, then we'd just build it all into one .ko
So you didn't actually mean omapdrm+omapdss, but omapdrm + omapdss + all-the-panel-drivers? And then have some kind of forced method to probe the panels at omapdrm start?
oh, yeah. Probably whenever I say 'omapdss' I actually mean 'everything under drivers/video/omap2 except omapfb'
That would be quite a horrible solution, in my opinion =). It would also be even worse when we get platform independent panel drivers, as all the drm drivers using those panel drivers would include all the panel drivers.
yeah, it isn't really ideal.. really we'd want them as separate modules but some sort of dependency to ensure that the panel drivers actually get loaded.
I'm no expert on this, but my understanding is that udev (or such) should load the modules for the devices that the board has. If it's a requirement that the drm drivers are loaded only by a call to drmOpen() (why is that?), then maybe udev could load only the panel drivers.
I'm not quite sure the entire history offhand.. maybe drmOpen() pre-dated that? At any rate, the main issue is just ensuring the panel modules (if they are split out into different .ko's) are loaded first
I think there should be a core display facility in the kernel to which the panel drivers register the displays in probe().
At boot time udev would go through /sys, find the panel devices somehow (how? modalias is probably somehow involved..), and load their respective modules.
Later, when X is started, omapdrm and omapdss are loaded, and at that point all the panels are already there.
yeah, as long as we get the panels (and encoders/bridges/etc) loaded first, I think it should be ok..
I'm not hugely fond of having it split out of drm, since I think that just results extra layering and glue, and it makes it harder to iteratively evolve the infrastructure for sharing panel stuff via different trees from drm. But that is a different discussion and is mostly about where the code lives.
BR, -R
Tomi
On 05/06/13 17:58, Rob Clark wrote:
could be udev or something else was triggering the module to load? Not 100% sure offhand.
Well in my case I load them manually. But if there are applications out there, that at the moment use drmOpen("omapdrm"), then the change would break those apps.
Why do you "need" drmOpen("omap")? Is it just a convention to use that kind of name, instead of "omapdrm" style name?
all of the /dev/dri/cardN get opened, and then DRM_IOCTL_VERSION ioctl to get the driver name/version, which is compared against the name passed in to drmOpen(). So drmOpen("omapdrm") shouldn't really work
Ok, so normally nobody will call drmOpen("omapdrm") explicitly, but somebody (X?) will just do what you described to get the modules loaded? If so, I guess it's unlikely that there would be an app with drmOpen("omapdrm").
Of course, we could also rename the name returned via DRM_IOCTL_VERSION to be "omapdrm", right? But looking at the names of other drivers, "omap" is probably better.
Tomi
On Thu, Jun 6, 2013 at 3:14 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:
On 05/06/13 17:58, Rob Clark wrote:
could be udev or something else was triggering the module to load? Not 100% sure offhand.
Well in my case I load them manually. But if there are applications out there, that at the moment use drmOpen("omapdrm"), then the change would break those apps.
well, drmOpen("omapdrm") shouldn't currently work
Why do you "need" drmOpen("omap")? Is it just a convention to use that kind of name, instead of "omapdrm" style name?
all of the /dev/dri/cardN get opened, and then DRM_IOCTL_VERSION ioctl to get the driver name/version, which is compared against the name passed in to drmOpen(). So drmOpen("omapdrm") shouldn't really work
Ok, so normally nobody will call drmOpen("omapdrm") explicitly, but somebody (X?) will just do what you described to get the modules loaded? If so, I guess it's unlikely that there would be an app with drmOpen("omapdrm").
Of course, we could also rename the name returned via DRM_IOCTL_VERSION to be "omapdrm", right? But looking at the names of other drivers, "omap" is probably better.
changing the name would also be an abi break ;-)
BR, -R
Tomi
On Thu, Jun 6, 2013 at 9:25 PM, Rob Clark robdclark@gmail.com wrote:
On Thu, Jun 6, 2013 at 3:\
Why do you "need" drmOpen("omap")? Is it just a convention to use that kind of name, instead of "omapdrm" style name?
all of the /dev/dri/cardN get opened, and then DRM_IOCTL_VERSION ioctl to get the driver name/version, which is compared against the name passed in to drmOpen(). So drmOpen("omapdrm") shouldn't really work
Ok, so normally nobody will call drmOpen("omapdrm") explicitly, but somebody (X?) will just do what you described to get the modules loaded? If so, I guess it's unlikely that there would be an app with drmOpen("omapdrm").
These days X shouldn't be loading anything, that is DRI1 left overs.
The kernel/udev should be loading all the drivers well before X starts.
Dave.
dri-devel@lists.freedesktop.org
-
Dave Airlie -
Rob Clark -
Tomi Valkeinen