diff mbox series

[v2,06/16] media: i2c: ov9282: Correct HTS register for configured pixel rate

Message ID 20221028160902.2696973-7-dave.stevenson@raspberrypi.com (mailing list archive)
State Accepted
Delegated to: Sakari Ailus
Headers
Series Updates to ov9282 sensor driver |

Commit Message

Dave Stevenson Oct. 28, 2022, 4:08 p.m. UTC 
  The calculations from pixel rate, width+hblank, and height+vblank
do not give the correct framerate - it's half the speed it should
be.

The datasheet lists the default for the TIMING_HTS registers (0x380c/d)
as being 0x2d8 (728) which is less than the width of the image, so
the units clearly can't be pixels.
If TIMING_HTS is considered to be units of 2-pixels, then the
resulting value of 0x5b0 (1456) makes all the calculations correct.

This driver is reporting an HBLANK value of 250, with an image width
of 1280, so TIMING_HTS is 1530 (0x5fa) pixels. However it was also
setting the register to 0x5fa, thereby not taking into account it
being units of 2-pixels.

Correct the register value to 0x2fd so that all the timing calculations
give the correct results.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/ov9282.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Oct. 31, 2022, 10:12 a.m. UTC | #1 
Hi Dave

On Fri, Oct 28, 2022 at 05:08:52PM +0100, Dave Stevenson wrote:
> The calculations from pixel rate, width+hblank, and height+vblank
> do not give the correct framerate - it's half the speed it should
> be.
>
> The datasheet lists the default for the TIMING_HTS registers (0x380c/d)
> as being 0x2d8 (728) which is less than the width of the image, so
> the units clearly can't be pixels.
> If TIMING_HTS is considered to be units of 2-pixels, then the
> resulting value of 0x5b0 (1456) makes all the calculations correct.
>
> This driver is reporting an HBLANK value of 250, with an image width
> of 1280, so TIMING_HTS is 1530 (0x5fa) pixels. However it was also
> setting the register to 0x5fa, thereby not taking into account it
> being units of 2-pixels.
>
> Correct the register value to 0x2fd so that all the timing calculations
> give the correct results.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Thanks, the commit message is now very clear

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  drivers/media/i2c/ov9282.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index 1524189cf3e5..7e0b12b89655 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -236,8 +236,8 @@ static const struct ov9282_reg mode_1280x720_regs[] = {
>  	{0x3809, 0x00},
>  	{0x380a, 0x02},
>  	{0x380b, 0xd0},
> -	{0x380c, 0x05},
> -	{0x380d, 0xfa},
> +	{0x380c, 0x02},
> +	{0x380d, 0xfd},
>  	{0x3810, 0x00},
>  	{0x3811, 0x08},
>  	{0x3812, 0x00},
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 1524189cf3e5..7e0b12b89655 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -236,8 +236,8 @@  static const struct ov9282_reg mode_1280x720_regs[] = {
 	{0x3809, 0x00},
 	{0x380a, 0x02},
 	{0x380b, 0xd0},
-	{0x380c, 0x05},
-	{0x380d, 0xfa},
+	{0x380c, 0x02},
+	{0x380d, 0xfd},
 	{0x3810, 0x00},
 	{0x3811, 0x08},
 	{0x3812, 0x00},