On Tue, Jun 8, 2021 at 8:25 AM Seongyong Park <euphoriccatface@xxxxxxxxx> wrote:
>
> On MLX90640, Each measurement step updates half of the pixels in the frame
> (every other pixel in default "chess mode", and every other row
> in "interleave mode"), while additional coefficient data (25th & 26th row)
> updates every step. The compensational coefficient data only corresponds
> with the pixels updated in the same step.
>
> Only way to know which "subpage" was updated on the last step is to read
> "status register" on address 0x8000. Without this data,
> compensation calculation may be able to detect which sets of pixels have
> been updated, but it will have to make assumptions when frame skip happens,
> and there is no way to do it correctly when the host simply cannot
> keep up with refresh rate.
>
> Signed-off-by: Seongyong Park <euphoriccatface@xxxxxxxxx>
> ---
> drivers/media/i2c/video-i2c.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> index 64ba96329..2b50a76f3 100644
> --- a/drivers/media/i2c/video-i2c.c
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -74,7 +74,8 @@ static const struct v4l2_fmtdesc mlx90640_format = {
>
> static const struct v4l2_frmsize_discrete mlx90640_size = {
> .width = 32,
> - .height = 26, /* 24 lines of pixel data + 2 lines of processing data */
> + .height = 27,
> + /* 24 lines of pixel data + 2 lines of processing data + 1 line of registers */
Guess you hit the 80 character line here and checkpatch.pl complained
.. But should all be one line since it is
much more clear on one line.
> };
>
> static const struct regmap_config amg88xx_regmap_config = {
> @@ -168,8 +169,12 @@ static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
>
> static int mlx90640_xfer(struct video_i2c_data *data, char *buf)
> {
> - return regmap_bulk_read(data->regmap, 0x400, buf,
> - data->chip->buffer_size);
> + int ret = regmap_bulk_read(data->regmap, 0x400, buf,
> + data->chip->buffer_size - 64);
> + if (ret)
> + return ret;
> + return regmap_bulk_read(data->regmap, 0x8000, buf + (data->chip->buffer_size - 64),
> + 64);
> }
>
> static int amg88xx_setup(struct video_i2c_data *data)
> @@ -375,7 +380,7 @@ static const struct video_i2c_chip video_i2c_chip[] = {
> .format = &mlx90640_format,
> .frame_intervals = mlx90640_frame_intervals,
> .num_frame_intervals = ARRAY_SIZE(mlx90640_frame_intervals),
> - .buffer_size = 1664,
> + .buffer_size = 1728,
Minus nitpick above looks good to me. You can keep the acked-by if
that is only change
Acked-by: Matt Ranostay <matt.ranostay@xxxxxxxxxxxx>
> .bpp = 16,
> .regmap_config = &mlx90640_regmap_config,
> .nvmem_config = &mlx90640_nvram_config,
> --
> 2.31.1
>
