cx18: Simplify the work handler for outgoing mailbox commands

From: Andy Walls <awalls@radix.net>

Simplify the way outgoing work handler gets scheduled to send empty buffers
back to the firmware for use.  Also reduced the memory required for scheduling
this outgoing work, by using a single, per stream work object.

Priority: normal

Signed-off-by: Andy Walls <awalls@radix.net>

6 files changed, 40 insertions, 130 deletions

diff --git a/linux/drivers/media/video/cx18/cx18-driver.c b/linux/drivers/media/video/cx18/cx18-driver.c
index db226f412..41997dd18 100644
--- a/linux/drivers/media/video/cx18/cx18-driver.c
+++ b/linux/drivers/media/video/cx18/cx18-driver.c
@@ -30,6 +30,7 @@
 #include "cx18-irq.h"
 #include "cx18-gpio.h"
 #include "cx18-firmware.h"
+#include "cx18-queue.h"
 #include "cx18-streams.h"
 #include "cx18-av-core.h"
 #include "cx18-scb.h"
@@ -585,18 +586,6 @@ static void __devinit cx18_init_in_work_orders(struct cx18 *cx)
 	}
 }
 
-static void __devinit cx18_init_out_work_orders(struct cx18 *cx)
-{
-	int i;
-	for (i = 0; i < CX18_MAX_OUT_WORK_ORDERS; i++)
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 20)
-		INIT_WORK(&cx->out_work_order[i].work, cx18_out_work_handler);
-#else
-		INIT_WORK(&cx->out_work_order[i].work, cx18_out_work_handler,
-			  &cx->out_work_order[i].work);
-#endif
-}
-
 /* Precondition: the cx18 structure has been memset to 0. Only
    the dev and instance fields have been filled in.
    No assumptions on the card type may be made here (see cx18_init_struct2
@@ -623,7 +612,6 @@ static int __devinit cx18_init_struct1(struct cx18 *cx)
 		return ret;
 	}
 
-	cx18_init_out_work_orders(cx);
 	cx18_init_in_work_orders(cx);
 
 	/* start counting open_id at 1 */
@@ -1114,6 +1102,14 @@ static void cx18_cancel_in_work_orders(struct cx18 *cx)
 		cancel_work_sync(&cx->in_work_order[i].work);
 }
 
+static void cx18_cancel_out_work_orders(struct cx18 *cx)
+{
+	int i;
+	for (i = 0; i < CX18_MAX_STREAMS; i++)
+		if (&cx->streams[i].video_dev != NULL)
+			cancel_work_sync(&cx->streams[i].out_work_order);
+}
+
 #endif
 static void cx18_remove(struct pci_dev *pci_dev)
 {
@@ -1134,16 +1130,11 @@ static void cx18_remove(struct pci_dev *pci_dev)
 	/* Incoming work can cause outgoing work, so clean up incoming first */
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 22)
 	cx18_cancel_in_work_orders(cx);
+	cx18_cancel_out_work_orders(cx);
 #else
 	flush_workqueue(cx->in_work_queue);
-#endif
-
-	/*
-	 * An outgoing work order can have the only pointer to a dynamically
-	 * allocated buffer, so we need to flush outgoing work and not just
-	 * cancel it, so we don't lose the pointer and leak memory.
-	 */
 	flush_workqueue(cx->out_work_queue);
+#endif
 
 	/* Stop ack interrupts that may have been needed for work to finish */
 	cx18_sw2_irq_disable(cx, IRQ_CPU_TO_EPU_ACK | IRQ_APU_TO_EPU_ACK);
diff --git a/linux/drivers/media/video/cx18/cx18-driver.h b/linux/drivers/media/video/cx18/cx18-driver.h
index 35a6758a7..f89b82367 100644
--- a/linux/drivers/media/video/cx18/cx18-driver.h
+++ b/linux/drivers/media/video/cx18/cx18-driver.h
@@ -326,33 +326,6 @@ struct cx18_in_work_order {
 	char *str;
 };
 
-/*
- * There are 2 types of deferrable tasks that send messages out to the firmware:
- * 1. Sending individual buffers back to the firmware
- * 2. Sending as many free buffers for a stream from q_free as we can to the fw
- *
- * The worst case scenario for multiple simultaneous streams is
- * TS, YUV, PCM, VBI, MPEG, and IDX all going at once.
- *
- * We try to load the firmware queue with as many free buffers as possible,
- * whenever we get a buffer back for a stream.  For the TS we return the single
- * buffer to the firmware at that time as well.  For all other streams, we
- * return single buffers to the firmware as the application drains them.
- *
- * 6 streams * 2 sets of orders * (1 single buf + 1 load fw from q_free)
- * = 24 work orders should cover our needs, provided the applications read
- * at a fairly steady rate.  If apps don't, we fall back to non-deferred
- * operation, when no cx18_out_work_orders are available for use.
- */
-#define CX18_MAX_OUT_WORK_ORDERS (24)
-
-struct cx18_out_work_order {
-	struct work_struct work;
-	atomic_t pending;
-	struct cx18_stream *s;
-	struct cx18_buffer *buf; /* buf == NULL, means load fw from q_free */
-};
-
 #define CX18_INVALID_TASK_HANDLE 0xffffffff
 
 struct cx18_stream {
@@ -381,6 +354,8 @@ struct cx18_stream {
 	struct cx18_queue q_busy;	/* busy buffers - in use by firmware */
 	struct cx18_queue q_full;	/* full buffers - data for user apps */
 
+	struct work_struct out_work_order;
+
 	/* DVB / Digital Transport */
 	struct cx18_dvb dvb;
 };
@@ -603,7 +578,6 @@ struct cx18 {
 
 	struct workqueue_struct *out_work_queue;
 	char out_workq_name[12]; /* "cx18-NN-out" */
-	struct cx18_out_work_order out_work_order[CX18_MAX_OUT_WORK_ORDERS];
 
 	/* i2c */
 	struct i2c_adapter i2c_adap[2];
diff --git a/linux/drivers/media/video/cx18/cx18-dvb.c b/linux/drivers/media/video/cx18/cx18-dvb.c
index 3b86f57cd..e7285a109 100644
--- a/linux/drivers/media/video/cx18/cx18-dvb.c
+++ b/linux/drivers/media/video/cx18/cx18-dvb.c
@@ -23,6 +23,7 @@
 #include "cx18-version.h"
 #include "cx18-dvb.h"
 #include "cx18-io.h"
+#include "cx18-queue.h"
 #include "cx18-streams.h"
 #include "cx18-cards.h"
 #include "s5h1409.h"
diff --git a/linux/drivers/media/video/cx18/cx18-queue.c b/linux/drivers/media/video/cx18/cx18-queue.c
index 693a745b0..fa1ed7897 100644
--- a/linux/drivers/media/video/cx18/cx18-queue.c
+++ b/linux/drivers/media/video/cx18/cx18-queue.c
@@ -23,8 +23,8 @@
  */
 
 #include "cx18-driver.h"
-#include "cx18-streams.h"
 #include "cx18-queue.h"
+#include "cx18-streams.h"
 #include "cx18-scb.h"
 
 void cx18_buf_swap(struct cx18_buffer *buf)
diff --git a/linux/drivers/media/video/cx18/cx18-streams.c b/linux/drivers/media/video/cx18/cx18-streams.c
index 5cd6f1e51..142302ba5 100644
--- a/linux/drivers/media/video/cx18/cx18-streams.c
+++ b/linux/drivers/media/video/cx18/cx18-streams.c
@@ -124,6 +124,13 @@ static void cx18_stream_init(struct cx18 *cx, int type)
 	cx18_queue_init(&s->q_busy);
 	spin_lock_init(&s->q_full.lock);
 	cx18_queue_init(&s->q_full);
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 20)
+	INIT_WORK(&s->out_work_order, cx18_out_work_handler);
+#else
+	INIT_WORK(&s->out_work_order, cx18_out_work_handler,
+		  &s->out_work_order);
+#endif
 }
 
 static int cx18_prep_dev(struct cx18 *cx, int type)
@@ -477,92 +484,18 @@ void _cx18_stream_load_fw_queue(struct cx18_stream *s)
 		 && q == &s->q_busy);
 }
 
-static inline
-void free_out_work_order(struct cx18_out_work_order *order)
-{
-	atomic_set(&order->pending, 0);
-}
-
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 20)
 void cx18_out_work_handler(struct work_struct *work)
 {
-	struct cx18_out_work_order *order =
-			container_of(work, struct cx18_out_work_order, work);
 #else
 void cx18_out_work_handler(void *arg)
 {
-	struct cx18_out_work_order *order = arg;
+	struct work_struct *work = arg;
 #endif
-	struct cx18_stream *s = order->s;
-	struct cx18_buffer *buf = order->buf;
-
-	free_out_work_order(order);
-
-	if (buf == NULL)
-		_cx18_stream_load_fw_queue(s);
-	else
-		_cx18_stream_put_buf_fw(s, buf);
-}
+	struct cx18_stream *s =
+			 container_of(work, struct cx18_stream, out_work_order);
 
-static
-struct cx18_out_work_order *alloc_out_work_order(struct cx18 *cx)
-{
-	int i;
-	struct cx18_out_work_order *order = NULL;
-
-	for (i = 0; i < CX18_MAX_OUT_WORK_ORDERS; i++) {
-		/*
-		 * We need "pending" to be atomic to inspect & set its contents
-		 * 1. "pending" is only set to 1 here, but needs multiple access
-		 * protection
-		 * 2. work handler threads only clear "pending" and only
-		 * on one, particular work order at a time, per handler thread.
-		 */
-		if (atomic_add_unless(&cx->out_work_order[i].pending, 1, 1)) {
-			order = &cx->out_work_order[i];
-			break;
-		}
-	}
-	return order;
-}
-
-struct cx18_queue *cx18_stream_put_buf_fw(struct cx18_stream *s,
-					  struct cx18_buffer *buf)
-{
-	struct cx18 *cx = s->cx;
-	struct cx18_out_work_order *order;
-
-	order = alloc_out_work_order(cx);
-	if (order == NULL) {
-		CX18_DEBUG_WARN("No blank, outgoing-mailbox, deferred-work, "
-				"order forms available; sending buffer %u back "
-				"to the firmware immediately for stream %s\n",
-				buf->id, s->name);
-		return _cx18_stream_put_buf_fw(s, buf);
-	}
-	order->s = s;
-	order->buf = buf;
-	queue_work(cx->out_work_queue, &order->work);
-	return NULL;
-}
-
-void cx18_stream_load_fw_queue(struct cx18_stream *s)
-{
-	struct cx18 *cx = s->cx;
-	struct cx18_out_work_order *order;
-
-	order = alloc_out_work_order(cx);
-	if (order == NULL) {
-		CX18_DEBUG_WARN("No blank, outgoing-mailbox, deferred-work, "
-				"order forms available; filling the firmware "
-				"buffer queue immediately for stream %s\n",
-				s->name);
-		_cx18_stream_load_fw_queue(s);
-		return;
-	}
-	order->s = s;
-	order->buf = NULL; /* Indicates to load the fw queue */
-	queue_work(cx->out_work_queue, &order->work);
+	_cx18_stream_load_fw_queue(s);
 }
 
 int cx18_start_v4l2_encode_stream(struct cx18_stream *s)
diff --git a/linux/drivers/media/video/cx18/cx18-streams.h b/linux/drivers/media/video/cx18/cx18-streams.h
index c0801d51d..b51a014b5 100644
--- a/linux/drivers/media/video/cx18/cx18-streams.h
+++ b/linux/drivers/media/video/cx18/cx18-streams.h
@@ -29,9 +29,20 @@ int cx18_streams_register(struct cx18 *cx);
 void cx18_streams_cleanup(struct cx18 *cx, int unregister);
 
 /* Related to submission of buffers to firmware */
-void cx18_stream_load_fw_queue(struct cx18_stream *s);
-struct cx18_queue *cx18_stream_put_buf_fw(struct cx18_stream *s,
-					  struct cx18_buffer *buf);
+static inline void cx18_stream_load_fw_queue(struct cx18_stream *s)
+{
+	struct cx18 *cx = s->cx;
+	queue_work(cx->out_work_queue, &s->out_work_order);
+}
+
+static inline void cx18_stream_put_buf_fw(struct cx18_stream *s,
+					  struct cx18_buffer *buf)
+{
+	/* Put buf on q_free; the out work handler will move buf(s) to q_busy */
+	cx18_enqueue(s, buf, &s->q_free);
+	cx18_stream_load_fw_queue(s);
+}
+
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 20)
 void cx18_out_work_handler(struct work_struct *work);
 #else