'struct timespec' works fine here, but we try to migrate away from it in favor of ktime_t or timespec64. In this case, using ktime_t produces the simplest code.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/media/usb/uvc/uvc_video.c | 11 ++++------- drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index fb86d6af398d..d6bee37cd1b8 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -725,7 +725,7 @@ static void uvc_video_stats_decode(struct uvc_streaming *stream,
if (stream->stats.stream.nb_frames == 0 && stream->stats.frame.nb_packets == 0) - ktime_get_ts(&stream->stats.stream.start_ts); + stream->stats.stream.start_ts = ktime_get();
switch (data[1] & (UVC_STREAM_PTS | UVC_STREAM_SCR)) { case UVC_STREAM_PTS | UVC_STREAM_SCR: @@ -865,16 +865,13 @@ size_t uvc_video_stats_dump(struct uvc_streaming *stream, char *buf, { unsigned int scr_sof_freq; unsigned int duration; - struct timespec ts; size_t count = 0;
- ts = timespec_sub(stream->stats.stream.stop_ts, - stream->stats.stream.start_ts); - /* Compute the SCR.SOF frequency estimate. At the nominal 1kHz SOF * frequency this will not overflow before more than 1h. */ - duration = ts.tv_sec * 1000 + ts.tv_nsec / 1000000; + duration = ktime_ms_delta(stream->stats.stream.stop_ts, + stream->stats.stream.start_ts); if (duration != 0) scr_sof_freq = stream->stats.stream.scr_sof_count * 1000 / duration; @@ -915,7 +912,7 @@ static void uvc_video_stats_start(struct uvc_streaming *stream)
static void uvc_video_stats_stop(struct uvc_streaming *stream) { - ktime_get_ts(&stream->stats.stream.stop_ts); + stream->stats.stream.stop_ts = ktime_get(); }
/* ------------------------------------------------------------------------ diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 05398784d1c8..a2c190937067 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -452,8 +452,8 @@ struct uvc_stats_frame { };
struct uvc_stats_stream { - struct timespec start_ts; /* Stream start timestamp */ - struct timespec stop_ts; /* Stream stop timestamp */ + ktime_t start_ts; /* Stream start timestamp */ + ktime_t stop_ts; /* Stream stop timestamp */
unsigned int nb_frames; /* Number of frames */
uvc_video_get_ts() returns a 'struct timespec', but all its users really want a nanoseconds variable anyway.
Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get and ktime_get_real simplifies the code noticeably, while keeping the resulting numbers unchanged.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/media/usb/uvc/uvc_video.c | 37 ++++++++++++------------------------- drivers/media/usb/uvc/uvcvideo.h | 2 +- 2 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index d6bee37cd1b8..f7a919490b2b 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -369,12 +369,12 @@ static int uvc_commit_video(struct uvc_streaming *stream, * Clocks and timestamps */
-static inline void uvc_video_get_ts(struct timespec *ts) +static inline ktime_t uvc_video_get_time(void) { if (uvc_clock_param == CLOCK_MONOTONIC) - ktime_get_ts(ts); + return ktime_get(); else - ktime_get_real_ts(ts); + return ktime_get_real(); }
static void @@ -386,7 +386,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, bool has_pts = false; bool has_scr = false; unsigned long flags; - struct timespec ts; + ktime_t time; u16 host_sof; u16 dev_sof;
@@ -436,7 +436,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, stream->clock.last_sof = dev_sof;
host_sof = usb_get_current_frame_number(stream->dev->udev); - uvc_video_get_ts(&ts); + time = uvc_video_get_time();
/* The UVC specification allows device implementations that can't obtain * the USB frame number to keep their own frame counters as long as they @@ -473,7 +473,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, sample->dev_stc = get_unaligned_le32(&data[header_size - 6]); sample->dev_sof = dev_sof; sample->host_sof = host_sof; - sample->host_ts = ts; + sample->host_time = time;
/* Update the sliding window head and count. */ stream->clock.head = (stream->clock.head + 1) % stream->clock.size; @@ -613,14 +613,12 @@ void uvc_video_clock_update(struct uvc_streaming *stream, struct uvc_clock_sample *first; struct uvc_clock_sample *last; unsigned long flags; - struct timespec ts; + u64 timestamp; u32 delta_stc; u32 y1, y2; u32 x1, x2; u32 mean; u32 sof; - u32 div; - u32 rem; u64 y;
if (!uvc_hw_timestamps_param) @@ -667,9 +665,8 @@ void uvc_video_clock_update(struct uvc_streaming *stream, if (x1 == x2) goto done;
- ts = timespec_sub(last->host_ts, first->host_ts); y1 = NSEC_PER_SEC; - y2 = (ts.tv_sec + 1) * NSEC_PER_SEC + ts.tv_nsec; + y2 = (u32)ktime_to_ns(ktime_sub(last->host_time, first->host_time)) + y1;
/* Interpolated and host SOF timestamps can wrap around at slightly * different times. Handle this by adding or removing 2048 to or from @@ -686,24 +683,18 @@ void uvc_video_clock_update(struct uvc_streaming *stream, - (u64)y2 * (u64)x1; y = div_u64(y, x2 - x1);
- div = div_u64_rem(y, NSEC_PER_SEC, &rem); - ts.tv_sec = first->host_ts.tv_sec - 1 + div; - ts.tv_nsec = first->host_ts.tv_nsec + rem; - if (ts.tv_nsec >= NSEC_PER_SEC) { - ts.tv_sec++; - ts.tv_nsec -= NSEC_PER_SEC; - } + timestamp = ktime_to_ns(first->host_time) + y - y1;
uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu " "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n", stream->dev->name, sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536), - y, timespec_to_ns(&ts), vbuf->vb2_buf.timestamp, + y, timestamp, vbuf->vb2_buf.timestamp, x1, first->host_sof, first->dev_sof, x2, last->host_sof, last->dev_sof, y1, y2);
/* Update the V4L2 buffer. */ - vbuf->vb2_buf.timestamp = timespec_to_ns(&ts); + vbuf->vb2_buf.timestamp = timestamp;
done: spin_unlock_irqrestore(&clock->lock, flags); @@ -1007,8 +998,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, * when the EOF bit is set to force synchronisation on the next packet. */ if (buf->state != UVC_BUF_STATE_ACTIVE) { - struct timespec ts; - if (fid == stream->last_fid) { uvc_trace(UVC_TRACE_FRAME, "Dropping payload (out of " "sync).\n"); @@ -1018,11 +1007,9 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, return -ENODATA; }
- uvc_video_get_ts(&ts); - buf->buf.field = V4L2_FIELD_NONE; buf->buf.sequence = stream->sequence; - buf->buf.vb2_buf.timestamp = timespec_to_ns(&ts); + buf->buf.vb2_buf.timestamp = uvc_video_get_time();
/* TODO: Handle PTS and SCR. */ buf->state = UVC_BUF_STATE_ACTIVE; diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index a2c190937067..d7797dfb6468 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -536,8 +536,8 @@ struct uvc_streaming { struct uvc_clock_sample { u32 dev_stc; u16 dev_sof; - struct timespec host_ts; u16 host_sof; + ktime_t host_time; } *samples;
unsigned int head;
Hi Arnd,
Thank you for the patch.
On Monday, 27 November 2017 15:19:54 EET Arnd Bergmann wrote:
uvc_video_get_ts() returns a 'struct timespec', but all its users really want a nanoseconds variable anyway.
Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get and ktime_get_real simplifies the code noticeably, while keeping the resulting numbers unchanged.
Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/media/usb/uvc/uvc_video.c | 37 ++++++++++++----------------------- drivers/media/usb/uvc/uvcvideo.h | 2 +- 2 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index d6bee37cd1b8..f7a919490b2b 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -369,12 +369,12 @@ static int uvc_commit_video(struct uvc_streaming *stream, * Clocks and timestamps */
-static inline void uvc_video_get_ts(struct timespec *ts) +static inline ktime_t uvc_video_get_time(void) { if (uvc_clock_param == CLOCK_MONOTONIC)
ktime_get_ts(ts);
return ktime_get();
ktime_get_real_ts(ts);
return ktime_get_real();}
static void @@ -386,7 +386,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, bool has_pts = false; bool has_scr = false; unsigned long flags;
- struct timespec ts;
- ktime_t time; u16 host_sof; u16 dev_sof;
@@ -436,7 +436,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, stream->clock.last_sof = dev_sof;
host_sof = usb_get_current_frame_number(stream->dev->udev);
- uvc_video_get_ts(&ts);
time = uvc_video_get_time();
/* The UVC specification allows device implementations that can't obtain
- the USB frame number to keep their own frame counters as long as they
@@ -473,7 +473,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, sample->dev_stc = get_unaligned_le32(&data[header_size - 6]); sample->dev_sof = dev_sof; sample->host_sof = host_sof;
- sample->host_ts = ts;
sample->host_time = time;
/* Update the sliding window head and count. */ stream->clock.head = (stream->clock.head + 1) % stream->clock.size;
@@ -613,14 +613,12 @@ void uvc_video_clock_update(struct uvc_streaming *stream, struct uvc_clock_sample *first; struct uvc_clock_sample *last; unsigned long flags;
- struct timespec ts;
- u64 timestamp; u32 delta_stc; u32 y1, y2; u32 x1, x2; u32 mean; u32 sof;
u32 div;
u32 rem; u64 y;
if (!uvc_hw_timestamps_param)
@@ -667,9 +665,8 @@ void uvc_video_clock_update(struct uvc_streaming *stream, if (x1 == x2) goto done;
- ts = timespec_sub(last->host_ts, first->host_ts); y1 = NSEC_PER_SEC;
- y2 = (ts.tv_sec + 1) * NSEC_PER_SEC + ts.tv_nsec;
y2 = (u32)ktime_to_ns(ktime_sub(last->host_time, first->host_time)) + y1;
/* Interpolated and host SOF timestamps can wrap around at slightly
- different times. Handle this by adding or removing 2048 to or from
@@ -686,24 +683,18 @@ void uvc_video_clock_update(struct uvc_streaming *stream, - (u64)y2 * (u64)x1; y = div_u64(y, x2 - x1);
- div = div_u64_rem(y, NSEC_PER_SEC, &rem);
- ts.tv_sec = first->host_ts.tv_sec - 1 + div;
- ts.tv_nsec = first->host_ts.tv_nsec + rem;
- if (ts.tv_nsec >= NSEC_PER_SEC) {
ts.tv_sec++;ts.tv_nsec -= NSEC_PER_SEC;- }
- timestamp = ktime_to_ns(first->host_time) + y - y1;
It took me a few minutes to see that the -1 and -y1 were equivalent. And then more time to re-read the code and the comments to understand what I had done. I'm impressed that you haven't blindly replaced the +1s and -1s by +NSEC_PER_SEC and -NSEC_PER_SEC, but used +y1 and -y1 which I think improves readability.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
with the highest honors :-)
Should I merge this through my tree ?
uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu " "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n", stream->dev->name, sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536),
y, timespec_to_ns(&ts), vbuf->vb2_buf.timestamp,
y, timestamp, vbuf->vb2_buf.timestamp,x1, first->host_sof, first->dev_sof, x2, last->host_sof, last->dev_sof, y1, y2);
/* Update the V4L2 buffer. */
- vbuf->vb2_buf.timestamp = timespec_to_ns(&ts);
- vbuf->vb2_buf.timestamp = timestamp;
done: spin_unlock_irqrestore(&clock->lock, flags); @@ -1007,8 +998,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, * when the EOF bit is set to force synchronisation on the next packet. */ if (buf->state != UVC_BUF_STATE_ACTIVE) {
struct timespec ts;- if (fid == stream->last_fid) { uvc_trace(UVC_TRACE_FRAME, "Dropping payload (out of " "sync).\n");
@@ -1018,11 +1007,9 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, return -ENODATA; }
uvc_video_get_ts(&ts);- buf->buf.field = V4L2_FIELD_NONE; buf->buf.sequence = stream->sequence;
buf->buf.vb2_buf.timestamp = timespec_to_ns(&ts);
buf->buf.vb2_buf.timestamp = uvc_video_get_time();/* TODO: Handle PTS and SCR. */ buf->state = UVC_BUF_STATE_ACTIVE;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index a2c190937067..d7797dfb6468 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -536,8 +536,8 @@ struct uvc_streaming { struct uvc_clock_sample { u32 dev_stc; u16 dev_sof;
struct timespec host_ts; u16 host_sof;
ktime_t host_time;} *samples;
unsigned int head;
Hi Arnd,
On Tuesday, 5 December 2017 02:37:27 EET Laurent Pinchart wrote:
On Monday, 27 November 2017 15:19:54 EET Arnd Bergmann wrote:
uvc_video_get_ts() returns a 'struct timespec', but all its users really want a nanoseconds variable anyway.
Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get and ktime_get_real simplifies the code noticeably, while keeping the resulting numbers unchanged.
Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/media/usb/uvc/uvc_video.c | 37 ++++++++++++--------------------- drivers/media/usb/uvc/uvcvideo.h | 2 +- 2 files changed, 13 insertions(+), 26 deletions(-)
[snip]
- struct timespec ts;
- u64 timestamp;
[snip]
uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu " "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n", stream->dev->name, sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536),
y, timespec_to_ns(&ts), vbuf->vb2_buf.timestamp,
y, timestamp, vbuf->vb2_buf.timestamp,
As you've done lots of work moving code away from timespec I figured out I would ask, what is the preferred way to print a ktime in secs.nsecs format ? Should I use ktime_to_timespec and print ts.tv_sec and ts.tv_nsec, or is there a better way ?
On Tue, Dec 5, 2017 at 1:58 AM, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Arnd,
On Tuesday, 5 December 2017 02:37:27 EET Laurent Pinchart wrote:
On Monday, 27 November 2017 15:19:54 EET Arnd Bergmann wrote:
uvc_video_get_ts() returns a 'struct timespec', but all its users really want a nanoseconds variable anyway.
Changing the deprecated ktime_get_ts/ktime_get_real_ts to ktime_get and ktime_get_real simplifies the code noticeably, while keeping the resulting numbers unchanged.
Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/media/usb/uvc/uvc_video.c | 37 ++++++++++++--------------------- drivers/media/usb/uvc/uvcvideo.h | 2 +- 2 files changed, 13 insertions(+), 26 deletions(-)
[snip]
- struct timespec ts;
- u64 timestamp;
[snip]
uvc_trace(UVC_TRACE_CLOCK, "%s: SOF %u.%06llu y %llu ts %llu " "buf ts %llu (x1 %u/%u/%u x2 %u/%u/%u y1 %u y2 %u)\n", stream->dev->name, sof >> 16, div_u64(((u64)sof & 0xffff) * 1000000LLU, 65536),
y, timespec_to_ns(&ts), vbuf->vb2_buf.timestamp,
y, timestamp, vbuf->vb2_buf.timestamp, x1, first->host_sof, first->dev_sof, x2, last->host_sof, last->dev_sof, y1, y2);As you've done lots of work moving code away from timespec I figured out I would ask, what is the preferred way to print a ktime in secs.nsecs format ? Should I use ktime_to_timespec and print ts.tv_sec and ts.tv_nsec, or is there a better way ?
We had patches under discussion to add a special printk format string that would pretty-print a date, but that never got merged. Usually there is a tradeoff between runtime to convert the nanoseconds into a different format and how useful you want it to be. ktime_to_timespec() can be a bit slow on some architectures, since it has to do a 64-bit division, but then again the sprintf logic also needs to do that. If the output isn't on a time-critical path, you can use time64_to_tm and print it in years/months/days/hours/ minutes/seconds, but depending on where it gets printed, that may not be easier to interpret than the seconds/nanoseconds or pure nanoseconds.
Arnd
solo6x10 correctly deals with time stamps and will never suffer from overflows, but it uses the deprecated 'struct timespec' type and 'ktime_get_ts()' interface to read the monotonic clock.
This changes it to use ktime_get_ts64() instead, so we can eventually remove ktime_get_ts().
Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/media/pci/solo6x10/solo6x10-core.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/media/pci/solo6x10/solo6x10-core.c b/drivers/media/pci/solo6x10/solo6x10-core.c index ca0873e47bea..19ffd2ed3cc7 100644 --- a/drivers/media/pci/solo6x10/solo6x10-core.c +++ b/drivers/media/pci/solo6x10/solo6x10-core.c @@ -47,18 +47,19 @@ MODULE_PARM_DESC(full_eeprom, "Allow access to full 128B EEPROM (dangerous)");
static void solo_set_time(struct solo_dev *solo_dev) { - struct timespec ts; + struct timespec64 ts;
- ktime_get_ts(&ts); + ktime_get_ts64(&ts);
- solo_reg_write(solo_dev, SOLO_TIMER_SEC, ts.tv_sec); - solo_reg_write(solo_dev, SOLO_TIMER_USEC, ts.tv_nsec / NSEC_PER_USEC); + /* no overflow because we use monotonic timestamps */ + solo_reg_write(solo_dev, SOLO_TIMER_SEC, (u32)ts.tv_sec); + solo_reg_write(solo_dev, SOLO_TIMER_USEC, (u32)ts.tv_nsec / NSEC_PER_USEC); }
static void solo_timer_sync(struct solo_dev *solo_dev) { u32 sec, usec; - struct timespec ts; + struct timespec64 ts; long diff;
if (solo_dev->type != SOLO_DEV_6110) @@ -72,11 +73,11 @@ static void solo_timer_sync(struct solo_dev *solo_dev) sec = solo_reg_read(solo_dev, SOLO_TIMER_SEC); usec = solo_reg_read(solo_dev, SOLO_TIMER_USEC);
- ktime_get_ts(&ts); + ktime_get_ts64(&ts);
- diff = (long)ts.tv_sec - (long)sec; + diff = (s32)ts.tv_sec - (s32)sec; diff = (diff * 1000000) - + ((long)(ts.tv_nsec / NSEC_PER_USEC) - (long)usec); + + ((s32)(ts.tv_nsec / NSEC_PER_USEC) - (s32)usec);
if (diff > 1000 || diff < -1000) { solo_set_time(solo_dev);
struct timeval is deprecated for in-kernel use, and converting this function to use ktime_t makes it simpler as well.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- .../vc04_services/bcm2835-camera/bcm2835-camera.c | 37 ++++++---------------- .../vc04_services/bcm2835-camera/bcm2835-camera.h | 2 +- 2 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c index be936b8fe317..d8766b166675 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c @@ -343,37 +343,18 @@ static void buffer_cb(struct vchiq_mmal_instance *instance, if (dev->capture.frame_count) { if (dev->capture.vc_start_timestamp != -1 && pts != 0) { - struct timeval timestamp; + ktime_t timestamp; s64 runtime_us = pts - dev->capture.vc_start_timestamp; - u32 div = 0; - u32 rem = 0; - - div = - div_u64_rem(runtime_us, USEC_PER_SEC, &rem); - timestamp.tv_sec = - dev->capture.kernel_start_ts.tv_sec + div; - timestamp.tv_usec = - dev->capture.kernel_start_ts.tv_usec + rem; - - if (timestamp.tv_usec >= - USEC_PER_SEC) { - timestamp.tv_sec++; - timestamp.tv_usec -= - USEC_PER_SEC; - } + timestamp = ktime_add_us(dev->capture.kernel_start_ts, + runtime_us); v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, - "Convert start time %d.%06d and %llu " - "with offset %llu to %d.%06d\n", - (int)dev->capture.kernel_start_ts. - tv_sec, - (int)dev->capture.kernel_start_ts. - tv_usec, + "Convert start time %llu and %llu " + "with offset %llu to %llu\n", + ktime_to_ns(dev->capture.kernel_start_ts), dev->capture.vc_start_timestamp, pts, - (int)timestamp.tv_sec, - (int)timestamp.tv_usec); - buf->vb.vb2_buf.timestamp = timestamp.tv_sec * 1000000000ULL + - timestamp.tv_usec * 1000ULL; + ktime_to_ns(timestamp)); + buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp); } else { buf->vb.vb2_buf.timestamp = ktime_get_ns(); } @@ -547,7 +528,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count) "Start time %lld size %d\n", dev->capture.vc_start_timestamp, parameter_size);
- v4l2_get_timestamp(&dev->capture.kernel_start_ts); + dev->capture.kernel_start_ts = ktime_get();
/* enable the camera port */ dev->capture.port->cb_ctx = dev; diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h index 404037476bc5..83920683a448 100644 --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h @@ -92,7 +92,7 @@ struct bm2835_mmal_dev { /* VC start timestamp for streaming */ s64 vc_start_timestamp; /* Kernel start timestamp for streaming */ - struct timeval kernel_start_ts; + ktime_t kernel_start_ts;
struct vchiq_mmal_port *port; /* port being used for capture */ /* camera port being used for capture */
C libraries with 64-bit time_t use an incompatible format for struct omap3isp_stat_data. This changes the kernel code to support either version, by moving over the normal handling to the 64-bit variant, and adding compatiblity code to handle the old binary format with the existing ioctl command code.
Fortunately, the command code includes the size of the structure, so the difference gets handled automatically. In the process of eliminating the references to 'struct timeval' from the kernel, I also change the way the timestamp is generated internally, basically by open-coding the v4l2_get_timestamp() call.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/media/platform/omap3isp/isph3a_aewb.c | 2 ++ drivers/media/platform/omap3isp/isph3a_af.c | 2 ++ drivers/media/platform/omap3isp/isphist.c | 2 ++ drivers/media/platform/omap3isp/ispstat.c | 21 +++++++++++++++++++-- drivers/media/platform/omap3isp/ispstat.h | 4 +++- include/uapi/linux/omap3isp.h | 22 ++++++++++++++++++++++ 6 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c b/drivers/media/platform/omap3isp/isph3a_aewb.c index d44626f20ac6..3c82dea4d375 100644 --- a/drivers/media/platform/omap3isp/isph3a_aewb.c +++ b/drivers/media/platform/omap3isp/isph3a_aewb.c @@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); case VIDIOC_OMAP3ISP_STAT_REQ: return omap3isp_stat_request_statistics(stat, arg); + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32: + return omap3isp_stat_request_statistics_time32(stat, arg); case VIDIOC_OMAP3ISP_STAT_EN: { unsigned long *en = arg; return omap3isp_stat_enable(stat, !!*en); diff --git a/drivers/media/platform/omap3isp/isph3a_af.c b/drivers/media/platform/omap3isp/isph3a_af.c index 99bd6cc21d86..4da25c84f0c6 100644 --- a/drivers/media/platform/omap3isp/isph3a_af.c +++ b/drivers/media/platform/omap3isp/isph3a_af.c @@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); case VIDIOC_OMAP3ISP_STAT_REQ: return omap3isp_stat_request_statistics(stat, arg); + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32: + return omap3isp_stat_request_statistics_time32(stat, arg); case VIDIOC_OMAP3ISP_STAT_EN: { int *en = arg; return omap3isp_stat_enable(stat, !!*en); diff --git a/drivers/media/platform/omap3isp/isphist.c b/drivers/media/platform/omap3isp/isphist.c index a4ed5d140d48..d4be3d0e06f9 100644 --- a/drivers/media/platform/omap3isp/isphist.c +++ b/drivers/media/platform/omap3isp/isphist.c @@ -435,6 +435,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); case VIDIOC_OMAP3ISP_STAT_REQ: return omap3isp_stat_request_statistics(stat, arg); + case VIDIOC_OMAP3ISP_STAT_REQ_TIME32: + return omap3isp_stat_request_statistics_time32(stat, arg); case VIDIOC_OMAP3ISP_STAT_EN: { int *en = arg; return omap3isp_stat_enable(stat, !!*en); diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c index 47cbc7e3d825..5967dfd0a9f7 100644 --- a/drivers/media/platform/omap3isp/ispstat.c +++ b/drivers/media/platform/omap3isp/ispstat.c @@ -18,6 +18,7 @@ #include <linux/dma-mapping.h> #include <linux/slab.h> #include <linux/uaccess.h> +#include <linux/timekeeping.h>
#include "isp.h"
@@ -237,7 +238,7 @@ static int isp_stat_buf_queue(struct ispstat *stat) if (!stat->active_buf) return STAT_NO_BUF;
- v4l2_get_timestamp(&stat->active_buf->ts); + ktime_get_ts64(&stat->active_buf->ts);
stat->active_buf->buf_size = stat->buf_size; if (isp_stat_buf_check_magic(stat, stat->active_buf)) { @@ -500,7 +501,8 @@ int omap3isp_stat_request_statistics(struct ispstat *stat, return PTR_ERR(buf); }
- data->ts = buf->ts; + data->ts.tv_sec = buf->ts.tv_sec; + data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC; data->config_counter = buf->config_counter; data->frame_number = buf->frame_number; data->buf_size = buf->buf_size; @@ -512,6 +514,21 @@ int omap3isp_stat_request_statistics(struct ispstat *stat, return 0; }
+int omap3isp_stat_request_statistics_time32(struct ispstat *stat, + struct omap3isp_stat_data_time32 *data) +{ + struct omap3isp_stat_data data64; + int ret; + + ret = omap3isp_stat_request_statistics(stat, &data64); + + data->ts.tv_sec = data64.ts.tv_sec; + data->ts.tv_usec = data64.ts.tv_usec; + memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts)); + + return ret; +} + /* * omap3isp_stat_config - Receives new statistic engine configuration. * @new_conf: Pointer to config structure. diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h index 6d9b0244f320..923b38cfc682 100644 --- a/drivers/media/platform/omap3isp/ispstat.h +++ b/drivers/media/platform/omap3isp/ispstat.h @@ -39,7 +39,7 @@ struct ispstat_buffer { struct sg_table sgt; void *virt_addr; dma_addr_t dma_addr; - struct timeval ts; + struct timespec64 ts; u32 buf_size; u32 frame_number; u16 config_counter; @@ -130,6 +130,8 @@ struct ispstat_generic_config { int omap3isp_stat_config(struct ispstat *stat, void *new_conf); int omap3isp_stat_request_statistics(struct ispstat *stat, struct omap3isp_stat_data *data); +int omap3isp_stat_request_statistics_time32(struct ispstat *stat, + struct omap3isp_stat_data_time32 *data); int omap3isp_stat_init(struct ispstat *stat, const char *name, const struct v4l2_subdev_ops *sd_ops); void omap3isp_stat_cleanup(struct ispstat *stat); diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h index 1a920145db04..87b55755f4ff 100644 --- a/include/uapi/linux/omap3isp.h +++ b/include/uapi/linux/omap3isp.h @@ -55,6 +55,8 @@ _IOWR('V', BASE_VIDIOC_PRIVATE + 5, struct omap3isp_h3a_af_config) #define VIDIOC_OMAP3ISP_STAT_REQ \ _IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data) +#define VIDIOC_OMAP3ISP_STAT_REQ_TIME32 \ + _IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data_time32) #define VIDIOC_OMAP3ISP_STAT_EN \ _IOWR('V', BASE_VIDIOC_PRIVATE + 7, unsigned long)
@@ -165,7 +167,14 @@ struct omap3isp_h3a_aewb_config { * @config_counter: Number of the configuration associated with the data. */ struct omap3isp_stat_data { +#ifdef __KERNEL__ + struct { + __s64 tv_sec; + __s64 tv_usec; + } ts; +#else struct timeval ts; +#endif void __user *buf; __u32 buf_size; __u16 frame_number; @@ -173,6 +182,19 @@ struct omap3isp_stat_data { __u16 config_counter; };
+#ifdef __KERNEL__ +struct omap3isp_stat_data_time32 { + struct { + __s32 tv_sec; + __s32 tv_usec; + } ts; + __u32 buf; + __u32 buf_size; + __u16 frame_number; + __u16 cur_frame; + __u16 config_counter; +}; +#endif
/* Histogram related structs */
Hi Arnd,
Thank you for the patch.
I'll try to review this without too much delay. In the meantime, I'm CC'ing Sakari Ailus who might be faster than me :-)
On Monday, 27 November 2017 15:19:57 EET Arnd Bergmann wrote:
C libraries with 64-bit time_t use an incompatible format for struct omap3isp_stat_data. This changes the kernel code to support either version, by moving over the normal handling to the 64-bit variant, and adding compatiblity code to handle the old binary format with the existing ioctl command code.
Fortunately, the command code includes the size of the structure, so the difference gets handled automatically. In the process of eliminating the references to 'struct timeval' from the kernel, I also change the way the timestamp is generated internally, basically by open-coding the v4l2_get_timestamp() call.
Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/media/platform/omap3isp/isph3a_aewb.c | 2 ++ drivers/media/platform/omap3isp/isph3a_af.c | 2 ++ drivers/media/platform/omap3isp/isphist.c | 2 ++ drivers/media/platform/omap3isp/ispstat.c | 21 +++++++++++++++++++-- drivers/media/platform/omap3isp/ispstat.h | 4 +++- include/uapi/linux/omap3isp.h | 22 ++++++++++++++++++++++ 6 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c b/drivers/media/platform/omap3isp/isph3a_aewb.c index d44626f20ac6..3c82dea4d375 100644 --- a/drivers/media/platform/omap3isp/isph3a_aewb.c +++ b/drivers/media/platform/omap3isp/isph3a_aewb.c @@ -250,6 +250,8 @@ static long h3a_aewb_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); case VIDIOC_OMAP3ISP_STAT_REQ: return omap3isp_stat_request_statistics(stat, arg);
- case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
return omap3isp_stat_request_statistics_time32(stat, arg);diff --git a/drivers/media/platform/omap3isp/isph3a_af.c b/drivers/media/platform/omap3isp/isph3a_af.c index 99bd6cc21d86..4da25c84f0c6 100644 --- a/drivers/media/platform/omap3isp/isph3a_af.c +++ b/drivers/media/platform/omap3isp/isph3a_af.c @@ -314,6 +314,8 @@ static long h3a_af_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); case VIDIOC_OMAP3ISP_STAT_REQ: return omap3isp_stat_request_statistics(stat, arg);
- case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
return omap3isp_stat_request_statistics_time32(stat, arg);diff --git a/drivers/media/platform/omap3isp/isphist.c b/drivers/media/platform/omap3isp/isphist.c index a4ed5d140d48..d4be3d0e06f9 100644 --- a/drivers/media/platform/omap3isp/isphist.c +++ b/drivers/media/platform/omap3isp/isphist.c @@ -435,6 +435,8 @@ static long hist_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg) return omap3isp_stat_config(stat, arg); case VIDIOC_OMAP3ISP_STAT_REQ: return omap3isp_stat_request_statistics(stat, arg);
- case VIDIOC_OMAP3ISP_STAT_REQ_TIME32:
return omap3isp_stat_request_statistics_time32(stat, arg);diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c index 47cbc7e3d825..5967dfd0a9f7 100644 --- a/drivers/media/platform/omap3isp/ispstat.c +++ b/drivers/media/platform/omap3isp/ispstat.c @@ -18,6 +18,7 @@ #include <linux/dma-mapping.h> #include <linux/slab.h> #include <linux/uaccess.h> +#include <linux/timekeeping.h>
#include "isp.h"
@@ -237,7 +238,7 @@ static int isp_stat_buf_queue(struct ispstat *stat) if (!stat->active_buf) return STAT_NO_BUF;
- v4l2_get_timestamp(&stat->active_buf->ts);
ktime_get_ts64(&stat->active_buf->ts);
stat->active_buf->buf_size = stat->buf_size; if (isp_stat_buf_check_magic(stat, stat->active_buf)) {
@@ -500,7 +501,8 @@ int omap3isp_stat_request_statistics(struct ispstat *stat, return PTR_ERR(buf); }
- data->ts = buf->ts;
- data->ts.tv_sec = buf->ts.tv_sec;
- data->ts.tv_usec = buf->ts.tv_nsec / NSEC_PER_USEC; data->config_counter = buf->config_counter; data->frame_number = buf->frame_number; data->buf_size = buf->buf_size;
@@ -512,6 +514,21 @@ int omap3isp_stat_request_statistics(struct ispstat *stat, return 0; }
+int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
struct omap3isp_stat_data_time32 *data)+{
- struct omap3isp_stat_data data64;
- int ret;
- ret = omap3isp_stat_request_statistics(stat, &data64);
- data->ts.tv_sec = data64.ts.tv_sec;
- data->ts.tv_usec = data64.ts.tv_usec;
- memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts));
- return ret;
+}
/*
- omap3isp_stat_config - Receives new statistic engine configuration.
- @new_conf: Pointer to config structure.
diff --git a/drivers/media/platform/omap3isp/ispstat.h b/drivers/media/platform/omap3isp/ispstat.h index 6d9b0244f320..923b38cfc682 100644 --- a/drivers/media/platform/omap3isp/ispstat.h +++ b/drivers/media/platform/omap3isp/ispstat.h @@ -39,7 +39,7 @@ struct ispstat_buffer { struct sg_table sgt; void *virt_addr; dma_addr_t dma_addr;
- struct timeval ts;
- struct timespec64 ts; u32 buf_size; u32 frame_number; u16 config_counter;
@@ -130,6 +130,8 @@ struct ispstat_generic_config { int omap3isp_stat_config(struct ispstat *stat, void *new_conf); int omap3isp_stat_request_statistics(struct ispstat *stat, struct omap3isp_stat_data *data); +int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
struct omap3isp_stat_data_time32 *data);int omap3isp_stat_init(struct ispstat *stat, const char *name, const struct v4l2_subdev_ops *sd_ops); void omap3isp_stat_cleanup(struct ispstat *stat); diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h index 1a920145db04..87b55755f4ff 100644 --- a/include/uapi/linux/omap3isp.h +++ b/include/uapi/linux/omap3isp.h @@ -55,6 +55,8 @@ _IOWR('V', BASE_VIDIOC_PRIVATE + 5, struct omap3isp_h3a_af_config) #define VIDIOC_OMAP3ISP_STAT_REQ \ _IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data) +#define VIDIOC_OMAP3ISP_STAT_REQ_TIME32 \
- _IOWR('V', BASE_VIDIOC_PRIVATE + 6, struct omap3isp_stat_data_time32)
#define VIDIOC_OMAP3ISP_STAT_EN \ _IOWR('V', BASE_VIDIOC_PRIVATE + 7, unsigned long)
@@ -165,7 +167,14 @@ struct omap3isp_h3a_aewb_config {
- @config_counter: Number of the configuration associated with the data.
*/ struct omap3isp_stat_data { +#ifdef __KERNEL__
- struct {
__s64 tv_sec;__s64 tv_usec;- } ts;
+#else struct timeval ts; +#endif void __user *buf; __u32 buf_size; __u16 frame_number; @@ -173,6 +182,19 @@ struct omap3isp_stat_data { __u16 config_counter; };
+#ifdef __KERNEL__ +struct omap3isp_stat_data_time32 {
- struct {
__s32 tv_sec;__s32 tv_usec;- } ts;
- __u32 buf;
- __u32 buf_size;
- __u16 frame_number;
- __u16 cur_frame;
- __u16 config_counter;
+}; +#endif
/* Histogram related structs */
On Tue, Dec 5, 2017 at 1:41 AM, Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
Hi Arnd,
Thank you for the patch.
I'll try to review this without too much delay. In the meantime, I'm CC'ing Sakari Ailus who might be faster than me :-)
Hi Laurent and Sakari,
I stumbled over this while cleaning out my backlog of patches. Any chance one of you can have a look at this one? It's not needed for the 4.16 merge window, but we do need it at some point and I would like to not see it again the next time I clean out my old patches ;-)
Arnd
timespec is generally deprecated because of the y2038 overflow. In vivid, the usage is fine, since we are dealing with monotonic timestamps, but we can also simplify the code by going to ktime_t.
Using ktime_divns() should be roughly as efficient as the old code, since the constant 64-bit division gets turned into a multiplication on modern platforms, and we save multiple 32-bit divisions that can be expensive e.g. on ARMv7.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- I hope I understood the VIVID_RDS_GEN_BLOCKS calculation right, please review carefully. --- drivers/media/platform/vivid/vivid-core.c | 2 +- drivers/media/platform/vivid/vivid-core.h | 2 +- drivers/media/platform/vivid/vivid-radio-rx.c | 11 +++++------ drivers/media/platform/vivid/vivid-radio-tx.c | 8 +++----- drivers/media/platform/vivid/vivid-rds-gen.h | 1 + 5 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c index 5f316a5e38db..a091cfd93164 100644 --- a/drivers/media/platform/vivid/vivid-core.c +++ b/drivers/media/platform/vivid/vivid-core.c @@ -995,7 +995,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
dev->edid_max_blocks = dev->edid_blocks = 2; memcpy(dev->edid, vivid_hdmi_edid, sizeof(vivid_hdmi_edid)); - ktime_get_ts(&dev->radio_rds_init_ts); + dev->radio_rds_init_time = ktime_get();
/* create all controls */ ret = vivid_create_controls(dev, ccs_cap == -1, ccs_out == -1, no_error_inj, diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h index 5cdf95bdc4d1..d8bff4dcefb7 100644 --- a/drivers/media/platform/vivid/vivid-core.h +++ b/drivers/media/platform/vivid/vivid-core.h @@ -510,7 +510,7 @@ struct vivid_dev {
/* Shared between radio receiver and transmitter */ bool radio_rds_loop; - struct timespec radio_rds_init_ts; + ktime_t radio_rds_init_time;
/* CEC */ struct cec_adapter *cec_rx_adap; diff --git a/drivers/media/platform/vivid/vivid-radio-rx.c b/drivers/media/platform/vivid/vivid-radio-rx.c index 47c36c26096b..1b96cbd7f2ea 100644 --- a/drivers/media/platform/vivid/vivid-radio-rx.c +++ b/drivers/media/platform/vivid/vivid-radio-rx.c @@ -38,9 +38,9 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user *buf, size_t size, loff_t *offset) { struct vivid_dev *dev = video_drvdata(file); - struct timespec ts; struct v4l2_rds_data *data = dev->rds_gen.data; bool use_alternates; + ktime_t timestamp; unsigned blk; int perc; int i; @@ -64,17 +64,16 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user *buf, }
retry: - ktime_get_ts(&ts); - use_alternates = ts.tv_sec % 10 >= 5; + timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time); + blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK); + use_alternates = blk % VIVID_RDS_GEN_BLOCKS; + if (dev->radio_rx_rds_last_block == 0 || dev->radio_rx_rds_use_alternates != use_alternates) { dev->radio_rx_rds_use_alternates = use_alternates; /* Re-init the RDS generator */ vivid_radio_rds_init(dev); } - ts = timespec_sub(ts, dev->radio_rds_init_ts); - blk = ts.tv_sec * 100 + ts.tv_nsec / 10000000; - blk = (blk * VIVID_RDS_GEN_BLOCKS) / 500; if (blk >= dev->radio_rx_rds_last_block + VIVID_RDS_GEN_BLOCKS) dev->radio_rx_rds_last_block = blk - VIVID_RDS_GEN_BLOCKS + 1;
diff --git a/drivers/media/platform/vivid/vivid-radio-tx.c b/drivers/media/platform/vivid/vivid-radio-tx.c index 0e8025b7b4dd..897b56195ca7 100644 --- a/drivers/media/platform/vivid/vivid-radio-tx.c +++ b/drivers/media/platform/vivid/vivid-radio-tx.c @@ -37,7 +37,7 @@ ssize_t vivid_radio_tx_write(struct file *file, const char __user *buf, { struct vivid_dev *dev = video_drvdata(file); struct v4l2_rds_data *data = dev->rds_gen.data; - struct timespec ts; + ktime_t timestamp; unsigned blk; int i;
@@ -58,10 +58,8 @@ ssize_t vivid_radio_tx_write(struct file *file, const char __user *buf, dev->radio_tx_rds_owner = file->private_data;
retry: - ktime_get_ts(&ts); - ts = timespec_sub(ts, dev->radio_rds_init_ts); - blk = ts.tv_sec * 100 + ts.tv_nsec / 10000000; - blk = (blk * VIVID_RDS_GEN_BLOCKS) / 500; + timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time); + blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK); if (blk - VIVID_RDS_GEN_BLOCKS >= dev->radio_tx_rds_last_block) dev->radio_tx_rds_last_block = blk - VIVID_RDS_GEN_BLOCKS + 1;
diff --git a/drivers/media/platform/vivid/vivid-rds-gen.h b/drivers/media/platform/vivid/vivid-rds-gen.h index eff4bf552ed3..e55e3b22b7ca 100644 --- a/drivers/media/platform/vivid/vivid-rds-gen.h +++ b/drivers/media/platform/vivid/vivid-rds-gen.h @@ -29,6 +29,7 @@ #define VIVID_RDS_GEN_GROUPS 57 #define VIVID_RDS_GEN_BLKS_PER_GRP 4 #define VIVID_RDS_GEN_BLOCKS (VIVID_RDS_GEN_BLKS_PER_GRP * VIVID_RDS_GEN_GROUPS) +#define VIVID_RDS_NSEC_PER_BLK (u32)(5ull * NSEC_PER_SEC / VIVID_RDS_GEN_BLOCKS)
struct vivid_rds_gen { struct v4l2_rds_data data[VIVID_RDS_GEN_BLOCKS];
Hi Arnd,
On 11/27/2017 02:19 PM, Arnd Bergmann wrote:
timespec is generally deprecated because of the y2038 overflow. In vivid, the usage is fine, since we are dealing with monotonic timestamps, but we can also simplify the code by going to ktime_t.
Using ktime_divns() should be roughly as efficient as the old code, since the constant 64-bit division gets turned into a multiplication on modern platforms, and we save multiple 32-bit divisions that can be expensive e.g. on ARMv7.
Signed-off-by: Arnd Bergmann arnd@arndb.de
I hope I understood the VIVID_RDS_GEN_BLOCKS calculation right, please review carefully.
drivers/media/platform/vivid/vivid-core.c | 2 +- drivers/media/platform/vivid/vivid-core.h | 2 +- drivers/media/platform/vivid/vivid-radio-rx.c | 11 +++++------ drivers/media/platform/vivid/vivid-radio-tx.c | 8 +++----- drivers/media/platform/vivid/vivid-rds-gen.h | 1 + 5 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c index 5f316a5e38db..a091cfd93164 100644 --- a/drivers/media/platform/vivid/vivid-core.c +++ b/drivers/media/platform/vivid/vivid-core.c @@ -995,7 +995,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst) dev->edid_max_blocks = dev->edid_blocks = 2; memcpy(dev->edid, vivid_hdmi_edid, sizeof(vivid_hdmi_edid));
- ktime_get_ts(&dev->radio_rds_init_ts);
- dev->radio_rds_init_time = ktime_get();
/* create all controls */ ret = vivid_create_controls(dev, ccs_cap == -1, ccs_out == -1, no_error_inj, diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h index 5cdf95bdc4d1..d8bff4dcefb7 100644 --- a/drivers/media/platform/vivid/vivid-core.h +++ b/drivers/media/platform/vivid/vivid-core.h @@ -510,7 +510,7 @@ struct vivid_dev { /* Shared between radio receiver and transmitter */ bool radio_rds_loop;
- struct timespec radio_rds_init_ts;
- ktime_t radio_rds_init_time;
/* CEC */ struct cec_adapter *cec_rx_adap; diff --git a/drivers/media/platform/vivid/vivid-radio-rx.c b/drivers/media/platform/vivid/vivid-radio-rx.c index 47c36c26096b..1b96cbd7f2ea 100644 --- a/drivers/media/platform/vivid/vivid-radio-rx.c +++ b/drivers/media/platform/vivid/vivid-radio-rx.c @@ -38,9 +38,9 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user *buf, size_t size, loff_t *offset) { struct vivid_dev *dev = video_drvdata(file);
- struct timespec ts; struct v4l2_rds_data *data = dev->rds_gen.data; bool use_alternates;
- ktime_t timestamp; unsigned blk; int perc; int i;
@@ -64,17 +64,16 @@ ssize_t vivid_radio_rx_read(struct file *file, char __user *buf, } retry:
- ktime_get_ts(&ts);
- use_alternates = ts.tv_sec % 10 >= 5;
- timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time);
- blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK);
- use_alternates = blk % VIVID_RDS_GEN_BLOCKS;
Almost right. This last line should be:
use_alternates = (blk / VIVID_RDS_GEN_BLOCKS) & 1;
With that in place it works and you can add my:
Tested-by: Hans Verkuil hans.verkuil@cisco.com
to this patch.
Regards,
Hans
if (dev->radio_rx_rds_last_block == 0 || dev->radio_rx_rds_use_alternates != use_alternates) { dev->radio_rx_rds_use_alternates = use_alternates; /* Re-init the RDS generator */ vivid_radio_rds_init(dev); }
- ts = timespec_sub(ts, dev->radio_rds_init_ts);
- blk = ts.tv_sec * 100 + ts.tv_nsec / 10000000;
- blk = (blk * VIVID_RDS_GEN_BLOCKS) / 500; if (blk >= dev->radio_rx_rds_last_block + VIVID_RDS_GEN_BLOCKS) dev->radio_rx_rds_last_block = blk - VIVID_RDS_GEN_BLOCKS + 1;
diff --git a/drivers/media/platform/vivid/vivid-radio-tx.c b/drivers/media/platform/vivid/vivid-radio-tx.c index 0e8025b7b4dd..897b56195ca7 100644 --- a/drivers/media/platform/vivid/vivid-radio-tx.c +++ b/drivers/media/platform/vivid/vivid-radio-tx.c @@ -37,7 +37,7 @@ ssize_t vivid_radio_tx_write(struct file *file, const char __user *buf, { struct vivid_dev *dev = video_drvdata(file); struct v4l2_rds_data *data = dev->rds_gen.data;
- struct timespec ts;
- ktime_t timestamp; unsigned blk; int i;
@@ -58,10 +58,8 @@ ssize_t vivid_radio_tx_write(struct file *file, const char __user *buf, dev->radio_tx_rds_owner = file->private_data; retry:
- ktime_get_ts(&ts);
- ts = timespec_sub(ts, dev->radio_rds_init_ts);
- blk = ts.tv_sec * 100 + ts.tv_nsec / 10000000;
- blk = (blk * VIVID_RDS_GEN_BLOCKS) / 500;
- timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time);
- blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK); if (blk - VIVID_RDS_GEN_BLOCKS >= dev->radio_tx_rds_last_block) dev->radio_tx_rds_last_block = blk - VIVID_RDS_GEN_BLOCKS + 1;
diff --git a/drivers/media/platform/vivid/vivid-rds-gen.h b/drivers/media/platform/vivid/vivid-rds-gen.h index eff4bf552ed3..e55e3b22b7ca 100644 --- a/drivers/media/platform/vivid/vivid-rds-gen.h +++ b/drivers/media/platform/vivid/vivid-rds-gen.h @@ -29,6 +29,7 @@ #define VIVID_RDS_GEN_GROUPS 57 #define VIVID_RDS_GEN_BLKS_PER_GRP 4 #define VIVID_RDS_GEN_BLOCKS (VIVID_RDS_GEN_BLKS_PER_GRP * VIVID_RDS_GEN_GROUPS) +#define VIVID_RDS_NSEC_PER_BLK (u32)(5ull * NSEC_PER_SEC / VIVID_RDS_GEN_BLOCKS) struct vivid_rds_gen { struct v4l2_rds_data data[VIVID_RDS_GEN_BLOCKS];
On Mon, Nov 27, 2017 at 4:14 PM, Hans Verkuil hverkuil@xs4all.nl wrote:
ktime_get_ts(&ts);use_alternates = ts.tv_sec % 10 >= 5;
timestamp = ktime_sub(ktime_get(), dev->radio_rds_init_time);blk = ktime_divns(timestamp, VIVID_RDS_NSEC_PER_BLK);use_alternates = blk % VIVID_RDS_GEN_BLOCKS;Almost right. This last line should be:
use_alternates = (blk / VIVID_RDS_GEN_BLOCKS) & 1;With that in place it works and you can add my:
Tested-by: Hans Verkuil hans.verkuil@cisco.com
Makes sense. Sending a fixed version now, thanks a lot for testing!
Arnd
timespec overflows in 2038 on 32-bit architectures, and the getnstimeofday() suffers from possible time jumps, so the timestamps here are better done using ktime_get(), which has neither of those problems.
In case of ov2680, we don't seem to use the timestamp at all, so I just remove it.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/staging/media/atomisp/i2c/ov2680.h | 1 - drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c | 15 ++++++--------- drivers/staging/media/atomisp/i2c/ov5693/ov5693.h | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h index bf4897347df7..03f75dd80f87 100644 --- a/drivers/staging/media/atomisp/i2c/ov2680.h +++ b/drivers/staging/media/atomisp/i2c/ov2680.h @@ -174,7 +174,6 @@ struct ov2680_format { struct mutex input_lock; struct v4l2_ctrl_handler ctrl_handler; struct camera_sensor_platform_data *platform_data; - struct timespec timestamp_t_focus_abs; int vt_pix_clk_freq_mhz; int fmt_idx; int run_mode; diff --git a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c index 3e7c3851280f..a715ea0e4230 100644 --- a/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c +++ b/drivers/staging/media/atomisp/i2c/ov5693/atomisp-ov5693.c @@ -973,7 +973,7 @@ static int ov5693_t_focus_abs(struct v4l2_subdev *sd, s32 value) if (ret == 0) { dev->number_of_steps = value - dev->focus; dev->focus = value; - getnstimeofday(&(dev->timestamp_t_focus_abs)); + dev->timestamp_t_focus_abs = ktime_get(); } else dev_err(&client->dev, "%s: i2c failed. ret %d\n", __func__, ret); @@ -993,16 +993,13 @@ static int ov5693_q_focus_status(struct v4l2_subdev *sd, s32 *value) { u32 status = 0; struct ov5693_device *dev = to_ov5693_sensor(sd); - struct timespec temptime; - const struct timespec timedelay = { - 0, + ktime_t temptime; + ktime_t timedelay = ns_to_ktime( min((u32)abs(dev->number_of_steps) * DELAY_PER_STEP_NS, - (u32)DELAY_MAX_PER_STEP_NS), - }; + (u32)DELAY_MAX_PER_STEP_NS));
- getnstimeofday(&temptime); - temptime = timespec_sub(temptime, (dev->timestamp_t_focus_abs)); - if (timespec_compare(&temptime, &timedelay) <= 0) { + temptime = ktime_sub(ktime_get(), (dev->timestamp_t_focus_abs)); + if (ktime_compare(temptime, timedelay) <= 0) { status |= ATOMISP_FOCUS_STATUS_MOVING; status |= ATOMISP_FOCUS_HP_IN_PROGRESS; } else { diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h index 2ea63807c56d..68cfcb4a6c3c 100644 --- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h +++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.h @@ -221,7 +221,7 @@ struct ov5693_device { struct v4l2_ctrl_handler ctrl_handler;
struct camera_sensor_platform_data *platform_data; - struct timespec timestamp_t_focus_abs; + ktime_t timestamp_t_focus_abs; int vt_pix_clk_freq_mhz; int fmt_idx; int run_mode;
On Mon, 2017-11-27 at 14:19 +0100, Arnd Bergmann wrote:
timespec overflows in 2038 on 32-bit architectures, and the getnstimeofday() suffers from possible time jumps, so the timestamps here are better done using ktime_get(), which has neither of those problems.
In case of ov2680, we don't seem to use the timestamp at all, so I just remove it.
- ktime_t timedelay = ns_to_ktime( min((u32)abs(dev->number_of_steps) *
DELAY_PER_STEP_NS,
(u32)DELAY_MAX_PER_STEP_NS),- };
(u32)DELAY_MAX_PER_STEP_NS));
Since you are touching this, it might make sense to convert to
min_t(u32, ...)
...and locate lines something like:
ktime_t timeday = ns_to_ktime(min_t(u32, param1, param2));
From my pov will make readability better.
On Mon, Nov 27, 2017 at 4:05 PM, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Mon, 2017-11-27 at 14:19 +0100, Arnd Bergmann wrote:
timespec overflows in 2038 on 32-bit architectures, and the getnstimeofday() suffers from possible time jumps, so the timestamps here are better done using ktime_get(), which has neither of those problems.
In case of ov2680, we don't seem to use the timestamp at all, so I just remove it.
ktime_t timedelay = ns_to_ktime( min((u32)abs(dev->number_of_steps) *DELAY_PER_STEP_NS,
(u32)DELAY_MAX_PER_STEP_NS),};
(u32)DELAY_MAX_PER_STEP_NS));Since you are touching this, it might make sense to convert to
min_t(u32, ...)
...and locate lines something like:
ktime_t timeday = ns_to_ktime(min_t(u32, param1, param2));
From my pov will make readability better.
Yes, good idea. Re-sending the patch now. Thanks for taking a look,
Arnd
The imx media driver passes around monotonic timestamps in the deprecated 'timespec' format. This is not a problem for the driver, as they won't overflow, but moving to either timespec64 or ktime_t is preferred.
I'm picking ktime_t for simplicity here. frame_interval_monitor() is the main function that changes, as it tries to compare a time interval in microseconds. The algorithm slightly changes here, to avoid 64-bit division. The code previously assumed that the error was at most 32-bit worth of microseconds here, so I'm making the same assumption but add an explicit test for it.
Signed-off-by: Arnd Bergmann arnd@arndb.de --- drivers/staging/media/imx/imx-media-csi.c | 8 ++------ drivers/staging/media/imx/imx-media-fim.c | 30 +++++++++++++++++------------- drivers/staging/media/imx/imx-media.h | 2 +- 3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index bb1d6dafca83..26994b429cf2 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -207,13 +207,9 @@ static irqreturn_t csi_idmac_eof_interrupt(int irq, void *dev_id) goto unlock; }
- if (priv->fim) { - struct timespec cur_ts; - - ktime_get_ts(&cur_ts); + if (priv->fim) /* call frame interval monitor */ - imx_media_fim_eof_monitor(priv->fim, &cur_ts); - } + imx_media_fim_eof_monitor(priv->fim, ktime_get());
csi_vb2_buf_done(priv);
diff --git a/drivers/staging/media/imx/imx-media-fim.c b/drivers/staging/media/imx/imx-media-fim.c index 47275ef803f3..6df189135db8 100644 --- a/drivers/staging/media/imx/imx-media-fim.c +++ b/drivers/staging/media/imx/imx-media-fim.c @@ -66,7 +66,7 @@ struct imx_media_fim { int icap_flags;
int counter; - struct timespec last_ts; + ktime_t last_ts; unsigned long sum; /* usec */ unsigned long nominal; /* usec */
@@ -147,22 +147,26 @@ static void send_fim_event(struct imx_media_fim *fim, unsigned long error) * (presumably random) interrupt latency. */ static void frame_interval_monitor(struct imx_media_fim *fim, - struct timespec *ts) + ktime_t timestamp) { - unsigned long interval, error, error_avg; + long long interval, error; + unsigned long error_avg; bool send_event = false; - struct timespec diff;
if (!fim->enabled || ++fim->counter <= 0) goto out_update_ts;
- diff = timespec_sub(*ts, fim->last_ts); - interval = diff.tv_sec * 1000 * 1000 + diff.tv_nsec / 1000; - error = abs(interval - fim->nominal); + /* max error is less than l00µs, so use 32-bit division or fail */ + interval = ktime_to_ns(ktime_sub(timestamp, fim->last_ts)); + error = abs(interval - NSEC_PER_USEC * (u64)fim->nominal); + if (error > U32_MAX) + error = U32_MAX; + else + error = abs((u32)error / NSEC_PER_USEC);
if (fim->tolerance_max && error >= fim->tolerance_max) { dev_dbg(fim->sd->dev, - "FIM: %lu ignored, out of tolerance bounds\n", + "FIM: %llu ignored, out of tolerance bounds\n", error); fim->counter--; goto out_update_ts; @@ -184,7 +188,7 @@ static void frame_interval_monitor(struct imx_media_fim *fim, }
out_update_ts: - fim->last_ts = *ts; + fim->last_ts = timestamp; if (send_event) send_fim_event(fim, error_avg); } @@ -195,14 +199,14 @@ static void frame_interval_monitor(struct imx_media_fim *fim, * to interrupt latency. */ static void fim_input_capture_handler(int channel, void *dev_id, - struct timespec *ts) + ktime_t timestamp) { struct imx_media_fim *fim = dev_id; unsigned long flags;
spin_lock_irqsave(&fim->lock, flags);
- frame_interval_monitor(fim, ts); + frame_interval_monitor(fim, timestamp);
if (!completion_done(&fim->icap_first_event)) complete(&fim->icap_first_event); @@ -405,14 +409,14 @@ static int init_fim_controls(struct imx_media_fim *fim) * the frame_interval_monitor() is called by the input capture event * callback handler in that case. */ -void imx_media_fim_eof_monitor(struct imx_media_fim *fim, struct timespec *ts) +void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp) { unsigned long flags;
spin_lock_irqsave(&fim->lock, flags);
if (!icap_enabled(fim)) - frame_interval_monitor(fim, ts); + frame_interval_monitor(fim, timestamp);
spin_unlock_irqrestore(&fim->lock, flags); } diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h index d409170632bd..ac3ab115394f 100644 --- a/drivers/staging/media/imx/imx-media.h +++ b/drivers/staging/media/imx/imx-media.h @@ -280,7 +280,7 @@ int imx_media_pipeline_set_stream(struct imx_media_dev *imxmd,
/* imx-media-fim.c */ struct imx_media_fim; -void imx_media_fim_eof_monitor(struct imx_media_fim *fim, struct timespec *ts); +void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp); int imx_media_fim_set_stream(struct imx_media_fim *fim, const struct v4l2_fract *frame_interval, bool on);
Hi Arnd,
This looks fine to me, except for a couple things...
On 11/27/2017 05:20 AM, Arnd Bergmann wrote:
The imx media driver passes around monotonic timestamps in the deprecated 'timespec' format. This is not a problem for the driver, as they won't overflow, but moving to either timespec64 or ktime_t is preferred.
I'm picking ktime_t for simplicity here. frame_interval_monitor() is the main function that changes, as it tries to compare a time interval in microseconds. The algorithm slightly changes here, to avoid 64-bit division. The code previously assumed that the error was at most 32-bit worth of microseconds here, so I'm making the same assumption but add an explicit test for it.
Signed-off-by: Arnd Bergmann arnd@arndb.de
drivers/staging/media/imx/imx-media-csi.c | 8 ++------ drivers/staging/media/imx/imx-media-fim.c | 30 +++++++++++++++++------------- drivers/staging/media/imx/imx-media.h | 2 +- 3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index bb1d6dafca83..26994b429cf2 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -207,13 +207,9 @@ static irqreturn_t csi_idmac_eof_interrupt(int irq, void *dev_id) goto unlock; }
- if (priv->fim) {
struct timespec cur_ts;ktime_get_ts(&cur_ts);
- if (priv->fim) /* call frame interval monitor */
imx_media_fim_eof_monitor(priv->fim, &cur_ts);- }
imx_media_fim_eof_monitor(priv->fim, ktime_get());csi_vb2_buf_done(priv); diff --git a/drivers/staging/media/imx/imx-media-fim.c b/drivers/staging/media/imx/imx-media-fim.c index 47275ef803f3..6df189135db8 100644 --- a/drivers/staging/media/imx/imx-media-fim.c +++ b/drivers/staging/media/imx/imx-media-fim.c @@ -66,7 +66,7 @@ struct imx_media_fim { int icap_flags; int counter;
- struct timespec last_ts;
- ktime_t last_ts; unsigned long sum; /* usec */ unsigned long nominal; /* usec */
@@ -147,22 +147,26 @@ static void send_fim_event(struct imx_media_fim *fim, unsigned long error)
- (presumably random) interrupt latency.
*/ static void frame_interval_monitor(struct imx_media_fim *fim,
struct timespec *ts)
ktime_t timestamp)
- unsigned long interval, error, error_avg;
- long long interval, error;
- unsigned long error_avg; bool send_event = false;
- struct timespec diff;
if (!fim->enabled || ++fim->counter <= 0) goto out_update_ts;
- diff = timespec_sub(*ts, fim->last_ts);
- interval = diff.tv_sec * 1000 * 1000 + diff.tv_nsec / 1000;
- error = abs(interval - fim->nominal);
- /* max error is less than l00µs, so use 32-bit division or fail */
This comment doesn't make sense. By "max error" maybe you are referring to the tolerance_max control, which is limited to 500 usec, but if set to zero (or less than tolerance_min), there is no max error, it is unbounded. Please just remove this comment.
- interval = ktime_to_ns(ktime_sub(timestamp, fim->last_ts));
- error = abs(interval - NSEC_PER_USEC * (u64)fim->nominal);
- if (error > U32_MAX)
error = U32_MAX;- else
error = abs((u32)error / NSEC_PER_USEC);
Why the second abs() ?
How about the following:
- if (error > U32_MAX) - error = U32_MAX; - else - error = abs((u32)error / NSEC_PER_USEC); + /* convert interval error to usec using 32-bit divide */ + error = (u32)min_t(long long, error, U32_MAX) / NSEC_PER_USEC;
Steve
if (fim->tolerance_max && error >= fim->tolerance_max) { dev_dbg(fim->sd->dev,
"FIM: %lu ignored, out of tolerance bounds\n",
"FIM: %llu ignored, out of tolerance bounds\n", error);@@ -184,7 +188,7 @@ static void frame_interval_monitor(struct imx_media_fim *fim, } out_update_ts:
- fim->last_ts = *ts;
- fim->last_ts = timestamp; if (send_event) send_fim_event(fim, error_avg); }
@@ -195,14 +199,14 @@ static void frame_interval_monitor(struct imx_media_fim *fim,
- to interrupt latency.
*/ static void fim_input_capture_handler(int channel, void *dev_id,
struct timespec *ts)
ktime_t timestamp)spin_lock_irqsave(&fim->lock, flags);
- frame_interval_monitor(fim, ts);
- frame_interval_monitor(fim, timestamp);
if (!completion_done(&fim->icap_first_event)) complete(&fim->icap_first_event); @@ -405,14 +409,14 @@ static int init_fim_controls(struct imx_media_fim *fim)
- the frame_interval_monitor() is called by the input capture event
- callback handler in that case.
*/ -void imx_media_fim_eof_monitor(struct imx_media_fim *fim, struct timespec *ts) +void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp) { unsigned long flags; spin_lock_irqsave(&fim->lock, flags); if (!icap_enabled(fim))
frame_interval_monitor(fim, ts);
frame_interval_monitor(fim, timestamp);spin_unlock_irqrestore(&fim->lock, flags); } diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h index d409170632bd..ac3ab115394f 100644 --- a/drivers/staging/media/imx/imx-media.h +++ b/drivers/staging/media/imx/imx-media.h @@ -280,7 +280,7 @@ int imx_media_pipeline_set_stream(struct imx_media_dev *imxmd, /* imx-media-fim.c */ struct imx_media_fim; -void imx_media_fim_eof_monitor(struct imx_media_fim *fim, struct timespec *ts); +void imx_media_fim_eof_monitor(struct imx_media_fim *fim, ktime_t timestamp); int imx_media_fim_set_stream(struct imx_media_fim *fim, const struct v4l2_fract *frame_interval, bool on);
Hi Arnd,
Thank you for the patch.
On Monday, 27 November 2017 15:19:53 EET Arnd Bergmann wrote:
'struct timespec' works fine here, but we try to migrate away from it in favor of ktime_t or timespec64. In this case, using ktime_t produces the simplest code.
Signed-off-by: Arnd Bergmann arnd@arndb.de
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/media/usb/uvc/uvc_video.c | 11 ++++------- drivers/media/usb/uvc/uvcvideo.h | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index fb86d6af398d..d6bee37cd1b8 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -725,7 +725,7 @@ static void uvc_video_stats_decode(struct uvc_streaming *stream,
if (stream->stats.stream.nb_frames == 0 && stream->stats.frame.nb_packets == 0)
ktime_get_ts(&stream->stats.stream.start_ts);
stream->stats.stream.start_ts = ktime_get();switch (data[1] & (UVC_STREAM_PTS | UVC_STREAM_SCR)) { case UVC_STREAM_PTS | UVC_STREAM_SCR:
@@ -865,16 +865,13 @@ size_t uvc_video_stats_dump(struct uvc_streaming *stream, char *buf, { unsigned int scr_sof_freq; unsigned int duration;
struct timespec ts; size_t count = 0;
ts = timespec_sub(stream->stats.stream.stop_ts,
stream->stats.stream.start_ts);/* Compute the SCR.SOF frequency estimate. At the nominal 1kHz SOF
- frequency this will not overflow before more than 1h.
*/
duration = ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
- duration = ktime_ms_delta(stream->stats.stream.stop_ts,
stream->stats.stream.start_ts);@@ -915,7 +912,7 @@ static void uvc_video_stats_start(struct uvc_streaming *stream)
static void uvc_video_stats_stop(struct uvc_streaming *stream) {
- ktime_get_ts(&stream->stats.stream.stop_ts);
- stream->stats.stream.stop_ts = ktime_get();
}
/* ------------------------------------------------------------------------ diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 05398784d1c8..a2c190937067 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -452,8 +452,8 @@ struct uvc_stats_frame { };
struct uvc_stats_stream {
- struct timespec start_ts; /* Stream start timestamp */
- struct timespec stop_ts; /* Stream stop timestamp */
ktime_t start_ts; /* Stream start timestamp */
ktime_t stop_ts; /* Stream stop timestamp */
unsigned int nb_frames; /* Number of frames */
-
Andy Shevchenko -
Arnd Bergmann -
Hans Verkuil -
Laurent Pinchart -
Steve Longerbeam