diff mbox

[v2,8/9] ov772x: Compute window size registers at runtime

Message ID Pine.LNX.4.64.1207201554130.5505@axis700.grange (mailing list archive)
State New, archived
Headers show

Commit Message

Guennadi Liakhovetski July 20, 2012, 1:58 p.m. UTC 
Hi Laurent

Thanks for the patch

On Wed, 18 Jul 2012, Laurent Pinchart wrote:

> Instead of hardcoding register arrays, compute the values at runtime.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/video/ov772x.c |  162 +++++++++++++++++-------------------------
>  1 files changed, 65 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index 3874dbc..aa2ba9e 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c

I'm afraid, I still don't quite agree with your changes to size macros. 
This is not a huge deal, but I'd preserve the current (Q)VGA_* naming for 
now until we find a better solution. How about this patch merged with yours:


Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Laurent Pinchart July 20, 2012, 2:14 p.m. UTC | #1 
Hi Guennadi,

On Friday 20 July 2012 15:58:25 Guennadi Liakhovetski wrote:
> On Wed, 18 Jul 2012, Laurent Pinchart wrote:
> > Instead of hardcoding register arrays, compute the values at runtime.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/video/ov772x.c |  162
> >  +++++++++++++++++------------------------- 1 files changed, 65
> >  insertions(+), 97 deletions(-)
> > 
> > diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> > index 3874dbc..aa2ba9e 100644
> > --- a/drivers/media/video/ov772x.c
> > +++ b/drivers/media/video/ov772x.c
> 
> I'm afraid, I still don't quite agree with your changes to size macros.
> This is not a huge deal, but I'd preserve the current (Q)VGA_* naming for
> now until we find a better solution. How about this patch merged with yours:

I still like my solution better, but you're the driver maintainer :-)

> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index a2dde04..76a80b6 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c
> @@ -360,8 +360,12 @@
>  #define SCAL0_ACTRL     0x08 /* Auto scaling factor control */
>  #define SCAL1_2_ACTRL   0x04 /* Auto scaling factor control */
> 
> -#define OV772X_DEFAULT_WIDTH	640
> -#define OV772X_DEFAULT_HEIGHT	480
> +#define VGA_WIDTH		640
> +#define VGA_HEIGHT		480
> +#define QVGA_WIDTH		320
> +#define QVGA_HEIGHT		240
> +#define OV772X_MAX_WIDTH	VGA_WIDTH
> +#define OV772X_MAX_HEIGHT	VGA_HEIGHT
> 
>  /*
>   * ID
> @@ -488,8 +492,8 @@ static const struct ov772x_win_size ov772x_win_sizes[] =
> { .rect = {
>  			.left = 140,
>  			.top = 14,
> -			.width = 640,
> -			.height = 480,
> +			.width = VGA_WIDTH,
> +			.height = VGA_HEIGHT,
>  		},
>  	}, {
>  		.name     = "QVGA",
> @@ -497,8 +501,8 @@ static const struct ov772x_win_size ov772x_win_sizes[] =
> { .rect = {
>  			.left = 252,
>  			.top = 6,
> -			.width = 320,
> -			.height = 240,
> +			.width = QVGA_WIDTH,
> +			.height = QVGA_HEIGHT,
>  		},
>  	},
>  };
> @@ -858,8 +862,8 @@ static int ov772x_g_crop(struct v4l2_subdev *sd, struct
> v4l2_crop *a) {
>  	a->c.left	= 0;
>  	a->c.top	= 0;
> -	a->c.width	= OV772X_DEFAULT_WIDTH;
> -	a->c.height	= OV772X_DEFAULT_HEIGHT;
> +	a->c.width	= VGA_WIDTH;
> +	a->c.height	= VGA_HEIGHT;
>  	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
> 
>  	return 0;
> @@ -869,8 +873,8 @@ static int ov772x_cropcap(struct v4l2_subdev *sd, struct
> v4l2_cropcap *a) {
>  	a->bounds.left			= 0;
>  	a->bounds.top			= 0;
> -	a->bounds.width			= OV772X_DEFAULT_WIDTH;
> -	a->bounds.height		= OV772X_DEFAULT_HEIGHT;
> +	a->bounds.width			= OV772X_MAX_WIDTH;
> +	a->bounds.height		= OV772X_MAX_HEIGHT;
>  	a->defrect			= a->bounds;
>  	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  	a->pixelaspect.numerator	= 1;
diff mbox

Patch

diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index a2dde04..76a80b6 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -360,8 +360,12 @@ 
 #define SCAL0_ACTRL     0x08 /* Auto scaling factor control */
 #define SCAL1_2_ACTRL   0x04 /* Auto scaling factor control */
 
-#define OV772X_DEFAULT_WIDTH	640
-#define OV772X_DEFAULT_HEIGHT	480
+#define VGA_WIDTH		640
+#define VGA_HEIGHT		480
+#define QVGA_WIDTH		320
+#define QVGA_HEIGHT		240
+#define OV772X_MAX_WIDTH	VGA_WIDTH
+#define OV772X_MAX_HEIGHT	VGA_HEIGHT
 
 /*
  * ID
@@ -488,8 +492,8 @@  static const struct ov772x_win_size ov772x_win_sizes[] = {
 		.rect = {
 			.left = 140,
 			.top = 14,
-			.width = 640,
-			.height = 480,
+			.width = VGA_WIDTH,
+			.height = VGA_HEIGHT,
 		},
 	}, {
 		.name     = "QVGA",
@@ -497,8 +501,8 @@  static const struct ov772x_win_size ov772x_win_sizes[] = {
 		.rect = {
 			.left = 252,
 			.top = 6,
-			.width = 320,
-			.height = 240,
+			.width = QVGA_WIDTH,
+			.height = QVGA_HEIGHT,
 		},
 	},
 };
@@ -858,8 +862,8 @@  static int ov772x_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 {
 	a->c.left	= 0;
 	a->c.top	= 0;
-	a->c.width	= OV772X_DEFAULT_WIDTH;
-	a->c.height	= OV772X_DEFAULT_HEIGHT;
+	a->c.width	= VGA_WIDTH;
+	a->c.height	= VGA_HEIGHT;
 	a->type		= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
 	return 0;
@@ -869,8 +873,8 @@  static int ov772x_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
 {
 	a->bounds.left			= 0;
 	a->bounds.top			= 0;
-	a->bounds.width			= OV772X_DEFAULT_WIDTH;
-	a->bounds.height		= OV772X_DEFAULT_HEIGHT;
+	a->bounds.width			= OV772X_MAX_WIDTH;
+	a->bounds.height		= OV772X_MAX_HEIGHT;
 	a->defrect			= a->bounds;
 	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	a->pixelaspect.numerator	= 1;