Re: [PATCH v3 2/2] media: video-i2c: append register data on MLX90640's frame

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Seongyong,

On Wed, Jun 09, 2021 at 12:24:51AM +0900, Seongyong Park 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 */

This device should actually use a multi-plane format as the data isn't
Y16_BE as the driver declares. That said, the format would be device
specific and using one would require specific support from applications.

I might just declare it produces fewer lines while an application that
knows the device could access the extra data close to the end of the
buffer.

An alternative would be to use that multi-plane format and keep driver
support for the plain Y16_BE as well. But I think this would be a bit
heavy-weight solution.

I wonder what Hans thinks.

>  };
>  
>  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),

Please wrap before 80 (unless there's a reason to do otherwise).

> +				64);
>  }
>  

-- 
Kind regards,

Sakari Ailus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux