Hi David, On Wed, Sep 12, 2012 at 7:30 PM, Prabhakar Lad <[email protected]> wrote: > On Wednesday 12 September 2012 07:23 PM, Hans Verkuil wrote: >> 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] :-) >>
I have posted the patch fixing this issue http://patchwork.linuxtv.org/patch/14418/ Regards, --Prabhakar Lad > Oops copy paste error :), thanks for correcting. > > Thanks, > --Prabhakar > >> 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 _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
