From patchwork Tue Sep 11 22:03:32 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Oleszkiewicz X-Patchwork-Id: 1440001 Return-Path: X-Original-To: patchwork-davinci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from bear.ext.ti.com (bear.ext.ti.com [192.94.94.41]) by patchwork2.kernel.org (Postfix) with ESMTP id D811EDFAF3 for ; Tue, 11 Sep 2012 22:03:44 +0000 (UTC) Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by bear.ext.ti.com (8.13.7/8.13.7) with ESMTP id q8BM3hv1001053 for ; Tue, 11 Sep 2012 17:03:43 -0500 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id q8BM3hXX010179 for ; Tue, 11 Sep 2012 17:03:43 -0500 Received: from dlelxv24.itg.ti.com (172.17.1.199) by dfle73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.1.323.3; Tue, 11 Sep 2012 17:03:43 -0500 Received: from linux.omap.com (dlelxs01.itg.ti.com [157.170.227.31]) by dlelxv24.itg.ti.com (8.13.8/8.13.8) with ESMTP id q8BM3hj6031905 for ; Tue, 11 Sep 2012 17:03:43 -0500 Received: from linux.omap.com (localhost [127.0.0.1]) by linux.omap.com (Postfix) with ESMTP id 9998C8062A for ; Tue, 11 Sep 2012 17:03:43 -0500 (CDT) X-Original-To: davinci-linux-open-source@linux.davincidsp.com Delivered-To: davinci-linux-open-source@linux.davincidsp.com Received: from dflp52.itg.ti.com (dflp52.itg.ti.com [128.247.22.96]) by linux.omap.com (Postfix) with ESMTP id 144C480626 for ; Tue, 11 Sep 2012 17:03:36 -0500 (CDT) Received: from white.ext.ti.com (white.ext.ti.com [192.94.93.38]) by dflp52.itg.ti.com (8.13.7/8.13.8) with ESMTP id q8BM3ZcH002393 for ; Tue, 11 Sep 2012 17:03:35 -0500 (CDT) Received: from psmtp.com (na3sys009amx241.postini.com [74.125.149.125]) by white.ext.ti.com (8.13.7/8.13.7) with SMTP id q8BM3Yh5028530 for ; Tue, 11 Sep 2012 17:03:35 -0500 Received: from smtp.omnis.com ([216.239.128.26]) by na3sys009amx241.postini.com ([74.125.148.10]) with SMTP; Tue, 11 Sep 2012 17:03:35 CDT Received: from mail-hub-e.omnis.com (mail-hub-e.omnis.com [216.239.128.245]) by smtp.omnis.com (Postfix) with ESMTP id 6CBA7457B8A for ; Tue, 11 Sep 2012 15:03:34 -0700 (PDT) Received: from Goby (wsip-70-184-66-200.oc.oc.cox.net [70.184.66.200]) (Authenticated sender: doleszki@adsyscontrols.com) by mail-hub-e.omnis.com (Postfix) with ESMTPA id 0CAAF127239 for ; Tue, 11 Sep 2012 15:03:34 -0700 (PDT) From: David Oleszkiewicz To: Subject: vpif_capture.c:vpif_start_streaming() race condition Date: Tue, 11 Sep 2012 15:03:32 -0700 Organization: Adsys Controls Message-ID: <00e201cd9069$4899d7a0$d9cd86e0$@com> MIME-Version: 1.0 X-Mailer: Microsoft Office Outlook 12.0 Thread-Index: Ac2QaUgj6hALAW9NQICEEfc3V6qtAg== Content-Language: en-us X-Virus-Scanned: clamav-milter 0.97.4 at mail-hub-e.omnis.com X-Virus-Status: Clean X-pstn-levels: (S:79.86212/99.90000 CV:99.9000 FC:95.5390 LC:95.5390 R:95.9108 P:95.9108 M:97.0282 C:98.6951 ) X-pstn-dkim: 0 skipped:not-enabled X-pstn-settings: 2 (0.5000:0.5000) s cv gt3 gt2 gt1 r p m c X-pstn-addresses: from [82/3] X-BeenThere: davinci-linux-open-source@linux.davincidsp.com X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces+patchwork-davinci=patchwork.kernel.org@linux.davincidsp.com All, I think I found a race condition with enabling interrupts in the streamon call that I wanted some feedback on. I am actually using the DaVinci PSP 03.03 GA Release (Build 37) kernel but I notice the same issue in the current linux-davinci kernel that I just pulled down from git://gitorious.org/linux-davinci/linux-davinci.git. I'll refer to the latest linux-davinci code in for a discussion of the issue. At the end of the vpif_streamon there is a call to vb2_streamon() which then invokes vpif_start_streaming. At the end of vpif_start_streaming() the channel_first_int[][] variable is set to 1 as a flag the ISR that the next time it runs. However this line of code follows the code that enables the interrupt. I was getting a race where the interrupt was enabled in streamon(), then the ISR ran prior to the channel_first_int getting set. This ended up hanging my user code in a Capture_get call. The problem crops up randomly but frequently enough that it wasn't viable. I have just started looking into the vpif kernel code recently to find out why my video input was hanging so I am not familiar with much of the vpif cod, but it kind of made sense to reset the channel_first_int flag that the interrupt handler queries prior to enabling the interrupts. I moved that setting of the flag prior to setting the interrupts and it fixed my issue. I made this change on my DaVinci PSP 03.03 GA Release (Build 37), but look forward to testing with the freshest linux-davinci kernel. Unfortunately it'll take some time to get my custom drivers for my video chips ported over to that kernel. Here's the diff: if ((VPIF_CHANNEL0_VIDEO == ch->channel_id)) { channel0_intr_assert(); channel0_intr_enable(1); @@ -350,7 +352,6 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) channel1_intr_enable(1); enable_channel1(1); } - channel_first_int[VPIF_VIDEO_INDEX][ch->channel_id] = 1; return 0; } --- David Oleszkiewicz Adsys Controls, Inc. 949-436-4848 This e-mail (including any attachments) is for the intended recipient. If you are not an intended recipient or an authorized representative of an intended recipient, you are prohibited from using, copying or distributing the information in this e-mail or its attachments. If you have received this e-mail in error, please notify the sender immediately by return e-mail and delete all copies of this message and any attachments. diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c index 266025e..2e13d8b 100644 --- a/drivers/media/video/davinci/vpif_capture.c +++ b/drivers/media/video/davinci/vpif_capture.c @@ -337,8 +337,10 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) /** * Set interrupt for both the fields in VPIF Register enable channel in - * VPIF register + * VPIF register. First set the channel_first_int flag for the interrupt + * handler to pickup when it is asserted. */ + channel_first_int[VPIF_VIDEO_INDEX][ch->channel_id] = 1;