From patchwork Mon Oct 25 15:34:40 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Birtz X-Patchwork-Id: 267222 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id o9PFggTp017280 for ; Mon, 25 Oct 2010 15:42:42 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753317Ab0JYPmi (ORCPT ); Mon, 25 Oct 2010 11:42:38 -0400 Received: from ip-209-172-57-244.static.privatedns.com ([209.172.57.244]:36175 "EHLO g-noc.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267Ab0JYPmi (ORCPT ); Mon, 25 Oct 2010 11:42:38 -0400 X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter1.kernel.org [140.211.167.41]); Mon, 25 Oct 2010 15:42:42 +0000 (UTC) X-Greylist: delayed 476 seconds by postgrey-1.27 at vger.kernel.org; Mon, 25 Oct 2010 11:42:37 EDT Received: from [IPv6:::1] (localhost [127.0.0.1]) by g-noc.net (Postfix) with ESMTP id 90FC8847BA1 for ; Mon, 25 Oct 2010 11:32:20 -0400 (EDT) Message-ID: <4CC5A390.9010800@usherbrooke.ca> Date: Mon, 25 Oct 2010 11:34:40 -0400 From: Laurent Birtz User-Agent: Thunderbird 2.0.0.24 (X11/20100411) MIME-Version: 1.0 To: linux-media@vger.kernel.org Subject: [PATCH] bttv driver memory corruption Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org diff -r ee9826bc7106 linux/drivers/media/video/bt8xx/bttv-driver.c --- a/linux/drivers/media/video/bt8xx/bttv-driver.c Thu Apr 29 23:31:06 2010 -0300 +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c Tue Aug 03 12:22:20 2010 -0400 @@ -3999,7 +3999,6 @@ if (NULL == wakeup) return; - spin_lock(&btv->s_lock); btv->curr.top_irq = 0; btv->curr.top = NULL; bttv_risc_hook(btv, RISC_SLOT_O_FIELD, NULL, 0); @@ -4008,7 +4007,6 @@ wakeup->vb.field_count = btv->field_count; wakeup->vb.state = VIDEOBUF_DONE; wake_up(&wakeup->vb.done); - spin_unlock(&btv->s_lock); } static inline int is_active(struct btcx_riscmem *risc, u32 rc) @@ -4025,21 +4023,9 @@ { struct bttv_buffer_set new; struct bttv_buffer_set old; - dma_addr_t rc; - - spin_lock(&btv->s_lock); /* new buffer set */ bttv_irq_next_video(btv, &new); - rc = btread(BT848_RISC_COUNT); - if ((btv->curr.top && is_active(&btv->curr.top->top, rc)) || - (btv->curr.bottom && is_active(&btv->curr.bottom->bottom, rc))) { - btv->framedrop++; - if (debug_latency) - bttv_irq_debug_low_latency(btv, rc); - spin_unlock(&btv->s_lock); - return; - } /* switch over */ old = btv->curr; @@ -4056,7 +4042,6 @@ /* wake up finished buffers */ bttv_irq_wakeup_video(btv, &old, &new, VIDEOBUF_DONE); - spin_unlock(&btv->s_lock); } static void @@ -4064,24 +4049,11 @@ { struct bttv_buffer *new = NULL; struct bttv_buffer *old; - u32 rc; - - spin_lock(&btv->s_lock); if (!list_empty(&btv->vcapture)) new = list_entry(btv->vcapture.next, struct bttv_buffer, vb.queue); old = btv->cvbi; - rc = btread(BT848_RISC_COUNT); - if (NULL != old && (is_active(&old->top, rc) || - is_active(&old->bottom, rc))) { - btv->framedrop++; - if (debug_latency) - bttv_irq_debug_low_latency(btv, rc); - spin_unlock(&btv->s_lock); - return; - } - /* switch */ btv->cvbi = new; btv->loop_irq &= ~4; @@ -4089,7 +4061,118 @@ bttv_set_dma(btv, 0); bttv_irq_wakeup_vbi(btv, old, VIDEOBUF_DONE); - spin_unlock(&btv->s_lock); +} + +/* Call the appropriate buffer switch handlers. */ +static void +bttv_irq_handle_switch(struct bttv *btv, u32 flags) +{ + if (flags & 4) bttv_irq_switch_vbi(btv); + if (flags & 2) bttv_irq_wakeup_top(btv); + if (flags & 1) bttv_irq_switch_video(btv); +} + +/* Called when a RISC instruction causes an interrupt. */ +static void +bttv_irq_handle_risci(struct bttv *btv, u32 stat) +{ + unsigned long flags; + int loop_seen_flag = 0; + u32 closed_offset = btv->closed_gate ? 10 : 6; + u32 open_offset = btv->closed_gate ? 6 : 10; + u32 sync_type = 0; + dma_addr_t rc = btread(BT848_RISC_COUNT); + dma_addr_t expected_rc = btv->main.dma + (closed_offset << 2); + dma_addr_t hook_target = 0; + dma_addr_t landing_target = btv->main.dma + (open_offset << 2); + dma_addr_t closed_target = btv->main.dma; + + /* Check if the DMA controller is looping in the closed gate. */ + if (rc < expected_rc || rc > expected_rc + (2 << 2)) { + btv->framedrop++; + if (debug_latency) + bttv_irq_debug_low_latency(btv, rc); + return; + } + + spin_lock_irqsave(&btv->s_lock,flags); + + /* Handle the requested buffer switches, if any. */ + bttv_irq_handle_switch(btv, btv->cur_irq); + btv->cur_irq = 0; + + /* Loop until we find something to do. */ + while (1) { + /* Pass to the next slot. */ + btv->sub_slot = (btv->sub_slot + 1) % 5; + + /* We are no longer synchronized when we pass those slots. */ + if (btv->sub_slot == RISC_SLOT_O_VBI || + btv->sub_slot == RISC_SLOT_E_VBI) { + btv->field_sync = 0; + } + + /* We're reached the loop IRQ slot. This is used by the code + * outside the IRQ handler to tell us that we need to start + * capturing buffers. + */ + if (btv->sub_slot == RISC_SLOT_LOOP) { + + /* If we don't have anything to do, the DMA should shut + * down soon. Insert a dummy sync instruction for the + * odd field and set the hook target to the landing pad. + */ + if (loop_seen_flag) { + sync_type = BT848_FIFO_STATUS_VRE; + hook_target = btv->main.dma + (4 << 2); + break; + } + + loop_seen_flag = 1; + bttv_irq_handle_switch(btv, btv->loop_irq); + } + + /* We have a valid subprogram. */ + else if (btv->sub_addr[btv->sub_slot] != NULL) { + if (!btv->field_sync) { + btv->field_sync = 1; + sync_type = (btv->sub_slot == RISC_SLOT_O_VBI || + btv->sub_slot == RISC_SLOT_O_FIELD) + ? BT848_FIFO_STATUS_VRE + : BT848_FIFO_STATUS_VRO; + } + + hook_target = btv->sub_addr[btv->sub_slot]->dma; + btv->cur_irq = btv->sub_irq[btv->sub_slot]; + break; + } + } + + /* Write the hook target and the landing pad target, close the open + * gate, write the sync instruction if needed and open the closed gate. + */ + btv->main.cpu[3] = cpu_to_le32(hook_target); + btv->main.cpu[5] = cpu_to_le32(landing_target); + btv->main.cpu[open_offset + 1] = cpu_to_le32(landing_target); + + if (sync_type) { + btv->main.cpu[0] = cpu_to_le32(BT848_RISC_SYNC | + BT848_RISC_RESYNC | + sync_type); + } + + else { + closed_target = btv->main.dma + (2 << 2); + } + + btv->closed_gate = !btv->closed_gate; + + /* Flush memory before we open the gate. */ + wmb(); + btv->main.cpu[closed_offset + 1] = cpu_to_le32(closed_target); + wmb(); + + spin_unlock_irqrestore(&btv->s_lock,flags); } #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 19) @@ -4153,14 +4236,9 @@ wake_up(&btv->i2c_queue); } - if ((astat & BT848_INT_RISCI) && (stat & (4<<28))) - bttv_irq_switch_vbi(btv); - - if ((astat & BT848_INT_RISCI) && (stat & (2<<28))) - bttv_irq_wakeup_top(btv); - - if ((astat & BT848_INT_RISCI) && (stat & (1<<28))) - bttv_irq_switch_video(btv); + if (astat & BT848_INT_RISCI) { + bttv_irq_handle_risci(btv, stat); + } if ((astat & BT848_INT_HLOCK) && btv->opt_automute) audio_mute(btv, btv->mute); /* trigger automute */ diff -r ee9826bc7106 linux/drivers/media/video/bt8xx/bttv-risc.c --- a/linux/drivers/media/video/bt8xx/bttv-risc.c Thu Apr 29 23:31:06 2010 -0300 +++ b/linux/drivers/media/video/bt8xx/bttv-risc.c Tue Aug 03 12:22:20 2010 -0400 @@ -463,7 +463,6 @@ void bttv_set_dma(struct bttv *btv, int override) { - unsigned long cmd; int capctl; btv->cap_ctl = 0; @@ -484,23 +483,19 @@ btv->cvbi ? (unsigned long long)btv->cvbi->bottom.dma : 0, btv->curr.bottom ? (unsigned long long)btv->curr.bottom->bottom.dma : 0); - cmd = BT848_RISC_JUMP; - if (btv->loop_irq) { - cmd |= BT848_RISC_IRQ; - cmd |= (btv->loop_irq & 0x0f) << 16; - cmd |= (~btv->loop_irq & 0x0f) << 20; - } if (btv->curr.frame_irq || btv->loop_irq || btv->cvbi) { mod_timer(&btv->timeout, jiffies+BTTV_TIMEOUT); } else { del_timer(&btv->timeout); } - btv->main.cpu[RISC_SLOT_LOOP] = cpu_to_le32(cmd); btaor(capctl, ~0x0f, BT848_CAP_CTL); if (capctl) { if (btv->dma_on) return; + btv->cur_irq = 0; + btv->sub_slot = RISC_SLOT_LOOP; + btv->field_sync = 0; btwrite(btv->main.dma, BT848_RISC_STRT_ADD); btor(3, BT848_GPIO_DMA_CTL); btv->dma_on = 1; @@ -523,59 +518,76 @@ dprintk(KERN_DEBUG "bttv%d: risc main @ %08Lx\n", btv->c.nr,(unsigned long long)btv->main.dma); + /* Main RISC program for the DMA controller. Note that the DMA + * controller keeps executing instructions even if we are in interrupt + * context! + */ + + /* Slot[0:1] holds a SYNC instruction for either the odd or the even + * field. It is not used if the field synchronization has already been + * done previously. + */ btv->main.cpu[0] = cpu_to_le32(BT848_RISC_SYNC | BT848_RISC_RESYNC | BT848_FIFO_STATUS_VRE); btv->main.cpu[1] = cpu_to_le32(0); + + /* Slot[2:3] is the hook that holds a JUMP instruction to a subprogram + * that captures a field. If there is no subprogram, the hook jumps + * directly to the landing pad below. Otherwise, the subprogram jumps to + * the landing pad when it is finished. + */ btv->main.cpu[2] = cpu_to_le32(BT848_RISC_JUMP); btv->main.cpu[3] = cpu_to_le32(btv->main.dma + (4<<2)); - /* top field */ - btv->main.cpu[4] = cpu_to_le32(BT848_RISC_JUMP); + /* Slot[4:5] is the landing pad that holds a JUMP instruction to one of + * the two gates below. One gate is left open to "release" the DMA + * controller and one gate is kept closed to "hold" the DMA controller + * while we are modifying the RISC program. The landing pad points to + * the closed gate when we release the DMA controller. We set the IRQ + * flag to cause an interrupt when the closed gate is reached. + */ + btv->main.cpu[4] = cpu_to_le32(BT848_RISC_JUMP | BT848_RISC_IRQ); btv->main.cpu[5] = cpu_to_le32(btv->main.dma + (6<<2)); + + /* Slot[6:7] and slot [10:11] hold a jump instruction whose target is + * itself when the gate is closed, or either the sync or the hook jump + * instruction when the gate is open. We leave an empty area between the + * two instructions because the DMA controller sometimes reports bogus + * PC values. + */ btv->main.cpu[6] = cpu_to_le32(BT848_RISC_JUMP); - btv->main.cpu[7] = cpu_to_le32(btv->main.dma + (8<<2)); + btv->main.cpu[7] = cpu_to_le32(btv->main.dma + (6<<2)); - btv->main.cpu[8] = cpu_to_le32(BT848_RISC_SYNC | BT848_RISC_RESYNC | - BT848_FIFO_STATUS_VRO); - btv->main.cpu[9] = cpu_to_le32(0); + /* No man's land. */ + btv->main.cpu[8] = cpu_to_le32(BT848_RISC_JUMP); + btv->main.cpu[9] = 0; - /* bottom field */ btv->main.cpu[10] = cpu_to_le32(BT848_RISC_JUMP); - btv->main.cpu[11] = cpu_to_le32(btv->main.dma + (12<<2)); - btv->main.cpu[12] = cpu_to_le32(BT848_RISC_JUMP); - btv->main.cpu[13] = cpu_to_le32(btv->main.dma + (14<<2)); + btv->main.cpu[11] = cpu_to_le32(btv->main.dma + (10<<2)); - /* jump back to top field */ - btv->main.cpu[14] = cpu_to_le32(BT848_RISC_JUMP); - btv->main.cpu[15] = cpu_to_le32(btv->main.dma + (0<<2)); + /* Flush memory. */ + wmb(); return 0; } +/* Hook/clear a RISC subprogram in the specified slot. */ int bttv_risc_hook(struct bttv *btv, int slot, struct btcx_riscmem *risc, int irqflags) { - unsigned long cmd; - unsigned long next = btv->main.dma + ((slot+2) << 2); + btv->sub_addr[slot] = risc; + btv->sub_irq[slot] = irqflags; - if (NULL == risc) { - d2printk(KERN_DEBUG "bttv%d: risc=%p slot[%d]=NULL\n", - btv->c.nr,risc,slot); - btv->main.cpu[slot+1] = cpu_to_le32(next); - } else { - d2printk(KERN_DEBUG "bttv%d: risc=%p slot[%d]=%08Lx irq=%d\n", - btv->c.nr,risc,slot,(unsigned long long)risc->dma,irqflags); - cmd = BT848_RISC_JUMP; - if (irqflags) { - cmd |= BT848_RISC_IRQ; - cmd |= (irqflags & 0x0f) << 16; - cmd |= (~irqflags & 0x0f) << 20; - } - risc->jmp[0] = cpu_to_le32(cmd); - risc->jmp[1] = cpu_to_le32(next); - btv->main.cpu[slot+1] = cpu_to_le32(risc->dma); + if (risc) { + /* Jump to landing pad. */ + risc->jmp[0] = cpu_to_le32(BT848_RISC_JUMP); + risc->jmp[1] = cpu_to_le32(btv->main.dma + (4 << 2)); + + /* Flush memory. */ + wmb(); } + return 0; } diff -r ee9826bc7106 linux/drivers/media/video/bt8xx/bttvp.h --- a/linux/drivers/media/video/bt8xx/bttvp.h Thu Apr 29 23:31:06 2010 -0300 +++ b/linux/drivers/media/video/bt8xx/bttvp.h Tue Aug 03 12:22:20 2010 -0400 @@ -58,11 +58,11 @@ #define FORMAT_FLAGS_RAW 0x08 #define FORMAT_FLAGS_CrCb 0x10 -#define RISC_SLOT_O_VBI 4 -#define RISC_SLOT_O_FIELD 6 -#define RISC_SLOT_E_VBI 10 -#define RISC_SLOT_E_FIELD 12 -#define RISC_SLOT_LOOP 14 +#define RISC_SLOT_O_VBI 0 +#define RISC_SLOT_O_FIELD 1 +#define RISC_SLOT_E_VBI 2 +#define RISC_SLOT_E_FIELD 3 +#define RISC_SLOT_LOOP 4 #define RESOURCE_OVERLAY 1 #define RESOURCE_VIDEO_STREAM 2 @@ -428,6 +428,24 @@ struct bttv_buffer *cvbi; /* active vbi buffer */ int loop_irq; int new_input; + int closed_gate; /* ID (0 or 1) of the gate in the + main RISC program that is + currently closed. */ + struct btcx_riscmem *sub_addr[4]; /* Address of the capture RISC + subprograms. */ + int sub_irq[4]; /* IRQ flags of the capture + RISC subprograms. */ + int cur_irq; /* IRQ flags that have been set for + the current slot. */ + int sub_slot; /* Current subprogram being + executed. + 0: VBI top. + 1: Video top. + 2: VBI bottom. + 3: Video bottom. + 4: IRQ loop scheduling. */ + int field_sync; /* True if the current field + has been synchronized. */ unsigned long cap_ctl; unsigned long dma_on;