Re: [PATCH] v4l2-ctrls.c: fix broken auto cluster handling

2018-07-02 Thread Hugues FRUCHET
Many thanks Hans,

This fix my issue with ov5640,

Best regards,
Hugues.

On 06/29/2018 12:12 PM, Hans Verkuil wrote:
> When you switch from auto to manual mode for an auto-cluster (e.g.
> autogain+gain controls), then the current HW value has to be copied
> to the current control value. However, has_changed was never set to
> true, so new_to_cur didn't actually copy this value.
> 
> Signed-off-by: Hans Verkuil 
> Reported-by: Hugues FRUCHET 

Tested-by: Hugues FRUCHET 

> ---
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d29e45516eb7..d1087573da34 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -3137,9 +3137,22 @@ static int try_or_set_cluster(struct v4l2_fh *fh, 
> struct v4l2_ctrl *master,
> 
>   /* If OK, then make the new values permanent. */
>   update_flag = is_cur_manual(master) != is_new_manual(master);
> - for (i = 0; i < master->ncontrols; i++)
> +
> + for (i = 0; i < master->ncontrols; i++) {
> + /*
> +  * If we switch from auto to manual mode, and this cluster
> +  * contains volatile controls, then all non-master controls
> +  * have to be marked as changed. The 'new' value contains
> +  * the volatile value (obtained by update_from_auto_cluster),
> +  * which now has to become the current value.
> +  */
> + if (i && update_flag && is_new_manual(master) &&
> + master->has_volatiles && master->cluster[i])
> + master->cluster[i]->has_changed = true;
> +
>   new_to_cur(fh, master->cluster[i], ch_flags |
>   ((update_flag && i > 0) ? V4L2_EVENT_CTRL_CH_FLAGS : 
> 0));
> + }
>   return 0;
>   }
> 

[PATCH] v4l2-ctrls.c: fix broken auto cluster handling

2018-06-29 Thread Hans Verkuil
When you switch from auto to manual mode for an auto-cluster (e.g.
autogain+gain controls), then the current HW value has to be copied
to the current control value. However, has_changed was never set to
true, so new_to_cur didn't actually copy this value.

Signed-off-by: Hans Verkuil 
Reported-by: Hugues FRUCHET 
---
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index d29e45516eb7..d1087573da34 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3137,9 +3137,22 @@ static int try_or_set_cluster(struct v4l2_fh *fh, struct 
v4l2_ctrl *master,

/* If OK, then make the new values permanent. */
update_flag = is_cur_manual(master) != is_new_manual(master);
-   for (i = 0; i < master->ncontrols; i++)
+
+   for (i = 0; i < master->ncontrols; i++) {
+   /*
+* If we switch from auto to manual mode, and this cluster
+* contains volatile controls, then all non-master controls
+* have to be marked as changed. The 'new' value contains
+* the volatile value (obtained by update_from_auto_cluster),
+* which now has to become the current value.
+*/
+   if (i && update_flag && is_new_manual(master) &&
+   master->has_volatiles && master->cluster[i])
+   master->cluster[i]->has_changed = true;
+
new_to_cur(fh, master->cluster[i], ch_flags |
((update_flag && i > 0) ? V4L2_EVENT_CTRL_CH_FLAGS : 
0));
+   }
return 0;
 }