Re: [RFC PATCH 07/11] v4l2-ctrls: implement 'ignore after use' support.

2014-11-17 Thread Hans Verkuil
On 11/15/2014 10:10 PM, Sakari Ailus wrote:
 Hi Hans,
 
 A few comments below.
 
 On Sun, Sep 21, 2014 at 04:48:25PM +0200, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 Sometimes you want to apply a value every time v4l2_ctrl_apply_store
 is called, and sometimes you want to apply that value only once.

 This adds support for that feature.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/v4l2-core/v4l2-ctrls.c | 75 
 
  drivers/media/v4l2-core/v4l2-ioctl.c | 14 +++
  include/media/v4l2-ctrls.h   | 12 ++
  3 files changed, 79 insertions(+), 22 deletions(-)

 diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
 b/drivers/media/v4l2-core/v4l2-ctrls.c
 index e5dccf2..21560b0 100644
 --- a/drivers/media/v4l2-core/v4l2-ctrls.c
 +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
 @@ -1475,6 +1475,7 @@ static int ptr_to_user(struct v4l2_ext_control *c,
  static int cur_to_user(struct v4l2_ext_control *c,
 struct v4l2_ctrl *ctrl)
  {
 +c-flags = 0;
  return ptr_to_user(c, ctrl, ctrl-p_cur);
  }
  
 @@ -1482,8 +1483,13 @@ static int cur_to_user(struct v4l2_ext_control *c,
  static int store_to_user(struct v4l2_ext_control *c,
 struct v4l2_ctrl *ctrl, unsigned store)
  {
 +c-flags = 0;
  if (store == 0)
  return ptr_to_user(c, ctrl, ctrl-p_new);
 +if (test_bit(store - 1, ctrl-cluster[0]-ignore_store_after_use))
 +c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
 +if (test_bit(store - 1, ctrl-cluster[0]-ignore_store))
 +c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE;
  return ptr_to_user(c, ctrl, ctrl-p_stores[store - 1]);
  }
  
 @@ -1491,6 +1497,7 @@ static int store_to_user(struct v4l2_ext_control *c,
  static int new_to_user(struct v4l2_ext_control *c,
 struct v4l2_ctrl *ctrl)
  {
 +c-flags = 0;
  return ptr_to_user(c, ctrl, ctrl-p_new);
  }
  
 @@ -1546,6 +1553,8 @@ static int user_to_ptr(struct v4l2_ext_control *c,
  static int user_to_new(struct v4l2_ext_control *c,
 struct v4l2_ctrl *ctrl)
  {
 +ctrl-cluster[0]-new_ignore_store_after_use =
 +c-flags  V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
  return user_to_ptr(c, ctrl, ctrl-p_new);
  }
  
 @@ -1597,8 +1606,11 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
 v4l2_ctrl *ctrl, u32 ch_flags)
  /* Helper function: copy the new control value to the store */
  static void new_to_store(struct v4l2_ctrl *ctrl)
  {
 +if (ctrl == NULL)
 +return;
 +
  /* has_changed is set by cluster_changed */
 -if (ctrl  ctrl-has_changed)
 +if (ctrl-has_changed)
  ptr_to_ptr(ctrl, ctrl-p_new, ctrl-p_stores[ctrl-store - 1]);
  }
  
 @@ -2328,6 +2340,12 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct 
 v4l2_ctrl **controls)
  
  for (i = 0; i  ncontrols; i++) {
  if (controls[i]) {
 +/*
 + * All controls in a cluster must have the same
 + * V4L2_CTRL_FLAG_CAN_STORE flag value.
 + */
 +WARN_ON((controls[0]-flags  controls[i]-flags) 
 +V4L2_CTRL_FLAG_CAN_STORE);
  controls[i]-cluster = controls;
  controls[i]-ncontrols = ncontrols;
  if (controls[i]-flags  V4L2_CTRL_FLAG_VOLATILE)
 @@ -2850,6 +2868,10 @@ static int extend_store(struct v4l2_ctrl *ctrl, 
 unsigned stores)
  unsigned s, idx;
  union v4l2_ctrl_ptr *p;
  
 +/* round up to the next multiple of 4 */
 +stores = (stores + 3)  ~3;
 
 You said it, round_up(). :-)
 
 The comment becomes redundant as a result, too.
 
 The above may also overflow. 

Will fix.

 
 +if (stores  V4L2_CTRLS_MAX_STORES)
 +return -EINVAL;
  p = kcalloc(stores, sizeof(union v4l2_ctrl_ptr), GFP_KERNEL);
  if (p == NULL)
  return -ENOMEM;
 @@ -2868,6 +2890,7 @@ static int extend_store(struct v4l2_ctrl *ctrl, 
 unsigned stores)
  memcpy(p, ctrl-p_stores, ctrl-nr_of_stores * sizeof(union 
 v4l2_ctrl_ptr));
  kfree(ctrl-p_stores);
  ctrl-p_stores = p;
 +bitmap_set(ctrl-ignore_store, ctrl-nr_of_stores, stores - 
 ctrl-nr_of_stores);
  ctrl-nr_of_stores = stores;
  return 0;
  }
 @@ -3081,21 +3104,33 @@ static int try_or_set_cluster(struct v4l2_fh *fh, 
 struct v4l2_ctrl *master,
  
  ret = call_op(master, try_ctrl);
  
 -/* Don't set if there is no change */
 -if (ret || !set || !cluster_changed(master))
 -return ret;
 -ret = call_op(master, s_ctrl);
 -if (ret)
 +if (ret || !set)
  return ret;
  
 -/* 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++) {
 -if (store)
 -

Re: [RFC PATCH 07/11] v4l2-ctrls: implement 'ignore after use' support.

2014-11-17 Thread Sakari Ailus
Hi Hans,

On Mon, Nov 17, 2014 at 10:02:03AM +0100, Hans Verkuil wrote:
 On 11/15/2014 10:10 PM, Sakari Ailus wrote:
  Hi Hans,
  
  A few comments below.
  
  On Sun, Sep 21, 2014 at 04:48:25PM +0200, Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
 
  Sometimes you want to apply a value every time v4l2_ctrl_apply_store
  is called, and sometimes you want to apply that value only once.
 
  This adds support for that feature.
 
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/v4l2-core/v4l2-ctrls.c | 75 
  
   drivers/media/v4l2-core/v4l2-ioctl.c | 14 +++
   include/media/v4l2-ctrls.h   | 12 ++
   3 files changed, 79 insertions(+), 22 deletions(-)
 
  diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
  b/drivers/media/v4l2-core/v4l2-ctrls.c
  index e5dccf2..21560b0 100644
  --- a/drivers/media/v4l2-core/v4l2-ctrls.c
  +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
  @@ -1475,6 +1475,7 @@ static int ptr_to_user(struct v4l2_ext_control *c,
   static int cur_to_user(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl)
   {
  +  c-flags = 0;
 return ptr_to_user(c, ctrl, ctrl-p_cur);
   }
   
  @@ -1482,8 +1483,13 @@ static int cur_to_user(struct v4l2_ext_control *c,
   static int store_to_user(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl, unsigned store)
   {
  +  c-flags = 0;
 if (store == 0)
 return ptr_to_user(c, ctrl, ctrl-p_new);
  +  if (test_bit(store - 1, ctrl-cluster[0]-ignore_store_after_use))
  +  c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
  +  if (test_bit(store - 1, ctrl-cluster[0]-ignore_store))
  +  c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE;
 return ptr_to_user(c, ctrl, ctrl-p_stores[store - 1]);
   }
   
  @@ -1491,6 +1497,7 @@ static int store_to_user(struct v4l2_ext_control *c,
   static int new_to_user(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl)
   {
  +  c-flags = 0;
 return ptr_to_user(c, ctrl, ctrl-p_new);
   }
   
  @@ -1546,6 +1553,8 @@ static int user_to_ptr(struct v4l2_ext_control *c,
   static int user_to_new(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl)
   {
  +  ctrl-cluster[0]-new_ignore_store_after_use =
  +  c-flags  V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
 return user_to_ptr(c, ctrl, ctrl-p_new);
   }
   
  @@ -1597,8 +1606,11 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
  v4l2_ctrl *ctrl, u32 ch_flags)
   /* Helper function: copy the new control value to the store */
   static void new_to_store(struct v4l2_ctrl *ctrl)
   {
  +  if (ctrl == NULL)
  +  return;
  +
 /* has_changed is set by cluster_changed */
  -  if (ctrl  ctrl-has_changed)
  +  if (ctrl-has_changed)
 ptr_to_ptr(ctrl, ctrl-p_new, ctrl-p_stores[ctrl-store - 1]);
   }
   
  @@ -2328,6 +2340,12 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct 
  v4l2_ctrl **controls)
   
 for (i = 0; i  ncontrols; i++) {
 if (controls[i]) {
  +  /*
  +   * All controls in a cluster must have the same
  +   * V4L2_CTRL_FLAG_CAN_STORE flag value.
  +   */
  +  WARN_ON((controls[0]-flags  controls[i]-flags) 
  +  V4L2_CTRL_FLAG_CAN_STORE);
 controls[i]-cluster = controls;
 controls[i]-ncontrols = ncontrols;
 if (controls[i]-flags  V4L2_CTRL_FLAG_VOLATILE)
  @@ -2850,6 +2868,10 @@ static int extend_store(struct v4l2_ctrl *ctrl, 
  unsigned stores)
 unsigned s, idx;
 union v4l2_ctrl_ptr *p;
   
  +  /* round up to the next multiple of 4 */
  +  stores = (stores + 3)  ~3;
  
  You said it, round_up(). :-)
  
  The comment becomes redundant as a result, too.
  
  The above may also overflow. 
 
 Will fix.

I just realised round_up() will naturally also overflow, but it'll overflow
correctly to zero. So the upper limit check must be before this. The
change then effectually only makes the comment unnecessary.

  
  +  if (stores  V4L2_CTRLS_MAX_STORES)
  +  return -EINVAL;
 p = kcalloc(stores, sizeof(union v4l2_ctrl_ptr), GFP_KERNEL);
 if (p == NULL)
 return -ENOMEM;
  @@ -2868,6 +2890,7 @@ static int extend_store(struct v4l2_ctrl *ctrl, 
  unsigned stores)
 memcpy(p, ctrl-p_stores, ctrl-nr_of_stores * sizeof(union 
  v4l2_ctrl_ptr));
 kfree(ctrl-p_stores);
 ctrl-p_stores = p;
  +  bitmap_set(ctrl-ignore_store, ctrl-nr_of_stores, stores - 
  ctrl-nr_of_stores);
 ctrl-nr_of_stores = stores;
 return 0;
   }
  @@ -3081,21 +3104,33 @@ static int try_or_set_cluster(struct v4l2_fh *fh, 
  struct v4l2_ctrl *master,
   
 ret = call_op(master, try_ctrl);
   
  -  /* Don't set if there is no change */
  -  if (ret || !set || !cluster_changed(master))
  -  return ret;
  -  ret = call_op(master, s_ctrl);
  -  if 

Re: [RFC PATCH 07/11] v4l2-ctrls: implement 'ignore after use' support.

2014-11-17 Thread Hans Verkuil
On 11/17/2014 10:31 AM, Sakari Ailus wrote:
 Hi Hans,
 
 On Mon, Nov 17, 2014 at 10:02:03AM +0100, Hans Verkuil wrote:
 On 11/15/2014 10:10 PM, Sakari Ailus wrote:
 @@ -197,6 +207,8 @@ struct v4l2_ctrl {
u32 nr_of_dims;
u16 nr_of_stores;
u16 store;
 +  DECLARE_BITMAP(ignore_store, V4L2_CTRLS_MAX_STORES);
 +  DECLARE_BITMAP(ignore_store_after_use, V4L2_CTRLS_MAX_STORES);

 I'd store this information next to the value itself. The reason is that
 stores are typically accessed one at a time, and thus keeping data related
 to a single store in a single contiguous location reduces cache misses.

 Hmm, sounds like overengineering to me. If I can do that without sacrificing
 readability, then I can more it around. It's likely that these datastructures
 will change anyway.
 
 The controls are accessed very often in practice so this kind of things
 count. There's already a lot of code which gets executed in order to set a
 single control that's relevant only in some cases, such as clusters.

Complexity is the biggest problem in video drivers, not speed. Optimizations for
the sake of speeding up code at the expense of complexity should only be 
implemented
if you can *prove* that there is a measurable speedup.

Personally I would be very surprised if you can measure this in this specific
case.

Anyway, it doesn't matter in this case since I intend to rework those data
structures in any case.

Regards,

Hans

 I think it'd probably be more readable as well if information related to a
 store was located in a single place. As a bonus you wouldn't need to set a
 global maximum for the number of stores one may have.

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH 07/11] v4l2-ctrls: implement 'ignore after use' support.

2014-11-15 Thread Sakari Ailus
Hi Hans,

A few comments below.

On Sun, Sep 21, 2014 at 04:48:25PM +0200, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Sometimes you want to apply a value every time v4l2_ctrl_apply_store
 is called, and sometimes you want to apply that value only once.
 
 This adds support for that feature.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/v4l2-core/v4l2-ctrls.c | 75 
 
  drivers/media/v4l2-core/v4l2-ioctl.c | 14 +++
  include/media/v4l2-ctrls.h   | 12 ++
  3 files changed, 79 insertions(+), 22 deletions(-)
 
 diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
 b/drivers/media/v4l2-core/v4l2-ctrls.c
 index e5dccf2..21560b0 100644
 --- a/drivers/media/v4l2-core/v4l2-ctrls.c
 +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
 @@ -1475,6 +1475,7 @@ static int ptr_to_user(struct v4l2_ext_control *c,
  static int cur_to_user(struct v4l2_ext_control *c,
  struct v4l2_ctrl *ctrl)
  {
 + c-flags = 0;
   return ptr_to_user(c, ctrl, ctrl-p_cur);
  }
  
 @@ -1482,8 +1483,13 @@ static int cur_to_user(struct v4l2_ext_control *c,
  static int store_to_user(struct v4l2_ext_control *c,
  struct v4l2_ctrl *ctrl, unsigned store)
  {
 + c-flags = 0;
   if (store == 0)
   return ptr_to_user(c, ctrl, ctrl-p_new);
 + if (test_bit(store - 1, ctrl-cluster[0]-ignore_store_after_use))
 + c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
 + if (test_bit(store - 1, ctrl-cluster[0]-ignore_store))
 + c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE;
   return ptr_to_user(c, ctrl, ctrl-p_stores[store - 1]);
  }
  
 @@ -1491,6 +1497,7 @@ static int store_to_user(struct v4l2_ext_control *c,
  static int new_to_user(struct v4l2_ext_control *c,
  struct v4l2_ctrl *ctrl)
  {
 + c-flags = 0;
   return ptr_to_user(c, ctrl, ctrl-p_new);
  }
  
 @@ -1546,6 +1553,8 @@ static int user_to_ptr(struct v4l2_ext_control *c,
  static int user_to_new(struct v4l2_ext_control *c,
  struct v4l2_ctrl *ctrl)
  {
 + ctrl-cluster[0]-new_ignore_store_after_use =
 + c-flags  V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
   return user_to_ptr(c, ctrl, ctrl-p_new);
  }
  
 @@ -1597,8 +1606,11 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
 v4l2_ctrl *ctrl, u32 ch_flags)
  /* Helper function: copy the new control value to the store */
  static void new_to_store(struct v4l2_ctrl *ctrl)
  {
 + if (ctrl == NULL)
 + return;
 +
   /* has_changed is set by cluster_changed */
 - if (ctrl  ctrl-has_changed)
 + if (ctrl-has_changed)
   ptr_to_ptr(ctrl, ctrl-p_new, ctrl-p_stores[ctrl-store - 1]);
  }
  
 @@ -2328,6 +2340,12 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct 
 v4l2_ctrl **controls)
  
   for (i = 0; i  ncontrols; i++) {
   if (controls[i]) {
 + /*
 +  * All controls in a cluster must have the same
 +  * V4L2_CTRL_FLAG_CAN_STORE flag value.
 +  */
 + WARN_ON((controls[0]-flags  controls[i]-flags) 
 + V4L2_CTRL_FLAG_CAN_STORE);
   controls[i]-cluster = controls;
   controls[i]-ncontrols = ncontrols;
   if (controls[i]-flags  V4L2_CTRL_FLAG_VOLATILE)
 @@ -2850,6 +2868,10 @@ static int extend_store(struct v4l2_ctrl *ctrl, 
 unsigned stores)
   unsigned s, idx;
   union v4l2_ctrl_ptr *p;
  
 + /* round up to the next multiple of 4 */
 + stores = (stores + 3)  ~3;

You said it, round_up(). :-)

The comment becomes redundant as a result, too.

The above may also overflow. 

 + if (stores  V4L2_CTRLS_MAX_STORES)
 + return -EINVAL;
   p = kcalloc(stores, sizeof(union v4l2_ctrl_ptr), GFP_KERNEL);
   if (p == NULL)
   return -ENOMEM;
 @@ -2868,6 +2890,7 @@ static int extend_store(struct v4l2_ctrl *ctrl, 
 unsigned stores)
   memcpy(p, ctrl-p_stores, ctrl-nr_of_stores * sizeof(union 
 v4l2_ctrl_ptr));
   kfree(ctrl-p_stores);
   ctrl-p_stores = p;
 + bitmap_set(ctrl-ignore_store, ctrl-nr_of_stores, stores - 
 ctrl-nr_of_stores);
   ctrl-nr_of_stores = stores;
   return 0;
  }
 @@ -3081,21 +3104,33 @@ static int try_or_set_cluster(struct v4l2_fh *fh, 
 struct v4l2_ctrl *master,
  
   ret = call_op(master, try_ctrl);
  
 - /* Don't set if there is no change */
 - if (ret || !set || !cluster_changed(master))
 - return ret;
 - ret = call_op(master, s_ctrl);
 - if (ret)
 + if (ret || !set)
   return ret;
  
 - /* 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++) {
 - if (store)
 - 

[RFC PATCH 07/11] v4l2-ctrls: implement 'ignore after use' support.

2014-09-21 Thread Hans Verkuil
From: Hans Verkuil hans.verk...@cisco.com

Sometimes you want to apply a value every time v4l2_ctrl_apply_store
is called, and sometimes you want to apply that value only once.

This adds support for that feature.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 75 
 drivers/media/v4l2-core/v4l2-ioctl.c | 14 +++
 include/media/v4l2-ctrls.h   | 12 ++
 3 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index e5dccf2..21560b0 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1475,6 +1475,7 @@ static int ptr_to_user(struct v4l2_ext_control *c,
 static int cur_to_user(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl)
 {
+   c-flags = 0;
return ptr_to_user(c, ctrl, ctrl-p_cur);
 }
 
@@ -1482,8 +1483,13 @@ static int cur_to_user(struct v4l2_ext_control *c,
 static int store_to_user(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl, unsigned store)
 {
+   c-flags = 0;
if (store == 0)
return ptr_to_user(c, ctrl, ctrl-p_new);
+   if (test_bit(store - 1, ctrl-cluster[0]-ignore_store_after_use))
+   c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
+   if (test_bit(store - 1, ctrl-cluster[0]-ignore_store))
+   c-flags |= V4L2_EXT_CTRL_FL_IGN_STORE;
return ptr_to_user(c, ctrl, ctrl-p_stores[store - 1]);
 }
 
@@ -1491,6 +1497,7 @@ static int store_to_user(struct v4l2_ext_control *c,
 static int new_to_user(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl)
 {
+   c-flags = 0;
return ptr_to_user(c, ctrl, ctrl-p_new);
 }
 
@@ -1546,6 +1553,8 @@ static int user_to_ptr(struct v4l2_ext_control *c,
 static int user_to_new(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl)
 {
+   ctrl-cluster[0]-new_ignore_store_after_use =
+   c-flags  V4L2_EXT_CTRL_FL_IGN_STORE_AFTER_USE;
return user_to_ptr(c, ctrl, ctrl-p_new);
 }
 
@@ -1597,8 +1606,11 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 ch_flags)
 /* Helper function: copy the new control value to the store */
 static void new_to_store(struct v4l2_ctrl *ctrl)
 {
+   if (ctrl == NULL)
+   return;
+
/* has_changed is set by cluster_changed */
-   if (ctrl  ctrl-has_changed)
+   if (ctrl-has_changed)
ptr_to_ptr(ctrl, ctrl-p_new, ctrl-p_stores[ctrl-store - 1]);
 }
 
@@ -2328,6 +2340,12 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct 
v4l2_ctrl **controls)
 
for (i = 0; i  ncontrols; i++) {
if (controls[i]) {
+   /*
+* All controls in a cluster must have the same
+* V4L2_CTRL_FLAG_CAN_STORE flag value.
+*/
+   WARN_ON((controls[0]-flags  controls[i]-flags) 
+   V4L2_CTRL_FLAG_CAN_STORE);
controls[i]-cluster = controls;
controls[i]-ncontrols = ncontrols;
if (controls[i]-flags  V4L2_CTRL_FLAG_VOLATILE)
@@ -2850,6 +2868,10 @@ static int extend_store(struct v4l2_ctrl *ctrl, unsigned 
stores)
unsigned s, idx;
union v4l2_ctrl_ptr *p;
 
+   /* round up to the next multiple of 4 */
+   stores = (stores + 3)  ~3;
+   if (stores  V4L2_CTRLS_MAX_STORES)
+   return -EINVAL;
p = kcalloc(stores, sizeof(union v4l2_ctrl_ptr), GFP_KERNEL);
if (p == NULL)
return -ENOMEM;
@@ -2868,6 +2890,7 @@ static int extend_store(struct v4l2_ctrl *ctrl, unsigned 
stores)
memcpy(p, ctrl-p_stores, ctrl-nr_of_stores * sizeof(union 
v4l2_ctrl_ptr));
kfree(ctrl-p_stores);
ctrl-p_stores = p;
+   bitmap_set(ctrl-ignore_store, ctrl-nr_of_stores, stores - 
ctrl-nr_of_stores);
ctrl-nr_of_stores = stores;
return 0;
 }
@@ -3081,21 +3104,33 @@ static int try_or_set_cluster(struct v4l2_fh *fh, 
struct v4l2_ctrl *master,
 
ret = call_op(master, try_ctrl);
 
-   /* Don't set if there is no change */
-   if (ret || !set || !cluster_changed(master))
-   return ret;
-   ret = call_op(master, s_ctrl);
-   if (ret)
+   if (ret || !set)
return ret;
 
-   /* 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++) {
-   if (store)
-   new_to_store(master-cluster[i]);
+   /* Don't set if there is no change */
+   if (cluster_changed(master)) {
+   ret = call_op(master, s_ctrl);
+   if (ret)
+   return ret;
+
+