On 11/05/2018 02:18 PM, Hans Verkuil wrote:
> On 11/05/2018 01:21 PM, Dave Stevenson wrote:
>> Hi All
>>
>> I'm testing with 4.19 and finding that testEvents in v4l2-compliance
>> is failing with ""failed to find event for control '%s' type %u", ie
>> it hasn't got the event for the inital values. This is with the
>> various BCM2835 drivers that I'm involved with.
>>
>> Having looked at the v4l2-core history I tried reverting "ad608fb
>> media: v4l: event: Prevent freeing event subscriptions while
>> accessed". The test passes again.
>>
>> Enabling all logging, and adding a couple of logging messages at the
>> beginning and end of v4l2_ctrl_add_event and __v4l2_event_queue_fh
>> error path, I get the following logs:
>>
>> [ 90.629999] v4l2_ctrl_add_event: ctrl a2b86fa8 "User Controls" type
>> 6, flags 0001
>> [ 90.630002] video0: VIDIOC_SUBSCRIBE_EVENT: type=0x3, id=0x980001, flags=0x1
>> [ 91.630166] videodev: v4l2_poll: video0: poll: 00000000
>> [ 91.630311] videodev: v4l2_poll: video0: poll: 00000000
>> [ 91.630325] video0: VIDIOC_UNSUBSCRIBE_EVENT: type=0x3,
>> id=0x980001, flags=0x1
>> [ 91.630396] v4l2_ctrl_add_event: ctrl 8f6fcc61 "Brightness" type 1,
>> flags 0001
>> [ 91.630403] __v4l2_event_queue_fh: Not subscribed to event type 3 id 0x980001
>> [ 91.630407] v4l2_ctrl_add_event: ctrl 8f6fcc61 "Brightness" type 1
>> - initial values queued
>> [ 91.630409] video0: VIDIOC_SUBSCRIBE_EVENT: type=0x3, id=0x980900, flags=0x1
>> [ 92.630513] videodev: v4l2_poll: video0: poll: 00000000
>> [ 92.630660] videodev: v4l2_poll: video0: poll: 00000000
>> [ 92.630729] videodev: v4l2_release: video0: release
>>
>> So __v4l2_event_queue_fh is dropping the event as we aren't subscribed
>> at the point that the initial values are queued.
>>
>> Sorry, I don't have any other devices that support subscribing for
>> events to hand (uvcvideo passes the test as it reports unsupported). I
>> don't have a media tree build immediately available either, but I
>> can't see anything related to this in the recent history. I can go
>> down that route if needed.
>> v4l-utils repo was synced today - head at "f9881444e8 cec-compliance:
>> wake-up on Active Source is warn for <2.0"
>>
>> Could someone test on other hardware to confirm that it's not the
>> drivers I'm using? I'm fairly certain it isn't as that patch does call
>> sev->ops->add(sev, elems); before list_add(&sev->list,
>> &fh->subscribed), and that is guaranteed to fail if sev->ops->add
>> immediately queues an event.
>
> Just to pitch in, I got similar issues when I tried to apply that commit
> to our Cisco code base. I've been away for a week and had no time to look
> into the cause, but it really appears that that commit was bad.
>
> Sakari, can you take a look at this?
Note: running v4l2-compliance with vivid (modprobe vivid no_error_inj=1) will
fail with this patch:
Control ioctls (Input 0):
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
fail: v4l2-test-controls.cpp(823): failed to find event for control 'Brightness'
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 15 Private Controls: 39
We really need regular vivid tests to find these regressions :-(
Regards,
Hans
