| Message ID | 20260107195537.2180637-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch. On Wed, Jan 07, 2026 at 08:55:37PM +0100, Barnabás Pőcze wrote: > When `poll()` returns multiple events for a given file descriptor, it is better > to service priority events first. So dispatch those first, and not last. > > For example, a v4l2 capture device might be a source of v4l2 events (e.g. frame s/v4l2/V4L2/g > start) as well as a source of dequeue-able buffers. In such cases, given the > appropriate scheduling, it is possible with the current event dispatch order > that dequeueing the buffer happens before processing the corresponding frame > start event. Such occurrence will most likely trip up any internal state machine. Hmmm... This is a tricky one. I think this patch is fine for now, but we may later need to make the order configurable per file descriptor. Despite the name, there's no universal rule that I'm aware of that indicates POLLPRI events always need to be processed first in all use cases. For V4L2 devices and our usage patterns I think this is fine. We don't use POLLPRI for any other device type at the moment, so I think we can just merge this and hope it won't come to bite us back. And when it will, we'll see how to address the issues. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > The above is suspected to contribute to [0], however, that is not confirmed. > > [0]: https://gitlab.freedesktop.org/camera/libcamera/-/issues/267 > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/base/event_dispatcher_poll.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp > index 52bfb34e2..006e223f8 100644 > --- a/src/libcamera/base/event_dispatcher_poll.cpp > +++ b/src/libcamera/base/event_dispatcher_poll.cpp > @@ -244,9 +244,9 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol > EventNotifier::Type type; > short events; > } events[] = { > + { EventNotifier::Exception, POLLPRI }, > { EventNotifier::Read, POLLIN }, > { EventNotifier::Write, POLLOUT }, > - { EventNotifier::Exception, POLLPRI }, > }; > > processingEvents_ = true;
Hi Barnabás, Thank you for this patch On Wed, 7 Jan 2026 at 19:55, Barnabás Pőcze <barnabas.pocze@ideasonboard.com> wrote: > > When `poll()` returns multiple events for a given file descriptor, it is better > to service priority events first. So dispatch those first, and not last. > > For example, a v4l2 capture device might be a source of v4l2 events (e.g. frame > start) as well as a source of dequeue-able buffers. In such cases, given the > appropriate scheduling, it is possible with the current event dispatch order > that dequeueing the buffer happens before processing the corresponding frame > start event. Such occurrence will most likely trip up any internal state machine. > > The above is suspected to contribute to [0], however, that is not confirmed. > > [0]: https://gitlab.freedesktop.org/camera/libcamera/-/issues/267 There is definitely an assumption in DelayedControls that the frame start v4l2 event occurs before the dequeue event. It should be quite easy to fix, but the behavior would be suboptimal in terms of setting sensor parameters. I think this fix should be applied regardless. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/base/event_dispatcher_poll.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp > index 52bfb34e2..006e223f8 100644 > --- a/src/libcamera/base/event_dispatcher_poll.cpp > +++ b/src/libcamera/base/event_dispatcher_poll.cpp > @@ -244,9 +244,9 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol > EventNotifier::Type type; > short events; > } events[] = { > + { EventNotifier::Exception, POLLPRI }, > { EventNotifier::Read, POLLIN }, > { EventNotifier::Write, POLLOUT }, > - { EventNotifier::Exception, POLLPRI }, > }; > > processingEvents_ = true; > -- > 2.52.0 >
diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp index 52bfb34e2..006e223f8 100644 --- a/src/libcamera/base/event_dispatcher_poll.cpp +++ b/src/libcamera/base/event_dispatcher_poll.cpp @@ -244,9 +244,9 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol EventNotifier::Type type; short events; } events[] = { + { EventNotifier::Exception, POLLPRI }, { EventNotifier::Read, POLLIN }, { EventNotifier::Write, POLLOUT }, - { EventNotifier::Exception, POLLPRI }, }; processingEvents_ = true;
When `poll()` returns multiple events for a given file descriptor, it is better to service priority events first. So dispatch those first, and not last. For example, a v4l2 capture device might be a source of v4l2 events (e.g. frame start) as well as a source of dequeue-able buffers. In such cases, given the appropriate scheduling, it is possible with the current event dispatch order that dequeueing the buffer happens before processing the corresponding frame start event. Such occurrence will most likely trip up any internal state machine. The above is suspected to contribute to [0], however, that is not confirmed. [0]: https://gitlab.freedesktop.org/camera/libcamera/-/issues/267 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/base/event_dispatcher_poll.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)