Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

V4L/DVB (9804): cx18: Avoid making firmware API calls with the queue lock held

cx18: Avoid making firmware API calls with the queue lock held. The source
of MPEG strem corruption when not holding the queue lock was found to be that
the MPEG buffer could be retrieved by the user app before it was sync'ed for
the host cpu. Incoming buffers are now sync'ed before being put on q_full and
releasing the queue lock. We can thus avoid the sometimes lengthy call to
the firmware for CPU_DE_SET_MDL while holding the queue lock, so we can get
better performance.

Signed-off-by: Andy Walls <awalls@radix.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

authored by

Andy Walls and committed by
Mauro Carvalho Chehab
abb096de 765f6f61

+29 -36
+9 -6
drivers/media/video/cx18/cx18-mailbox.c
··· 163 163 * it's filled in). 164 164 * 165 165 * cx18_queue_get buf() will detect the lost buffers 166 - * and put them back in rotation eventually. 166 + * and send them back to q_free for fw rotation eventually. 167 167 */ 168 168 if ((order->flags & CX18_F_EWO_MB_STALE_UPON_RECEIPT) && 169 169 !(id >= s->mdl_offset && ··· 174 174 break; 175 175 } 176 176 buf = cx18_queue_get_buf(s, id, mdl_ack->data_used); 177 + 177 178 CX18_DEBUG_HI_DMA("DMA DONE for %s (buffer %d)\n", s->name, id); 178 179 if (buf == NULL) { 179 180 CX18_WARN("Could not find buf %d for stream %s\n", 180 181 id, s->name); 182 + /* Put as many buffers as possible back into fw use */ 183 + cx18_stream_load_fw_queue(s); 181 184 continue; 182 185 } 183 186 184 - cx18_buf_sync_for_cpu(s, buf); 185 187 if (s->type == CX18_ENC_STREAM_TYPE_TS && s->dvb.enabled) { 186 188 CX18_DEBUG_HI_DMA("TS recv bytesused = %d\n", 187 189 buf->bytesused); 188 - 189 190 dvb_dmx_swfilter(&s->dvb.demux, buf->buf, 190 191 buf->bytesused); 191 - 192 + } 193 + /* Put as many buffers as possible back into fw use */ 194 + cx18_stream_load_fw_queue(s); 195 + /* Put back TS buffer, since it was removed from all queues */ 196 + if (s->type == CX18_ENC_STREAM_TYPE_TS) 192 197 cx18_stream_put_buf_fw(s, buf); 193 - } else 194 - set_bit(CX18_F_B_NEED_BUF_SWAP, &buf->b_flags); 195 198 } 196 199 wake_up(&cx->dma_waitq); 197 200 if (s->id != -1)
+6 -7
drivers/media/video/cx18/cx18-queue.c
··· 117 117 } 118 118 119 119 buf->bytesused = bytesused; 120 + /* Sync the buffer before we release the qlock */ 121 + cx18_buf_sync_for_cpu(s, buf); 120 122 if (s->type == CX18_ENC_STREAM_TYPE_TS) { 121 123 /* 122 - * TS doesn't use q_full, but for sweeping up lost 123 - * buffers, we want the TS to requeue the buffer just 124 - * before sending the MDL back to the firmware, so we 125 - * pull it off the list here. 124 + * TS doesn't use q_full. As we pull the buffer off of 125 + * the queue here, the caller will have to put it back. 126 126 */ 127 127 list_del_init(&buf->list); 128 128 } else { 129 + /* Move buffer from q_busy to q_full */ 129 130 list_move_tail(&buf->list, &s->q_full.list); 131 + set_bit(CX18_F_B_NEED_BUF_SWAP, &buf->b_flags); 130 132 s->q_full.bytesused += buf->bytesused; 131 133 atomic_inc(&s->q_full.buffers); 132 134 } ··· 137 135 ret = buf; 138 136 break; 139 137 } 140 - 141 - /* Put more buffers into the transfer rotation from q_free, if we can */ 142 - cx18_stream_load_fw_queue_nolock(s); 143 138 mutex_unlock(&s->qlock); 144 139 return ret; 145 140 }
+13 -22
drivers/media/video/cx18/cx18-streams.c
··· 419 419 return q; 420 420 } 421 421 422 - /* Must hold s->qlock when calling */ 423 - void cx18_stream_load_fw_queue_nolock(struct cx18_stream *s) 422 + void cx18_stream_load_fw_queue(struct cx18_stream *s) 424 423 { 424 + struct cx18_queue *q; 425 425 struct cx18_buffer *buf; 426 - struct cx18 *cx = s->cx; 427 426 428 - /* Move from q_free to q_busy notifying the firmware: 63 buf limit */ 429 - while (s->handle != CX18_INVALID_TASK_HANDLE && 430 - test_bit(CX18_F_S_STREAMING, &s->s_flags) && 431 - atomic_read(&s->q_busy.buffers) < 63 && 432 - !list_empty(&s->q_free.list)) { 427 + if (atomic_read(&s->q_free.buffers) == 0 || 428 + atomic_read(&s->q_busy.buffers) >= 63) 429 + return; 433 430 434 - /* Move from q_free to q_busy */ 435 - buf = list_entry(s->q_free.list.next, struct cx18_buffer, list); 436 - list_move_tail(&buf->list, &s->q_busy.list); 437 - buf->bytesused = buf->readpos = buf->b_flags = buf->skipped = 0; 438 - atomic_dec(&s->q_free.buffers); 439 - atomic_inc(&s->q_busy.buffers); 440 - 441 - /* Notify firmware */ 442 - cx18_buf_sync_for_device(s, buf); 443 - cx18_vapi(cx, CX18_CPU_DE_SET_MDL, 5, s->handle, 444 - (void __iomem *) &cx->scb->cpu_mdl[buf->id] - cx->enc_mem, 445 - 1, buf->id, s->buf_size); 446 - } 431 + /* Move from q_free to q_busy notifying the firmware, until the limit */ 432 + do { 433 + buf = cx18_dequeue(s, &s->q_free); 434 + if (buf == NULL) 435 + break; 436 + q = cx18_stream_put_buf_fw(s, buf); 437 + } while (atomic_read(&s->q_busy.buffers) < 63 && q == &s->q_busy); 447 438 } 448 439 449 440 int cx18_start_v4l2_encode_stream(struct cx18_stream *s) ··· 534 543 &cx->scb->cpu_mdl[buf->id].paddr); 535 544 cx18_writel(cx, s->buf_size, &cx->scb->cpu_mdl[buf->id].length); 536 545 } 537 - cx18_stream_load_fw_queue_nolock(s); 538 546 mutex_unlock(&s->qlock); 547 + cx18_stream_load_fw_queue(s); 539 548 540 549 /* begin_capture */ 541 550 if (cx18_vapi(cx, CX18_CPU_CAPTURE_START, 1, s->handle)) {
+1 -1
drivers/media/video/cx18/cx18-streams.h
··· 29 29 void cx18_streams_cleanup(struct cx18 *cx, int unregister); 30 30 31 31 /* Capture related */ 32 - void cx18_stream_load_fw_queue_nolock(struct cx18_stream *s); 32 + void cx18_stream_load_fw_queue(struct cx18_stream *s); 33 33 struct cx18_queue *cx18_stream_put_buf_fw(struct cx18_stream *s, 34 34 struct cx18_buffer *buf); 35 35 int cx18_start_v4l2_encode_stream(struct cx18_stream *s);