* [PATCH] mt9p031: Really disable Black Level Calibration in test pattern mode
@ 2014-05-07 15:40 Laurent Pinchart
2014-05-08 8:44 ` Hans Verkuil
0 siblings, 1 reply; 3+ messages in thread
From: Laurent Pinchart @ 2014-05-07 15:40 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil
The digital side of the Black Level Calibration (BLC) function must be
disabled when generating a test pattern to avoid artifacts in the image.
The driver disables BLC correctly at the hardware level, but the feature
gets reenabled by v4l2_ctrl_handler_setup() the next time the device is
powered on.
Fix this by marking the BLC controls as inactive when generating a test
pattern, and ignoring control set requests on inactive controls.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/i2c/mt9p031.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 33daace..9102b23 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -655,6 +655,9 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
u16 data;
int ret;
+ if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
+ return 0;
+
switch (ctrl->id) {
case V4L2_CID_EXPOSURE:
ret = mt9p031_write(client, MT9P031_SHUTTER_WIDTH_UPPER,
@@ -709,8 +712,16 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
MT9P031_READ_MODE_2_ROW_MIR, 0);
case V4L2_CID_TEST_PATTERN:
+ /* The digital side of the Black Level Calibration function must
+ * be disabled when generating a test pattern to avoid artifacts
+ * in the image. Activate (deactivate) the BLC-related controls
+ * when the test pattern is enabled (disabled).
+ */
+ v4l2_ctrl_activate(mt9p031->blc_auto, ctrl->val == 0);
+ v4l2_ctrl_activate(mt9p031->blc_offset, ctrl->val == 0);
+
if (!ctrl->val) {
- /* Restore the black level compensation settings. */
+ /* Restore the BLC settings. */
if (mt9p031->blc_auto->cur.val != 0) {
ret = mt9p031_s_ctrl(mt9p031->blc_auto);
if (ret < 0)
@@ -735,9 +746,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
if (ret < 0)
return ret;
- /* Disable digital black level compensation when using a test
- * pattern.
- */
+ /* Disable digital BLC when generating a test pattern. */
ret = mt9p031_set_mode2(mt9p031, MT9P031_READ_MODE_2_ROW_BLC,
0);
if (ret < 0)
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mt9p031: Really disable Black Level Calibration in test pattern mode
2014-05-07 15:40 [PATCH] mt9p031: Really disable Black Level Calibration in test pattern mode Laurent Pinchart
@ 2014-05-08 8:44 ` Hans Verkuil
2014-05-08 12:55 ` Laurent Pinchart
0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2014-05-08 8:44 UTC (permalink / raw)
To: Laurent Pinchart, linux-media; +Cc: Hans Verkuil
Hi Laurent,
The patch is correct, but I noticed a pre-existing bug that should be
fixed. See below.
On 05/07/14 17:40, Laurent Pinchart wrote:
> The digital side of the Black Level Calibration (BLC) function must be
> disabled when generating a test pattern to avoid artifacts in the image.
> The driver disables BLC correctly at the hardware level, but the feature
> gets reenabled by v4l2_ctrl_handler_setup() the next time the device is
> powered on.
>
> Fix this by marking the BLC controls as inactive when generating a test
> pattern, and ignoring control set requests on inactive controls.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/i2c/mt9p031.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index 33daace..9102b23 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -655,6 +655,9 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> u16 data;
> int ret;
>
> + if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
> + return 0;
> +
> switch (ctrl->id) {
> case V4L2_CID_EXPOSURE:
> ret = mt9p031_write(client, MT9P031_SHUTTER_WIDTH_UPPER,
> @@ -709,8 +712,16 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> MT9P031_READ_MODE_2_ROW_MIR, 0);
>
> case V4L2_CID_TEST_PATTERN:
> + /* The digital side of the Black Level Calibration function must
> + * be disabled when generating a test pattern to avoid artifacts
> + * in the image. Activate (deactivate) the BLC-related controls
> + * when the test pattern is enabled (disabled).
> + */
> + v4l2_ctrl_activate(mt9p031->blc_auto, ctrl->val == 0);
> + v4l2_ctrl_activate(mt9p031->blc_offset, ctrl->val == 0);
> +
> if (!ctrl->val) {
> - /* Restore the black level compensation settings. */
> + /* Restore the BLC settings. */
> if (mt9p031->blc_auto->cur.val != 0) {
> ret = mt9p031_s_ctrl(mt9p031->blc_auto);
This doesn't do what you expect. What you want is to set the blc_auto
control to the current value, but mt9p031_s_ctrl(mt9p031->blc_auto) will
set it to the *new* value, which may not be the same. Ditto for doing the
same for blc_offset.
It's best to just call mt9p031_write directly, rather than going through
mt9p031_s_ctrl.
Regards,
Hans
> if (ret < 0)
> @@ -735,9 +746,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> if (ret < 0)
> return ret;
>
> - /* Disable digital black level compensation when using a test
> - * pattern.
> - */
> + /* Disable digital BLC when generating a test pattern. */
> ret = mt9p031_set_mode2(mt9p031, MT9P031_READ_MODE_2_ROW_BLC,
> 0);
> if (ret < 0)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mt9p031: Really disable Black Level Calibration in test pattern mode
2014-05-08 8:44 ` Hans Verkuil
@ 2014-05-08 12:55 ` Laurent Pinchart
0 siblings, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2014-05-08 12:55 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil
Hi Hans,
On Thursday 08 May 2014 10:44:12 Hans Verkuil wrote:
> Hi Laurent,
>
> The patch is correct, but I noticed a pre-existing bug that should be
> fixed. See below.
>
> On 05/07/14 17:40, Laurent Pinchart wrote:
> > The digital side of the Black Level Calibration (BLC) function must be
> > disabled when generating a test pattern to avoid artifacts in the image.
> > The driver disables BLC correctly at the hardware level, but the feature
> > gets reenabled by v4l2_ctrl_handler_setup() the next time the device is
> > powered on.
> >
> > Fix this by marking the BLC controls as inactive when generating a test
> > pattern, and ignoring control set requests on inactive controls.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/media/i2c/mt9p031.c | 17 +++++++++++++----
> > 1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> > index 33daace..9102b23 100644
> > --- a/drivers/media/i2c/mt9p031.c
> > +++ b/drivers/media/i2c/mt9p031.c
> > @@ -655,6 +655,9 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> > u16 data;
> > int ret;
> >
> > + if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
> > + return 0;
> > +
> > switch (ctrl->id) {
> > case V4L2_CID_EXPOSURE:
> > ret = mt9p031_write(client, MT9P031_SHUTTER_WIDTH_UPPER,
> > @@ -709,8 +712,16 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> > MT9P031_READ_MODE_2_ROW_MIR, 0);
> >
> > case V4L2_CID_TEST_PATTERN:
> > + /* The digital side of the Black Level Calibration function must
> > + * be disabled when generating a test pattern to avoid artifacts
> > + * in the image. Activate (deactivate) the BLC-related controls
> > + * when the test pattern is enabled (disabled).
> > + */
> > + v4l2_ctrl_activate(mt9p031->blc_auto, ctrl->val == 0);
> > + v4l2_ctrl_activate(mt9p031->blc_offset, ctrl->val == 0);
> > +
> > if (!ctrl->val) {
> > - /* Restore the black level compensation settings. */
> > + /* Restore the BLC settings. */
> > if (mt9p031->blc_auto->cur.val != 0) {
> > ret = mt9p031_s_ctrl(mt9p031->blc_auto);
>
> This doesn't do what you expect. What you want is to set the blc_auto
> control to the current value, but mt9p031_s_ctrl(mt9p031->blc_auto) will
> set it to the *new* value, which may not be the same. Ditto for doing the
> same for blc_offset.
>
> It's best to just call mt9p031_write directly, rather than going through
> mt9p031_s_ctrl.
Thanks for catching the problem. I'll fix that in a follow-up patch.
> > if (ret < 0)
> >
> > @@ -735,9 +746,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> > if (ret < 0)
> > return ret;
> >
> > - /* Disable digital black level compensation when using a test
> > - * pattern.
> > - */
> > + /* Disable digital BLC when generating a test pattern. */
> > ret = mt9p031_set_mode2(mt9p031, MT9P031_READ_MODE_2_ROW_BLC,
> > 0);
> > if (ret < 0)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-05-08 12:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-07 15:40 [PATCH] mt9p031: Really disable Black Level Calibration in test pattern mode Laurent Pinchart
2014-05-08 8:44 ` Hans Verkuil
2014-05-08 12:55 ` Laurent Pinchart
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).