Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5e8c50cdc031bffd96b19929508f034d1263c8b7.camel@ndufresne.ca>
Subject: Re: [PATCH 1/3] media: videobuf2: Add a transfer error event
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Michael Rodin <mrodin@de.adit-jv.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Niklas =?ISO-8859-1?Q?S=F6derlund?= 
        <niklas.soderlund@ragnatech.se>, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Cc:     michael@rodin.online, erosca@de.adit-jv.com,
        Niklas =?ISO-8859-1?Q?S=F6derlund?= 
        <niklas.soderlund+renesas@ragnatech.se>
Date:   Mon, 04 Jul 2022 11:59:58 -0400
In-Reply-To: <20220628180024.451258-2-mrodin@de.adit-jv.com>
References: <YqEO3/KekkZhVjW+@oden.dyn.berto.se>
         <20220628180024.451258-1-mrodin@de.adit-jv.com>
         <20220628180024.451258-2-mrodin@de.adit-jv.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 (3.44.2-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

Hi Micheal,

thanks for your work, I have some questions below ...

Le mardi 28 juin 2022 =C3=A0 20:00 +0200, Michael Rodin a =C3=A9crit=C2=A0:
> From: Niklas S=C3=B6derlund <niklas.soderlund+renesas@ragnatech.se>
>=20
> Add a new V4L2_EVENT_XFER_ERROR event to signal if an error happens durin=
g
> video transfer.
>=20
> 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.
>=20
> Signed-off-by: Niklas S=C3=B6derlund <niklas.soderlund+renesas@ragnatech.=
se>
> [mrodin@de.adit-jv.com: adapted information what to do if this new event =
is received]
> Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com>
> ---
>  .../userspace-api/media/v4l/vidioc-dqevent.rst         | 10 ++++++++++
>  .../userspace-api/media/videodev2.h.rst.exceptions     |  1 +
>  include/uapi/linux/videodev2.h                         |  1 +
>  3 files changed, 12 insertions(+)
>=20
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/D=
ocumentation/userspace-api/media/v4l/vidioc-dqevent.rst
> index 6eb40073c906..3cf0b4859784 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> @@ -182,6 +182,16 @@ call.
>  	the regions changes. This event has a struct
>  	:c:type:`v4l2_event_motion_det`
>  	associated with it.
> +    * - ``V4L2_EVENT_XFER_ERROR``

I'm not sure why this event is specific to XFER. Is there uses cases were a
future implementation would have both XFER and RECEIVER error ?

> +      - 7
> +      - This event is triggered when an transfer error is detected while
> +	streaming. For example if an error is detected on a video bus in
> +	the pipeline. If a driver receives this event from an upstream
> +	subdevice, it has to forward the event to userspace. The streaming
> +	application has to check if the transfer error is unrecoverable,
> +	i.e. no new buffers can be dequeued from the kernel after the
> +	expected time. If the error is unrecoverable, the streaming
> +	application should restart streaming if it wants to continue.

The process to determine if an error is recoverable or not isn't clear to m=
e. As
an application developer, I would not know what to do here. Recoverable err=
or
already have a designed mechanism, it consist of marking done a buffer with=
 the
flag V4L2_BUF_FLAG_ERROR. I would like to understand what the existing mech=
anism
needed to be replaced, and the placement should be documented.

Nicolas

>      * - ``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 9cbb7a0c354a..25bde61a1519 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -500,6 +500,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
> =20
>  replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev=
2.h
> index 5311ac4fde35..44db724d4541 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2385,6 +2385,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
> =20
>  /* Payload for V4L2_EVENT_VSYNC */