2019-04-15 14:15:29

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH v2 0/4] media: atmel: atmel-isc: new features

From: Eugen Hristev <[email protected]>

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


2019-04-15 14:14:51

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH v2 1/4] media: atmel: atmel-isc: reworked white balance feature

From: Eugen Hristev <[email protected]>

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 <[email protected]>
---
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

2019-04-15 14:15:02

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH v2 3/4] media: atmel: atmel-isc: add support for DO_WHITE_BALANCE

From: Eugen Hristev <[email protected]>

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 <[email protected]>
---
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

2019-04-15 14:15:12

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH v2 4/4] media: atmel: atmel-isc: make try_fmt error less verbose

From: Eugen Hristev <[email protected]>

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 <[email protected]>
---
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

2019-04-15 14:16:03

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH v2 2/4] media: v4l2-ctrl: fix flags for DO_WHITE_BALANCE

From: Eugen Hristev <[email protected]>

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 <[email protected]>
---
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