diff options
author | Tomi Valkeinen <tomi.valkeinen@nokia.com> | 2010-04-12 02:57:19 -0400 |
---|---|---|
committer | Tomi Valkeinen <tomi.valkeinen@nokia.com> | 2010-08-03 08:18:46 -0400 |
commit | 0f16aa0ae6b84d7ae72fbe8999e6a94cb78edd4e (patch) | |
tree | ff277d1cfa93a6ea5b6602d93462a704b79be5ca /drivers/video | |
parent | 86a7867ebff675f5f5816222c5a2c64b35f8bea6 (diff) |
OMAP: DSS2: DSI: use a private workqueue
Using the shared workqueue led to to a deadlock in the case where the
display was unblanked via keyboard.
What happens is something like this:
- User presses a key
context 1:
- drivers/char/keyboard.c calls schedule_console_callback()
- fb_unblank takes the console semaphore
- dsi bus lock is taken, and frame transfer is started (dsi bus lock is
left on)
- Unblank code tries to set the panel backlight, which tries to take dsi
bus lock, but is blocked while the frame transfer is going on
context 2, shared workqueue, console_callback in drivers/char/vt.c:
- Tries to take console semaphore
- Blocks, as console semaphore is being held by context 1
- No other shared workqueue work can be run
context 3, HW irq, caused by FRAMEDONE interrupt:
- Interrupt handler schedules framedone-work in shared workqueue
- Framedone-work is never ran, as the shared workqueue is blocked. This
means that the unblank thread stays blocked, which means that context 2
stays blocked.
While I think the real problem is in keyboard/virtual terminal code, using
a private workqueue in the DSI driver is perhaps safer and more robust
than using the shared one. The DSI works should not be delayed more than a
millisecond or so, and even if the private workqueue gives us no hard
promise of doing so, it's still safer bet than the shared workqueue.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@nokia.com>
Diffstat (limited to 'drivers/video')
-rw-r--r-- | drivers/video/omap2/dss/dsi.c | 22 |
1 files changed, 20 insertions, 2 deletions
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c index 542c6e2a3ff1..de3fdab46f38 100644 --- a/drivers/video/omap2/dss/dsi.c +++ b/drivers/video/omap2/dss/dsi.c | |||
@@ -238,6 +238,8 @@ static struct | |||
238 | 238 | ||
239 | bool te_enabled; | 239 | bool te_enabled; |
240 | 240 | ||
241 | struct workqueue_struct *workqueue; | ||
242 | |||
241 | struct work_struct framedone_work; | 243 | struct work_struct framedone_work; |
242 | void (*framedone_callback)(int, void *); | 244 | void (*framedone_callback)(int, void *); |
243 | void *framedone_data; | 245 | void *framedone_data; |
@@ -2759,6 +2761,7 @@ static void dsi_update_screen_dispc(struct omap_dss_device *dssdev, | |||
2759 | unsigned packet_payload; | 2761 | unsigned packet_payload; |
2760 | unsigned packet_len; | 2762 | unsigned packet_len; |
2761 | u32 l; | 2763 | u32 l; |
2764 | int r; | ||
2762 | const unsigned channel = dsi.update_channel; | 2765 | const unsigned channel = dsi.update_channel; |
2763 | /* line buffer is 1024 x 24bits */ | 2766 | /* line buffer is 1024 x 24bits */ |
2764 | /* XXX: for some reason using full buffer size causes considerable TX | 2767 | /* XXX: for some reason using full buffer size causes considerable TX |
@@ -2809,8 +2812,9 @@ static void dsi_update_screen_dispc(struct omap_dss_device *dssdev, | |||
2809 | 2812 | ||
2810 | dsi_perf_mark_start(); | 2813 | dsi_perf_mark_start(); |
2811 | 2814 | ||
2812 | schedule_delayed_work(&dsi.framedone_timeout_work, | 2815 | r = queue_delayed_work(dsi.workqueue, &dsi.framedone_timeout_work, |
2813 | msecs_to_jiffies(250)); | 2816 | msecs_to_jiffies(250)); |
2817 | BUG_ON(r == 0); | ||
2814 | 2818 | ||
2815 | dss_start_update(dssdev); | 2819 | dss_start_update(dssdev); |
2816 | 2820 | ||
@@ -2841,6 +2845,11 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work) | |||
2841 | 2845 | ||
2842 | DSSERR("Framedone not received for 250ms!\n"); | 2846 | DSSERR("Framedone not received for 250ms!\n"); |
2843 | 2847 | ||
2848 | /* XXX While extremely unlikely, we could get FRAMEDONE interrupt after | ||
2849 | * 250ms which would conflict with this timeout work. What should be | ||
2850 | * done is first cancel the transfer on the HW, and then cancel the | ||
2851 | * possibly scheduled framedone work */ | ||
2852 | |||
2844 | /* SIDLEMODE back to smart-idle */ | 2853 | /* SIDLEMODE back to smart-idle */ |
2845 | dispc_enable_sidle(); | 2854 | dispc_enable_sidle(); |
2846 | 2855 | ||
@@ -2873,6 +2882,7 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work) | |||
2873 | 2882 | ||
2874 | static void dsi_framedone_irq_callback(void *data, u32 mask) | 2883 | static void dsi_framedone_irq_callback(void *data, u32 mask) |
2875 | { | 2884 | { |
2885 | int r; | ||
2876 | /* Note: We get FRAMEDONE when DISPC has finished sending pixels and | 2886 | /* Note: We get FRAMEDONE when DISPC has finished sending pixels and |
2877 | * turns itself off. However, DSI still has the pixels in its buffers, | 2887 | * turns itself off. However, DSI still has the pixels in its buffers, |
2878 | * and is sending the data. | 2888 | * and is sending the data. |
@@ -2881,7 +2891,8 @@ static void dsi_framedone_irq_callback(void *data, u32 mask) | |||
2881 | /* SIDLEMODE back to smart-idle */ | 2891 | /* SIDLEMODE back to smart-idle */ |
2882 | dispc_enable_sidle(); | 2892 | dispc_enable_sidle(); |
2883 | 2893 | ||
2884 | schedule_work(&dsi.framedone_work); | 2894 | r = queue_work(dsi.workqueue, &dsi.framedone_work); |
2895 | BUG_ON(r == 0); | ||
2885 | } | 2896 | } |
2886 | 2897 | ||
2887 | static void dsi_handle_framedone(void) | 2898 | static void dsi_handle_framedone(void) |
@@ -3292,6 +3303,10 @@ int dsi_init(struct platform_device *pdev) | |||
3292 | mutex_init(&dsi.lock); | 3303 | mutex_init(&dsi.lock); |
3293 | sema_init(&dsi.bus_lock, 1); | 3304 | sema_init(&dsi.bus_lock, 1); |
3294 | 3305 | ||
3306 | dsi.workqueue = create_singlethread_workqueue("dsi"); | ||
3307 | if (dsi.workqueue == NULL) | ||
3308 | return -ENOMEM; | ||
3309 | |||
3295 | INIT_WORK(&dsi.framedone_work, dsi_framedone_work_callback); | 3310 | INIT_WORK(&dsi.framedone_work, dsi_framedone_work_callback); |
3296 | INIT_DELAYED_WORK_DEFERRABLE(&dsi.framedone_timeout_work, | 3311 | INIT_DELAYED_WORK_DEFERRABLE(&dsi.framedone_timeout_work, |
3297 | dsi_framedone_timeout_work_callback); | 3312 | dsi_framedone_timeout_work_callback); |
@@ -3328,6 +3343,7 @@ int dsi_init(struct platform_device *pdev) | |||
3328 | err2: | 3343 | err2: |
3329 | iounmap(dsi.base); | 3344 | iounmap(dsi.base); |
3330 | err1: | 3345 | err1: |
3346 | destroy_workqueue(dsi.workqueue); | ||
3331 | return r; | 3347 | return r; |
3332 | } | 3348 | } |
3333 | 3349 | ||
@@ -3335,6 +3351,8 @@ void dsi_exit(void) | |||
3335 | { | 3351 | { |
3336 | iounmap(dsi.base); | 3352 | iounmap(dsi.base); |
3337 | 3353 | ||
3354 | destroy_workqueue(dsi.workqueue); | ||
3355 | |||
3338 | DSSDBG("omap_dsi_exit\n"); | 3356 | DSSDBG("omap_dsi_exit\n"); |
3339 | } | 3357 | } |
3340 | 3358 | ||