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@xxxxxxxx>
> ---
> 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@xxxxxxxxx>
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];
>
