Hi Alexandre, On 04/19/2018 09:48 AM, Paul Kocialkowski wrote: > Hi, > > On Wed, 2018-04-18 at 02:06 +0000, Alexandre Courbot wrote: >> On Tue, Apr 17, 2018 at 8:41 PM Paul Kocialkowski < >> paul.kocialkowski@xxxxxxxxxxx> wrote: >> On Tue, 2018-04-17 at 06:17 +0000, Alexandre Courbot wrote: >>>> On Tue, Apr 17, 2018 at 3:12 PM Hans Verkuil <hverkuil@xxxxxxxxx> >>>> wrote: >>>> >>>>> On 04/17/2018 06:33 AM, Alexandre Courbot wrote: >>>>>> On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil <hverkuil@xs4all. >>>>>> nl> >>>>>> wrote: >>>>>> >>>>>>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> >>>>>>> Hi all, >>>>>>> This is a cleaned up version of the v10 series (never posted >>>>>>> to >>>>>>> the list since it was messy). >>>>>> >>>>>> Hi Hans, >>>>>> >>>>>> It took me a while to test and review this, but finally have >>>>>> been >>>>>> able >>>> >>>> to >>>>>> do it. >>>>>> >>>>>> First the result of the test: I have tried porting my dummy >>>>>> vim2m >>>>>> test >>>>>> program >>>>>> (https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a >>>>>> 42 >>>>>> for >>>>>> reference), >>>>>> and am getting a hang when trying to queue the second OUTPUT >>>>>> buffer >>>> >>>> (right >>>>>> after >>>>>> queuing the first request). If I move the calls the >>>>>> VIDIOC_STREAMON >>>> >>>> after >>>>>> the >>>>>> requests are queued, the hang seems to happen at that moment. >>>>>> Probably a >>>>>> deadlock, haven't looked in detail yet. >>>>>> >>>>>> I have a few other comments, will follow up per-patch. >>>>>> >>>>> >>>>> I had a similar/same (?) report about this from Paul: >>>>> https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg1291 >>>>> 77.h >>>>> tml >>>> >>>> I saw this and tried to move the call to STREAMON to after the >>>> requests are queued in my example program, but it then hanged >>>> there. >>>> So there is probably something more intricate taking place. >>> I figured out the issue (but forgot to report back to the list): >>> Hans' >>> version of the request API doesn't set the POLLIN bit but POLLPRI >>> instead, so you need to select for expect_fds instead of read_fds in >>> the >>> select call. That's pretty much all there is to it. >> >> I am not using select() but poll() in my test program (see the gist >> link >> above) and have set POLLPRI as the event to poll for. I may be missing >> something but this looks correct to me? > > You're right, I overlooked your email and assumed you were hitting the > same issue that I had at first. > > Anyway, I also had a similar issue when calling the STREAMON ioctl > *before* enqueuing the request. What happens here is that > v4l2_m2m_streamon (called from the ioctl) will also try to schedule a > device run (v4l2_m2m_try_schedule). When the ioctl is called and the > request was not queued yet, the lack of buffers will delay the streamon > call. Then, when the request is later submitted, vb2_streamon is called > with the queue directly, and there is no m2m-specific provision there, > so no device run is scheduled and nothing happens. If the STREAMON ioctl > happens after queuing the request, then things should be fine (but > that's definitely not what we want userspace to be doing) since > the vb2_streamon call from the ioctl handler will take effect > and v4l2_m2m_try_schedule will be called. > > The way that I have solved this with the Sunxi-Cedrus driver is to add a > req_complete callback function pointer (holding a call > to v4l2_m2m_try_schedule) in media_device_ops and call it (if available) > from media_request_ioctl_queue. I initially put this in req_queue > directly, but since it is wrapped by the request queue mutex lock and > provided that device_run needs to access the request queue, we need an > extra op called out of this lock, before completing the ioctl handler. > > I will be sending v2 of my driver with preliminary patches to fix this > (perhaps I should also fix vim2m that way along the way). Can you verify that this indeed solves your problem? Or is there still something else going on? Regards, Hans
