Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

media: atmel: atmel-isc: rework component offsets

The component offsets were computed in a negative way: they were
subtracted from the actual color component value.
So, a higher offset was reducing the component value.
This is not really desirable, as the offset is a 2's complements
number with 1 bit for sign and 12 value bits, so we would like to be able
to also add to the component, not only subtract.
The reported number in v4l2 is fine, a range from -4095 to +4095.
However when configuring a negative value for the offset, this would in
fact not function, because with the old code, the number was subtracted
from the max value. By setting something negative, it was overflowing in
fact. Reworked the component offsets by placing the real value as the
v4l2 ctrls.

Now, the values are the real number that is added or subtracted from
the component.
The negative values received from v4l2 are already in 2's complements, so
there is no need for conversion.
This actually simplifies a lot the computation procedure, eliminating the
need for the macros that convert from v4l2 values to ISC values and
viceversa.
Also the ZERO_VAL is eliminated, as 0 is now 0, as it's supposed to be.

Example after this change:

# v4l2-ctl --set-ctrl=red_component_offset=-150 -L

User Controls

brightness 0x00980900 (int) : min=-1024 max=1023 step=1 default=0 value=0 flags=slider
contrast 0x00980901 (int) : min=-2048 max=2047 step=1 default=16 value=20 flags=slider
white_balance_automatic 0x0098090c (bool) : default=1 value=0 flags=update
do_white_balance 0x0098090d (button) : flags=inactive, write-only, execute-on-write
gamma 0x00980910 (int) : min=0 max=3 step=1 default=3 value=3 flags=slider
red_component_gain 0x009819c0 (int) : min=0 max=8191 step=1 default=512 value=512 flags=slider
blue_component_gain 0x009819c1 (int) : min=0 max=8191 step=1 default=512 value=512 flags=slider
green_red_component_gain 0x009819c2 (int) : min=0 max=8191 step=1 default=512 value=512 flags=slider
green_blue_component_gain 0x009819c3 (int) : min=0 max=8191 step=1 default=512 value=512 flags=slider
red_component_offset 0x009819c4 (int) : min=-4095 max=4095 step=1 default=0 value=-150 flags=slider
blue_component_offset 0x009819c5 (int) : min=-4095 max=4095 step=1 default=0 value=0 flags=slider
green_red_component_offset 0x009819c6 (int) : min=-4095 max=4095 step=1 default=0 value=0 flags=slider
green_blue_component_offset 0x009819c7 (int) : min=-4095 max=4095 step=1 default=0 value=0 flags=slider

The auto white balance algorithm is unchanged, but the obtained value to
'subtract' is now converted to negative and saved as a v4l2 control and
displayed properly.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

authored by

Eugen Hristev and committed by
Mauro Carvalho Chehab
2e7ee8bb bb79974c

+25 -39
+24 -36
drivers/media/platform/atmel/atmel-isc-base.c
··· 225 225 (((mbus_code) == MEDIA_BUS_FMT_Y10_1X10) | \ 226 226 (((mbus_code) == MEDIA_BUS_FMT_Y8_1X8))) 227 227 228 - #define ISC_CTRL_ISC_TO_V4L2(x) ((x) == ISC_WB_O_ZERO_VAL ? 0 : (x)) 229 - #define ISC_CTRL_V4L2_TO_ISC(x) ((x) ? (x) : ISC_WB_O_ZERO_VAL) 230 - 231 228 static inline void isc_update_v4l2_ctrls(struct isc_device *isc) 232 229 { 233 230 struct isc_ctrls *ctrls = &isc->ctrls; ··· 235 238 v4l2_ctrl_s_ctrl(isc->gr_gain_ctrl, ctrls->gain[ISC_HIS_CFG_MODE_GR]); 236 239 v4l2_ctrl_s_ctrl(isc->gb_gain_ctrl, ctrls->gain[ISC_HIS_CFG_MODE_GB]); 237 240 238 - v4l2_ctrl_s_ctrl(isc->r_off_ctrl, 239 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_R])); 240 - v4l2_ctrl_s_ctrl(isc->b_off_ctrl, 241 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_B])); 242 - v4l2_ctrl_s_ctrl(isc->gr_off_ctrl, 243 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_GR])); 244 - v4l2_ctrl_s_ctrl(isc->gb_off_ctrl, 245 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_GB])); 241 + v4l2_ctrl_s_ctrl(isc->r_off_ctrl, ctrls->offset[ISC_HIS_CFG_MODE_R]); 242 + v4l2_ctrl_s_ctrl(isc->b_off_ctrl, ctrls->offset[ISC_HIS_CFG_MODE_B]); 243 + v4l2_ctrl_s_ctrl(isc->gr_off_ctrl, ctrls->offset[ISC_HIS_CFG_MODE_GR]); 244 + v4l2_ctrl_s_ctrl(isc->gb_off_ctrl, ctrls->offset[ISC_HIS_CFG_MODE_GB]); 246 245 } 247 246 248 247 static inline void isc_update_awb_ctrls(struct isc_device *isc) ··· 248 255 /* In here we set our actual hw pipeline config */ 249 256 250 257 regmap_write(isc->regmap, ISC_WB_O_RGR, 251 - (ISC_WB_O_ZERO_VAL - (ctrls->offset[ISC_HIS_CFG_MODE_R])) | 252 - ((ISC_WB_O_ZERO_VAL - ctrls->offset[ISC_HIS_CFG_MODE_GR]) << 16)); 258 + ((ctrls->offset[ISC_HIS_CFG_MODE_R])) | 259 + ((ctrls->offset[ISC_HIS_CFG_MODE_GR]) << 16)); 253 260 regmap_write(isc->regmap, ISC_WB_O_BGB, 254 - (ISC_WB_O_ZERO_VAL - (ctrls->offset[ISC_HIS_CFG_MODE_B])) | 255 - ((ISC_WB_O_ZERO_VAL - ctrls->offset[ISC_HIS_CFG_MODE_GB]) << 16)); 261 + ((ctrls->offset[ISC_HIS_CFG_MODE_B])) | 262 + ((ctrls->offset[ISC_HIS_CFG_MODE_GB]) << 16)); 256 263 regmap_write(isc->regmap, ISC_WB_G_RGR, 257 264 ctrls->gain[ISC_HIS_CFG_MODE_R] | 258 265 (ctrls->gain[ISC_HIS_CFG_MODE_GR] << 16)); ··· 268 275 for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) { 269 276 /* gains have a fixed point at 9 decimals */ 270 277 isc->ctrls.gain[c] = 1 << 9; 271 - /* offsets are in 2's complements, the value 272 - * will be substracted from ISC_WB_O_ZERO_VAL to obtain 273 - * 2's complement of a value between 0 and 274 - * ISC_WB_O_ZERO_VAL >> 1 275 - */ 276 - isc->ctrls.offset[c] = ISC_WB_O_ZERO_VAL; 278 + /* offsets are in 2's complements */ 279 + isc->ctrls.offset[c] = 0; 277 280 } 278 281 } 279 282 ··· 1763 1774 */ 1764 1775 ctrls->offset[c] = (offset[c] - 1) << 3; 1765 1776 1766 - /* the offset is then taken and converted to 2's complements */ 1767 - if (!ctrls->offset[c]) 1768 - ctrls->offset[c] = ISC_WB_O_ZERO_VAL; 1777 + /* 1778 + * the offset is then taken and converted to 2's complements, 1779 + * and must be negative, as we subtract this value from the 1780 + * color components 1781 + */ 1782 + ctrls->offset[c] = -ctrls->offset[c]; 1769 1783 1770 1784 /* 1771 1785 * the stretch gain is the total number of histogram bins ··· 1930 1938 ctrls->gain[ISC_HIS_CFG_MODE_GB] = isc->gb_gain_ctrl->val; 1931 1939 1932 1940 if (ctrl->cluster[ISC_CTRL_R_OFF]->is_new) 1933 - ctrls->offset[ISC_HIS_CFG_MODE_R] = 1934 - ISC_CTRL_V4L2_TO_ISC(isc->r_off_ctrl->val); 1941 + ctrls->offset[ISC_HIS_CFG_MODE_R] = isc->r_off_ctrl->val; 1935 1942 if (ctrl->cluster[ISC_CTRL_B_OFF]->is_new) 1936 - ctrls->offset[ISC_HIS_CFG_MODE_B] = 1937 - ISC_CTRL_V4L2_TO_ISC(isc->b_off_ctrl->val); 1943 + ctrls->offset[ISC_HIS_CFG_MODE_B] = isc->b_off_ctrl->val; 1938 1944 if (ctrl->cluster[ISC_CTRL_GR_OFF]->is_new) 1939 - ctrls->offset[ISC_HIS_CFG_MODE_GR] = 1940 - ISC_CTRL_V4L2_TO_ISC(isc->gr_off_ctrl->val); 1945 + ctrls->offset[ISC_HIS_CFG_MODE_GR] = isc->gr_off_ctrl->val; 1941 1946 if (ctrl->cluster[ISC_CTRL_GB_OFF]->is_new) 1942 - ctrls->offset[ISC_HIS_CFG_MODE_GB] = 1943 - ISC_CTRL_V4L2_TO_ISC(isc->gb_off_ctrl->val); 1947 + ctrls->offset[ISC_HIS_CFG_MODE_GB] = isc->gb_off_ctrl->val; 1944 1948 1945 1949 isc_update_awb_ctrls(isc); 1946 1950 ··· 1998 2010 ctrls->gain[ISC_HIS_CFG_MODE_GB]; 1999 2011 2000 2012 ctrl->cluster[ISC_CTRL_R_OFF]->val = 2001 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_R]); 2013 + ctrls->offset[ISC_HIS_CFG_MODE_R]; 2002 2014 ctrl->cluster[ISC_CTRL_B_OFF]->val = 2003 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_B]); 2015 + ctrls->offset[ISC_HIS_CFG_MODE_B]; 2004 2016 ctrl->cluster[ISC_CTRL_GR_OFF]->val = 2005 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_GR]); 2017 + ctrls->offset[ISC_HIS_CFG_MODE_GR]; 2006 2018 ctrl->cluster[ISC_CTRL_GB_OFF]->val = 2007 - ISC_CTRL_ISC_TO_V4L2(ctrls->offset[ISC_HIS_CFG_MODE_GB]); 2019 + ctrls->offset[ISC_HIS_CFG_MODE_GB]; 2008 2020 break; 2009 2021 } 2010 2022 return 0;
-2
drivers/media/platform/atmel/atmel-isc-regs.h
··· 108 108 /* ISC White Balance Gain for B, GB Register */ 109 109 #define ISC_WB_G_BGB 0x0000006c 110 110 111 - #define ISC_WB_O_ZERO_VAL (1 << 13) 112 - 113 111 /* ISC Color Filter Array Control Register */ 114 112 #define ISC_CFA_CTRL 0x00000070 115 113
+1 -1
drivers/media/platform/atmel/atmel-isc.h
··· 133 133 134 134 /* one for each component : GR, R, GB, B */ 135 135 u32 gain[HIST_BAYER]; 136 - u32 offset[HIST_BAYER]; 136 + s32 offset[HIST_BAYER]; 137 137 138 138 u32 hist_entry[HIST_ENTRIES]; 139 139 u32 hist_count[HIST_BAYER];