On 4/9/19 1:07 PM, Eugen.Hristev@xxxxxxxxxxxxx wrote:
> From: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
>
> 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 disable auto white balance feature (first step)
> v4l2-ctl --set-ctrl=white_balance_automatic=0
>
> To start the one time white balance procedure:
> v4l2-ctl --set-ctrl=do_white_balance=1
>
> 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=1
> 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@xxxxxxxxxxxxx>
> ---
> drivers/media/platform/atmel/atmel-isc.c | 74 +++++++++++++++++++++++++++++---
> 1 file changed, 69 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> index f6b8b00e..e516805 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;
> @@ -809,7 +813,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);
> @@ -1928,7 +1932,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);
> @@ -1937,7 +1941,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.
> @@ -1947,6 +1951,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);
> @@ -1974,10 +1988,56 @@ 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 (ctrl->val == 1) {
> + ctrls->awb = ISC_WB_AUTO;
> + v4l2_ctrl_activate(isc->do_wb_ctrl, false);
> + } else {
> + ctrls->awb = ISC_WB_NONE;
> + v4l2_ctrl_activate(isc->do_wb_ctrl, true);
> + }
> + /* we did not configure ISC yet */
> + if (!isc->config.sd_format)
> + break;
> +
> + if (!ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code)) {
> + v4l2_err(&isc->v4l2_dev,
> + "White balance adjustments available only if sensor is in RAW mode.\n");
This isn't an error, instead if the format isn't raw, then deactivate
the control (see v4l2_ctrl_activate()). That way the control framework
will handle this.
> + return 0;
> + }
> +
> if (ctrls->hist_stat != HIST_ENABLED) {
> isc_reset_awb_ctrls(isc);
> }
> +
> + if (isc->ctrls.awb && 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:
> + /* we did not configure ISC yet */
> + if (!isc->config.sd_format)
> + break;
> +
> + if (ctrls->awb == ISC_WB_AUTO) {
> + v4l2_err(&isc->v4l2_dev,
> + "To use one time white-balance adjustment, disable auto white balance first.\n");
I'd do this differently: if auto whitebalance is already on, then just do
nothing for V4L2_CID_DO_WHITE_BALANCE.
> + return -EAGAIN;
> + }
> + if (!vb2_is_streaming(&isc->vb2_vidq)) {
> + v4l2_err(&isc->v4l2_dev,
> + "One time white-balance adjustment requires streaming to be enabled.\n");
This too should use v4l2_ctrl_activate(): activate the control in start_streaming,
deactivate in stop_streaming (and when the control is created).
> + return -EAGAIN;
> + }
> +
> + if (!ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code)) {
> + v4l2_err(&isc->v4l2_dev,
> + "White balance adjustments available only if sensor is in RAW mode.\n");
Same note as above: use v4l2_ctrl_activate() for this.
> + return -EAGAIN;
> + }
> + ctrls->awb = ISC_WB_ONETIME;
> + isc_set_histogram(isc, true);
> + v4l2_info(&isc->v4l2_dev, "One time white-balance started.\n");
Make this v4l2_dbg.
> break;
> default:
> return -EINVAL;
> @@ -2000,7 +2060,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;
>
> @@ -2012,6 +2072,10 @@ 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_handler_setup(hdl);
>
> return 0;
>
Regards,
Hans
