Hi Niklas, thank you for addressing the problem, which I tried to solve in [1]! I have a few concerns about this patch... [1] https://lore.kernel.org/linux-renesas-soc/1592588777-100596-1-git-send-email-mrodin@xxxxxxxxxxxxxx/ On Mon, Nov 08, 2021 at 05:02:18PM +0100, Niklas Söderlund wrote: > Add a new V4L2_EVENT_XFER_ERROR event to signal if a unrecoverable error > happens during video transfer. > > The use-case that sparked this new event is to signal to the video > device driver that an error has happen on the CSI-2 bus from the CSI-2 > receiver subdevice. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > --- > Documentation/userspace-api/media/v4l/vidioc-dqevent.rst | 5 +++++ > Documentation/userspace-api/media/videodev2.h.rst.exceptions | 1 + > include/uapi/linux/videodev2.h | 1 + > 3 files changed, 7 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst > index 6eb40073c906deba..84984641c9351aa9 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst > @@ -182,6 +182,11 @@ call. > the regions changes. This event has a struct > :c:type:`v4l2_event_motion_det` > associated with it. > + * - ``V4L2_EVENT_XFER_ERROR`` > + - 7 > + - This event is triggered when an transfer error is detected while > + streaming. For example if a unrecoverable error is detected on a video > + bus in the pipeline. Could you please mention, what a userspace application and a video capture driver are supposed to do after reception of this event? (e.g. should video capture driver execute vb2_queue_error as done in the patch 3?) I have a few concerns depending on the kind of error which this event will report: 1. If an error is "unrecoverable" as you mentioned, then the overall video pipeline is in an unusable state, so sending this event to userspace probably does not make sense, since we can already signal IO error to userspace via vb2_queue_error. 2. If this event is also for reporting recoverable errors, e.g. which can be recovered by restarting the video pipeline, then probably it makes sense to explicitly mention, who shall restart the video pipeline via streamoff/streamon: application or video capture driver? > * - ``V4L2_EVENT_PRIVATE_START`` > - 0x08000000 > - Base event number for driver-private events. > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > index eb0b1cd37abd9381..7ed9884b879c888e 100644 > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > @@ -498,6 +498,7 @@ replace define V4L2_EVENT_CTRL event-type > replace define V4L2_EVENT_FRAME_SYNC event-type > replace define V4L2_EVENT_SOURCE_CHANGE event-type > replace define V4L2_EVENT_MOTION_DET event-type > +replace define V4L2_EVENT_XFER_ERROR event-type > replace define V4L2_EVENT_PRIVATE_START event-type > > replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index f118fe7a9f58d240..48d4738eb862418b 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -2373,6 +2373,7 @@ struct v4l2_streamparm { > #define V4L2_EVENT_FRAME_SYNC 4 > #define V4L2_EVENT_SOURCE_CHANGE 5 > #define V4L2_EVENT_MOTION_DET 6 > +#define V4L2_EVENT_XFER_ERROR 7 > #define V4L2_EVENT_PRIVATE_START 0x08000000 > > /* Payload for V4L2_EVENT_VSYNC */ > -- > 2.33.1 > -- Best Regards, Michael
