[PATCH] omap3isp: Fix histogram regions

[PATCH] omap3isp: Fix histogram regions

[Johannes Schellen]

From: Johannes Schellen <Johannes.Schellen@rwth-aachen.de>

This patch fixes a bug which causes all histogram regions to start in the
top left corner of the image. The histogram region coordinates are 16 bit
values which share a 32 bit register. The bug is due to the region end
value assignments overwriting the region start values with zero.
Signed-off-by: Johannes Schellen <Johannes.Schellen@rwth-aachen.de>
---
The patch is against v3.8-rc3

--- linux-3.8-rc3/drivers/media/platform/omap3isp/isphist.c.orig
+++ linux-3.8-rc3/drivers/media/platform/omap3isp/isphist.c
@@ -114,14 +114,14 @@ static void hist_setup_regs(struct ispst
 	/* Regions size and position */
 	for (c = 0; c < OMAP3ISP_HIST_MAX_REGIONS; c++) {
 		if (c < conf->num_regions) {
-			reg_hor[c] = conf->region[c].h_start <<
-				     ISPHIST_REG_START_SHIFT;
-			reg_hor[c] = conf->region[c].h_end <<
-				     ISPHIST_REG_END_SHIFT;
-			reg_ver[c] = conf->region[c].v_start <<
-				     ISPHIST_REG_START_SHIFT;
-			reg_ver[c] = conf->region[c].v_end <<
-				     ISPHIST_REG_END_SHIFT;
+			reg_hor[c] = (conf->region[c].h_start <<
+				     ISPHIST_REG_START_SHIFT)
+			           | (conf->region[c].h_end <<
+				     ISPHIST_REG_END_SHIFT);
+			reg_ver[c] = (conf->region[c].v_start <<
+				     ISPHIST_REG_START_SHIFT)
+			           | (conf->region[c].v_end <<
+				     ISPHIST_REG_END_SHIFT);
 		} else {
 			reg_hor[c] = 0;
 			reg_ver[c] = 0;

Re: [PATCH] omap3isp: Fix histogram regions

[Laurent Pinchart]

Hi Johannes,

On Friday 11 January 2013 16:00:19 Johannes Schellen wrote:
> From: Johannes Schellen <Johannes.Schellen@rwth-aachen.de>
> 
> This patch fixes a bug which causes all histogram regions to start in the
> top left corner of the image. The histogram region coordinates are 16 bit
> values which share a 32 bit register. The bug is due to the region end
> value assignments overwriting the region start values with zero.
> Signed-off-by: Johannes Schellen <Johannes.Schellen@rwth-aachen.de>

Good catch, thanks.

I've applied the patch to my tree.

> ---
> The patch is against v3.8-rc3
> 
> --- linux-3.8-rc3/drivers/media/platform/omap3isp/isphist.c.orig
> +++ linux-3.8-rc3/drivers/media/platform/omap3isp/isphist.c
> @@ -114,14 +114,14 @@ static void hist_setup_regs(struct ispst
>  	/* Regions size and position */
>  	for (c = 0; c < OMAP3ISP_HIST_MAX_REGIONS; c++) {
>  		if (c < conf->num_regions) {
> -			reg_hor[c] = conf->region[c].h_start <<
> -				     ISPHIST_REG_START_SHIFT;
> -			reg_hor[c] = conf->region[c].h_end <<
> -				     ISPHIST_REG_END_SHIFT;
> -			reg_ver[c] = conf->region[c].v_start <<
> -				     ISPHIST_REG_START_SHIFT;
> -			reg_ver[c] = conf->region[c].v_end <<
> -				     ISPHIST_REG_END_SHIFT;
> +			reg_hor[c] = (conf->region[c].h_start <<
> +				     ISPHIST_REG_START_SHIFT)
> +			           | (conf->region[c].h_end <<
> +				     ISPHIST_REG_END_SHIFT);
> +			reg_ver[c] = (conf->region[c].v_start <<
> +				     ISPHIST_REG_START_SHIFT)
> +			           | (conf->region[c].v_end <<
> +				     ISPHIST_REG_END_SHIFT);
>  		} else {
>  			reg_hor[c] = 0;
>  			reg_ver[c] = 0;

-- 
Regards,

Laurent Pinchart