Commit 0f16aa0a authored by Tomi Valkeinen's avatar Tomi Valkeinen

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: default avatarTomi Valkeinen <tomi.valkeinen@nokia.com>
parent 86a7867e
...@@ -238,6 +238,8 @@ static struct ...@@ -238,6 +238,8 @@ static struct
bool te_enabled; bool te_enabled;
struct workqueue_struct *workqueue;
struct work_struct framedone_work; struct work_struct framedone_work;
void (*framedone_callback)(int, void *); void (*framedone_callback)(int, void *);
void *framedone_data; void *framedone_data;
...@@ -2759,6 +2761,7 @@ static void dsi_update_screen_dispc(struct omap_dss_device *dssdev, ...@@ -2759,6 +2761,7 @@ static void dsi_update_screen_dispc(struct omap_dss_device *dssdev,
unsigned packet_payload; unsigned packet_payload;
unsigned packet_len; unsigned packet_len;
u32 l; u32 l;
int r;
const unsigned channel = dsi.update_channel; const unsigned channel = dsi.update_channel;
/* line buffer is 1024 x 24bits */ /* line buffer is 1024 x 24bits */
/* XXX: for some reason using full buffer size causes considerable TX /* 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,8 +2812,9 @@ static void dsi_update_screen_dispc(struct omap_dss_device *dssdev,
dsi_perf_mark_start(); dsi_perf_mark_start();
schedule_delayed_work(&dsi.framedone_timeout_work, r = queue_delayed_work(dsi.workqueue, &dsi.framedone_timeout_work,
msecs_to_jiffies(250)); msecs_to_jiffies(250));
BUG_ON(r == 0);
dss_start_update(dssdev); dss_start_update(dssdev);
...@@ -2841,6 +2845,11 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work) ...@@ -2841,6 +2845,11 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work)
DSSERR("Framedone not received for 250ms!\n"); DSSERR("Framedone not received for 250ms!\n");
/* XXX While extremely unlikely, we could get FRAMEDONE interrupt after
* 250ms which would conflict with this timeout work. What should be
* done is first cancel the transfer on the HW, and then cancel the
* possibly scheduled framedone work */
/* SIDLEMODE back to smart-idle */ /* SIDLEMODE back to smart-idle */
dispc_enable_sidle(); dispc_enable_sidle();
...@@ -2873,6 +2882,7 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work) ...@@ -2873,6 +2882,7 @@ static void dsi_framedone_timeout_work_callback(struct work_struct *work)
static void dsi_framedone_irq_callback(void *data, u32 mask) static void dsi_framedone_irq_callback(void *data, u32 mask)
{ {
int r;
/* Note: We get FRAMEDONE when DISPC has finished sending pixels and /* Note: We get FRAMEDONE when DISPC has finished sending pixels and
* turns itself off. However, DSI still has the pixels in its buffers, * turns itself off. However, DSI still has the pixels in its buffers,
* and is sending the data. * and is sending the data.
...@@ -2881,7 +2891,8 @@ static void dsi_framedone_irq_callback(void *data, u32 mask) ...@@ -2881,7 +2891,8 @@ static void dsi_framedone_irq_callback(void *data, u32 mask)
/* SIDLEMODE back to smart-idle */ /* SIDLEMODE back to smart-idle */
dispc_enable_sidle(); dispc_enable_sidle();
schedule_work(&dsi.framedone_work); r = queue_work(dsi.workqueue, &dsi.framedone_work);
BUG_ON(r == 0);
} }
static void dsi_handle_framedone(void) static void dsi_handle_framedone(void)
...@@ -3292,6 +3303,10 @@ int dsi_init(struct platform_device *pdev) ...@@ -3292,6 +3303,10 @@ int dsi_init(struct platform_device *pdev)
mutex_init(&dsi.lock); mutex_init(&dsi.lock);
sema_init(&dsi.bus_lock, 1); sema_init(&dsi.bus_lock, 1);
dsi.workqueue = create_singlethread_workqueue("dsi");
if (dsi.workqueue == NULL)
return -ENOMEM;
INIT_WORK(&dsi.framedone_work, dsi_framedone_work_callback); INIT_WORK(&dsi.framedone_work, dsi_framedone_work_callback);
INIT_DELAYED_WORK_DEFERRABLE(&dsi.framedone_timeout_work, INIT_DELAYED_WORK_DEFERRABLE(&dsi.framedone_timeout_work,
dsi_framedone_timeout_work_callback); dsi_framedone_timeout_work_callback);
...@@ -3328,6 +3343,7 @@ int dsi_init(struct platform_device *pdev) ...@@ -3328,6 +3343,7 @@ int dsi_init(struct platform_device *pdev)
err2: err2:
iounmap(dsi.base); iounmap(dsi.base);
err1: err1:
destroy_workqueue(dsi.workqueue);
return r; return r;
} }
...@@ -3335,6 +3351,8 @@ void dsi_exit(void) ...@@ -3335,6 +3351,8 @@ void dsi_exit(void)
{ {
iounmap(dsi.base); iounmap(dsi.base);
destroy_workqueue(dsi.workqueue);
DSSDBG("omap_dsi_exit\n"); DSSDBG("omap_dsi_exit\n");
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment