Re: [PATCH 4/6 v3] [media] ov9740: Remove hardcoded resolution regs

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

 



Hi Andrew,

Thanks for the patch.

On Friday 24 June 2011 01:19:42 achew@xxxxxxxxxx wrote:
> From: Andrew Chew <achew@xxxxxxxxxx>
> 
> Derive resolution-dependent register settings programmatically.
> 
> Signed-off-by: Andrew Chew <achew@xxxxxxxxxx>
> ---
>  drivers/media/video/ov9740.c |  210
> +++++++++++++++++++++++------------------- 1 files changed, 114
> insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c
> index 72c6ac1d..decd706 100644
> --- a/drivers/media/video/ov9740.c
> +++ b/drivers/media/video/ov9740.c
> @@ -181,27 +181,8 @@
>  #define OV9740_MIPI_CTRL_3012		0x3012
>  #define OV9740_SC_CMMM_MIPI_CTR		0x3014
> 
> -/* supported resolutions */
> -enum {
> -	OV9740_VGA,
> -	OV9740_720P,
> -};
> -
> -struct ov9740_resolution {
> -	unsigned int width;
> -	unsigned int height;
> -};
> -
> -static struct ov9740_resolution ov9740_resolutions[] = {
> -	[OV9740_VGA] = {
> -		.width	= 640,
> -		.height	= 480,
> -	},
> -	[OV9740_720P] = {
> -		.width	= 1280,
> -		.height	= 720,
> -	},
> -};
> +#define OV9740_MAX_WIDTH		1280
> +#define OV9740_MAX_HEIGHT		720
> 
>  /* Misc. structures */
>  struct ov9740_reg {
> @@ -403,54 +384,6 @@ static const struct ov9740_reg ov9740_defaults[] = {
>  	{ OV9740_ISP_CTRL19,		0x02 },
>  };
> 
> -static const struct ov9740_reg ov9740_regs_vga[] = {
> -	{ OV9740_X_ADDR_START_HI,	0x00 },
> -	{ OV9740_X_ADDR_START_LO,	0xa0 },
> -	{ OV9740_Y_ADDR_START_HI,	0x00 },
> -	{ OV9740_Y_ADDR_START_LO,	0x00 },
> -	{ OV9740_X_ADDR_END_HI,		0x04 },
> -	{ OV9740_X_ADDR_END_LO,		0x63 },
> -	{ OV9740_Y_ADDR_END_HI,		0x02 },
> -	{ OV9740_Y_ADDR_END_LO,		0xd3 },
> -	{ OV9740_X_OUTPUT_SIZE_HI,	0x02 },
> -	{ OV9740_X_OUTPUT_SIZE_LO,	0x80 },
> -	{ OV9740_Y_OUTPUT_SIZE_HI,	0x01 },
> -	{ OV9740_Y_OUTPUT_SIZE_LO,	0xe0 },
> -	{ OV9740_ISP_CTRL1E,		0x03 },
> -	{ OV9740_ISP_CTRL1F,		0xc0 },
> -	{ OV9740_ISP_CTRL20,		0x02 },
> -	{ OV9740_ISP_CTRL21,		0xd0 },
> -	{ OV9740_VFIFO_READ_START_HI,	0x01 },
> -	{ OV9740_VFIFO_READ_START_LO,	0x40 },
> -	{ OV9740_ISP_CTRL00,		0xff },
> -	{ OV9740_ISP_CTRL01,		0xff },
> -	{ OV9740_ISP_CTRL03,		0xff },
> -};
> -
> -static const struct ov9740_reg ov9740_regs_720p[] = {
> -	{ OV9740_X_ADDR_START_HI,	0x00 },
> -	{ OV9740_X_ADDR_START_LO,	0x00 },
> -	{ OV9740_Y_ADDR_START_HI,	0x00 },
> -	{ OV9740_Y_ADDR_START_LO,	0x00 },
> -	{ OV9740_X_ADDR_END_HI,		0x05 },
> -	{ OV9740_X_ADDR_END_LO,		0x03 },
> -	{ OV9740_Y_ADDR_END_HI,		0x02 },
> -	{ OV9740_Y_ADDR_END_LO,		0xd3 },
> -	{ OV9740_X_OUTPUT_SIZE_HI,	0x05 },
> -	{ OV9740_X_OUTPUT_SIZE_LO,	0x00 },
> -	{ OV9740_Y_OUTPUT_SIZE_HI,	0x02 },
> -	{ OV9740_Y_OUTPUT_SIZE_LO,	0xd0 },
> -	{ OV9740_ISP_CTRL1E,		0x05 },
> -	{ OV9740_ISP_CTRL1F,		0x00 },
> -	{ OV9740_ISP_CTRL20,		0x02 },
> -	{ OV9740_ISP_CTRL21,		0xd0 },
> -	{ OV9740_VFIFO_READ_START_HI,	0x02 },
> -	{ OV9740_VFIFO_READ_START_LO,	0x30 },
> -	{ OV9740_ISP_CTRL00,		0xff },
> -	{ OV9740_ISP_CTRL01,		0xef },
> -	{ OV9740_ISP_CTRL03,		0xff },
> -};
> -
>  static enum v4l2_mbus_pixelcode ov9740_codes[] = {
>  	V4L2_MBUS_FMT_YUYV8_2X8,
>  };
> @@ -727,39 +660,124 @@ static int ov9740_set_register(struct v4l2_subdev
> *sd, /* select nearest higher resolution for capture */
>  static void ov9740_res_roundup(u32 *width, u32 *height)
>  {
> -	int i;
> +	/* Width must be a multiple of 4 pixels. */
> +	*width = ALIGN(*width, 4);
> 
> -	for (i = 0; i < ARRAY_SIZE(ov9740_resolutions); i++)
> -		if ((ov9740_resolutions[i].width >= *width) &&
> -		    (ov9740_resolutions[i].height >= *height)) {
> -			*width = ov9740_resolutions[i].width;
> -			*height = ov9740_resolutions[i].height;
> -			return;
> -		}
> +	/* Max resolution is 1280x720 (720p). */
> +	if (*width > OV9740_MAX_WIDTH)
> +		*width = OV9740_MAX_WIDTH;
> 
> -	*width = ov9740_resolutions[OV9740_720P].width;
> -	*height = ov9740_resolutions[OV9740_720P].height;
> +	if (*height > OV9740_MAX_HEIGHT)
> +		*height = OV9740_MAX_HEIGHT;
>  }
> 
>  /* Setup registers according to resolution and color encoding */
> -static int ov9740_set_res(struct i2c_client *client, u32 width)
> +static int ov9740_set_res(struct i2c_client *client, u32 width, u32
> height) {
> +	u32 x_start;
> +	u32 y_start;
> +	u32 x_end;
> +	u32 y_end;
> +	bool scaling = 0;
> +	u32 scale_input_x;
> +	u32 scale_input_y;
>  	int ret;
> 
> -	/* select register configuration for given resolution */
> -	if (width == ov9740_resolutions[OV9740_VGA].width) {
> -		dev_dbg(&client->dev, "Setting image size to 640x480\n");
> -		ret = ov9740_reg_write_array(client, ov9740_regs_vga,
> -					     ARRAY_SIZE(ov9740_regs_vga));
> -	} else if (width == ov9740_resolutions[OV9740_720P].width) {
> -		dev_dbg(&client->dev, "Setting image size to 1280x720\n");
> -		ret = ov9740_reg_write_array(client, ov9740_regs_720p,
> -					     ARRAY_SIZE(ov9740_regs_720p));
> +	if ((width != OV9740_MAX_WIDTH) || (height != OV9740_MAX_HEIGHT))
> +		scaling = 1;
> +
> +	/*
> +	 * Try to use as much of the sensor area as possible when supporting
> +	 * smaller resolutions.  Depending on the aspect ratio of the
> +	 * chosen resolution, we can either use the full width of the sensor,
> +	 * or the full height of the sensor (or both if the aspect ratio is
> +	 * the same as 1280x720.
> +	 */
> +	if ((OV9740_MAX_WIDTH * height) > (OV9740_MAX_HEIGHT * width)) {
> +		scale_input_x = (OV9740_MAX_HEIGHT * width) / height;
> +		scale_input_y = OV9740_MAX_HEIGHT;
>  	} else {
> -		dev_err(&client->dev, "Failed to select resolution!\n");
> -		return -EINVAL;
> +		scale_input_x = OV9740_MAX_WIDTH;
> +		scale_input_y = (OV9740_MAX_WIDTH * height) / width;
>  	}
> 
> +	/* These describe the area of the sensor to use. */
> +	x_start = (OV9740_MAX_WIDTH - scale_input_x) / 2;
> +	y_start = (OV9740_MAX_HEIGHT - scale_input_y) / 2;
> +	x_end = x_start + scale_input_x - 1;
> +	y_end = y_start + scale_input_y - 1;
> +
> +	ret = ov9740_reg_write(client, OV9740_X_ADDR_START_HI, x_start >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_X_ADDR_START_LO, x_start & 0xff);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_Y_ADDR_START_HI, y_start >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_Y_ADDR_START_LO, y_start & 0xff);
> +	if (ret)
> +		goto done;
> +
> +	ret = ov9740_reg_write(client, OV9740_X_ADDR_END_HI, x_end >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_X_ADDR_END_LO, x_end & 0xff);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_Y_ADDR_END_HI, y_end >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_Y_ADDR_END_LO, y_end & 0xff);
> +	if (ret)
> +		goto done;
> +
> +	ret = ov9740_reg_write(client, OV9740_X_OUTPUT_SIZE_HI, width >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_X_OUTPUT_SIZE_LO, width & 0xff);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_Y_OUTPUT_SIZE_HI, height >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_Y_OUTPUT_SIZE_LO, height & 0xff);
> +	if (ret)
> +		goto done;
> +
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL1E, scale_input_x >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL1F, scale_input_x & 0xff);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL20, scale_input_y >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL21, scale_input_y & 0xff);
> +	if (ret)
> +		goto done;

Now that we know what control registers 1e to 21 are for, what about renaming 
them ? Are they called CTRL1E to CTRL21 in the vendor documentation ?

> +
> +	ret = ov9740_reg_write(client, OV9740_VFIFO_READ_START_HI,
> +			       (scale_input_x - width) >> 8);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_VFIFO_READ_START_LO,
> +			       (scale_input_x - width) & 0xff);
> +	if (ret)
> +		goto done;
> +
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL00, 0xff);
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL01, 0xef |
> +							  (scaling << 4));
> +	if (ret)
> +		goto done;
> +	ret = ov9740_reg_write(client, OV9740_ISP_CTRL03, 0xff);
> +
> +done:
>  	return ret;
>  }
> 
> @@ -787,7 +805,7 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
>  	if (ret < 0)
>  		return ret;
> 
> -	ret = ov9740_set_res(client, mf->width);
> +	ret = ov9740_set_res(client, mf->width, mf->height);
>  	if (ret < 0)
>  		return ret;
> 
> @@ -824,8 +842,8 @@ static int ov9740_cropcap(struct v4l2_subdev *sd,
> struct v4l2_cropcap *a) {
>  	a->bounds.left		= 0;
>  	a->bounds.top		= 0;
> -	a->bounds.width		= ov9740_resolutions[OV9740_720P].width;
> -	a->bounds.height	= ov9740_resolutions[OV9740_720P].height;
> +	a->bounds.width		= OV9740_MAX_WIDTH;
> +	a->bounds.height	= OV9740_MAX_HEIGHT;
>  	a->defrect		= a->bounds;
>  	a->type			= V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  	a->pixelaspect.numerator	= 1;
> @@ -838,8 +856,8 @@ static int ov9740_g_crop(struct v4l2_subdev *sd, struct
> v4l2_crop *a) {
>  	a->c.left		= 0;
>  	a->c.top		= 0;
> -	a->c.width		= ov9740_resolutions[OV9740_720P].width;
> -	a->c.height		= ov9740_resolutions[OV9740_720P].height;
> +	a->c.width		= OV9740_MAX_WIDTH;
> +	a->c.height		= OV9740_MAX_HEIGHT;
>  	a->type			= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> 
>  	return 0;

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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