linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] media: atmel: atmel-isc: new features
@ 2019-04-15 14:13 Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 1/4] media: atmel: atmel-isc: reworked white balance feature Eugen.Hristev
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Eugen.Hristev @ 2019-04-15 14:13 UTC (permalink / raw)
  To: linux-media, hverkuil, linux-arm-kernel, linux-kernel
  Cc: Nicolas.Ferre, mchehab, ksloat, Eugen.Hristev

From: Eugen Hristev <eugen.hristev@microchip.com>

This series is based on top of my previous commits/series, namely:
media: atmel: atmel-isc: fix asd memory allocation
media: atmel: atmel-isc: fix INIT_WORK misplacement
media: atmel: atmel-isc: limit incoming pixels per frame
media: atmel: atmel-isc: removed ARGB32 added ABGR32 and XBGR32
media: atmel: atmel-isc: reworked driver and formats

it will not build if applied directly

This series include the rework of the white balance,
the one time white balance adjustment, and a change in try_fmt error message
handling.

Changes in v2:
 - reworked error messages for do_white_balance according to review from Hans

v4l2-compliance:

~# v4l2-compliance
v4l2-compliance SHA: 32cf495ff5da24df54936fae3bf0eb91fba77f3a, 32 bits

Compliance test for atmel_isc device /devatmelo0:_isc f0008000.isc: =================  START STATUS  =================

atDriver Info:
        Driver name      : atmel_isc
        Card type        : Atmel Image Sensor Controller
        Bus info         : platform:atmel_isc f0008000.isc
        Driver version   : 5.1.0
        Capabilities     : 0x84200001
                Video Capture
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x04200001
                Video Capture
                Streaming
                Extended Pix Format

Required ioctls:
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second /dev/video0 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
mel_isc f0008000.isc: Brightness: 0
atmel_isc f0008000.isc: Contrast: 256
atmel_isc f0008000.isc: Gamma: 2
atmel_isc f0008000.isc: White Balance, Automatic: true
atmel_isc f0008000.isc: ==================  END STATUS  ==================
        test VIDIOC_LOG_STATUS: OK

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
        test VIDIOC_QUERYCTRL: OK
        test VIDIOC_G/S_CTRL: OK
        test VIDIOC_G/S/TRY_EXT_CTRLS: OK
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 6 Private Controls: 0

Format ioctls (Input 0):
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
        test VIDIOC_G/S_PARM: OK
        test VIDIOC_G_FBUF: OK (Not Supported)
        test VIDIOC_G_FMT: OK
        test VIDIOC_TRY_FMT: OK
        test VIDIOC_S_FMT: OK
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls (Input 0):
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Total for atmel_isc device /dev/video0: 44, Succeeded: 44, Failed: 0, Warnings: 0
root@sama5d2-xplained:~#



Eugen Hristev (4):
  media: atmel: atmel-isc: reworked white balance feature
  media: v4l2-ctrl: fix flags for DO_WHITE_BALANCE
  media: atmel: atmel-isc: add support for DO_WHITE_BALANCE
  media: atmel: atmel-isc: make try_fmt error less verbose

 drivers/media/platform/atmel/atmel-isc-regs.h |   6 +-
 drivers/media/platform/atmel/atmel-isc.c      | 263 +++++++++++++++++++++++---
 drivers/media/v4l2-core/v4l2-ctrls.c          |   1 +
 3 files changed, 240 insertions(+), 30 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/4] media: atmel: atmel-isc: reworked white balance feature
  2019-04-15 14:13 [PATCH v2 0/4] media: atmel: atmel-isc: new features Eugen.Hristev
@ 2019-04-15 14:13 ` Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 2/4] media: v4l2-ctrl: fix flags for DO_WHITE_BALANCE Eugen.Hristev
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Eugen.Hristev @ 2019-04-15 14:13 UTC (permalink / raw)
  To: linux-media, hverkuil, linux-arm-kernel, linux-kernel
  Cc: Nicolas.Ferre, mchehab, ksloat, Eugen.Hristev

From: Eugen Hristev <eugen.hristev@microchip.com>

Reworked auto white balance feature (awb) to cope with all four channels.
Implemented stretching and grey world algorithms.
Using the histogram, the ISC will auto adjust the white balance during
frame captures.
Because each histogram needs a frame, it will take 4 frames for one adjustment.
When the gains were updated by previous code, the registers for the gains
were updated only on new streaming start. Now, after each full histogram the
registers are updated with new gains.
Also, on previous code, if the streaming stopped but not all 3 histograms
finished, a new histogram was started either way. This used to lead to an
error "timeout to update profile" when streaming was stopped.
According to the hardware, histogram can only work together with the capture,
not independently.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/media/platform/atmel/atmel-isc-regs.h |   6 +-
 drivers/media/platform/atmel/atmel-isc.c      | 200 +++++++++++++++++++++++---
 2 files changed, 181 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h b/drivers/media/platform/atmel/atmel-isc-regs.h
index 8f7f8ef..c1283fb 100644
--- a/drivers/media/platform/atmel/atmel-isc-regs.h
+++ b/drivers/media/platform/atmel/atmel-isc-regs.h
@@ -100,13 +100,15 @@
 #define ISC_WB_O_RGR	0x00000060
 
 /* ISC White Balance Offset for B, GB Register */
-#define ISC_WB_O_BGR	0x00000064
+#define ISC_WB_O_BGB	0x00000064
 
 /* ISC White Balance Gain for R, GR Register */
 #define ISC_WB_G_RGR	0x00000068
 
 /* ISC White Balance Gain for B, GB Register */
-#define ISC_WB_G_BGR	0x0000006c
+#define ISC_WB_G_BGB	0x0000006c
+
+#define ISC_WB_O_ZERO_VAL	(1 << 13)
 
 /* ISC Color Filter Array Control Register */
 #define ISC_CFA_CTRL    0x00000070
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index 94cb309..0ac5953 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -169,13 +169,17 @@ struct isc_ctrls {
 	u8 gamma_index;
 	u8 awb;
 
-	u32 r_gain;
-	u32 b_gain;
+	/* one for each component : GR, R, GB, B */
+	u32 gain[HIST_BAYER];
+	u32 offset[HIST_BAYER];
 
 	u32 hist_entry[HIST_ENTRIES];
 	u32 hist_count[HIST_BAYER];
 	u8 hist_id;
 	u8 hist_stat;
+#define HIST_MIN_INDEX		0
+#define HIST_MAX_INDEX		1
+	u32 hist_minmax[HIST_BAYER][2];
 };
 
 #define ISC_PIPE_LINE_NODE_NUM	11
@@ -209,6 +213,7 @@ struct isc_device {
 	struct work_struct	awb_work;
 
 	struct mutex		lock;
+	spinlock_t		awb_lock;
 
 	struct regmap_field	*pipeline[ISC_PIPE_LINE_NODE_NUM];
 
@@ -395,6 +400,40 @@ module_param(sensor_preferred, uint, 0644);
 MODULE_PARM_DESC(sensor_preferred,
 		 "Sensor is preferred to output the specified format (1-on 0-off), default 1");
 
+static inline void isc_update_awb_ctrls(struct isc_device *isc)
+{
+	struct isc_ctrls *ctrls = &isc->ctrls;
+
+	regmap_write(isc->regmap, ISC_WB_O_RGR,
+		     (ISC_WB_O_ZERO_VAL - (ctrls->offset[ISC_HIS_CFG_MODE_R])) |
+		     ((ISC_WB_O_ZERO_VAL - ctrls->offset[ISC_HIS_CFG_MODE_GR]) << 16));
+	regmap_write(isc->regmap, ISC_WB_O_BGB,
+		     (ISC_WB_O_ZERO_VAL - (ctrls->offset[ISC_HIS_CFG_MODE_B])) |
+		     ((ISC_WB_O_ZERO_VAL - ctrls->offset[ISC_HIS_CFG_MODE_GB]) << 16));
+	regmap_write(isc->regmap, ISC_WB_G_RGR,
+		     ctrls->gain[ISC_HIS_CFG_MODE_R] |
+		     (ctrls->gain[ISC_HIS_CFG_MODE_GR] << 16));
+	regmap_write(isc->regmap, ISC_WB_G_BGB,
+		     ctrls->gain[ISC_HIS_CFG_MODE_B] |
+		     (ctrls->gain[ISC_HIS_CFG_MODE_GB] << 16));
+}
+
+static inline void isc_reset_awb_ctrls(struct isc_device *isc)
+{
+	int c;
+
+	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
+		/* gains have a fixed point at 9 decimals */
+		isc->ctrls.gain[c] = 1 << 9;
+		/* offsets are in 2's complements, the value
+		 * will be substracted from ISC_WB_O_ZERO_VAL to obtain
+		 * 2's complement of a value between 0 and
+		 * ISC_WB_O_ZERO_VAL >> 1
+		 */
+		isc->ctrls.offset[c] = ISC_WB_O_ZERO_VAL;
+	}
+}
+
 static int isc_wait_clk_stable(struct clk_hw *hw)
 {
 	struct isc_clk *isc_clk = to_isc_clk(hw);
@@ -775,7 +814,9 @@ static void isc_start_dma(struct isc_device *isc)
 	dctrl_dview = isc->config.dctrl_dview;
 
 	regmap_write(regmap, ISC_DCTRL, dctrl_dview | ISC_DCTRL_IE_IS);
+	spin_lock(&isc->awb_lock);
 	regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_CAPTURE);
+	spin_unlock(&isc->awb_lock);
 }
 
 static void isc_set_pipeline(struct isc_device *isc, u32 pipeline)
@@ -797,11 +838,11 @@ static void isc_set_pipeline(struct isc_device *isc, u32 pipeline)
 
 	bay_cfg = isc->config.sd_format->cfa_baycfg;
 
+	if (!ctrls->awb)
+		isc_reset_awb_ctrls(isc);
+
 	regmap_write(regmap, ISC_WB_CFG, bay_cfg);
-	regmap_write(regmap, ISC_WB_O_RGR, 0x0);
-	regmap_write(regmap, ISC_WB_O_BGR, 0x0);
-	regmap_write(regmap, ISC_WB_G_RGR, ctrls->r_gain | (0x1 << 25));
-	regmap_write(regmap, ISC_WB_G_BGR, ctrls->b_gain | (0x1 << 25));
+	isc_update_awb_ctrls(isc);
 
 	regmap_write(regmap, ISC_CFA_CFG, bay_cfg | ISC_CFA_CFG_EITPOL);
 
@@ -851,13 +892,13 @@ static void isc_set_histogram(struct isc_device *isc, bool enable)
 
 	if (enable) {
 		regmap_write(regmap, ISC_HIS_CFG,
-			     ISC_HIS_CFG_MODE_R |
+			     ISC_HIS_CFG_MODE_GR |
 			     (isc->config.sd_format->cfa_baycfg
 					<< ISC_HIS_CFG_BAYSEL_SHIFT) |
 					ISC_HIS_CFG_RAR);
 		regmap_write(regmap, ISC_HIS_CTRL, ISC_HIS_CTRL_EN);
 		regmap_write(regmap, ISC_INTEN, ISC_INT_HISDONE);
-		ctrls->hist_id = ISC_HIS_CFG_MODE_R;
+		ctrls->hist_id = ISC_HIS_CFG_MODE_GR;
 		isc_update_profile(isc);
 		regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
 
@@ -900,7 +941,7 @@ static int isc_configure(struct isc_device *isc)
 	isc_set_pipeline(isc, pipeline);
 
 	/*
-	 * The current implemented histogram is available for RAW R, B, GB
+	 * The current implemented histogram is available for RAW R, B, GB, GR
 	 * channels. We need to check if sensor is outputting RAW BAYER
 	 */
 	if (isc->ctrls.awb &&
@@ -1475,6 +1516,12 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
 		return ret;
 
 	isc->fmt = *f;
+
+	if (isc->try_config.sd_format && isc->config.sd_format &&
+	    isc->try_config.sd_format != isc->config.sd_format) {
+		isc->ctrls.hist_stat = HIST_INIT;
+		isc_reset_awb_ctrls(isc);
+	}
 	/* make the try configuration active */
 	isc->config = isc->try_config;
 
@@ -1758,7 +1805,7 @@ static irqreturn_t isc_interrupt(int irq, void *dev_id)
 	return ret;
 }
 
-static void isc_hist_count(struct isc_device *isc)
+static void isc_hist_count(struct isc_device *isc, u32 *min, u32 *max)
 {
 	struct regmap *regmap = isc->regmap;
 	struct isc_ctrls *ctrls = &isc->ctrls;
@@ -1766,25 +1813,99 @@ static void isc_hist_count(struct isc_device *isc)
 	u32 *hist_entry = &ctrls->hist_entry[0];
 	u32 i;
 
+	*min = 0;
+	*max = HIST_ENTRIES;
+
 	regmap_bulk_read(regmap, ISC_HIS_ENTRY, hist_entry, HIST_ENTRIES);
 
 	*hist_count = 0;
-	for (i = 0; i < HIST_ENTRIES; i++)
+	/*
+	 * we deliberately ignore the end of the histogram,
+	 * the most white pixels
+	 */
+	for (i = 1; i < HIST_ENTRIES; i++) {
+		if (*hist_entry && !*min)
+			*min = i;
+		if (*hist_entry)
+			*max = i;
 		*hist_count += i * (*hist_entry++);
+	}
+
+	if (!*min)
+		*min = 1;
 }
 
 static void isc_wb_update(struct isc_ctrls *ctrls)
 {
 	u32 *hist_count = &ctrls->hist_count[0];
-	u64 g_count = (u64)hist_count[ISC_HIS_CFG_MODE_GB] << 9;
-	u32 hist_r = hist_count[ISC_HIS_CFG_MODE_R];
-	u32 hist_b = hist_count[ISC_HIS_CFG_MODE_B];
+	u32 c, offset[4];
+	u64 avg = 0;
+	/* We compute two gains, stretch gain and grey world gain */
+	u32 s_gain[4], gw_gain[4];
 
-	if (hist_r)
-		ctrls->r_gain = div_u64(g_count, hist_r);
+	/*
+	 * According to Grey World, we need to set gains for R/B to normalize
+	 * them towards the green channel.
+	 * Thus we want to keep Green as fixed and adjust only Red/Blue
+	 * Compute the average of the both green channels first
+	 */
+	avg = (u64)hist_count[ISC_HIS_CFG_MODE_GR] +
+		(u64)hist_count[ISC_HIS_CFG_MODE_GB];
+	avg >>= 1;
+
+	/* Green histogram is null, nothing to do */
+	if (!avg)
+		return;
 
-	if (hist_b)
-		ctrls->b_gain = div_u64(g_count, hist_b);
+	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
+		/*
+		 * the color offset is the minimum value of the histogram.
+		 * we stretch this color to the full range by substracting
+		 * this value from the color component.
+		 */
+		offset[c] = ctrls->hist_minmax[c][HIST_MIN_INDEX];
+		/*
+		 * The offset is always at least 1. If the offset is 1, we do
+		 * not need to adjust it, so our result must be zero.
+		 * the offset is computed in a histogram on 9 bits (0..512)
+		 * but the offset in register is based on
+		 * 12 bits pipeline (0..4096).
+		 * we need to shift with the 3 bits that the histogram is
+		 * ignoring
+		 */
+		ctrls->offset[c] = (offset[c] - 1) << 3;
+
+		/* the offset is then taken and converted to 2's complements */
+		if (!ctrls->offset[c])
+			ctrls->offset[c] = ISC_WB_O_ZERO_VAL;
+
+		/*
+		 * the stretch gain is the total number of histogram bins
+		 * divided by the actual range of color component (Max - Min)
+		 * If we compute gain like this, the actual color component
+		 * will be stretched to the full histogram.
+		 * We need to shift 9 bits for precision, we have 9 bits for
+		 * decimals
+		 */
+		s_gain[c] = (HIST_ENTRIES << 9) /
+			(ctrls->hist_minmax[c][HIST_MAX_INDEX] -
+			ctrls->hist_minmax[c][HIST_MIN_INDEX] + 1);
+
+		/*
+		 * Now we have to compute the gain w.r.t. the average.
+		 * Add/lose gain to the component towards the average.
+		 * If it happens that the component is zero, use the
+		 * fixed point value : 1.0 gain.
+		 */
+		if (hist_count[c])
+			gw_gain[c] = div_u64(avg << 9, hist_count[c]);
+		else
+			gw_gain[c] = 1 << 9;
+
+		/* multiply both gains and adjust for decimals */
+		ctrls->gain[c] = s_gain[c] * gw_gain[c];
+		ctrls->gain[c] >>= 9;
+	}
 }
 
 static void isc_awb_work(struct work_struct *w)
@@ -1795,27 +1916,56 @@ static void isc_awb_work(struct work_struct *w)
 	struct isc_ctrls *ctrls = &isc->ctrls;
 	u32 hist_id = ctrls->hist_id;
 	u32 baysel;
+	unsigned long flags;
+	u32 min, max;
+
+	/* streaming is not active anymore */
+	if (isc->stop)
+		return;
 
 	if (ctrls->hist_stat != HIST_ENABLED)
 		return;
 
-	isc_hist_count(isc);
+	isc_hist_count(isc, &min, &max);
+	ctrls->hist_minmax[hist_id][HIST_MIN_INDEX] = min;
+	ctrls->hist_minmax[hist_id][HIST_MAX_INDEX] = max;
 
 	if (hist_id != ISC_HIS_CFG_MODE_B) {
 		hist_id++;
 	} else {
 		isc_wb_update(ctrls);
-		hist_id = ISC_HIS_CFG_MODE_R;
+		hist_id = ISC_HIS_CFG_MODE_GR;
 	}
 
 	ctrls->hist_id = hist_id;
 	baysel = isc->config.sd_format->cfa_baycfg << ISC_HIS_CFG_BAYSEL_SHIFT;
 
+	/* if no more auto white balance, reset controls. */
+	if (!ctrls->awb)
+		isc_reset_awb_ctrls(isc);
+
 	pm_runtime_get_sync(isc->dev);
 
+	/*
+	 * only update if we have all the required histograms and controls
+	 * if awb has been disabled, we need to reset registers as well.
+	 */
+	if (hist_id == ISC_HIS_CFG_MODE_GR || !ctrls->awb) {
+		/*
+		 * It may happen that DMA Done IRQ will trigger while we are
+		 * updating white balance registers here.
+		 * In that case, only parts of the controls have been updated.
+		 * We can avoid that by locking the section.
+		 */
+		spin_lock_irqsave(&isc->awb_lock, flags);
+		isc_update_awb_ctrls(isc);
+		spin_unlock_irqrestore(&isc->awb_lock, flags);
+	}
 	regmap_write(regmap, ISC_HIS_CFG, hist_id | baysel | ISC_HIS_CFG_RAR);
 	isc_update_profile(isc);
-	regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
+	/* if awb has been disabled, we don't need to start another histogram */
+	if (ctrls->awb)
+		regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
 
 	pm_runtime_put_sync(isc->dev);
 }
@@ -1839,8 +1989,7 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_AUTO_WHITE_BALANCE:
 		ctrls->awb = ctrl->val;
 		if (ctrls->hist_stat != HIST_ENABLED) {
-			ctrls->r_gain = 0x1 << 9;
-			ctrls->b_gain = 0x1 << 9;
+			isc_reset_awb_ctrls(isc);
 		}
 		break;
 	default:
@@ -1862,11 +2011,15 @@ static int isc_ctrl_init(struct isc_device *isc)
 	int ret;
 
 	ctrls->hist_stat = HIST_INIT;
+	isc_reset_awb_ctrls(isc);
 
 	ret = v4l2_ctrl_handler_init(hdl, 4);
 	if (ret < 0)
 		return ret;
 
+	ctrls->brightness = 0;
+	ctrls->contrast = 256;
+
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -1024, 1023, 1, 0);
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_CONTRAST, -2048, 2047, 1, 256);
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, GAMMA_MAX, 1, 2);
@@ -2034,6 +2187,7 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
 	/* Init video dma queues */
 	INIT_LIST_HEAD(&isc->dma_queue);
 	spin_lock_init(&isc->dma_queue_lock);
+	spin_lock_init(&isc->awb_lock);
 
 	ret = isc_formats_init(isc);
 	if (ret < 0) {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/4] media: v4l2-ctrl: fix flags for DO_WHITE_BALANCE
  2019-04-15 14:13 [PATCH v2 0/4] media: atmel: atmel-isc: new features Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 1/4] media: atmel: atmel-isc: reworked white balance feature Eugen.Hristev
@ 2019-04-15 14:13 ` Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 3/4] media: atmel: atmel-isc: add support " Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 4/4] media: atmel: atmel-isc: make try_fmt error less verbose Eugen.Hristev
  3 siblings, 0 replies; 5+ messages in thread
From: Eugen.Hristev @ 2019-04-15 14:13 UTC (permalink / raw)
  To: linux-media, hverkuil, linux-arm-kernel, linux-kernel
  Cc: Nicolas.Ferre, mchehab, ksloat, Eugen.Hristev

From: Eugen Hristev <eugen.hristev@microchip.com>

Control DO_WHITE_BALANCE is a button, with read only and execute-on-write flags.
Adding this control in the proper list in the fill function.

After adding it here, we can see output of v4l2-ctl -L
do_white_balance 0x0098090d (button) : flags=write-only, execute-on-write

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index b1ae2e5..388ea90 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1153,6 +1153,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_FLASH_STROBE_STOP:
 	case V4L2_CID_AUTO_FOCUS_START:
 	case V4L2_CID_AUTO_FOCUS_STOP:
+	case V4L2_CID_DO_WHITE_BALANCE:
 		*type = V4L2_CTRL_TYPE_BUTTON;
 		*flags |= V4L2_CTRL_FLAG_WRITE_ONLY |
 			  V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/4] media: atmel: atmel-isc: add support for DO_WHITE_BALANCE
  2019-04-15 14:13 [PATCH v2 0/4] media: atmel: atmel-isc: new features Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 1/4] media: atmel: atmel-isc: reworked white balance feature Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 2/4] media: v4l2-ctrl: fix flags for DO_WHITE_BALANCE Eugen.Hristev
@ 2019-04-15 14:13 ` Eugen.Hristev
  2019-04-15 14:13 ` [PATCH v2 4/4] media: atmel: atmel-isc: make try_fmt error less verbose Eugen.Hristev
  3 siblings, 0 replies; 5+ messages in thread
From: Eugen.Hristev @ 2019-04-15 14:13 UTC (permalink / raw)
  To: linux-media, hverkuil, linux-arm-kernel, linux-kernel
  Cc: Nicolas.Ferre, mchehab, ksloat, Eugen.Hristev

From: Eugen Hristev <eugen.hristev@microchip.com>

This adds support for the 'button' control DO_WHITE_BALANCE
This feature will enable the ISC to compute the white balance coefficients
in a one time shot, at the user discretion.
This can be used if a color chart/grey chart is present in front of the camera.
The ISC will adjust the coefficients and have them fixed until next balance
or until sensor mode is changed.
This is particularly useful for white balance adjustment in different
lighting scenarios, and then taking photos to similar scenery.
The old auto white balance stays in place, where the ISC will adjust every
4 frames to the current scenery lighting, if the scenery is approximately
grey in average, otherwise grey world algorithm fails.
One time white balance adjustments needs streaming to be enabled, such that
capture is enabled and the histogram has data to work with.
Histogram without capture does not work in this hardware module.

To start the one time white balance procedure:
v4l2-ctl --set-ctrl=do_white_balance=1

This feature works only if the sensor is streaming RAW data, as the hardware
supports a histogram only for RAW bayer components.

If the auto white balance is enabled, do_white_balance does nothing.
If the streaming is disabled, or the sensor does not output RAW data, the
control is inactive.

User controls now include the do_white_balance ctrl:
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=256 value=256 flags=slider
        white_balance_automatic 0x0098090c (bool)   : default=1 value=0
               do_white_balance 0x0098090d (button) : flags=write-only, execute-on-write
                          gamma 0x00980910 (int)    : min=0 max=2 step=1 default=2 value=2 flags=slider

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
Changes in v2:
 - changed according to review by Hans: DO_WB works evne if AWB is enabled and
does nothing; use active/inactive on do_wb ctrl to make it inactive if
not streaming or not raw; do not do anything on inactive ctrls in ctrls
callback.

 drivers/media/platform/atmel/atmel-isc.c | 66 ++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index 0ac5953..777e27f 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -167,6 +167,9 @@ struct isc_ctrls {
 	u32 brightness;
 	u32 contrast;
 	u8 gamma_index;
+#define ISC_WB_NONE	0
+#define ISC_WB_AUTO	1
+#define ISC_WB_ONETIME	2
 	u8 awb;
 
 	/* one for each component : GR, R, GB, B */
@@ -210,6 +213,7 @@ struct isc_device {
 	struct fmt_config	try_config;
 
 	struct isc_ctrls	ctrls;
+	struct v4l2_ctrl	*do_wb_ctrl;
 	struct work_struct	awb_work;
 
 	struct mutex		lock;
@@ -838,7 +842,7 @@ static void isc_set_pipeline(struct isc_device *isc, u32 pipeline)
 
 	bay_cfg = isc->config.sd_format->cfa_baycfg;
 
-	if (!ctrls->awb)
+	if (ctrls->awb == ISC_WB_NONE)
 		isc_reset_awb_ctrls(isc);
 
 	regmap_write(regmap, ISC_WB_CFG, bay_cfg);
@@ -993,6 +997,10 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
 
 	spin_unlock_irqrestore(&isc->dma_queue_lock, flags);
 
+	/* if we streaming from RAW, we can do one-shot white balance adj */
+	if (ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code))
+		v4l2_ctrl_activate(isc->do_wb_ctrl, true);
+
 	return 0;
 
 err_configure:
@@ -1017,6 +1025,8 @@ static void isc_stop_streaming(struct vb2_queue *vq)
 	struct isc_buffer *buf;
 	int ret;
 
+	v4l2_ctrl_activate(isc->do_wb_ctrl, false);
+
 	isc->stop = true;
 
 	/* Wait until the end of the current frame */
@@ -1941,7 +1951,7 @@ static void isc_awb_work(struct work_struct *w)
 	baysel = isc->config.sd_format->cfa_baycfg << ISC_HIS_CFG_BAYSEL_SHIFT;
 
 	/* if no more auto white balance, reset controls. */
-	if (!ctrls->awb)
+	if (ctrls->awb == ISC_WB_NONE)
 		isc_reset_awb_ctrls(isc);
 
 	pm_runtime_get_sync(isc->dev);
@@ -1950,7 +1960,7 @@ static void isc_awb_work(struct work_struct *w)
 	 * only update if we have all the required histograms and controls
 	 * if awb has been disabled, we need to reset registers as well.
 	 */
-	if (hist_id == ISC_HIS_CFG_MODE_GR || !ctrls->awb) {
+	if (hist_id == ISC_HIS_CFG_MODE_GR || ctrls->awb == ISC_WB_NONE) {
 		/*
 		 * It may happen that DMA Done IRQ will trigger while we are
 		 * updating white balance registers here.
@@ -1960,6 +1970,16 @@ static void isc_awb_work(struct work_struct *w)
 		spin_lock_irqsave(&isc->awb_lock, flags);
 		isc_update_awb_ctrls(isc);
 		spin_unlock_irqrestore(&isc->awb_lock, flags);
+
+		/*
+		 * if we are doing just the one time white balance adjustment,
+		 * we are basically done.
+		 */
+		if (ctrls->awb == ISC_WB_ONETIME) {
+			v4l2_info(&isc->v4l2_dev,
+				  "Completed one time white-balance adjustment.\n");
+			ctrls->awb = ISC_WB_NONE;
+		}
 	}
 	regmap_write(regmap, ISC_HIS_CFG, hist_id | baysel | ISC_HIS_CFG_RAR);
 	isc_update_profile(isc);
@@ -1976,6 +1996,9 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
 					     struct isc_device, ctrls.handler);
 	struct isc_ctrls *ctrls = &isc->ctrls;
 
+	if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
+		return 0;
+
 	switch (ctrl->id) {
 	case V4L2_CID_BRIGHTNESS:
 		ctrls->brightness = ctrl->val & ISC_CBC_BRIGHT_MASK;
@@ -1987,10 +2010,33 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
 		ctrls->gamma_index = ctrl->val;
 		break;
 	case V4L2_CID_AUTO_WHITE_BALANCE:
-		ctrls->awb = ctrl->val;
-		if (ctrls->hist_stat != HIST_ENABLED) {
+		if (ctrl->val == 1)
+			ctrls->awb = ISC_WB_AUTO;
+		else
+			ctrls->awb = ISC_WB_NONE;
+
+		/* we did not configure ISC yet */
+		if (!isc->config.sd_format)
+			break;
+
+		if (ctrls->hist_stat != HIST_ENABLED)
 			isc_reset_awb_ctrls(isc);
-		}
+
+		if (isc->ctrls.awb == ISC_WB_AUTO &&
+		    vb2_is_streaming(&isc->vb2_vidq) &&
+		    ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code))
+			isc_set_histogram(isc, true);
+
+		break;
+	case V4L2_CID_DO_WHITE_BALANCE:
+		/* if AWB is enabled, do nothing */
+		if (ctrls->awb == ISC_WB_AUTO)
+			return 0;
+
+		ctrls->awb = ISC_WB_ONETIME;
+		isc_set_histogram(isc, true);
+		v4l2_dbg(1, debug, &isc->v4l2_dev,
+			 "One time white-balance started.\n");
 		break;
 	default:
 		return -EINVAL;
@@ -2013,7 +2059,7 @@ static int isc_ctrl_init(struct isc_device *isc)
 	ctrls->hist_stat = HIST_INIT;
 	isc_reset_awb_ctrls(isc);
 
-	ret = v4l2_ctrl_handler_init(hdl, 4);
+	ret = v4l2_ctrl_handler_init(hdl, 5);
 	if (ret < 0)
 		return ret;
 
@@ -2025,6 +2071,12 @@ static int isc_ctrl_init(struct isc_device *isc)
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, GAMMA_MAX, 1, 2);
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
 
+	/* do_white_balance is a button, so min,max,step,default are ignored */
+	isc->do_wb_ctrl = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DO_WHITE_BALANCE,
+					    0, 0, 0, 0);
+
+	v4l2_ctrl_activate(isc->do_wb_ctrl, false);
+
 	v4l2_ctrl_handler_setup(hdl);
 
 	return 0;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 4/4] media: atmel: atmel-isc: make try_fmt error less verbose
  2019-04-15 14:13 [PATCH v2 0/4] media: atmel: atmel-isc: new features Eugen.Hristev
                   ` (2 preceding siblings ...)
  2019-04-15 14:13 ` [PATCH v2 3/4] media: atmel: atmel-isc: add support " Eugen.Hristev
@ 2019-04-15 14:13 ` Eugen.Hristev
  3 siblings, 0 replies; 5+ messages in thread
From: Eugen.Hristev @ 2019-04-15 14:13 UTC (permalink / raw)
  To: linux-media, hverkuil, linux-arm-kernel, linux-kernel
  Cc: Nicolas.Ferre, mchehab, ksloat, Eugen.Hristev

From: Eugen Hristev <eugen.hristev@microchip.com>

In case the sensor refuses to set the format, avoid printing the error
message that no compatible format was found.
This means that the try_fmt will be less verbose.
The error will be printed only if really a format cannot be found.
Some application try all possible formats in a row (gstreamer e.g.)
which will flood the console with error messages until a working one is found.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/media/platform/atmel/atmel-isc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index 777e27f..da3b441 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1487,7 +1487,7 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
 	ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
 			       &pad_cfg, &format);
 	if (ret < 0)
-		goto isc_try_fmt_err;
+		goto isc_try_fmt_subdev_err;
 
 	v4l2_fill_pix_format(pixfmt, &format.format);
 
@@ -1502,6 +1502,7 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
 
 isc_try_fmt_err:
 	v4l2_err(&isc->v4l2_dev, "Could not find any possible format for a working pipeline\n");
+isc_try_fmt_subdev_err:
 	memset(&isc->try_config, 0, sizeof(isc->try_config));
 
 	return ret;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-15 14:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15 14:13 [PATCH v2 0/4] media: atmel: atmel-isc: new features Eugen.Hristev
2019-04-15 14:13 ` [PATCH v2 1/4] media: atmel: atmel-isc: reworked white balance feature Eugen.Hristev
2019-04-15 14:13 ` [PATCH v2 2/4] media: v4l2-ctrl: fix flags for DO_WHITE_BALANCE Eugen.Hristev
2019-04-15 14:13 ` [PATCH v2 3/4] media: atmel: atmel-isc: add support " Eugen.Hristev
2019-04-15 14:13 ` [PATCH v2 4/4] media: atmel: atmel-isc: make try_fmt error less verbose Eugen.Hristev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).