| Message ID | aec7e5c30902130214k6a0fc8ck74b412f41fa63385@mail.gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi Magnus > Morimoto-san, can you check the attached patch? I've tested it on my > Migo-R board together with mplayer and it seems to work well here. I > don't think using mplayer triggers this error case though, so maybe we > should try some other application. I checked this patch with following case. Migo-R + ov772x + mplayer Migo-R + tw9910 + mplayer AP325 + ov772x + mplayer It works well on all cases. And I can agree with your opinion "so maybe we should try some other application." Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Morimoto-san On Mon, 16 Feb 2009, morimoto.kuninori@renesas.com wrote: > > Hi Magnus > > > Morimoto-san, can you check the attached patch? I've tested it on my > > Migo-R board together with mplayer and it seems to work well here. I > > don't think using mplayer triggers this error case though, so maybe we > > should try some other application. > > I checked this patch with following case. > > Migo-R + ov772x + mplayer > Migo-R + tw9910 + mplayer > AP325 + ov772x + mplayer > > It works well on all cases. So I can add your "Tested-by:"? > And I can agree with your opinion > "so maybe we should try some other application." You can try to trigger the race with the capture.c example. Reduce the "count" variable in mainloop() and run capture.c in a loop for a while... Try without this patch and then with this patch. But I think this is a correct fix, so, unless you say, it crashes without it and with it, I'll apply it. Thanks Guennadi > > Best regards > -- > Kuninori Morimoto > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 13 Feb 2009, Magnus Damm wrote: > Hi Matthieu, > > [CC Morimoto-san] > [Changed list to linux-media] > > On Tue, Jan 20, 2009 at 6:27 PM, Matthieu CASTET > <matthieu.castet@parrot.com> wrote: > > Magnus Damm a écrit : > >> On Mon, Jan 19, 2009 at 11:02 PM, Matthieu CASTET > >>> But we didn't do stop_capture, so as far I understand the controller is > >>> still writing data in memory. What prevent us to free the buffer we are > >>> writing. > >> > >> I have not looked into this in great detail, but isn't this handled by > >> the videobuf state? The videobuf has state VIDEOBUF_ACTIVE while it is > >> in use. I don't think such a buffer is freed. > > Well from my understanding form videobuf_queue_cancel [1], we call > > buf_release on all buffer. > > Yeah, you are correct. I guess waiting for the buffer before freeing > is the correct way to do this. I guess vivi doesn't have to do this > since it's not using DMA. > > Morimoto-san, can you check the attached patch? I've tested it on my > Migo-R board together with mplayer and it seems to work well here. I > don't think using mplayer triggers this error case though, so maybe we > should try some other application. Magnus, can you, please, submit it with an Sob, after Morimoto-san has tested it with capture.c? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Guennadi > > It works well on all cases. > So I can add your "Tested-by:"? Yes please. > You can try to trigger the race with the capture.c example. Reduce the > "count" variable in mainloop() and run capture.c in a loop for a while... > Try without this patch and then with this patch. But I think this is a > correct fix, so, unless you say, it crashes without it and with it, I'll > apply it. I tried with following command option # capture_example -d /dev/videoX -f -c 1000 I used -f option, I think you already know the reason =). I think -c 1000 is enough. # Because tw9910 can not use YUYV, # I fixed capture_example to use VUYU for tw9910. sh_mobile_ceu_camera didn't crashe with and without Magnus patch. MigoR + ov772x + capture_example MigoR + tw9910 + capture_example (VUYU fix) AP325 + ov772x + capture_example Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 19 Feb 2009, morimoto.kuninori@renesas.com wrote: > Dear Guennadi > > > > It works well on all cases. > > So I can add your "Tested-by:"? > > Yes please. > > > You can try to trigger the race with the capture.c example. Reduce the > > "count" variable in mainloop() and run capture.c in a loop for a while... > > Try without this patch and then with this patch. But I think this is a > > correct fix, so, unless you say, it crashes without it and with it, I'll > > apply it. > > I tried with following command option > > # capture_example -d /dev/videoX -f -c 1000 > > I used -f option, I think you already know the reason =). Yes, and I'm working on that. > I think -c 1000 is enough. No, sorry, this is not the test I meant. "-c" doesn't really stress the path we need. You really have to execute capture_example multiple times completely. The race we're trying to catch happens on STREAMOFF, and for that you have to run the example completely through. So, I would suggest something like for (( i=0; i<100; i++ )); do capture_example -d /dev/videoX -f -c 4; done Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Guennadi > No, sorry, this is not the test I meant. "-c" doesn't really stress the > path we need. You really have to execute capture_example multiple times > completely. The race we're trying to catch happens on STREAMOFF, and for > that you have to run the example completely through. So, I would suggest > something like > > for (( i=0; i<100; i++ )); do capture_example -d /dev/videoX -f -c 4; done wow !! sorry miss understanding. OK. I re-tried test with your script (300 times). sh_mobile_ceu_camera didn't crashe with and without Magnus patch. MigoR + ov772x + capture_example MigoR + tw9910 + capture_example (VUYU fix) AP325 + ov772x + capture_example Best regards -- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- 0001/drivers/media/video/sh_mobile_ceu_camera.c +++ work/drivers/media/video/sh_mobile_ceu_camera.c 2009-02-13 18:55:55.000000000 +0900 @@ -150,6 +150,7 @@ static void free_buffer(struct videobuf_ if (in_interrupt()) BUG(); + videobuf_waiton(&buf->vb, 0, 0); videobuf_dma_contig_free(vq, &buf->vb); dev_dbg(&icd->dev, "%s freed\n", __func__); buf->vb.state = VIDEOBUF_NEEDS_INIT;