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] :-)
>
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
