Hi Hans: Appreciate your comments on this patch. On Tue, 2020-01-07 at 15:10 +0100, Hans Verkuil wrote: > On 12/19/19 6:49 AM, Jungo Lin wrote: > > For Camera AR(Augmented Reality) application requires camera timestamps > > to be reported with CLOCK_BOOTTIME to sync timestamp with other sensor > > sources. > > > > The boottime timestamp is identical to monotonic timestamp, > > except it also includes any time that the system is suspended. > > > > Signed-off-by: Jungo Lin <jungo.lin@xxxxxxxxxxxx> > > --- > > Changes from v6: > > - No change. > > --- > > Documentation/media/uapi/v4l/buffer.rst | 11 ++++++++++- > > include/uapi/linux/videodev2.h | 2 ++ > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst > > index 9149b57728e5..f45bfce7fddd 100644 > > --- a/Documentation/media/uapi/v4l/buffer.rst > > +++ b/Documentation/media/uapi/v4l/buffer.rst > > @@ -662,13 +662,22 @@ Buffer Flags > > - 0x00002000 > > - The buffer timestamp has been taken from the ``CLOCK_MONOTONIC`` > > clock. To access the same clock outside V4L2, use > > - :c:func:`clock_gettime`. > > + :c:func:`clock_gettime` using clock IDs ``CLOCK_MONOTONIC``. > > IDs -> ID > Ok, fix in next version. > > * .. _`V4L2-BUF-FLAG-TIMESTAMP-COPY`: > > > > - ``V4L2_BUF_FLAG_TIMESTAMP_COPY`` > > - 0x00004000 > > - The CAPTURE buffer timestamp has been taken from the corresponding > > OUTPUT buffer. This flag applies only to mem2mem devices. > > + * .. _`V4L2_BUF_FLAG_TIMESTAMP_BOOTIME`: > > You mistyped BOOTTIME as BOOTIME in a lot of places. Please check. > Ok, fix this typo in next version. > > + > > + - ``V4L2_BUF_FLAG_TIMESTAMP_BOOTIME`` > > + - 0x00008000 > > + - The buffer timestamp has been taken from the ``CLOCK_BOOTTIME`` > > + clock. To access the same clock outside V4L2, use > > + :c:func:`clock_gettime` using clock IDs ``CLOCK_BOOTTIME``. > > IDs -> ID > Ditto. > > + Identical to CLOCK_MONOTONIC, except it also includes any time that > > + the system is suspended. > > * .. _`V4L2-BUF-FLAG-TSTAMP-SRC-MASK`: > > > > - ``V4L2_BUF_FLAG_TSTAMP_SRC_MASK`` > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > index 04481c717fee..74ef9472e702 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -1060,6 +1060,8 @@ static inline __u64 v4l2_timeval_to_ns(const struct timeval *tv) > > #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN 0x00000000 > > #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC 0x00002000 > > #define V4L2_BUF_FLAG_TIMESTAMP_COPY 0x00004000 > > +#define V4L2_BUF_FLAG_TIMESTAMP_BOOTIME 0x00008000 > > This should be 0x00006000. > > (flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) is a value that determines the timestamp > source, so these timestamp defines are values, not bitmasks. > > However, I don't like your approach. Whether to use MONOTONIC or BOOTTIME is really > a userspace decision, and locking a driver to one of these two options seems > wrong to me. > > Instead add new V4L2_BUF_FLAG_USE_BOOTTIME flag that userspace can set when queuing > the buffer and that indicates that instead of the MONOTONIC timestamp, it should return > the BOOTTIME timestamp. This requires a simple helper function that returns either > ktime_get_ns or ktime_get_boottime_ns based on the vb2_v4l2_buffer flags field. > > It's definitely more work (although it can be limited to drivers that use vb2), > but much more useful. > > Regards, > > Hans > Agree. We will add new V4L2_BUF_FLAG_USE_BOOTTIME flag (0x00006000.) to replace this V4L2_BUF_FLAG_TIMESTAMP_BOOTIME flag for better usage. > > + > > /* Timestamp sources. */ > > #define V4L2_BUF_FLAG_TSTAMP_SRC_MASK 0x00070000 > > #define V4L2_BUF_FLAG_TSTAMP_SRC_EOF 0x00000000 > > > Sincerely Jungo
