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.
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
