Hi Kamil, On Monday 30 January 2012 14:39:22 Kamil Debski wrote: > On 30 January 2012 13:12 Laurent Pinchart wrote: > > On Monday 30 January 2012 10:58:43 Sachin Kamat wrote: > > > This patch adds support for flipping the image horizontally and > > > vertically. [snip] > > > + v4l2_ctrl_new_std(&ctx->ctrl_handler, &g2d_ctrl_ops, > > > + V4L2_CID_HFLIP, 0, 1, 1, 0); > > > + if (ctx->ctrl_handler.error) > > > + goto error; > > > + > > > + v4l2_ctrl_new_std(&ctx->ctrl_handler, &g2d_ctrl_ops, > > > + V4L2_CID_VFLIP, 0, 1, 1, 0); > > > > As a single register controls hflip and vflip, you should group the two > > controls in a cluster. > > I think it doesn't matter in this use case. As register are not written > in the g2d_s_ctrl. Because the driver uses multiple context it modifies > the appropriate values in its context structure and registers are written > when the transaction is run. > > Also there is no logical connection between horizontal and vertical flip. > I think this is the case when using clusters. Here one is independent from > another. As the value is only written to hardware registers later, not in the s_ctrl() handler, a cluster is (probably) not mandatory if the driver uses proper locking. Otherwise there will be no guarantee that setting both hflip and vflip in a single VIDIOC_S_EXT_CTRLS call will not result in one frame with only hflip or vflip applied. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html