On Wed 12 September 2012 15:50:56 Prabhakar Lad wrote: > 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]).
That's [email protected] :-) Regards, Hans > > 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
