Hi David, Thanks for the findings :)
On Wednesday 12 September 2012 03:05 PM, Hans Verkuil wrote: > Hi David, > > On Wed 12 September 2012 00:03:32 David Oleszkiewicz wrote: >> 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. > > I agree with your analysis. The same problem is present in vpif_display.c as > well. > > You are very unlucky as well with your timings to actually hit this race > condition. > Can you submit a patch for this also making changes to display driver as pointed by Hans ? Note: The folder structure for video drivers is changed rebase the patch on this branch http://git.linuxtv.org/media_tree.git/shortlog/refs/heads/staging/for_v3.7 and make sure you CC the patches to media mailing list too ([email protected]). Regards, --Prabhakar > Regards, > > Hans > >> Here's the diff: >> >> 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; >> 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. >> >> >> >> _______________________________________________ >> Davinci-linux-open-source mailing list >> [email protected] >> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source >> > _______________________________________________ > Davinci-linux-open-source mailing list > [email protected] > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source > _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
