diff mbox series

[v3,15/15] media: i2c: Add vblank control to ov7251 driver

Message ID 20220504223027.3480287-16-djrscally@gmail.com
State Superseded
Headers show
Series None | expand

Commit Message

Daniel Scally May 4, 2022, 10:30 p.m. UTC 
Add a vblank control to the ov7251 driver.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v3:

	- New patch

 drivers/media/i2c/ov7251.c | 53 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

Comments

Daniel Scally May 5, 2022, 10:04 p.m. UTC | #1 
Hi Andy


On 05/05/2022 11:31, Andy Shevchenko wrote:
> On Wed, May 04, 2022 at 11:30:27PM +0100, Daniel Scally wrote:
>> Add a vblank control to the ov7251 driver.
>> +static int ov7251_vts_configure(struct ov7251 *ov7251, s32 vblank)
>> +{
>> +	u8 vts[2];
>> +
>> +	vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
>> +	vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
> __be16 vts;
>
> cpu_to_be16();
Most places that do this seem to do the conversion in the i2c read/write
functions, so in this case within ov7251_write_seq_regs(). Can I do it
there, as an extra patch? I actually have more changes to make on this
driver but they're not remotely read yet so there'll be another series
in the future
>> +	return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
>> +}
Andy Shevchenko May 6, 2022, 9:22 p.m. UTC | #2 
On Fri, May 6, 2022 at 1:46 PM Daniel Scally <djrscally@gmail.com> wrote:
> On 05/05/2022 11:31, Andy Shevchenko wrote:
> > On Wed, May 04, 2022 at 11:30:27PM +0100, Daniel Scally wrote:

...

> >> +    u8 vts[2];
> >> +
> >> +    vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
> >> +    vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
> > __be16 vts;
> >
> > cpu_to_be16();

> Most places that do this seem to do the conversion in the i2c read/write
> functions, so in this case within ov7251_write_seq_regs(). Can I do it
> there, as an extra patch? I actually have more changes to make on this
> driver but they're not remotely read yet so there'll be another series
> in the future

Ideally you should first convert them and then add your patch with
this change in mind.

> >> +    return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 003a7a5ae038..dc9d4e08efae 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -62,6 +62,10 @@ 
 #define OV7251_ACTIVE_HEIGHT		488
 
 #define OV7251_FIXED_PPL		928
+#define OV7251_TIMING_VTS_REG		0x380e
+#define OV7251_TIMING_MIN_VTS		1
+#define OV7251_TIMING_MAX_VTS		0xffff
+#define OV7251_INTEGRATION_MARGIN	20
 
 struct reg_value {
 	u16 reg;
@@ -71,6 +75,7 @@  struct reg_value {
 struct ov7251_mode_info {
 	u32 width;
 	u32 height;
+	u32 vts;
 	const struct reg_value *data;
 	u32 data_size;
 	u32 pixel_clock;
@@ -142,6 +147,7 @@  struct ov7251 {
 	struct v4l2_ctrl *exposure;
 	struct v4l2_ctrl *gain;
 	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *vblank;
 
 	/* Cached register values */
 	u8 aec_pk_manual;
@@ -637,6 +643,7 @@  static const struct ov7251_mode_info ov7251_mode_info_data[] = {
 	{
 		.width = 640,
 		.height = 480,
+		.vts = 1724,
 		.data = ov7251_setting_vga_30fps,
 		.data_size = ARRAY_SIZE(ov7251_setting_vga_30fps),
 		.exposure_max = 1704,
@@ -649,6 +656,7 @@  static const struct ov7251_mode_info ov7251_mode_info_data[] = {
 	{
 		.width = 640,
 		.height = 480,
+		.vts = 860,
 		.data = ov7251_setting_vga_60fps,
 		.data_size = ARRAY_SIZE(ov7251_setting_vga_60fps),
 		.exposure_max = 840,
@@ -661,6 +669,7 @@  static const struct ov7251_mode_info ov7251_mode_info_data[] = {
 	{
 		.width = 640,
 		.height = 480,
+		.vts = 572,
 		.data = ov7251_setting_vga_90fps,
 		.data_size = ARRAY_SIZE(ov7251_setting_vga_90fps),
 		.exposure_max = 552,
@@ -1001,12 +1010,36 @@  static const char * const ov7251_test_pattern_menu[] = {
 	"Vertical Pattern Bars",
 };
 
+static int ov7251_vts_configure(struct ov7251 *ov7251, s32 vblank)
+{
+	u8 vts[2];
+
+	vts[0] = ((ov7251->current_mode->height + vblank) & 0xff00) >> 8;
+	vts[1] = ((ov7251->current_mode->height + vblank) & 0x00ff);
+
+	return ov7251_write_seq_regs(ov7251, OV7251_TIMING_VTS_REG, vts, 2);
+}
+
 static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov7251 *ov7251 = container_of(ctrl->handler,
 					     struct ov7251, ctrls);
 	int ret;
 
+	/* If VBLANK is altered we need to update exposure to compensate */
+	if (ctrl->id == V4L2_CID_VBLANK) {
+		int exposure_max;
+
+		exposure_max = ov7251->current_mode->height + ctrl->val -
+			       OV7251_INTEGRATION_MARGIN;
+		__v4l2_ctrl_modify_range(ov7251->exposure,
+					 ov7251->exposure->minimum,
+					 exposure_max,
+					 ov7251->exposure->step,
+					 min(ov7251->exposure->val,
+					     exposure_max));
+	}
+
 	/* v4l2_ctrl_lock() locks our mutex */
 
 	if (!pm_runtime_get_if_in_use(ov7251->dev))
@@ -1028,6 +1061,9 @@  static int ov7251_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_VFLIP:
 		ret = ov7251_set_vflip(ov7251, ctrl->val);
 		break;
+	case V4L2_CID_VBLANK:
+		ret = ov7251_vts_configure(ov7251, ctrl->val);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -1179,6 +1215,7 @@  static int ov7251_set_format(struct v4l2_subdev *sd,
 {
 	struct ov7251 *ov7251 = to_ov7251(sd);
 	struct v4l2_mbus_framefmt *__format;
+	int vblank_max, vblank_def;
 	struct v4l2_rect *__crop;
 	const struct ov7251_mode_info *new_mode;
 	int ret = 0;
@@ -1212,6 +1249,14 @@  static int ov7251_set_format(struct v4l2_subdev *sd,
 		if (ret < 0)
 			goto exit;
 
+		vblank_max = OV7251_TIMING_MAX_VTS - new_mode->height;
+		vblank_def = new_mode->vts - new_mode->height;
+		ret = __v4l2_ctrl_modify_range(ov7251->vblank,
+					       OV7251_TIMING_MIN_VTS,
+					       vblank_max, 1, vblank_max);
+		if (ret < 0)
+			goto exit;
+
 		ov7251->current_mode = new_mode;
 	}
 
@@ -1492,6 +1537,7 @@  static int ov7251_detect_chip(struct ov7251 *ov7251)
 
 static int ov7251_init_ctrls(struct ov7251 *ov7251)
 {
+	int vblank_max, vblank_def;
 	s64 pixel_rate;
 	int hblank;
 
@@ -1535,6 +1581,13 @@  static int ov7251_init_ctrls(struct ov7251 *ov7251)
 	if (ov7251->hblank)
 		ov7251->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
+	vblank_max = OV7251_TIMING_MAX_VTS - ov7251->current_mode->height;
+	vblank_def = ov7251->current_mode->vts - ov7251->current_mode->height;
+	ov7251->vblank = v4l2_ctrl_new_std(&ov7251->ctrls, &ov7251_ctrl_ops,
+					   V4L2_CID_VBLANK,
+					   OV7251_TIMING_MIN_VTS, vblank_max, 1,
+					   vblank_def);
+
 	ov7251->sd.ctrl_handler = &ov7251->ctrls;
 
 	if (ov7251->ctrls.error) {